[Disable Sudo] Supporting DisableSudoAccessForDefaultUser config Opti…

…on using Cookbook (#6004) * [Disable Sudo] Removing DisableSudoAccessForDefaultUser from cloud-config in user-data.sh * [Disable Sudo] Supporting DisableSudoAccessForDefaultUser config Option using Cookbook Recipies * [Disable Sudo] Update Unit Tests for checking value of DisableSudoAccessForDefaultUser in dna.json * [Disable Sudo] Update Unit Tests for checking value of DisableSudoAccessForDefaultUser in dna.json --------- Co-authored-by: Himani Deshpande <[email protected]>
aws · Jan 12, 2024 · 70ebb8c · 70ebb8c
1 parent a89a70b
commit 70ebb8c
Show file tree

Hide file tree

Showing 18 changed files with 19 additions and 99 deletions.
diff --git a/cli/src/pcluster/constants.py b/cli/src/pcluster/constants.py
@@ -339,6 +339,3 @@ class Operation(Enum):
 IAM_ROLE_REGEX = "^arn:.*:role/"
 IAM_INSTANCE_PROFILE_REGEX = "^arn:.*:instance-profile/"
 IAM_POLICY_REGEX = "^arn:.*:policy/"
-
-# Section of Cloud-Init(cloud-config) which will be added for Disabling the Sudo access of Default user
-DISABLE_SUDO_ACCESS_FOR_DEFAULT_USER_CONFIG = {"system_info": {"default_user": {"sudo": ["ALL=(ALL) !ALL"]}}}
diff --git a/cli/src/pcluster/resources/compute_node/user_data.sh b/cli/src/pcluster/resources/compute_node/user_data.sh
@@ -35,10 +35,6 @@ export NO_PROXY="localhost,127.0.0.1,169.254.169.254"
 PROXY
 fi
 
-if [ "${DisableSudoAccessForDefault}" == "true" ]; then
-  sed -n -i "/"${OSUser}" ALL=(ALL) NOPASSWD:ALL/d" /etc/sudoers
-fi
-
 --==BOUNDARY==
 Content-Type: text/cloud-config; charset=us-ascii
 MIME-Version: 1.0
@@ -60,8 +56,6 @@ repo_upgrade: none
 
 datasource_list: [ Ec2, None ]
 
-${DisableSudoAccessForDefaultUserConfig}
-
 output:
   all: "| tee -a /var/log/cloud-init-output.log | logger -t user-data -s 2>/dev/console"
 write_files:
@@ -112,7 +106,8 @@ write_files:
           "head_node_private_ip": "${HeadNodePrivateIp}",
           "directory_service": {
             "enabled": "${DirectoryServiceEnabled}"
-          }
+          },
+          "disable_sudo_access_for_default_user":"${DisableSudoAccessForDefault}"
         }
       }
   - path: /etc/chef/client.rb

diff --git a/cli/src/pcluster/resources/head_node/user_data.sh b/cli/src/pcluster/resources/head_node/user_data.sh
@@ -35,10 +35,6 @@ export NO_PROXY="localhost,127.0.0.1,169.254.169.254"
 PROXY
 fi
 
-if [ "${DisableSudoAccessForDefault}" == "true" ]; then
-  sed -n -i "/"${OSUser}" ALL=(ALL) NOPASSWD:ALL/d" /etc/sudoers
-fi
-
 --==BOUNDARY==
 Content-Type: text/cloud-config; charset=us-ascii
 MIME-Version: 1.0
@@ -60,8 +56,6 @@ repo_upgrade: none
 
 datasource_list: [ Ec2, None ]
 
-${DisableSudoAccessForDefaultUserConfig}
-
 --==BOUNDARY==
 Content-Type: text/x-shellscript; charset="us-ascii"
 MIME-Version: 1.0

diff --git a/cli/src/pcluster/resources/login_node/user_data.sh b/cli/src/pcluster/resources/login_node/user_data.sh
@@ -35,10 +35,6 @@ export NO_PROXY="localhost,127.0.0.1,169.254.169.254"
 PROXY
 fi
 
-if [ "${DisableSudoAccessForDefault}" == "true" ]; then
-  sed -n -i "/"${OSUser}" ALL=(ALL) NOPASSWD:ALL/d" /etc/sudoers
-fi
-
 --==BOUNDARY==
 Content-Type: text/cloud-config; charset=us-ascii
 MIME-Version: 1.0
@@ -49,8 +45,6 @@ repo_upgrade: none
 
 datasource_list: [ Ec2, None ]
 
-${DisableSudoAccessForDefaultUserConfig}
-
 output:
   all: "| tee -a /var/log/cloud-init-output.log | logger -t user-data -s 2>/dev/console"
 write_files:
@@ -98,7 +92,8 @@ write_files:
           "scheduler": "${Scheduler}",
           "stack_name": "${AWS::StackName}",
           "stack_arn": "${AWS::StackId}",
-          "use_private_hostname": "${UsePrivateHostname}"
+          "use_private_hostname": "${UsePrivateHostname}",
+          "disable_sudo_access_for_default_user":"${DisableSudoAccessForDefault}"
         }
       }
   - path: /etc/chef/client.rb

diff --git a/cli/src/pcluster/templates/cdk_builder_utils.py b/cli/src/pcluster/templates/cdk_builder_utils.py
@@ -15,7 +15,6 @@
 from typing import List, Union
 
 import pkg_resources
-import yaml
 from aws_cdk import aws_ec2 as ec2
 from aws_cdk import aws_iam as iam
 from aws_cdk import aws_lambda as awslambda
@@ -37,7 +36,6 @@
 from pcluster.constants import (
     COOKBOOK_PACKAGES_VERSIONS,
     CW_LOGS_RETENTION_DAYS_DEFAULT,
-    DISABLE_SUDO_ACCESS_FOR_DEFAULT_USER_CONFIG,
     IAM_ROLE_PATH,
     LAMBDA_VPC_ACCESS_MANAGED_POLICY,
     PCLUSTER_CLUSTER_NAME_TAG,
@@ -132,11 +130,6 @@ def get_directory_service_dna_json_for_head_node(config: BaseClusterConfig) -> d
     )
 
 
-def get_cloud_config_for_default_user(disable_sudo_access_default_user: bool):
-    """Return Cloud-Init Section (cloud-config) in YAML format if DisableSudoAccessForDefaultUser is True."""
-    return yaml.dump(DISABLE_SUDO_ACCESS_FOR_DEFAULT_USER_CONFIG) if disable_sudo_access_default_user else ""
-
-
 def to_comma_separated_string(list, use_lower_case=False):
     result = ",".join(str(item) for item in list)
     if use_lower_case:

diff --git a/cli/src/pcluster/templates/cluster_stack.py b/cli/src/pcluster/templates/cluster_stack.py
@@ -86,7 +86,6 @@
     apply_permissions_boundary,
     convert_deletion_policy,
     create_hash_suffix,
-    get_cloud_config_for_default_user,
     get_cloud_watch_logs_policy_statement,
     get_cloud_watch_logs_retention_days,
     get_common_user_data_env,
@@ -1214,13 +1213,6 @@ def _add_head_node(self):
                                 if head_node.disable_simultaneous_multithreading_manually
                                 else "false",
                                 "CloudFormationUrl": cloudformation_url,
-                                "DisableSudoAccessForDefaultUserConfig": get_cloud_config_for_default_user(
-                                    self.config.disable_sudo_access_default_user
-                                ),
-                                "OSUser": OS_MAPPING[self.config.image.os]["user"],
-                                "DisableSudoAccessForDefault": "true"
-                                if self.config.disable_sudo_access_default_user
-                                else "false",
                             },
                             **get_common_user_data_env(head_node, self.config),
                         },
@@ -1320,6 +1312,9 @@ def _add_head_node(self):
                     "install_intel_python": str(
                         get_attr(self.config, "additional_packages.intel_software.python")
                     ).lower(),
+                    "disable_sudo_access_for_default_user": "true"
+                    if self.config.disable_sudo_access_default_user
+                    else "false",
                     **(
                         get_slurm_specific_dna_json_for_head_node(self.config, self.scheduler_resources)
                         if self._condition_is_slurm()

diff --git a/cli/src/pcluster/templates/login_nodes_stack.py b/cli/src/pcluster/templates/login_nodes_stack.py
@@ -18,7 +18,6 @@
     CdkLaunchTemplateBuilder,
     LoginNodesIamResources,
     _get_resource_combination_name,
-    get_cloud_config_for_default_user,
     get_common_user_data_env,
     get_custom_tags,
     get_default_instance_tags,
@@ -208,9 +207,6 @@ def _add_login_nodes_pool_launch_template(self):
                                 "LaunchingLifecycleHookName": (
                                     f"{self._login_nodes_stack_id}-LoginNodesLaunchingLifecycleHook"
                                 ),
-                                "DisableSudoAccessForDefaultUserConfig": get_cloud_config_for_default_user(
-                                    self._config.disable_sudo_access_default_user
-                                ),
                                 "DisableSudoAccessForDefault": "true"
                                 if self._config.disable_sudo_access_default_user
                                 else "false",

diff --git a/cli/src/pcluster/templates/queues_stack.py b/cli/src/pcluster/templates/queues_stack.py
@@ -19,7 +19,6 @@
     ComputeNodeIamResources,
     create_hash_suffix,
     dict_to_cfn_tags,
-    get_cloud_config_for_default_user,
     get_common_user_data_env,
     get_custom_tags,
     get_default_instance_tags,
@@ -305,9 +304,6 @@ def _add_compute_resource_launch_template(
                                         default=False,
                                     )
                                 ),
-                                "DisableSudoAccessForDefaultUserConfig": get_cloud_config_for_default_user(
-                                    self._config.disable_sudo_access_default_user
-                                ),
                                 "DisableSudoAccessForDefault": "true"
                                 if self._config.disable_sudo_access_default_user
                                 else "false",

diff --git a/cli/tests/pcluster/templates/test_cluster_stack.py b/cli/tests/pcluster/templates/test_cluster_stack.py
@@ -362,7 +362,7 @@ def get_generated_template_and_cdk_assets(
 
 
 @pytest.mark.parametrize(
-    "config_file_name, lt_assertions, disable_sudo_access_default_user, expected_cloud_config_for_default_user",
+    "config_file_name, lt_assertions",
     [
         (
             "cluster-using-flexible-instance-types.yaml",
@@ -371,8 +371,6 @@ def get_generated_template_and_cdk_assets(
                 InstanceTypeLTAssertion(has_instance_type=False),
                 EbsLTAssertion(includes_ebs_optimized=False, is_ebs_optimized=None),
             ],
-            True,
-            "system_info:\n  default_user:\n    sudo:\n    - ALL=(ALL) !ALL\n",
         ),
         (
             "cluster-using-single-instance-type.yaml",
@@ -381,23 +379,18 @@ def get_generated_template_and_cdk_assets(
                 InstanceTypeLTAssertion(has_instance_type=True),
                 EbsLTAssertion(includes_ebs_optimized=True, is_ebs_optimized=True),
             ],
-            False,
-            "",
         ),
     ],
 )
 def test_compute_launch_template_properties(
     mocker,
     config_file_name,
     lt_assertions,
-    disable_sudo_access_default_user,
-    expected_cloud_config_for_default_user,
     pcluster_config_reader,
     test_datadir,
 ):
     rendered_config_file = pcluster_config_reader(
         config_file_name,
-        disable_sudo_access_default_user=disable_sudo_access_default_user,
     )
     generated_template, cdk_assets = get_generated_template_and_cdk_assets(
         mocker,
@@ -409,12 +402,6 @@ def test_compute_launch_template_properties(
 
     for lt_assertion in lt_assertions:
         lt_assertion.assert_lt_properties(asset_content, launch_template_logical_id)
-    # Checking for DisableSudoAccessForDefaultUser
-    assert_that(
-        asset_content["Resources"][launch_template_logical_id]["Properties"]["LaunchTemplateData"]["UserData"][
-            "Fn::Base64"
-        ]["Fn::Sub"][1]["DisableSudoAccessForDefaultUserConfig"]
-    ).is_equal_to(expected_cloud_config_for_default_user)
 
 
 class LoginNodeLTAssertion:
@@ -453,7 +440,7 @@ def assert_lt_properties(self, generated_template, resource_type):
 
 
 @pytest.mark.parametrize(
-    "config_file_name, lt_assertions, disable_sudo_access_default_user, expected_cloud_config_for_default_user",
+    "config_file_name, lt_assertions",
     [
         (
             "test-login-nodes-stack.yaml",
@@ -468,8 +455,6 @@ def assert_lt_properties(self, generated_template, resource_type):
                 NetworkInterfaceLTAssertion(no_of_network_interfaces=3, subnet_id="subnet-12345678"),
                 InstanceTypeLTAssertion(has_instance_type=True),
             ],
-            True,
-            "system_info:\n  default_user:\n    sudo:\n    - ALL=(ALL) !ALL\n",
         ),
         (
             "test-login-nodes-stack-without-ssh.yaml",
@@ -484,8 +469,6 @@ def assert_lt_properties(self, generated_template, resource_type):
                 NetworkInterfaceLTAssertion(no_of_network_interfaces=3, subnet_id="subnet-12345678"),
                 InstanceTypeLTAssertion(has_instance_type=True),
             ],
-            False,
-            "",
         ),
         (
             "test-login-nodes-stack-with-custom-tags.yaml",
@@ -504,23 +487,18 @@ def assert_lt_properties(self, generated_template, resource_type):
                 NetworkInterfaceLTAssertion(no_of_network_interfaces=3, subnet_id="subnet-12345678"),
                 InstanceTypeLTAssertion(has_instance_type=True),
             ],
-            True,
-            "system_info:\n  default_user:\n    sudo:\n    - ALL=(ALL) !ALL\n",
         ),
     ],
 )
 def test_login_nodes_launch_template_properties(
     mocker,
     config_file_name,
     lt_assertions,
-    disable_sudo_access_default_user,
-    expected_cloud_config_for_default_user,
     pcluster_config_reader,
     test_datadir,
 ):
     rendered_config_file = pcluster_config_reader(
         config_file_name,
-        disable_sudo_access_default_user=disable_sudo_access_default_user,
     )
     generated_template, cdk_assets = get_generated_template_and_cdk_assets(
         mocker,
@@ -532,13 +510,6 @@ def test_login_nodes_launch_template_properties(
     for lt_assertion in lt_assertions:
         lt_assertion.assert_lt_properties(asset_content, launch_template_logical_id)
 
-    # Checking for DisableSudoAccessForDefaultUser
-    assert_that(
-        asset_content["Resources"][launch_template_logical_id]["Properties"]["LaunchTemplateData"]["UserData"][
-            "Fn::Base64"
-        ]["Fn::Sub"][1]["DisableSudoAccessForDefaultUserConfig"]
-    ).is_equal_to(expected_cloud_config_for_default_user)
-
 
 class AutoScalingGroupAssertion:
     def __init__(self, min_size: int, max_size: int, desired_capacity: int, expected_lifecycle_specification: List):
@@ -765,7 +736,10 @@ def test_login_nodes_traffic_management_resources_values_properties(
 @pytest.mark.parametrize(
     "config_file_name, expected_head_node_dna_json_fields",
     [
-        ("slurm-imds-secured-true.yaml", {"scheduler": "slurm", "head_node_imds_secured": "true"}),
+        (
+            "slurm-imds-secured-true.yaml",
+            {"scheduler": "slurm", "head_node_imds_secured": "true", "disable_sudo_access_for_default_user": "true"},
+        ),
         (
             "slurm-imds-secured-false.yaml",
             {"scheduler": "slurm", "head_node_imds_secured": "false", "compute_node_bootstrap_timeout": 1000},
@@ -838,13 +812,15 @@ def test_head_node_dna_json(mocker, test_datadir, config_file_name, expected_hea
                 "node_type": "LoginNode",
                 '"proxy"': "NONE",
                 "scheduler": "slurm",
+                "disable_sudo_access_for_default_user": "true",
             },
         ),
         (
             "login-with-directory-service.yaml",
             {
                 "domain_read_only_user": "cn=ReadOnlyUser,ou=Users,ou=CORP,dc=corp,dc=sirena,dc=com",
                 "generate_ssh_keys_for_users": "true",
+                "disable_sudo_access_for_default_user": "false",
             },
         ),
     ],
@@ -910,8 +886,7 @@ def _get_cfn_init_file_content(template, resource, file):
 
 @freeze_time("2021-01-01T01:01:01")
 @pytest.mark.parametrize(
-    "config_file_name, expected_instance_tags, expected_volume_tags,"
-    "disable_sudo_access_default_user, expected_cloud_config_for_default_user,",
+    "config_file_name, expected_instance_tags, expected_volume_tags,",
     [
         (
             "slurm.full.yaml",
@@ -925,8 +900,6 @@ def _get_cfn_init_file_content(template, resource, file):
                 "String": "String",
                 "two": "two22",
             },
-            True,
-            "system_info:\n  default_user:\n    sudo:\n    - ALL=(ALL) !ALL\n",
         ),
         (
             "awsbatch.full.yaml",
@@ -940,8 +913,6 @@ def _get_cfn_init_file_content(template, resource, file):
                 "String": "String",
                 "two": "two22",
             },
-            True,
-            "",
         ),
     ],
 )
@@ -950,8 +921,6 @@ def test_head_node_tags_from_launch_template(
     config_file_name,
     expected_instance_tags,
     expected_volume_tags,
-    disable_sudo_access_default_user,
-    expected_cloud_config_for_default_user,
 ):
     mock_aws_api(mocker)
     mock_bucket(mocker)
@@ -980,13 +949,6 @@ def test_head_node_tags_from_launch_template(
     actual_volume_tags = {tag["Key"]: tag["Value"] for tag in volume_tags}
     assert_that(actual_volume_tags).is_equal_to(expected_volume_tags)
 
-    # Checking for DisableSudoAccessForDefaultUser
-    assert_that(
-        generated_template["Resources"]["HeadNodeLaunchTemplate"]["Properties"]["LaunchTemplateData"]["UserData"][
-            "Fn::Base64"
-        ]["Fn::Sub"][1]["DisableSudoAccessForDefaultUserConfig"]
-    ).is_equal_to(expected_cloud_config_for_default_user)
-
 
 @freeze_time("2021-01-01T01:01:01")
 @pytest.mark.parametrize(

diff --git a/..._stack/test_compute_launch_template_properties/cluster-using-flexible-instance-types.yaml b/..._stack/test_compute_launch_template_properties/cluster-using-flexible-instance-types.yaml
@@ -23,4 +23,3 @@ Scheduling:
     Networking:
       SubnetIds:
       - subnet-12345678
-DisableSudoAccessForDefaultUser: {{ disable_sudo_access_default_user }}
diff --git a/...ter_stack/test_compute_launch_template_properties/cluster-using-single-instance-type.yaml b/...ter_stack/test_compute_launch_template_properties/cluster-using-single-instance-type.yaml
@@ -21,4 +21,3 @@ Scheduling:
     Networking:
       SubnetIds:
       - subnet-12345678
-DisableSudoAccessForDefaultUser: {{ disable_sudo_access_default_user }}
diff --git a/.../pcluster/templates/test_cluster_stack/test_head_node_dna_json/head_node_default.dna.json b/.../pcluster/templates/test_cluster_stack/test_head_node_dna_json/head_node_default.dna.json
@@ -13,6 +13,7 @@
     "dcv_enabled": "false",
     "dcv_port": "NONE",
     "ddb_table": "NONE",
+    "disable_sudo_access_for_default_user": "false",
     "ebs_shared_dirs": "",
     "efs_encryption_in_transits": "",
     "efs_fs_ids": "",