From c099e1f289474f43a27154ff81d35d331979aa7d Mon Sep 17 00:00:00 2001 From: Justin Webster Date: Tue, 11 Jan 2022 19:37:03 -0800 Subject: [PATCH] Support multiple Application Gateways (#644) Note: This increases/restores the symmetry between the LB and AGW functionality (and configuration), by enhancing AGW support with new (for AGWs) functionality which had already been previously supported for LBs (by #541 for #111 and #638 for #328). Restoring this level of symmetry partially resolves #644. These changes include: - refactoring of existing code/implementation (and tests). E.g. Renamed some vars and methods to more accurately reflect their current values/behavior. - significant configuration API enhancements, to enable the (previously LB-only) functionality of #111 and #328 to be used with both LB and/or AGW configs. E.g. Allow the `application_gateway` config to be configured as a Hash; allow the `application_gateway` config to be configured as an Array (new in this PR for both LBs and AGWs); added support for `application_gateway/resource_roup_name` config. --- SQUASHED COMMIT HISTORY: issue-644: multi-AGW: Refactored the `AzureClient.get_application_gateway_by_name` method to make the `resource_group_name` param a positional param (instead of an optional keyword param). Note: This change was made so that the `AzureClient.get_application_gateway_by_name` method's signature is consistent with the signatures of the other `AzureClient.get_*` methods in requiring the `resource_group_name` param to be passed as the first positional param. issue-644: multi-AGW: Modified the `VMCloudProps._parse_application_gateway_config` method to allow the `application_gateway` config to be configured as an Array. Note: When specifying an Array of AGWs, each element of the Array should be a String and/or Hash of the same format which was previously (and still is) valid when configuring a single AGW (as a non-Array). issue-644: multi-AGW: Modified the `VMCloudProps._parse_application_gateway_config` method to accept either String or Hash config (instead of String only). issue-644: multi-AGW: Fixed the `AzureClient.parse_network_interface` method to correctly return all of the NIC's application_gateways' info (instead of only the first LB's info). NOTE: All unit test specs are now passing again. issue-644: multi-AGW: Update `VMManager._get_application_gateways` method to support multi-AGW (and also to use the new `ApplicationGatewayConfig.resource_group_name` property/data). issue-644: multi-AGW: Modified the `VMCloudProps._parse_application_gateway_config` method to init `ApplicationGatewayConfig.resource_group_name` as `nil`, unless a value is explicitly configured. (See code comment for details.) issue-644: multi-AGW: Update `AzureClient.parse_network_interface` method to use the new `resource_group_name` keyword param of the `AzureClient.get_application_gateway_by_name` method. issue-644: multi-AGW: Added a new, optional `resource_group_name` keyword param to the `AzureClient.get_application_gateway_by_name` method. issue-644: multi-AGW: Updated `AzureClient.create_network_interface` method to support multi-AGW. Renamed a spec example in the Integration test suite, to make the meaning clearer: application_gateway_spec.rb. COMMENTS: Added/updated comments and doc comments: AzureClient, VMManager. issue-644: multi-AGW: Renamed a key within the `nic_params` Hash (from "application_gateway" to "application_gateways"). issue-644: multi-AGW: Rename and update `VMManager._get_application_gateways` method issue-644: multi-AGW: Renamed various vars/keys (from `application_gateway` to `application_gateways`) issue-644: multi-AGW: Added additional specs for the `VMCloudProps._parse_application_gateway_config` method. issue-644: multi-AGW: Add new `VMCloudProps._parse_application_gateway_config` method. NOTE: This change is part of a multi-commit set of incremental changes. Some of the existing unit test specs are now failing. They will be fixed/updated by the time the incremental changes are completed. --- .../lib/cloud/azure/models/config.rb | 14 ++ .../lib/cloud/azure/models/vm_cloud_props.rb | 35 ++++- .../lib/cloud/azure/restapi/azure_client.rb | 47 +++--- .../lib/cloud/azure/vms/vm_manager_network.rb | 33 +++-- .../resources/application_gateway_spec.rb | 2 +- .../create_network_interface_spec.rb | 14 +- .../unit/azure_client/get_operation_spec.rb | 52 +++++-- ...list_network_interfaces_by_keyword_spec.rb | 2 +- .../models/application_gateway_config_spec.rb | 18 +++ .../spec/unit/models/vm_cloud_props_spec.rb | 140 ++++++++++++++++++ .../create/application_security_group_spec.rb | 4 +- .../create/dynamic_public_ip_spec.rb | 4 +- .../vm_manager/create/invalid_option_spec.rb | 14 +- .../unit/vm_manager/create/shared_stuff.rb | 2 +- .../spec/unit/vm_manager/delete_spec.rb | 2 +- 15 files changed, 312 insertions(+), 71 deletions(-) create mode 100644 src/bosh_azure_cpi/spec/unit/models/application_gateway_config_spec.rb diff --git a/src/bosh_azure_cpi/lib/cloud/azure/models/config.rb b/src/bosh_azure_cpi/lib/cloud/azure/models/config.rb index 09666f544..20f974bd9 100644 --- a/src/bosh_azure_cpi/lib/cloud/azure/models/config.rb +++ b/src/bosh_azure_cpi/lib/cloud/azure/models/config.rb @@ -13,6 +13,20 @@ def to_s end end + class ApplicationGatewayConfig + # TODO: issue-644: multi-BEPool-AGW: Add support for explicitly specified BEPool name. + attr_reader :name, :resource_group_name + + def initialize(resource_group_name, name) + @resource_group_name = resource_group_name + @name = name + end + + def to_s + "name: #{@name}, resource_group_name: #{@resource_group_name}" + end + end + class AvailabilitySetConfig attr_reader :name attr_reader :platform_update_domain_count, :platform_fault_domain_count diff --git a/src/bosh_azure_cpi/lib/cloud/azure/models/vm_cloud_props.rb b/src/bosh_azure_cpi/lib/cloud/azure/models/vm_cloud_props.rb index 6820ed6f6..2ef17e539 100644 --- a/src/bosh_azure_cpi/lib/cloud/azure/models/vm_cloud_props.rb +++ b/src/bosh_azure_cpi/lib/cloud/azure/models/vm_cloud_props.rb @@ -10,7 +10,7 @@ class VMCloudProps attr_reader :availability_zone attr_reader :availability_set attr_reader :load_balancers - attr_reader :application_gateway + attr_reader :application_gateways attr_reader :managed_identity attr_reader :security_group attr_reader :application_security_groups @@ -26,6 +26,7 @@ class VMCloudProps AVAILABILITY_SET_KEY = 'availability_set' LOAD_BALANCER_KEY = 'load_balancer' + APPLICATION_GATEWAY_KEY = 'application_gateway' RESOURCE_GROUP_NAME_KEY = 'resource_group_name' NAME_KEY = 'name' @@ -57,7 +58,8 @@ def initialize(vm_properties, global_azure_config) cloud_error("Only one of 'availability_zone' and 'availability_set' is allowed to be configured for the VM but you have configured both.") if !@availability_zone.nil? && !@availability_set.name.nil? @load_balancers = _parse_load_balancer_config(vm_properties, global_azure_config) - @application_gateway = vm_properties['application_gateway'] + + @application_gateways = _parse_application_gateway_config(vm_properties) @managed_identity = global_azure_config.default_managed_identity managed_identity_hash = vm_properties.fetch('managed_identity', nil) @@ -126,6 +128,35 @@ def _parse_load_balancer_config(vm_properties, global_azure_config) load_balancers.compact end + # @return [Array,nil] + def _parse_application_gateway_config(vm_properties) + application_gateway_config = vm_properties[APPLICATION_GATEWAY_KEY] + + return nil unless application_gateway_config + + cloud_error("Property '#{APPLICATION_GATEWAY_KEY}' must be a String, Hash, or Array.") unless application_gateway_config.is_a?(String) || application_gateway_config.is_a?(Hash) || application_gateway_config.is_a?(Array) + + application_gateway_configs = application_gateway_config.is_a?(Array) ? application_gateway_config : [application_gateway_config] + application_gateways = Array(application_gateway_configs).flat_map do |agwc| + if agwc.is_a?(Hash) + application_gateway_names = agwc[NAME_KEY] + resource_group_name = agwc[RESOURCE_GROUP_NAME_KEY] + else + application_gateway_names = agwc + resource_group_name = nil + end + String(application_gateway_names).split(',').map do |application_gateway_name| + Bosh::AzureCloud::ApplicationGatewayConfig.new( + # NOTE: It is OK for the resource_group_name to be `nil` here. The nil will be defaulted elsewhere (if needed). And leaving it nil makes the specs simpler. + # resource_group_name || global_azure_config.resource_group_name, + resource_group_name, + application_gateway_name + ) + end + end + application_gateways.compact + end + # @return [Bosh::AzureCloud::AvailabilitySetConfig] def _parse_availability_set_config(vm_properties, global_azure_config) if vm_properties[AVAILABILITY_SET_KEY].is_a?(Hash) diff --git a/src/bosh_azure_cpi/lib/cloud/azure/restapi/azure_client.rb b/src/bosh_azure_cpi/lib/cloud/azure/restapi/azure_client.rb index 0d1e9f453..8638bde5a 100644 --- a/src/bosh_azure_cpi/lib/cloud/azure/restapi/azure_client.rb +++ b/src/bosh_azure_cpi/lib/cloud/azure/restapi/azure_client.rb @@ -1335,13 +1335,13 @@ def delete_public_ip(resource_group_name, name) # Network/Load Balancer # Get a load balancer's information + # @param [String,nil] resource_group_name - The load balancer's resource group name. # @param [String] name - Name of load balancer. # # @return [Hash] # # @See https://docs.microsoft.com/en-us/rest/api/load-balancer/loadbalancers/get # - def get_load_balancer_by_name(resource_group_name, name) url = rest_api_url(REST_API_PROVIDER_NETWORK, REST_API_LOAD_BALANCERS, resource_group_name: resource_group_name, name: name) _get_load_balancer(url) @@ -1356,6 +1356,7 @@ def get_load_balancer_by_name(resource_group_name, name) # def _get_load_balancer(url) load_balancer = nil + # see: https://docs.microsoft.com/en-us/rest/api/load-balancer/load-balancers/get#loadbalancer result = get_resource_by_id(url) unless result.nil? load_balancer = {} @@ -1380,6 +1381,7 @@ def _get_load_balancer(url) load_balancer[:frontend_ip_configurations].push(ip) end + # see: https://docs.microsoft.com/en-us/rest/api/load-balancer/load-balancers/get#backendaddresspool backend = properties['backendAddressPools'] load_balancer[:backend_address_pools] = [] backend.each do |backend_ip| @@ -1418,8 +1420,8 @@ def _get_load_balancer(url) # * +:dns_servers - Array. DNS servers. # * +:network_security_group - Hash. The network security group which the network interface is bound to. # * +:application_security_groups - Array. The application security groups which the network interface is bound to. - # * +:load_balancers - Array. The load balancers which the network interface is bound to. - # * +:application_gateway - Hash. The application gateway which the network interface is bound to. + # * +:load_balancers - Array. The load balancers which the network interface is bound to. (see: Bosh::AzureCloud::VMManager._get_load_balancers) + # * +:application_gateways - Array. The application gateways which the network interface is bound to. (see: Bosh::AzureCloud::VMManager._get_application_gateways) # # @return [Boolean] # @@ -1462,6 +1464,7 @@ def create_network_interface(resource_group_name, nic_params) end interface['properties']['ipConfigurations'][0]['properties']['applicationSecurityGroups'] = application_security_groups unless application_security_groups.empty? + # see: Bosh::AzureCloud::VMManager._get_load_balancers load_balancers = nic_params[:load_balancers] unless load_balancers.nil? backend_pools = load_balancers.map { |load_balancer| {:id => load_balancer[:backend_address_pools][0][:id]} } @@ -1475,13 +1478,12 @@ def create_network_interface(resource_group_name, nic_params) interface['properties']['ipConfigurations'][0]['properties']['loadBalancerInboundNatRules'] = inbound_nat_rules end - application_gateway = nic_params[:application_gateway] - unless application_gateway.nil? - interface['properties']['ipConfigurations'][0]['properties']['applicationGatewayBackendAddressPools'] = [ - { - 'id' => application_gateway[:backend_address_pools][0][:id] - } - ] + # see: Bosh::AzureCloud::VMManager._get_application_gateways + application_gateways = nic_params[:application_gateways] + unless application_gateways.nil? + # NOTE: backend_address_pools[0] should always be used. (When `application_gateway/backend_pool_name` is specified, the named pool will always be first here.) + backend_pools = application_gateways.map { |application_gateway| {:id => application_gateway[:backend_address_pools][0][:id]} } + interface['properties']['ipConfigurations'][0]['properties']['applicationGatewayBackendAddressPools'] = backend_pools end http_put(url, interface) @@ -1620,14 +1622,15 @@ def get_network_subnet(url) # Network/Application Gateway # Get an application gateway's information + # @param [String,nil] resource_group_name - The application gateway's resource group name. # @param [String] name - Name of application gateway. # # @return [Hash] # # @See https://docs.microsoft.com/en-us/rest/api/application-gateway/applicationgateways/get # - def get_application_gateway_by_name(name) - url = rest_api_url(REST_API_PROVIDER_NETWORK, REST_API_APPLICATION_GATEWAYS, name: name) + def get_application_gateway_by_name(resource_group_name, name) + url = rest_api_url(REST_API_PROVIDER_NETWORK, REST_API_APPLICATION_GATEWAYS, resource_group_name: resource_group_name, name: name) get_application_gateway(url) end @@ -1640,6 +1643,7 @@ def get_application_gateway_by_name(name) # def get_application_gateway(url) application_gateway = nil + # see: https://docs.microsoft.com/en-us/rest/api/application-gateway/application-gateways/get#applicationgateway result = get_resource_by_id(url) unless result.nil? application_gateway = {} @@ -1649,6 +1653,8 @@ def get_application_gateway(url) application_gateway[:tags] = result['tags'] properties = result['properties'] + # TODO: issue-644: multi-BEPool-AGW: Review: Already supports multiple ApplicationGateway Backend Address Pools? + # see: https://docs.microsoft.com/en-us/rest/api/application-gateway/application-gateways/get#applicationgatewaybackendaddresspool backend = properties['backendAddressPools'] application_gateway[:backend_address_pools] = [] backend.each do |backend_ip| @@ -2065,13 +2071,18 @@ def parse_network_interface(result, recursive: true) end interface[:load_balancers] = load_balancers end - unless ip_configuration_properties['applicationGatewayBackendAddressPools'].nil? - if recursive - names = _parse_name_from_id(ip_configuration_properties['applicationGatewayBackendAddressPools'][0]['id']) - interface[:application_gateway] = get_application_gateway_by_name(names[:resource_name]) - else - interface[:application_gateway] = { id: ip_configuration_properties['applicationGatewayBackendAddressPools'][0]['id'] } + application_gateway_backend_pools = ip_configuration_properties['applicationGatewayBackendAddressPools'] + unless application_gateway_backend_pools.nil? + application_gateways = application_gateway_backend_pools.map do |agw_backend_pool| + if recursive + names = _parse_name_from_id(agw_backend_pool['id']) + application_gateway = get_application_gateway_by_name(names[:resource_group_name], names[:resource_name]) + else + application_gateway = { id: agw_backend_pool['id'] } + end + application_gateway end + interface[:application_gateways] = application_gateways end unless ip_configuration_properties['applicationSecurityGroups'].nil? asgs_properties = ip_configuration_properties['applicationSecurityGroups'] diff --git a/src/bosh_azure_cpi/lib/cloud/azure/vms/vm_manager_network.rb b/src/bosh_azure_cpi/lib/cloud/azure/vms/vm_manager_network.rb index 54e735cb6..d695980ee 100644 --- a/src/bosh_azure_cpi/lib/cloud/azure/vms/vm_manager_network.rb +++ b/src/bosh_azure_cpi/lib/cloud/azure/vms/vm_manager_network.rb @@ -89,6 +89,7 @@ def _get_public_ip(vip_network) end # @return [Array] + # @see Bosh::AzureCloud::AzureClient.create_network_interface def _get_load_balancers(vm_props) load_balancers = nil load_balancer_configs = vm_props.load_balancers @@ -102,14 +103,19 @@ def _get_load_balancers(vm_props) load_balancers end - def _get_application_gateway(vm_props) - application_gateway = nil - unless vm_props.application_gateway.nil? - application_gateway_name = vm_props.application_gateway - application_gateway = @azure_client.get_application_gateway_by_name(application_gateway_name) - cloud_error("Cannot find the application gateway '#{application_gateway_name}'") if application_gateway.nil? + # @return [Array] + # @see Bosh::AzureCloud::AzureClient.create_network_interface + def _get_application_gateways(vm_props) + application_gateways = nil + application_gateway_configs = vm_props.application_gateways + unless application_gateway_configs.nil? + application_gateways = application_gateway_configs.map do |application_gateway_config| + application_gateway = @azure_client.get_application_gateway_by_name(application_gateway_config.resource_group_name, application_gateway_config.name) + cloud_error("Cannot find the application gateway '#{application_gateway_config.name}'") if application_gateway.nil? + application_gateway + end end - application_gateway + application_gateways end def _get_or_create_public_ip(resource_group_name, vm_name, location, vm_props, network_configurator) @@ -131,11 +137,12 @@ def _get_or_create_public_ip(resource_group_name, vm_name, location, vm_props, n public_ip end + # @return [Array] one Hash (returned by Bosh::AzureCloud::AzureClient.get_network_interface_by_name) per network interface created def _create_network_interfaces(resource_group_name, vm_name, location, vm_props, network_configurator, primary_nic_tags = AZURE_TAGS) # Tasks to prepare before creating NICs: # * prepare public ip # * prepare load balancer(s) - # * prepare application gateway + # * prepare application gateway(s) tasks_preparing = [] tasks_preparing.push( @@ -149,8 +156,8 @@ def _create_network_interfaces(resource_group_name, vm_name, location, vm_props, end ) tasks_preparing.push( - task_get_application_gateway = Concurrent::Future.execute do - _get_application_gateway(vm_props) + task_get_application_gateways = Concurrent::Future.execute do + _get_application_gateways(vm_props) end ) @@ -159,7 +166,7 @@ def _create_network_interfaces(resource_group_name, vm_name, location, vm_props, public_ip = task_get_or_create_public_ip.value! load_balancers = task_get_load_balancers.value! - application_gateway = task_get_application_gateway.value! + application_gateways = task_get_application_gateways.value! # tasks to create NICs, NICs will be created in different threads tasks_creating = [] @@ -187,12 +194,12 @@ def _create_network_interfaces(resource_group_name, vm_name, location, vm_props, nic_params[:public_ip] = public_ip nic_params[:tags] = primary_nic_tags nic_params[:load_balancers] = load_balancers - nic_params[:application_gateway] = application_gateway + nic_params[:application_gateways] = application_gateways else nic_params[:public_ip] = nil nic_params[:tags] = AZURE_TAGS nic_params[:load_balancers] = nil - nic_params[:application_gateway] = nil + nic_params[:application_gateways] = nil end tasks_creating.push( Concurrent::Future.execute do diff --git a/src/bosh_azure_cpi/spec/integration/resources/application_gateway_spec.rb b/src/bosh_azure_cpi/spec/integration/resources/application_gateway_spec.rb index 0b2a1960d..0a1b5c33e 100644 --- a/src/bosh_azure_cpi/spec/integration/resources/application_gateway_spec.rb +++ b/src/bosh_azure_cpi/spec/integration/resources/application_gateway_spec.rb @@ -7,7 +7,7 @@ @application_gateway_name = ENV.fetch('BOSH_AZURE_APPLICATION_GATEWAY_NAME') end - context 'when application_gateway is specified in resource pool' do + context 'when single application_gateway is specified in resource pool' do let(:network_spec) do { 'network_a' => { diff --git a/src/bosh_azure_cpi/spec/unit/azure_client/create_network_interface_spec.rb b/src/bosh_azure_cpi/spec/unit/azure_client/create_network_interface_spec.rb index 646b71f4b..8f4374f4e 100644 --- a/src/bosh_azure_cpi/spec/unit/azure_client/create_network_interface_spec.rb +++ b/src/bosh_azure_cpi/spec/unit/azure_client/create_network_interface_spec.rb @@ -56,7 +56,7 @@ network_security_group: { id: nsg_id }, application_security_groups: [], load_balancers: nil, - application_gateway: nil + application_gateways: nil } end let(:request_body) do @@ -131,7 +131,7 @@ network_security_group: { id: nsg_id }, application_security_groups: [], load_balancers: nil, - application_gateway: nil + application_gateways: nil } end let(:request_body) do @@ -209,7 +209,7 @@ network_security_group: nil, application_security_groups: [], load_balancers: nil, - application_gateway: nil + application_gateways: nil } end let(:request_body) do @@ -296,7 +296,7 @@ } ] }], - application_gateway: nil + application_gateways: nil } end @@ -381,7 +381,7 @@ network_security_group: { id: nsg_id }, application_security_groups: [{ id: 'fake-asg-id-1' }, { id: 'fake-asg-id-2' }], load_balancers: nil, - application_gateway: nil + application_gateways: nil } end let(:request_body) do @@ -467,13 +467,13 @@ network_security_group: { id: nsg_id }, application_security_groups: [], load_balancers: nil, - application_gateway: { + application_gateways: [{ backend_address_pools: [ { id: 'fake-id-2' } ] - } + }] } end let(:request_body) do diff --git a/src/bosh_azure_cpi/spec/unit/azure_client/get_operation_spec.rb b/src/bosh_azure_cpi/spec/unit/azure_client/get_operation_spec.rb index 16662127f..09d3eeace 100644 --- a/src/bosh_azure_cpi/spec/unit/azure_client/get_operation_spec.rb +++ b/src/bosh_azure_cpi/spec/unit/azure_client/get_operation_spec.rb @@ -655,24 +655,44 @@ headers: {} ) expect( - azure_client.get_application_gateway_by_name(application_gateway_name) + azure_client.get_application_gateway_by_name(nil, application_gateway_name) ).to be_nil end - it 'should return the resource if response body is not null' do - stub_request(:get, application_gateway_uri).to_return( - status: 200, - body: application_gateway_response_body, - headers: {} - ) - stub_request(:get, public_ip_uri).to_return( - status: 200, - body: public_ip_response_body.to_json, - headers: {} - ) - expect( - azure_client.get_application_gateway_by_name(application_gateway_name) - ).to eq(fake_application_gateway) + context 'when resource_group_name is not specified' do + it 'should return the resource if response body is not null' do + stub_request(:get, application_gateway_uri).to_return( + status: 200, + body: application_gateway_response_body, + headers: {} + ) + stub_request(:get, public_ip_uri).to_return( + status: 200, + body: public_ip_response_body.to_json, + headers: {} + ) + expect( + azure_client.get_application_gateway_by_name(nil, application_gateway_name) + ).to eq(fake_application_gateway) + end + end + + context 'when resource_group_name is specified' do + it 'should return the resource if response body is not null' do + stub_request(:get, application_gateway_uri).to_return( + status: 200, + body: application_gateway_response_body, + headers: {} + ) + stub_request(:get, public_ip_uri).to_return( + status: 200, + body: public_ip_response_body.to_json, + headers: {} + ) + expect( + azure_client.get_application_gateway_by_name(default_resource_group_name, application_gateway_name) + ).to eq(fake_application_gateway) + end end end end @@ -880,7 +900,7 @@ ip_configuration_id: 'fake-id', private_ip: '10.0.0.100', private_ip_allocation_method: 'Dynamic', - application_gateway: fake_application_gateway + application_gateways: [fake_application_gateway] } end it 'should return the network interface with load balancer' do diff --git a/src/bosh_azure_cpi/spec/unit/azure_client/list_network_interfaces_by_keyword_spec.rb b/src/bosh_azure_cpi/spec/unit/azure_client/list_network_interfaces_by_keyword_spec.rb index cc1fe5103..e9f3d97d9 100644 --- a/src/bosh_azure_cpi/spec/unit/azure_client/list_network_interfaces_by_keyword_spec.rb +++ b/src/bosh_azure_cpi/spec/unit/azure_client/list_network_interfaces_by_keyword_spec.rb @@ -168,7 +168,7 @@ network_security_group: { id: 'i' }, public_ip: { id: 'j' }, load_balancers: [{ id: 'k' }], - application_gateway: { id: 'l' }, + application_gateways: [{ id: 'l' }], application_security_groups: [{ id: 'asg-id-1' }] } end diff --git a/src/bosh_azure_cpi/spec/unit/models/application_gateway_config_spec.rb b/src/bosh_azure_cpi/spec/unit/models/application_gateway_config_spec.rb new file mode 100644 index 000000000..5d8af5579 --- /dev/null +++ b/src/bosh_azure_cpi/spec/unit/models/application_gateway_config_spec.rb @@ -0,0 +1,18 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe Bosh::AzureCloud::ApplicationGatewayConfig do + describe '#to_s' do + context 'when called' do + let(:name) { 'fake_name' } + let(:resource_group_name) { 'fake_rg' } + let(:lb_string) { "name: #{name}, resource_group_name: #{resource_group_name}" } + let(:application_gateway_config) { Bosh::AzureCloud::ApplicationGatewayConfig.new(resource_group_name, name) } + + it 'should return the correct string' do + expect(application_gateway_config.to_s).to eq(lb_string) + end + end + end +end diff --git a/src/bosh_azure_cpi/spec/unit/models/vm_cloud_props_spec.rb b/src/bosh_azure_cpi/spec/unit/models/vm_cloud_props_spec.rb index 08a7ff389..2bc7b25fe 100644 --- a/src/bosh_azure_cpi/spec/unit/models/vm_cloud_props_spec.rb +++ b/src/bosh_azure_cpi/spec/unit/models/vm_cloud_props_spec.rb @@ -279,6 +279,146 @@ end end + context 'when application_gateway is not specified' do + let(:vm_cloud_props) do + Bosh::AzureCloud::VMCloudProps.new( + { + 'instance_type' => 'Standard_D1' + }, azure_config_managed + ) + end + + it 'should return the correct config' do + expect(vm_cloud_props.application_gateways).to be_nil + end + end + + context 'when application_gateway is a string' do + let(:agw_name) { 'fake_agw_name' } + + let(:vm_cloud_props) do + Bosh::AzureCloud::VMCloudProps.new( + { + 'instance_type' => 'Standard_D1', + 'application_gateway' => agw_name + }, azure_config_managed + ) + end + + it 'should return the correct config' do + expect(vm_cloud_props.application_gateways.length).to eq(1) + application_gateway = vm_cloud_props.application_gateways.first + expect(application_gateway.name).to eq(agw_name) + expect(application_gateway.resource_group_name).to be_nil + end + end + + context 'when application_gateway is a hash' do + let(:agw_name) { 'fake_agw_name' } + + context 'when resource group not empty' do + let(:resource_group_name) { 'fake_resource_group' } + let(:vm_cloud_props) do + Bosh::AzureCloud::VMCloudProps.new( + { + 'instance_type' => 'Standard_D1', + 'application_gateway' => { + 'name' => agw_name, + 'resource_group_name' => resource_group_name + } + }, azure_config_managed + ) + end + + it 'should return the correct config' do + expect(vm_cloud_props.application_gateways.length).to eq(1) + application_gateway = vm_cloud_props.application_gateways.first + expect(application_gateway.name).to eq(agw_name) + expect(application_gateway.resource_group_name).to eq(resource_group_name) + end + end + + context 'when resource group empty' do + let(:vm_cloud_props) do + Bosh::AzureCloud::VMCloudProps.new( + { + 'instance_type' => 'Standard_D1', + 'application_gateway' => { 'name' => agw_name } + }, azure_config_managed + ) + end + + it 'should return the correct config' do + expect(vm_cloud_props.application_gateways.length).to eq(1) + application_gateway = vm_cloud_props.application_gateways.first + expect(application_gateway.name).to eq(agw_name) + expect(application_gateway.resource_group_name).to be_nil + end + end + end + + context 'when application_gateway is an array' do + let(:resource_group_name) { 'fake_resource_group' } + + let(:vm_cloud_props) do + Bosh::AzureCloud::VMCloudProps.new( + { + 'instance_type' => 'Standard_D1', + 'application_gateway' => [ + 'fake_agw1_name', # String + { + 'name' => 'fake_agw2_name' + # 'resource_group_name' => resource_group_name + }, # Hash without resource_group_name + 'fake_agw3_name,fake_agw4_name', # delimited String + { + 'name' => 'fake_agw5_name,fake_agw6_name', + 'resource_group_name' => resource_group_name + } # Hash with delimited String and explicit resource_group_name + ] + }, azure_config_managed + ) + end + + it 'should return the correct config' do + application_gateways = vm_cloud_props.application_gateways + expect(application_gateways.length).to eq(6) + + expect(application_gateways[0].name).to eq('fake_agw1_name') + expect(application_gateways[0].resource_group_name).to be_nil + + expect(application_gateways[1].name).to eq('fake_agw2_name') + expect(application_gateways[1].resource_group_name).to be_nil + + expect(application_gateways[2].name).to eq('fake_agw3_name') + expect(application_gateways[2].resource_group_name).to be_nil + + expect(application_gateways[3].name).to eq('fake_agw4_name') + expect(application_gateways[3].resource_group_name).to be_nil + + expect(application_gateways[4].name).to eq('fake_agw5_name') + expect(application_gateways[4].resource_group_name).to eq(resource_group_name) + + expect(application_gateways[5].name).to eq('fake_agw6_name') + expect(application_gateways[5].resource_group_name).to eq(resource_group_name) + end + end + + context 'when application_gateway is an int' do + let(:vm_cloud_properties) do + { + 'application_gateway' => 123, + 'instance_type' => 't' + } + end + + it 'should raise an error' do + expect do + Bosh::AzureCloud::VMCloudProps.new(vm_cloud_properties, azure_config_managed) + end.to raise_error('Property \'application_gateway\' must be a String, Hash, or Array.') + end + end + context '#managed_identity' do context 'when default_managed_identity is not specified in global configurations' do context 'when managed_identity is not specified in vm_extensions' do diff --git a/src/bosh_azure_cpi/spec/unit/vm_manager/create/application_security_group_spec.rb b/src/bosh_azure_cpi/spec/unit/vm_manager/create/application_security_group_spec.rb index 8b9b4b63b..fb3f78f29 100644 --- a/src/bosh_azure_cpi/spec/unit/vm_manager/create/application_security_group_spec.rb +++ b/src/bosh_azure_cpi/spec/unit/vm_manager/create/application_security_group_spec.rb @@ -598,7 +598,7 @@ subnet: subnet, tags: tags, load_balancers: [ load_balancer ], - application_gateway: application_gateway + application_gateways: [application_gateway] )).once _, vm_params = vm_manager_for_pip.create(bosh_vm_meta, location, vm_props, disk_cids, network_configurator, env, agent_util, network_spec, config) @@ -626,7 +626,7 @@ subnet: subnet, tags: tags, load_balancers: [ load_balancer ], - application_gateway: application_gateway + application_gateways: [application_gateway] )) _, vm_params = vm_manager.create(bosh_vm_meta, location, vm_props, disk_cids, network_configurator, env, agent_util, network_spec, config) diff --git a/src/bosh_azure_cpi/spec/unit/vm_manager/create/dynamic_public_ip_spec.rb b/src/bosh_azure_cpi/spec/unit/vm_manager/create/dynamic_public_ip_spec.rb index df72a98d6..28298695e 100644 --- a/src/bosh_azure_cpi/spec/unit/vm_manager/create/dynamic_public_ip_spec.rb +++ b/src/bosh_azure_cpi/spec/unit/vm_manager/create/dynamic_public_ip_spec.rb @@ -68,7 +68,7 @@ subnet: subnet, tags: tags, load_balancers: [ load_balancer ], - application_gateway: application_gateway + application_gateways: [application_gateway] )).once _, vm_params = vm_manager_for_pip.create(bosh_vm_meta, location, vm_props, disk_cids, network_configurator, env, agent_util, network_spec, config) @@ -96,7 +96,7 @@ subnet: subnet, tags: tags, load_balancers: [ load_balancer ], - application_gateway: application_gateway + application_gateways: [application_gateway] )) _, vm_params = vm_manager.create(bosh_vm_meta, location, vm_props, disk_cids, network_configurator, env, agent_util, network_spec, config) diff --git a/src/bosh_azure_cpi/spec/unit/vm_manager/create/invalid_option_spec.rb b/src/bosh_azure_cpi/spec/unit/vm_manager/create/invalid_option_spec.rb index ca1888a97..f9d3f8bff 100644 --- a/src/bosh_azure_cpi/spec/unit/vm_manager/create/invalid_option_spec.rb +++ b/src/bosh_azure_cpi/spec/unit/vm_manager/create/invalid_option_spec.rb @@ -25,7 +25,7 @@ .with(vm_props.load_balancers.first.resource_group_name, vm_props.load_balancers.first.name) .and_return(load_balancer) allow(azure_client).to receive(:get_application_gateway_by_name) - .with(vm_props.application_gateway) + .with(vm_props.application_gateways.first.resource_group_name, vm_props.application_gateways.first.name) .and_return(application_gateway) allow(azure_client).to receive(:list_public_ips) .and_return([{ @@ -52,7 +52,7 @@ .with(vm_props.load_balancers.first.resource_group_name, vm_props.load_balancers.first.name) .and_return(load_balancer) allow(azure_client).to receive(:get_application_gateway_by_name) - .with(vm_props.application_gateway) + .with(vm_props.application_gateways.first.resource_group_name, vm_props.application_gateways.first.name) .and_return(application_gateway) allow(azure_client).to receive(:list_public_ips) .and_return([{ @@ -82,7 +82,7 @@ .with(vm_props.load_balancers.first.resource_group_name, vm_props.load_balancers.first.name) .and_return(load_balancer) allow(azure_client).to receive(:get_application_gateway_by_name) - .with(vm_props.application_gateway) + .with(vm_props.application_gateways.first.resource_group_name, vm_props.application_gateways.first.name) .and_return(application_gateway) allow(azure_client).to receive(:list_public_ips) .and_return([{ @@ -131,7 +131,7 @@ .with(vm_props.load_balancers.first.resource_group_name, vm_props.load_balancers.first.name) .and_return(load_balancer) allow(azure_client).to receive(:get_application_gateway_by_name) - .with(vm_props.application_gateway) + .with(vm_props.application_gateways.first.resource_group_name, vm_props.application_gateways.first.name) .and_return(application_gateway) end @@ -214,7 +214,7 @@ .with(vm_props.load_balancers.first.resource_group_name, vm_props.load_balancers.first.name) .and_return(load_balancer) allow(azure_client).to receive(:get_application_gateway_by_name) - .with(vm_props.application_gateway) + .with(vm_props.application_gateways.first.resource_group_name, vm_props.application_gateways.first.name) .and_return(nil) expect(azure_client).not_to receive(:delete_virtual_machine) @@ -234,7 +234,7 @@ .with(vm_props.load_balancers.first.resource_group_name, vm_props.load_balancers.first.name) .and_return(load_balancer) allow(azure_client).to receive(:get_application_gateway_by_name) - .with(vm_props.application_gateway) + .with(vm_props.application_gateways.first.resource_group_name, vm_props.application_gateways.first.name) .and_return(application_gateway) allow(azure_client).to receive(:list_public_ips) .and_return([{ @@ -278,7 +278,7 @@ .with(vm_props.load_balancers.first.resource_group_name, vm_props.load_balancers.first.name) .and_return(load_balancer) allow(azure_client).to receive(:get_application_gateway_by_name) - .with(vm_props.application_gateway) + .with(vm_props.application_gateways.first.resource_group_name, vm_props.application_gateways.first.name) .and_return(application_gateway) allow(azure_client).to receive(:list_public_ips) .and_return([{ diff --git a/src/bosh_azure_cpi/spec/unit/vm_manager/create/shared_stuff.rb b/src/bosh_azure_cpi/spec/unit/vm_manager/create/shared_stuff.rb index 2c6a3afe1..85a0a98f0 100644 --- a/src/bosh_azure_cpi/spec/unit/vm_manager/create/shared_stuff.rb +++ b/src/bosh_azure_cpi/spec/unit/vm_manager/create/shared_stuff.rb @@ -132,7 +132,7 @@ .with(MOCK_RESOURCE_GROUP_NAME, vm_properties['load_balancer']) .and_return(load_balancer) allow(azure_client).to receive(:get_application_gateway_by_name) - .with(vm_properties['application_gateway']) + .with(nil, vm_properties['application_gateway']) .and_return(application_gateway) allow(azure_client).to receive(:get_public_ip_by_name) .with(MOCK_RESOURCE_GROUP_NAME, vm_name) diff --git a/src/bosh_azure_cpi/spec/unit/vm_manager/delete_spec.rb b/src/bosh_azure_cpi/spec/unit/vm_manager/delete_spec.rb index c97361828..b2235e8f5 100644 --- a/src/bosh_azure_cpi/spec/unit/vm_manager/delete_spec.rb +++ b/src/bosh_azure_cpi/spec/unit/vm_manager/delete_spec.rb @@ -57,7 +57,7 @@ allow(azure_client).to receive(:get_load_balancer_by_name) .with(resource_group_name, vm_name).and_return(load_balancer) allow(azure_client).to receive(:get_application_gateway_by_name) - .with(vm_name).and_return(application_gateway) + .with(nil, vm_name).and_return(application_gateway) allow(azure_client).to receive(:get_network_interface_by_name) .with(resource_group_name, vm_name).and_return(network_interface) allow(azure_client).to receive(:get_public_ip_by_name)