From da2c636041299c26bfeeab60e4c0c49e03cf8313 Mon Sep 17 00:00:00 2001 From: Tony Vu Date: Fri, 9 Apr 2021 13:11:21 -0700 Subject: [PATCH] (CODEMGMT-1323) Prefer purge_allowlist This change deprecates the setting `purge_whitelist` and prefers the setting `purge_allowlist`. The setting `purge_whitelist` will still work until a future change removes it entirely. --- CHANGELOG.mkd | 1 + doc/dynamic-environments/configuration.mkd | 14 +++---- lib/r10k/action/deploy/environment.rb | 22 ++++++++++- lib/r10k/settings.rb | 7 +++- spec/unit/action/deploy/environment_spec.rb | 42 +++++++++++++++++++++ 5 files changed, 76 insertions(+), 10 deletions(-) diff --git a/CHANGELOG.mkd b/CHANGELOG.mkd index 8917e5988..d68cade3a 100644 --- a/CHANGELOG.mkd +++ b/CHANGELOG.mkd @@ -4,6 +4,7 @@ CHANGELOG Unreleased ---------- +- Deprecate 'purge_whitelist' and favor usage of 'purge_allowlist'. [#1144](https://github.com/puppetlabs/r10k/pull/1144) - Add 'strip\_component' environment source configuration setting, to allow deploying Git branches named like "env/production" as Puppet environments named like "production". [#1128](https://github.com/puppetlabs/r10k/pull/1128) - A warning will be emitted when the user supplies conflicting arguments to module definitions in a Puppetfile, such as when specifying both :commit and :branch [#1130](https://github.com/puppetlabs/r10k/pull/1130) - Add optional standard module and environment specification interface: name, type, source, version. These options can be used when specifying environments and/or modules in a yaml/exec source, as well as when specifying modules in a Puppetfile. Providing the standard interface simplifies integrations with external services [#1131](https://github.com/puppetlabs/r10k/pull/1131) diff --git a/doc/dynamic-environments/configuration.mkd b/doc/dynamic-environments/configuration.mkd index 48c5a0a3a..50a24e60a 100644 --- a/doc/dynamic-environments/configuration.mkd +++ b/doc/dynamic-environments/configuration.mkd @@ -266,31 +266,31 @@ managed by a Puppetfile include the configured `moduledir` (which defaults to "modules") as well as alternate directories specified as an `install_path` option to any Puppetfile content declarations. -#### purge\_whitelist +#### purge\_allowlist -The `purge_whitelist` setting exempts the specified filename patterns from +The `purge_allowlist` setting exempts the specified filename patterns from being purged. This setting is currently only considered during `environment` level purging. (See above.) Given value must be a list of shell style filename patterns in string format. See the Ruby [documentation for the `fnmatch` method](http://ruby-doc.org/core-2.2.0/File.html#method-c-fnmatch) for more details on valid patterns. Note that the `FNM_PATHNAME` and -`FNM_DOTMATCH` flags are in effect when r10k considers the whitelist. +`FNM_DOTMATCH` flags are in effect when r10k considers the allowlist. Patterns are relative to the root of the environment being purged and *do -not match recursively* by default. For example, a whitelist value of +not match recursively* by default. For example, a allowlist value of `*myfile*` would only preserve a matching file at the root of the environment. To preserve the file throughout the deployed environment, a recursive pattern such as `**/*myfile*` would be required. -Files matching a whitelist pattern may still be removed if they exist in +Files matching a allowlist pattern may still be removed if they exist in a folder that is otherwise subject to purging. In this case, an additional -whitelist rule to preserve the containing folder is required. +allowlist rule to preserve the containing folder is required. ```yaml --- deploy: - purge_whitelist: [ 'custom.json', '**/*.xpp' ] + purge_allowlist: [ 'custom.json', '**/*.xpp' ] ``` diff --git a/lib/r10k/action/deploy/environment.rb b/lib/r10k/action/deploy/environment.rb index bbe3baeeb..0c7039b91 100644 --- a/lib/r10k/action/deploy/environment.rb +++ b/lib/r10k/action/deploy/environment.rb @@ -18,7 +18,8 @@ class Environment < R10K::Action::Base def initialize(opts, argv, settings = nil) settings ||= {} @purge_levels = settings.fetch(:deploy, {}).fetch(:purge_levels, []) - @user_purge_whitelist = settings.fetch(:deploy, {}).fetch(:purge_whitelist, []) + @user_purge_allowlist = read_purge_allowlist(settings.fetch(:deploy, {}).fetch(:purge_whitelist, []), + settings.fetch(:deploy, {}).fetch(:purge_allowlist, [])) @generate_types = settings.fetch(:deploy, {}).fetch(:generate_types, false) super @@ -43,6 +44,23 @@ def call private + def read_purge_allowlist (whitelist, allowlist) + whitelist_has_content = !whitelist.empty? + allowlist_has_content = !allowlist.empty? + case + when whitelist_has_content == false && allowlist_has_content == false + [] + when whitelist_has_content && allowlist_has_content + raise R10K::Error.new "Values found for both purge_whitelist and purge_allowlist. Setting " << + "purge_whitelist is deprecated, please only use purge_allowlist." + when allowlist_has_content + allowlist + else + logger.warn "Setting purge_whitelist is deprecated; please use purge_allowlist instead." + whitelist + end + end + def visit_deployment(deployment) # Ensure that everything can be preloaded. If we cannot preload all # sources then we can't fully enumerate all environments which @@ -110,7 +128,7 @@ def visit_environment(environment) if @purge_levels.include?(:environment) if @visit_ok logger.debug("Purging unmanaged content for environment '#{environment.dirname}'...") - environment.purge!(:recurse => true, :whitelist => environment.whitelist(@user_purge_whitelist)) + environment.purge!(:recurse => true, :whitelist => environment.whitelist(@user_purge_allowlist)) else logger.debug("Not purging unmanaged content for environment '#{environment.dirname}' due to prior deploy failures.") end diff --git a/lib/r10k/settings.rb b/lib/r10k/settings.rb index 579b74470..e7c622e0a 100644 --- a/lib/r10k/settings.rb +++ b/lib/r10k/settings.rb @@ -122,11 +122,16 @@ def self.deploy_settings end, }), - Definition.new(:purge_whitelist, { + Definition.new(:purge_allowlist, { :desc => "A list of filename patterns to be excluded from any purge operations. Patterns are matched relative to the root of each deployed environment, if you want a pattern to match recursively you need to use the '**' glob in your pattern. Basic shell style globs are supported.", :default => [], }), + Definition.new(:purge_whitelist, { + :desc => "Deprecated; please use purge_allowlist instead. This setting will be removed in a future version.", + :default => [], + }), + Definition.new(:generate_types, { :desc => "Controls whether to generate puppet types after deploying an environment. Defaults to false.", :default => false, diff --git a/spec/unit/action/deploy/environment_spec.rb b/spec/unit/action/deploy/environment_spec.rb index 39695bb22..24313e136 100644 --- a/spec/unit/action/deploy/environment_spec.rb +++ b/spec/unit/action/deploy/environment_spec.rb @@ -42,6 +42,17 @@ it 'can accept a token option' do described_class.new({ 'oauth-token': '/nonexistent' }, []) end + + describe "initializing errors" do + let (:settings) { { deploy: { purge_levels: [:environment], + purge_whitelist: ['coolfile', 'coolfile2'], + purge_allowlist: ['anothercoolfile']}}} + + subject { described_class.new({config: "/some/nonexistent/path"}, [], settings)} + it 'errors out when both purge_whitelist and purge_allowlist are set' do + expect{subject}.to raise_error(R10K::Error, /Values found for both purge_whitelist and purge_allowlist./) + end + end end describe "when called" do @@ -175,6 +186,37 @@ end + describe "Purging white/allowlist" do + + let(:settings) { { deploy: { purge_levels: [:environment], purge_allowlist: ['coolfile', 'coolfile2'] } } } + + let(:deployment) do + R10K::Deployment.new(mock_config.merge(settings)) + end + + before do + expect(R10K::Deployment).to receive(:new).and_return(deployment) + end + + subject { described_class.new({ config: "/some/nonexistent/path", puppetfile: true }, %w[PREFIX_first], settings) } + + it "reads in the purge_allowlist setting and purges accordingly" do + expect(subject.logger).to receive(:debug).with(/purging unmanaged content for environment/i) + expect(subject.instance_variable_get(:@user_purge_allowlist)).to eq(['coolfile', 'coolfile2']) + subject.call + end + + describe "purge_whitelist" do + let (:settings) { { deploy: { purge_levels: [:environment], purge_whitelist: ['coolfile', 'coolfile2'] } } } + + it "reads in the purge_whitelist setting and still sets it to purge_allowlist and purges accordingly" do + expect(subject.logger).to receive(:debug).with(/purging unmanaged content for environment/i) + expect(subject.instance_variable_get(:@user_purge_allowlist)).to eq(['coolfile', 'coolfile2']) + subject.call + end + end + end + describe "purge_levels" do let(:settings) { { deploy: { purge_levels: purge_levels } } }