From 69a7a9460bb94e9cd2ae961bba080bfcda4815a0 Mon Sep 17 00:00:00 2001 From: Tanmay Satam Date: Thu, 21 Sep 2023 12:51:02 -0400 Subject: [PATCH] Revert "Enable preconfigured NSG flag on azure-cli (#3112)" This reverts commit 118f1a8fbcb1f500a9509afa399241529dc37d69. --- python/az/aro/azext_aro/_client_factory.py | 2 +- .../az/aro/azext_aro/_dynamic_validators.py | 2 +- python/az/aro/azext_aro/_params.py | 2 - python/az/aro/azext_aro/commands.py | 2 +- python/az/aro/azext_aro/custom.py | 43 +++---------------- .../latest/unit/test_dynamic_validators.py | 21 +-------- python/az/aro/linter_exclusions.yml | 3 -- 7 files changed, 11 insertions(+), 64 deletions(-) diff --git a/python/az/aro/azext_aro/_client_factory.py b/python/az/aro/azext_aro/_client_factory.py index 7bd20999e26..1dda842fc26 100644 --- a/python/az/aro/azext_aro/_client_factory.py +++ b/python/az/aro/azext_aro/_client_factory.py @@ -4,7 +4,7 @@ import urllib3 from azext_aro.custom import rp_mode_development -from azext_aro.vendored_sdks.azure.mgmt.redhatopenshift.v2023_09_04 import AzureRedHatOpenShiftClient +from azext_aro.vendored_sdks.azure.mgmt.redhatopenshift.v2023_04_01 import AzureRedHatOpenShiftClient from azure.cli.core.commands.client_factory import get_mgmt_service_client diff --git a/python/az/aro/azext_aro/_dynamic_validators.py b/python/az/aro/azext_aro/_dynamic_validators.py index 3dda7b78404..c9dd364f2fc 100644 --- a/python/az/aro/azext_aro/_dynamic_validators.py +++ b/python/az/aro/azext_aro/_dynamic_validators.py @@ -184,7 +184,7 @@ def _validate_subnet(cmd, namespace): "Microsoft.Network/routeTables/read", "Microsoft.Network/routeTables/write"]) - if not namespace.enable_preconfigured_nsg and subnet_obj.get('networkSecurityGroup', None): + if subnet_obj.get('networkSecurityGroup', None): message = f"A Network Security Group \"{subnet_obj['networkSecurityGroup']['id']}\" "\ "is already assigned to this subnet. Ensure there are no Network "\ "Security Groups assigned to cluster subnets before cluster creation" diff --git a/python/az/aro/azext_aro/_params.py b/python/az/aro/azext_aro/_params.py index 94e355e9057..d548130fb27 100644 --- a/python/az/aro/azext_aro/_params.py +++ b/python/az/aro/azext_aro/_params.py @@ -68,8 +68,6 @@ def load_arguments(self, _): c.argument('outbound_type', help='Outbound type of cluster. Must be "Loadbalancer" (default) or "UserDefinedRouting".', validator=validate_outbound_type) - c.argument('enable_preconfigured_nsg', arg_type=get_three_state_flag(), - help='Use Preconfigured NSGs. Allowed values: false, true.') c.argument('disk_encryption_set', help='ResourceID of the DiskEncryptionSet to be used for master and worker VMs.', validator=validate_disk_encryption_set) diff --git a/python/az/aro/azext_aro/commands.py b/python/az/aro/azext_aro/commands.py index 0965220baa1..e11171dcace 100644 --- a/python/az/aro/azext_aro/commands.py +++ b/python/az/aro/azext_aro/commands.py @@ -11,7 +11,7 @@ def load_command_table(self, _): aro_sdk = CliCommandType( - operations_tmpl='azext_aro.vendored_sdks.azure.mgmt.redhatopenshift.v2023_09_04.operations#OpenShiftClustersOperations.{}', # pylint: disable=line-too-long + operations_tmpl='azext_aro.vendored_sdks.azure.mgmt.redhatopenshift.v2023_04_01.operations#OpenShiftClustersOperations.{}', # pylint: disable=line-too-long client_factory=cf_aro) with self.command_group('aro', aro_sdk, client_factory=cf_aro) as g: diff --git a/python/az/aro/azext_aro/custom.py b/python/az/aro/azext_aro/custom.py index a2ad63e0b7e..6561d799a98 100644 --- a/python/az/aro/azext_aro/custom.py +++ b/python/az/aro/azext_aro/custom.py @@ -7,7 +7,7 @@ from base64 import b64decode import textwrap -import azext_aro.vendored_sdks.azure.mgmt.redhatopenshift.v2023_09_04.models as openshiftcluster +import azext_aro.vendored_sdks.azure.mgmt.redhatopenshift.v2023_04_01.models as openshiftcluster from azure.cli.command_modules.role import GraphError from azure.cli.core.commands.client_factory import get_mgmt_service_client @@ -49,7 +49,6 @@ def aro_create(cmd, # pylint: disable=too-many-locals worker_subnet, vnet=None, # pylint: disable=unused-argument vnet_resource_group_name=None, # pylint: disable=unused-argument - enable_preconfigured_nsg=None, location=None, pull_secret=None, domain=None, @@ -89,7 +88,6 @@ def aro_create(cmd, # pylint: disable=too-many-locals master_subnet, worker_subnet, vnet=vnet, - enable_preconfigured_nsg=enable_preconfigured_nsg, cluster_resource_group=cluster_resource_group, client_id=client_id, client_secret=client_secret, @@ -147,7 +145,6 @@ def aro_create(cmd, # pylint: disable=too-many-locals pod_cidr=pod_cidr or '10.128.0.0/14', service_cidr=service_cidr or '172.30.0.0/16', outbound_type=outbound_type or '', - preconfigured_nsg='Enabled' if enable_preconfigured_nsg else 'Disabled', ), master_profile=openshiftcluster.MasterProfile( vm_size=master_vm_size or 'Standard_D8s_v3', @@ -193,7 +190,6 @@ def validate(cmd, # pylint: disable=too-many-locals,too-many-statements master_subnet, worker_subnet, vnet=None, - enable_preconfigured_nsg=None, cluster_resource_group=None, # pylint: disable=unused-argument client_id=None, client_secret=None, # pylint: disable=unused-argument @@ -206,10 +202,7 @@ def validate(cmd, # pylint: disable=too-many-locals,too-many-statements warnings_as_text=False): class mockoc: # pylint: disable=too-few-public-methods - def __init__(self, disk_encryption_id, master_subnet_id, worker_subnet_id, preconfigured_nsg): - self.network_profile = openshiftcluster.NetworkProfile( - preconfigured_nsg='Enabled' if preconfigured_nsg else 'Disabled' - ) + def __init__(self, disk_encryption_id, master_subnet_id, worker_subnet_id): self.master_profile = openshiftcluster.MasterProfile( subnet_id=master_subnet_id, disk_encryption_set_id=disk_encryption_id @@ -217,7 +210,6 @@ def __init__(self, disk_encryption_id, master_subnet_id, worker_subnet_id, preco self.worker_profiles = [openshiftcluster.WorkerProfile( subnet_id=worker_subnet_id )] - self.worker_profiles_status = None aad = AADManager(cmd.cli_ctx) @@ -230,7 +222,7 @@ def __init__(self, disk_encryption_id, master_subnet_id, worker_subnet_id, preco if client_id is not None: sp_obj_ids.append(aad.get_service_principal_id(client_id)) - cluster = mockoc(disk_encryption_set, master_subnet, worker_subnet, enable_preconfigured_nsg) + cluster = mockoc(disk_encryption_set, master_subnet, worker_subnet) try: # Get cluster resources we need to assign permissions on, sort to ensure the same order of operations resources = {ROLE_NETWORK_CONTRIBUTOR: sorted(get_cluster_network_resources(cmd.cli_ctx, cluster, True)), @@ -440,9 +432,8 @@ def generate_random_id(): return random_id -def get_network_resources_from_subnets(cli_ctx, subnets, fail, oc): +def get_network_resources_from_subnets(cli_ctx, subnets, fail): subnet_resources = set() - subnets_with_no_nsg_attached = set() for sn in subnets: sid = parse_resource_id(sn) @@ -464,21 +455,6 @@ def get_network_resources_from_subnets(cli_ctx, subnets, fail, oc): if subnet.get("natGateway", None): subnet_resources.add(subnet['natGateway']['id']) - if oc.network_profile.preconfigured_nsg == 'Enabled': - if subnet.get("networkSecurityGroup", None): - subnet_resources.add(subnet['networkSecurityGroup']['id']) - else: - subnets_with_no_nsg_attached.add(sn) - - # when preconfiguredNSG feature is Enabled we either have all subnets NSG attached or none. - if oc.network_profile.preconfigured_nsg == 'Enabled' and \ - len(subnets_with_no_nsg_attached) != 0 and \ - len(subnets_with_no_nsg_attached) != len(subnets): - raise ValidationError(f"(ValidationError) preconfiguredNSG feature is enabled but an NSG is\ - not attached for all required subnets. Please make sure all the following\ - subnets have a network security groups attached and retry.\ - {subnets_with_no_nsg_attached}") - return subnet_resources @@ -491,11 +467,6 @@ def get_cluster_network_resources(cli_ctx, oc, fail): if oc.worker_profiles is not None: worker_subnets = {w.subnet_id for w in oc.worker_profiles} - # Ensure that worker_profiles_status exists - # it will not be returned if the cluster resources do not exist - if oc.worker_profiles_status is not None: - worker_subnets |= {w.subnet_id for w in oc.worker_profiles_status} - master_parts = parse_resource_id(master_subnet) vnet = resource_id( subscription=master_parts['subscription'], @@ -505,11 +476,11 @@ def get_cluster_network_resources(cli_ctx, oc, fail): name=master_parts['name'], ) - return get_network_resources(cli_ctx, worker_subnets | {master_subnet}, vnet, fail, oc) + return get_network_resources(cli_ctx, worker_subnets | {master_subnet}, vnet, fail) -def get_network_resources(cli_ctx, subnets, vnet, fail, oc): - subnet_resources = get_network_resources_from_subnets(cli_ctx, subnets, fail, oc) +def get_network_resources(cli_ctx, subnets, vnet, fail): + subnet_resources = get_network_resources_from_subnets(cli_ctx, subnets, fail) resources = set() resources.add(vnet) diff --git a/python/az/aro/azext_aro/tests/latest/unit/test_dynamic_validators.py b/python/az/aro/azext_aro/tests/latest/unit/test_dynamic_validators.py index a71b64fbc02..3327abb9193 100644 --- a/python/az/aro/azext_aro/tests/latest/unit/test_dynamic_validators.py +++ b/python/az/aro/azext_aro/tests/latest/unit/test_dynamic_validators.py @@ -190,7 +190,7 @@ def test_validate_cidr( ( "should return message when network security group is already attached to subnet", Mock(cli_ctx=Mock(get_progress_controller=Mock(add=Mock(), end=Mock()))), - Mock(vnet='', key="192.168.0.1/32", master_subnet='', worker_subnet='', pod_cidr=None, service_cidr=None, enable_preconfigured_nsg=None), + Mock(vnet='', key="192.168.0.1/32", master_subnet='', worker_subnet='', pod_cidr=None, service_cidr=None), {'networkSecurityGroup': {'id': 'test'}, 'routeTable': {'id': 'test'}}, Mock(**{"permissions.list_for_resource.return_value": [Permission(actions=["Microsoft.Network/routeTables/*"], not_actions=[])]}), { @@ -207,25 +207,6 @@ def test_validate_cidr( "A Network Security Group \"test\" is already assigned to this subnet. " "Ensure there are no Network Security Groups assigned to cluster " "subnets before cluster creation" - ), - ( - "should not return message when Preconfigured NSG is enabled and network security group is attached to subnet", - Mock(cli_ctx=Mock(get_progress_controller=Mock(add=Mock(), end=Mock()))), - Mock(vnet='', key="192.168.0.1/32", master_subnet='', worker_subnet='', pod_cidr=None, service_cidr=None, enable_preconfigured_nsg=True), - {'networkSecurityGroup': {'id': 'test'}, 'routeTable': {'id': 'test'}}, - Mock(**{"permissions.list_for_resource.return_value": [Permission(actions=["Microsoft.Network/routeTables/*"], not_actions=[])]}), - { - "subscription": "subscription", - "namespace": "MICROSOFT.NETWORK", - "type": "virtualnetworks", - "last_child_num": 1, - "child_type_1": "subnets", - "resource_group": None, - "name": None, - "child_name_1": None - }, - Mock(), - None ) ] diff --git a/python/az/aro/linter_exclusions.yml b/python/az/aro/linter_exclusions.yml index 0c64d6db6de..22cc6d562e1 100644 --- a/python/az/aro/linter_exclusions.yml +++ b/python/az/aro/linter_exclusions.yml @@ -11,6 +11,3 @@ aro create: vnet_resource_group_name: rule_exclusions: - parameter_should_not_end_in_resource_group - enable_preconfigured_nsg: - rule_exclusions: - - option_length_too_long