Revert "Enable preconfigured NSG flag on azure-cli (Azure#3112)"

This reverts commit 118f1a8.

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
 
 

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"

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)

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:

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,18 +202,14 @@ 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
             )
             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)

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
     )
 ]
 

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