[Disable Sudo] Support Update of DisableSudoAccessForDefaultUser Conf…

…ig Option (aws#2616) * [Disable Sudo]Adding logs in sudo_access resource * [Disable Sudo]Allowing sudo_access resource to update * [Disable Sudo]Update tests for Sudo_access resource * [Code Quality] Ignore bandit rules B038 --------- Co-authored-by: Himani Deshpande <[email protected]>
hgreebe · Jan 16, 2024 · 517e55d · 517e55d
1 parent 4886bfc
commit 517e55d
Show file tree

Hide file tree

Showing 7 changed files with 25 additions and 8 deletions.
diff --git a/cookbooks/aws-parallelcluster-entrypoints/recipes/update.rb b/cookbooks/aws-parallelcluster-entrypoints/recipes/update.rb
@@ -31,3 +31,5 @@
 if is_custom_node?
   include_recipe 'aws-parallelcluster-computefleet::update_parallelcluster_node'
 end
+
+sudo_access "Update Sudo Access" if node['cluster']['scheduler'] == 'slurm'
diff --git a/cookbooks/aws-parallelcluster-platform/resources/sudo_access/partial/_sudo_access_common.rb b/cookbooks/aws-parallelcluster-platform/resources/sudo_access/partial/_sudo_access_common.rb
@@ -15,13 +15,14 @@
 unified_mode true
 default_action :setup
 
-property :sudo_access, String
+property :user_name, String, default: node['cluster']['cluster_user']
 
 action :setup do
   node['cluster']['disable_sudo_access_for_default_user'] == 'true' ? action_disable : action_enable
 end
 
 action :enable do
+  Chef::Log.info("Enabling Sudo Access for #{new_resource.user_name}")
   # Enable sudo access for default user
   template '/etc/sudoers.d/99-parallelcluster-revoke-sudo-access' do
     only_if { ::File.exist? "/etc/sudoers.d/99-parallelcluster-revoke-sudo-access" }
@@ -32,9 +33,10 @@
 end
 
 action :disable do
-  replace_or_add "Disable Sudo Access for #{node['cluster']['cluster_user']}" do
+  Chef::Log.info("Disabling Sudo Access for #{new_resource.user_name}")
+  replace_or_add "Disable Sudo Access for #{new_resource.user_name}" do
     path "/etc/sudoers"
-    pattern "^#{node['cluster']['cluster_user']}*"
+    pattern "^#{new_resource.user_name}*"
     line ""
     remove_duplicates true
     replace_only true
@@ -47,6 +49,9 @@
     owner 'root'
     group 'root'
     mode '0600'
+    variables(
+      user_name: new_resource.user_name
+    )
     action :create
   end
 end
diff --git a/cookbooks/aws-parallelcluster-platform/spec/unit/resources/sudo_access_spec.rb b/cookbooks/aws-parallelcluster-platform/spec/unit/resources/sudo_access_spec.rb
@@ -13,7 +13,7 @@ def self.setup(chef_run)
 describe 'sudo_access:setup' do
   for_all_oses do |platform, version|
     context "on #{platform}#{version}" do
-      cached(:default_user) { 'ec2-user' }
+      cached(:default_user) { 'ubuntu' }
       let(:chef_run) do
         runner(platform: platform, version: version, step_into: ['sudo_access']) do |node|
           node.override['cluster']['cluster_user'] = default_user
@@ -34,6 +34,16 @@ def self.setup(chef_run)
             remove_duplicates: true,
             replace_only: true
           )
+          is_expected.to create_template("/etc/sudoers.d/99-parallelcluster-revoke-sudo-access").with(
+            source: 'sudo_access/99-parallelcluster-revoke-sudo.erb',
+            cookbook: 'aws-parallelcluster-platform',
+            user: 'root',
+            group: 'root',
+            mode: '0600',
+            variables: {
+              user_name: default_user,
+            }
+          )
         end
       end
 

diff --git a/...oks/aws-parallelcluster-platform/templates/sudo_access/99-parallelcluster-revoke-sudo.erb b/...oks/aws-parallelcluster-platform/templates/sudo_access/99-parallelcluster-revoke-sudo.erb
@@ -1 +1 @@
-<%= node['cluster']['cluster_user'] %> ALL=(ALL) !ALL
+<%= @user_name %> ALL=(ALL) !ALL
diff --git a/cookbooks/aws-parallelcluster-platform/test/controls/sudo_access_spec.rb b/cookbooks/aws-parallelcluster-platform/test/controls/sudo_access_spec.rb
@@ -42,7 +42,7 @@
 
   describe file('/etc/sudoers.d/90-cloud-init-users') do
     it { should exist }
-    its('content') { should match /^[\-#\.,\:\+\w\s]*(rocky ALL=\(ALL\) NOPASSWD:ALL)/ }
+    its('content') { should match /^[\-#\.,\:\+\w\s]*(#{node['cluster']['cluster_user']} ALL=\(ALL\) NOPASSWD:ALL)/ }
   end unless os_properties.on_docker?
 
   describe bash("sudo -l -U #{node['cluster']['cluster_user']} | tail -1 | awk '{$1=$1};1'") do

diff --git a/test/unit/clusterstatusmgtd/test_clusterstatusmgtd.py b/test/unit/clusterstatusmgtd/test_clusterstatusmgtd.py
@@ -228,7 +228,7 @@ def test_config_parsing(self, config_file, expected_attributes, test_datadir):
         """Test config_parsing method."""
         sync_config = ClusterstatusmgtdConfig(test_datadir / config_file)
         for key in expected_attributes:
-            assert_that(sync_config.__dict__.get(key)).is_equal_to(expected_attributes.get(key))
+            assert_that(sync_config.__dict__.get(key)).is_equal_to(expected_attributes.get(key))  # noqa: B038
 
     def test_config_comparison(self, test_datadir):
         """Test configs comparison."""

diff --git a/test/unit/health_check/test_health_check_manager.py b/test/unit/health_check/test_health_check_manager.py
@@ -88,7 +88,7 @@ def test_config_parsing(self, config_file, expected_attributes, test_datadir):
         """Test config_parsing method."""
         sync_config = HealthCheckManagerConfig(test_datadir / config_file)
         for key in expected_attributes:
-            assert_that(sync_config.__dict__.get(key)).is_equal_to(expected_attributes.get(key))
+            assert_that(sync_config.__dict__.get(key)).is_equal_to(expected_attributes.get(key))  # noqa: B038
 
     def test_config_comparison(self, test_datadir):
         """Test configs comparison."""
Original file line number	Diff line number	Diff line change
		@@ -1 +1 @@
		<%= node['cluster']['cluster_user'] %> ALL=(ALL) !ALL
		<%= @user_name %> ALL=(ALL) !ALL