Skip to content

Add check for vnet location for Azure Spring Cloud #2255

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.

Sign up for GitHub

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

Merged
yungezz merged 4 commits into from
Sep 2, 2020
Merged

Add check for vnet location for Azure Spring Cloud #2255

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
4dc4b59
Add check for vnet location
gavinfish Sep 1, 2020
d86231e
Fix ut
gavinfish Sep 1, 2020
c58d1f1
Resolve comments
gavinfish Sep 2, 2020
b4d4ef5
Merge branch 'master' into jieshe/vnet-location
gavinfish Sep 2, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 10 additions & 0 deletions src/spring-cloud/azext_spring_cloud/_validators.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
from msrestazure.tools import resource_id
from knack.log import get_logger
from ._utils import ApiType
from ._utils import _get_rg_location

logger = get_logger(__name__)

Expand Down Expand Up @@ -180,6 +181,15 @@ def validate_vnet(cmd, namespace):
raise CLIError('--app-subnet and --service-runtime-subnet should not be the same.')

vnet_obj = _get_vnet(cmd, vnet_id)
instance_location = namespace.location
Copy link
Member

@yungezz yungezz Sep 1, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

will there be format difference? like west us vs westus. :)

Copy link
Contributor Author

@gavinfish gavinfish Sep 2, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed.

if instance_location is None:
instance_location = _get_rg_location(cmd.cli_ctx, namespace.resource_group)
else:
instance_location_slice = instance_location.split(" ")
instance_location = "".join([piece.lower()
for piece in instance_location_slice])
if vnet_obj.location.lower() != instance_location.lower():
raise CLIError('--vnet and Azure Spring Cloud instance should be in the same location.')
for subnet in vnet_obj.subnets:
_validate_subnet(namespace, subnet)

Expand Down
33 changes: 25 additions & 8 deletions src/spring-cloud/azext_spring_cloud/tests/latest/test_asc_validator.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ def _get_test_cmd():

def _mock_get_vnet(cmd, vnet_id):
def _mock_get(id):
def _get_subnet(vnet_id, name, address_prefix=None, route_table=None, ip_configurations=None):
def _get_subnet(vnet_id, name, address_prefix=None, route_table=None, ip_configurations=None, location=None):
subnet = mock.MagicMock()
subnet.id = '{0}/subnets/{1}'.format(vnet_id, name)
subnet.name = name
Expand All @@ -39,24 +39,29 @@ def _get_subnet(vnet_id, name, address_prefix=None, route_table=None, ip_configu
subnet.ip_configurations = ip_configurations
return subnet

def _get_mock_vnet(id, **kwargs):
def _get_mock_vnet(id, location, **kwargs):
vnet = mock.MagicMock()
vnet.id = id
vnet.location = location
vnet.subnets = [_get_subnet(id, x, **kwargs) for x in ['app', 'svc']]
return vnet

all_mocks = [
_get_mock_vnet(
"/subscriptions/00000000-0000-0000-0000-000000000000/resourceGroups/test/providers/Microsoft.Network/virtualNetworks/test-vnet",
'eastus',
address_prefix='10.0.0.0/24'),
_get_mock_vnet(
"/subscriptions/11111111-0000-0000-0000-000000000000/resourceGroups/test/providers/Microsoft.Network/VirtualNetworks/test-vnet",
'eastus',
address_prefix='10.0.0.0/24'),
_get_mock_vnet(
"/subscriptions/22222222-0000-0000-0000-000000000000/resourceGroups/test/providers/Microsoft.Network/VirtualNetworks/test-vnet",
'eastus',
route_table=mock.MagicMock(), address_prefix='10.0.0.0/24'),
_get_mock_vnet(
"/subscriptions/33333333-0000-0000-0000-000000000000/resourceGroups/test/providers/Microsoft.Network/VirtualNetworks/test-vnet",
'eastus',
address_prefix='10.0.0.0/24'),
]
for x in all_mocks:
Expand Down Expand Up @@ -113,7 +118,7 @@ def _get_mock_role_assignment(scope, object_id):
class TestValidateIPRanges(unittest.TestCase):
def test_lack_of_parameters(self):
ns = Namespace(vnet='test-vnet', app_subnet='app', service_runtime_subnet=None, resource_group='test',
reserved_cidr_range='10.0.0.0/14', sku=None)
reserved_cidr_range='10.0.0.0/14', sku=None, location='eastus')
with self.assertRaises(CLIError) as context:
validate_vnet_required_parameters(ns)
self.assertEqual('--app-subnet, --service-runtime-subnet must be set when deploying to VNet',
Expand Down Expand Up @@ -148,7 +153,7 @@ def test_multi_cidr_with_overlaps(self):
@mock.patch('azext_spring_cloud._validators._get_graph_rbac_management_client',
_mock_get_graph_rbac_management_client)
def test_valid_vnet(self):
ns = Namespace(reserved_cidr_range='10.0.0.0/8,20.0.0.0/16,30.0.0.0/16', vnet='test-vnet', app_subnet='app',
ns = Namespace(reserved_cidr_range='10.0.0.0/8,20.0.0.0/16,30.0.0.0/16', vnet='test-vnet', app_subnet='app', location='eastus',
service_runtime_subnet='svc', resource_group='test', sku=None)
validate_vnet(_get_test_cmd(), ns)
self.assertEqual(ns.app_subnet.lower(),
Expand Down Expand Up @@ -176,7 +181,7 @@ def test_only_subnet_name(self):
@mock.patch('azext_spring_cloud._validators._get_graph_rbac_management_client',
_mock_get_graph_rbac_management_client)
def test_valid_subnets(self):
ns = Namespace(reserved_cidr_range='10.0.0.0/8,20.0.0.0/16,30.0.0.0/16', resource_group='test', vnet=None, sku=None,
ns = Namespace(reserved_cidr_range='10.0.0.0/8,20.0.0.0/16,30.0.0.0/16', resource_group='test', vnet=None, sku=None, location='eastus',
app_subnet='/subscriptions/11111111-0000-0000-0000-000000000000/resourceGroups/test/providers/Microsoft.Network/VirtualNetworks/test-vnet/subnets/app',
service_runtime_subnet='/subscriptions/11111111-0000-0000-0000-000000000000/resourceGroups/test/providers/Microsoft.Network/VirtualNetworks/test-vnet/subnets/svc')
validate_vnet(_get_test_cmd(), ns)
Expand All @@ -190,23 +195,23 @@ def test_valid_subnets(self):
@mock.patch('azext_spring_cloud._validators._get_graph_rbac_management_client',
_mock_get_graph_rbac_management_client)
def test_subnet_with_route_table(self):
ns = Namespace(reserved_cidr_range='10.0.0.0/8,20.0.0.0/16,30.0.0.0/16', resource_group='test', vnet=None, sku=None,
ns = Namespace(reserved_cidr_range='10.0.0.0/8,20.0.0.0/16,30.0.0.0/16', resource_group='test', vnet=None, sku=None, location='eastus',
app_subnet='/subscriptions/22222222-0000-0000-0000-000000000000/resourceGroups/test/providers/Microsoft.Network/VirtualNetworks/test-vnet/subnets/app',
service_runtime_subnet='/subscriptions/22222222-0000-0000-0000-000000000000/resourceGroups/test/providers/Microsoft.Network/VirtualNetworks/test-vnet/subnets/svc')
with self.assertRaises(CLIError) as context:
validate_vnet(_get_test_cmd(), ns)
self.assertTrue('subnet should not associate with any route tables.' in str(context.exception))

def test_subnets_same(self):
ns = Namespace(reserved_cidr_range='10.0.0.0/8,20.0.0.0/16,30.0.0.0/16', resource_group='test', vnet=None, sku=None,
ns = Namespace(reserved_cidr_range='10.0.0.0/8,20.0.0.0/16,30.0.0.0/16', resource_group='test', vnet=None, sku=None, location='eastus',
app_subnet='/subscriptions/11111111-0000-0000-0000-000000000000/resourceGroups/test/providers/Microsoft.Network/virtualnetworks/test-Vnet/subnets/app',
service_runtime_subnet='/subscriptions/11111111-0000-0000-0000-000000000000/resourceGroups/test/providers/Microsoft.Network/VirtualNetworks/test-vnet/subnets/app')
with self.assertRaises(CLIError) as context:
validate_vnet(_get_test_cmd(), ns)
self.assertEqual('--app-subnet and --service-runtime-subnet should not be same.', str(context.exception))

def test_subnets_in_different_vnet(self):
ns = Namespace(reserved_cidr_range='10.0.0.0/8,20.0.0.0/16,30.0.0.0/16', resource_group='test', vnet=None, sku=None,
ns = Namespace(reserved_cidr_range='10.0.0.0/8,20.0.0.0/16,30.0.0.0/16', resource_group='test', vnet=None, sku=None, location='eastus',
app_subnet='/subscriptions/11111111-0000-0000-0000-000000000000/resourceGroups/test/providers/Microsoft.Network/virtualnetworks/test-Vnet/subnets/app',
service_runtime_subnet='/subscriptions/11111111-0000-0000-0000-000000000000/resourceGroups/test/providers/Microsoft.Network/VirtualNetworks/test-vnet1/subnets/svc')
with self.assertRaises(CLIError) as context:
Expand All @@ -232,3 +237,15 @@ def test_set_default_cidr_range(self):
self.assertEqual(
'Cannot set "reserved-cidr-range" automatically.Please specify "--reserved-cidr-range" with 3 unused CIDR ranges in your network environment.',
str(context.exception))

@mock.patch('azext_spring_cloud._validators._get_vnet', _mock_get_vnet)
@mock.patch('azext_spring_cloud._validators._get_authorization_client', _mock_get_authorization_client)
@mock.patch('azext_spring_cloud._validators._get_graph_rbac_management_client',
_mock_get_graph_rbac_management_client)
def test_vnet_location(self):
ns = Namespace(reserved_cidr_range='10.0.0.0/8,20.0.0.0/16,30.0.0.1/16', resource_group='test', vnet=None, sku=None, location='westus',
app_subnet='/subscriptions/00000000-0000-0000-0000-000000000000/resourceGroups/test/providers/Microsoft.Network/virtualNetworks/test-vnet/subnets/app',
service_runtime_subnet='/subscriptions/00000000-0000-0000-0000-000000000000/resourceGroups/test/providers/Microsoft.Network/virtualNetworks/test-vnet/subnets/svc')
with self.assertRaises(CLIError) as context:
validate_vnet(_get_test_cmd(), ns)
self.assertTrue('--vnet and Azure Spring Cloud instance should be in the same location.' in str(context.exception))