[JEX-687] Update sensitive property #200

jaymalasinha · 2017-09-25T16:23:04Z

Signed-off-by: Jaymala Sinha [email protected]

Description

sensitive property set at top level resources will be passed down to resources

Issues Resolved

#198

Check List

All tests pass. See https://github.com/chef-cookbooks/community_cookbook_documentation/blob/master/TESTING.MD
New functionality includes testing.
New functionality has been documented in the README if applicable
All commits have been signed for the Developer Certificate of Origin. See https://github.com/chef-cookbooks/community_cookbook_documentation/blob/master/CONTRIBUTING.MD

wrightp

@jaymalasinha, let's pair on this. A lot of these should not be masked by sensitive properties and there are several occurrences of noops.

wrightp · 2017-09-25T19:58:30Z

libraries/omnitruck_handler.rb

@@ -57,6 +57,7 @@ def configure_version(installer)
        end
      else
        execute install_command_resource do
+          sensitive new_resource.sensitive if new_resource.sensitive


Not needed here

wrightp · 2017-09-25T20:00:06Z

resources/automate.rb

@@ -109,6 +114,7 @@
  end

  file '/var/opt/delivery/nginx/etc/addon.d/99-installer_internal.conf' do
+    sensitive new_resource.sensitive if new_resource.sensitive


Not needed here

wrightp · 2017-09-25T20:00:12Z

resources/automate.rb

@@ -122,6 +128,7 @@

  Array(new_resource.enterprise).each do |ent|
    execute "create enterprise #{ent}" do
+      sensitive new_resource.sensitive if new_resource.sensitive


Not needed here

wrightp · 2017-09-25T20:01:39Z

resources/backend.rb

@@ -72,6 +75,7 @@

  peers.each do |peer|
    begin
+      sensitive new_resource.sensitive if new_resource.sensitive


remove. this is not a chef resource

wrightp · 2017-09-25T20:01:46Z

resources/backend.rb

@@ -82,11 +86,13 @@
  Chef::Config['http_retry_count'] = http_retry_count

  execute 'chef-backend-ctl create-cluster --accept-license --yes' do
+    sensitive new_resource.sensitive if new_resource.sensitive


Not needed here

wrightp · 2017-09-25T20:08:14Z

resources/wf_builder.rb

@@ -66,6 +67,7 @@
    new_resource.supermarket_fqdn,
  ].each do |server|
    execute "fetch ssl cert for #{server}" do
+      sensitive new_resource.sensitive if new_resource.sensitive


Not needed here

wrightp · 2017-09-25T20:08:37Z

resources/wf_builder.rb

    owner 'root'
    group 'dbuild'
    mode '0640'
  end

  file new_resource.chef_config_path do
+    sensitive new_resource.sensitive if new_resource.sensitive


Not needed here

wrightp · 2017-09-25T20:08:48Z

resources/wf_builder.rb

    mode '0644'
  end

  Dir.glob('/etc/chef/trusted_certs/*').each do |fn|
    file fn do
+      sensitive new_resource.sensitive if new_resource.sensitive


Not needed here

wrightp · 2017-09-25T20:08:52Z

resources/wf_builder.rb

      mode '0644'
    end
  end

  case new_resource.job_dispatch_version
  when 'v1'
    execute 'tag node as legacy build-node' do
+      sensitive new_resource.sensitive if new_resource.sensitive


Not needed here

wrightp · 2017-09-25T20:09:08Z

resources/wf_builder.rb

@@ -217,6 +232,7 @@
    home_dir = '/home/job_runner'

    execute 'tag node as job-runner' do
+      sensitive new_resource.sensitive if new_resource.sensitive


Not needed here

jaymalasinha · 2017-09-26T14:53:44Z

@wrightp updated as per review comments.

wrightp

chef_server and chef_supermarket resources have not been updated with the necessary sensitive properties.

Please note the "remove" lines.We do not need to protect resources that do not write any private info.

wrightp · 2017-09-26T16:16:43Z

resources/client.rb

@@ -81,6 +81,7 @@
  end

  template 'client.rb' do
+    sensitive new_resource.sensitive if new_resource.sensitive


wrightp · 2017-09-26T16:19:23Z

resources/client.rb

@@ -143,6 +147,7 @@
  # end

  file 'delete validation.pem' do
+    sensitive new_resource.sensitive if new_resource.sensitive


I don't think this is required here because of the delete action. Doesn't hurt I suppose, but I don't think the log will display the deleted file's content.

wrightp · 2017-09-26T16:21:31Z

resources/wf_builder.rb

@@ -47,6 +47,7 @@

 action :create do
  chef_ingredient 'chefdk' do
+    sensitive new_resource.sensitive if new_resource.sensitive


wrightp · 2017-09-26T16:21:59Z

resources/wf_builder.rb

@@ -97,6 +99,7 @@
  group 'dbuild'

  user 'dbuild' do
+    sensitive new_resource.sensitive if new_resource.sensitive


wrightp · 2017-09-26T16:23:09Z

resources/wf_builder.rb

@@ -129,6 +134,7 @@

  %w(etc/delivery.rb .chef/knife.rb).each do |dir|
    file "#{workspace}/#{dir}" do
+      sensitive new_resource.sensitive if new_resource.sensitive


wrightp · 2017-09-26T16:23:47Z

resources/wf_builder.rb

@@ -204,6 +214,7 @@
    end

    remote_file init_file do
+      sensitive new_resource.sensitive if new_resource.sensitive


wrightp · 2017-09-26T16:24:07Z

resources/wf_builder.rb

@@ -238,6 +249,7 @@
    end

    file "#{home_dir}/.ssh/authorized_keys" do
+      sensitive new_resource.sensitive if new_resource.sensitive


wrightp · 2017-09-26T16:24:21Z

resources/wf_builder.rb

@@ -272,6 +284,7 @@
    end

    file ::File.join('/etc/sudoers.d', build_user) do
+      sensitive new_resource.sensitive if new_resource.sensitive


wrightp · 2017-09-26T16:24:32Z

resources/wf_builder.rb

@@ -287,12 +300,14 @@
    end

    file ::File.join(home_dir, '.ssh/authorized_keys') do


wrightp · 2017-09-26T16:24:44Z

resources/wf_builder.rb

@@ -287,12 +300,14 @@
    end

    file ::File.join(home_dir, '.ssh/authorized_keys') do
+      sensitive new_resource.sensitive if new_resource.sensitive
      owner build_user
      group build_user
      mode '0600'
    end

    file '/usr/local/bin/delivery-cmd' do


Signed-off-by: Jaymala Sinha <[email protected]> [JEX-687] Address review comments on sensitive property Signed-off-by: Jaymala Sinha <[email protected]> Updated with review comments Signed-off-by: Jaymala Sinha <[email protected]> add tests Signed-off-by: Patrick Wright <[email protected]> remove sensitive props Signed-off-by: Patrick Wright <[email protected]> Fix travis errors Signed-off-by: Jaymala Sinha <[email protected]>

wrightp suggested changes Sep 25, 2017

View reviewed changes

wrightp suggested changes Sep 26, 2017

View reviewed changes

jaymalasinha force-pushed the jsinha/JEX-687 branch from c15b862 to e80aa81 Compare September 29, 2017 17:59

wrightp approved these changes Oct 2, 2017

View reviewed changes

wrightp merged commit 9af1f6e into master Oct 2, 2017

tas50 deleted the jsinha/JEX-687 branch November 30, 2017 18:00

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[JEX-687] Update sensitive property #200

[JEX-687] Update sensitive property #200

jaymalasinha commented Sep 25, 2017 •

edited

Loading

wrightp left a comment

wrightp Sep 25, 2017

wrightp Sep 25, 2017

wrightp Sep 25, 2017

wrightp Sep 25, 2017

wrightp Sep 25, 2017

wrightp Sep 25, 2017

wrightp Sep 25, 2017

wrightp Sep 25, 2017

wrightp Sep 25, 2017

wrightp Sep 25, 2017

jaymalasinha commented Sep 26, 2017

wrightp left a comment

wrightp Sep 26, 2017

wrightp Sep 26, 2017

wrightp Sep 26, 2017

wrightp Sep 26, 2017

wrightp Sep 26, 2017

wrightp Sep 26, 2017

wrightp Sep 26, 2017

wrightp Sep 26, 2017

wrightp Sep 26, 2017

wrightp Sep 26, 2017

		@@ -287,12 +300,14 @@
		end

		file ::File.join(home_dir, '.ssh/authorized_keys') do

[JEX-687] Update sensitive property #200

[JEX-687] Update sensitive property #200

Conversation

jaymalasinha commented Sep 25, 2017 • edited Loading

Description

Issues Resolved

Check List

wrightp left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jaymalasinha commented Sep 26, 2017

wrightp left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jaymalasinha commented Sep 25, 2017 •

edited

Loading