Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Enable preconfigured NSG flag on azure-cli #3112

Merged
merged 11 commits into from
Sep 19, 2023
2 changes: 1 addition & 1 deletion python/az/aro/azext_aro/_client_factory.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
import urllib3

from azext_aro.custom import rp_mode_development
from azext_aro.vendored_sdks.azure.mgmt.redhatopenshift.v2023_04_01 import AzureRedHatOpenShiftClient
from azext_aro.vendored_sdks.azure.mgmt.redhatopenshift.v2023_09_04 import AzureRedHatOpenShiftClient
from azure.cli.core.commands.client_factory import get_mgmt_service_client


Expand Down
2 changes: 1 addition & 1 deletion python/az/aro/azext_aro/_dynamic_validators.py
Original file line number Diff line number Diff line change
Expand Up @@ -185,7 +185,7 @@ def _validate_subnet(cmd, namespace):
"Microsoft.Network/routeTables/read",
"Microsoft.Network/routeTables/write"])

if subnet_obj.get('networkSecurityGroup', None):
if not namespace.enable_preconfigured_nsg and 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"
Expand Down
2 changes: 2 additions & 0 deletions python/az/aro/azext_aro/_params.py
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,8 @@ 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',
gouthamMN marked this conversation as resolved.
Show resolved Hide resolved
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)
Expand Down
2 changes: 1 addition & 1 deletion python/az/aro/azext_aro/commands.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@

def load_command_table(self, _):
aro_sdk = CliCommandType(
operations_tmpl='azext_aro.vendored_sdks.azure.mgmt.redhatopenshift.v2023_04_01.operations#OpenShiftClustersOperations.{}', # pylint: disable=line-too-long
operations_tmpl='azext_aro.vendored_sdks.azure.mgmt.redhatopenshift.v2023_09_04.operations#OpenShiftClustersOperations.{}', # pylint: disable=line-too-long
client_factory=cf_aro)

with self.command_group('aro', aro_sdk, client_factory=cf_aro) as g:
Expand Down
43 changes: 36 additions & 7 deletions python/az/aro/azext_aro/custom.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
from base64 import b64decode
import textwrap

import azext_aro.vendored_sdks.azure.mgmt.redhatopenshift.v2023_04_01.models as openshiftcluster
import azext_aro.vendored_sdks.azure.mgmt.redhatopenshift.v2023_09_04.models as openshiftcluster

from azure.cli.command_modules.role import GraphError
from azure.cli.core.commands.client_factory import get_mgmt_service_client
Expand Down Expand Up @@ -44,6 +44,7 @@ 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,
Expand Down Expand Up @@ -73,6 +74,14 @@ def aro_create(cmd, # pylint: disable=too-many-locals
if provider.registration_state != 'Registered':
raise UnauthorizedError('Microsoft.RedHatOpenShift provider is not registered.',
'Run `az provider register -n Microsoft.RedHatOpenShift --wait`.')
if enable_preconfigured_nsg:
feature_client = get_mgmt_service_client(
cmd.cli_ctx, ResourceType.MGMT_RESOURCE_FEATURES)
feature = feature_client.features.get('Microsoft.RedHatOpenShift', 'PreconfiguredNSG')
if feature.properties.state != 'Registered':
raise ValidationError('PreconfiguredNSG feature is not registered. \
Run `az feature register --namespace Microsoft.RedHatOpenShift \
-n PreconfiguredNSG`.')
gouthamMN marked this conversation as resolved.
Show resolved Hide resolved

validate_subnets(master_subnet, worker_subnet)

Expand All @@ -83,6 +92,7 @@ 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,
Expand Down Expand Up @@ -141,6 +151,7 @@ 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',
gouthamMN marked this conversation as resolved.
Show resolved Hide resolved
),
master_profile=openshiftcluster.MasterProfile(
vm_size=master_vm_size or 'Standard_D8s_v3',
Expand Down Expand Up @@ -186,6 +197,7 @@ 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
Expand All @@ -198,7 +210,10 @@ 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):
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'
)
self.master_profile = openshiftcluster.MasterProfile(
subnet_id=master_subnet_id,
disk_encryption_set_id=disk_encryption_id
Expand All @@ -218,7 +233,7 @@ def __init__(self, disk_encryption_id, master_subnet_id, worker_subnet_id):
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)
cluster = mockoc(disk_encryption_set, master_subnet, worker_subnet, enable_preconfigured_nsg)
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)),
Expand Down Expand Up @@ -436,8 +451,9 @@ def generate_random_id():
return random_id


def get_network_resources_from_subnets(cli_ctx, subnets, fail):
def get_network_resources_from_subnets(cli_ctx, subnets, fail, oc):
subnet_resources = set()
subnet_with_nsg_preattached = 0
for sn in subnets:
sid = parse_resource_id(sn)

Expand All @@ -458,6 +474,19 @@ def get_network_resources_from_subnets(cli_ctx, subnets, fail):
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'])
subnet_with_nsg_preattached = subnet_with_nsg_preattached + 1

# when preconfiguredNSG feature is Enabled we either have all subnets NSG attached or none.
if oc.network_profile.preconfigured_nsg == 'Enabled' and \
subnet_with_nsg_preattached != 0 and \
subnet_with_nsg_preattached != len(subnets):
raise ValidationError("""(ValidationError) preconfiguredNSG feature is enabled but NSG not attached for subnets.
Please makesure all the subnets have network security group attached and retry.
gouthamMN marked this conversation as resolved.
Show resolved Hide resolved
If issue persists: raise azure support ticket""")
gouthamMN marked this conversation as resolved.
Show resolved Hide resolved

return subnet_resources


Expand All @@ -479,11 +508,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)
return get_network_resources(cli_ctx, worker_subnets | {master_subnet}, vnet, fail, oc)


def get_network_resources(cli_ctx, subnets, vnet, fail):
subnet_resources = get_network_resources_from_subnets(cli_ctx, subnets, fail)
def get_network_resources(cli_ctx, subnets, vnet, fail, oc):
subnet_resources = get_network_resources_from_subnets(cli_ctx, subnets, fail, oc)

resources = set()
resources.add(vnet)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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),
Mock(vnet='', key="192.168.0.1/32", master_subnet='', worker_subnet='', pod_cidr=None, service_cidr=None, enable_preconfigured_nsg=None),
{'networkSecurityGroup': {'id': 'test'}, 'routeTable': {'id': 'test'}},
Mock(**{"permissions.list_for_resource.return_value": [Permission(actions=["Microsoft.Network/routeTables/*"], not_actions=[])]}),
{
Expand All @@ -207,6 +207,25 @@ 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
)
]

Expand Down
3 changes: 3 additions & 0 deletions python/az/aro/linter_exclusions.yml
Original file line number Diff line number Diff line change
Expand Up @@ -11,3 +11,6 @@ aro create:
vnet_resource_group_name:
rule_exclusions:
- parameter_should_not_end_in_resource_group
enable_preconfigured_nsg:
rule_exclusions:
- option_length_too_long