diff --git a/functional_tests/duplicate_ports.json b/functional_tests/duplicate_ports.json new file mode 100644 index 000000000..16922b34f --- /dev/null +++ b/functional_tests/duplicate_ports.json @@ -0,0 +1,239 @@ +{ + "properties": { + "backendAddressPools": [ + { + "id": "/subscriptions/--subscription--/resourceGroups/--resource-group--/providers/Microsoft.Network/applicationGateways/--app-gw-name--/backendAddressPools/defaultaddresspool", + "name": "defaultaddresspool", + "properties": { + "backendAddresses": [] + } + }, + { + "etag": "*", + "id": "/subscriptions/--subscription--/resourceGroups/--resource-group--/providers/Microsoft.Network/applicationGateways/--app-gw-name--/backendAddressPools/pool---namespace---hello-world-80-bp-80", + "name": "pool---namespace---hello-world-80-bp-80", + "properties": { + "backendAddresses": [ + { + "ipAddress": "1.1.1.1" + }, + { + "ipAddress": "1.1.1.2" + }, + { + "ipAddress": "1.1.1.3" + } + ] + } + } + ], + "backendHttpSettingsCollection": [ + { + "etag": "*", + "id": "/subscriptions/--subscription--/resourceGroups/--resource-group--/providers/Microsoft.Network/applicationGateways/--app-gw-name--/backendHttpSettingsCollection/bp---namespace---hello-world-80-80---name--", + "name": "bp---namespace---hello-world-80-80---name--", + "properties": { + "port": 80, + "probe": { + "id": "/subscriptions/--subscription--/resourceGroups/--resource-group--/providers/Microsoft.Network/applicationGateways/--app-gw-name--/probes/pb---namespace---hello-world-80---name--" + }, + "protocol": "Http" + } + }, + { + "id": "/subscriptions/--subscription--/resourceGroups/--resource-group--/providers/Microsoft.Network/applicationGateways/--app-gw-name--/backendHttpSettingsCollection/defaulthttpsetting", + "name": "defaulthttpsetting", + "properties": { + "port": 80, + "probe": { + "id": "/subscriptions/--subscription--/resourceGroups/--resource-group--/providers/Microsoft.Network/applicationGateways/--app-gw-name--/probes/defaultprobe-Http" + }, + "protocol": "Http" + } + } + ], + "frontendIPConfigurations": [ + { + "etag": "xx2", + "id": "--front-end-ip-id-1--", + "name": "xx3", + "properties": { + "publicIPAddress": { + "id": "xyz" + } + }, + "type": "xx1" + }, + { + "etag": "yy2", + "id": "--front-end-ip-id-2--", + "name": "yy3", + "properties": { + "privateIPAddress": "abc" + }, + "type": "yy1" + } + ], + "frontendPorts": [ + { + "etag": "*", + "id": "/subscriptions/--subscription--/resourceGroups/--resource-group--/providers/Microsoft.Network/applicationGateways/--app-gw-name--/frontendPorts/fp-443", + "name": "fp-443", + "properties": { + "port": 443 + } + }, + { + "etag": "*", + "id": "/subscriptions/--subscription--/resourceGroups/--resource-group--/providers/Microsoft.Network/applicationGateways/--app-gw-name--/frontendPorts/fp-80", + "name": "fp-80", + "properties": { + "port": 80 + } + } + ], + "httpListeners": [ + { + "etag": "*", + "id": "/subscriptions/--subscription--/resourceGroups/--resource-group--/providers/Microsoft.Network/applicationGateways/--app-gw-name--/httpListeners/fl-foo.baz-443", + "name": "fl-foo.baz-443", + "properties": { + "frontendIPConfiguration": { + "id": "--front-end-ip-id-1--" + }, + "frontendPort": { + "id": "/subscriptions/--subscription--/resourceGroups/--resource-group--/providers/Microsoft.Network/applicationGateways/--app-gw-name--/frontendPorts/fp-443" + }, + "hostName": "foo.baz", + "protocol": "Https", + "sslCertificate": { + "id": "/subscriptions/--subscription--/resourceGroups/--resource-group--/providers/Microsoft.Network/applicationGateways/--app-gw-name--/sslCertificates/--namespace-----the-name-of-the-secret--" + } + } + }, + { + "etag": "*", + "id": "/subscriptions/--subscription--/resourceGroups/--resource-group--/providers/Microsoft.Network/applicationGateways/--app-gw-name--/httpListeners/fl-foo.baz-80", + "name": "fl-foo.baz-80", + "properties": { + "frontendIPConfiguration": { + "id": "--front-end-ip-id-1--" + }, + "frontendPort": { + "id": "/subscriptions/--subscription--/resourceGroups/--resource-group--/providers/Microsoft.Network/applicationGateways/--app-gw-name--/frontendPorts/fp-80" + }, + "hostName": "foo.baz", + "protocol": "Http" + } + } + ], + "probes": [ + { + "id": "/subscriptions/--subscription--/resourceGroups/--resource-group--/providers/Microsoft.Network/applicationGateways/--app-gw-name--/probes/defaultprobe-Http", + "name": "defaultprobe-Http", + "properties": { + "host": "localhost", + "interval": 30, + "path": "/", + "protocol": "Http", + "timeout": 30, + "unhealthyThreshold": 3 + } + }, + { + "id": "/subscriptions/--subscription--/resourceGroups/--resource-group--/providers/Microsoft.Network/applicationGateways/--app-gw-name--/probes/defaultprobe-Https", + "name": "defaultprobe-Https", + "properties": { + "host": "localhost", + "interval": 30, + "path": "/", + "protocol": "Https", + "timeout": 30, + "unhealthyThreshold": 3 + } + }, + { + "id": "/subscriptions/--subscription--/resourceGroups/--resource-group--/providers/Microsoft.Network/applicationGateways/--app-gw-name--/probes/pb---namespace---hello-world-80---name--", + "name": "pb---namespace---hello-world-80---name--", + "properties": { + "host": "foo.baz", + "interval": 30, + "path": "/", + "protocol": "Http", + "timeout": 30, + "unhealthyThreshold": 3 + } + } + ], + "redirectConfigurations": [ + { + "etag": "*", + "id": "/subscriptions/--subscription--/resourceGroups/--resource-group--/providers/Microsoft.Network/applicationGateways/--app-gw-name--/redirectConfigurations/sslr-fl-foo.baz-443", + "name": "sslr-fl-foo.baz-443", + "properties": { + "includePath": true, + "includeQueryString": true, + "redirectType": "Permanent", + "targetListener": { + "id": "/subscriptions/--subscription--/resourceGroups/--resource-group--/providers/Microsoft.Network/applicationGateways/--app-gw-name--/httpListeners/fl-foo.baz-443" + } + } + } + ], + "requestRoutingRules": [ + { + "etag": "*", + "id": "/subscriptions/--subscription--/resourceGroups/--resource-group--/providers/Microsoft.Network/applicationGateways/--app-gw-name--/requestRoutingRules/rr-foo.baz-443", + "name": "rr-foo.baz-443", + "properties": { + "backendAddressPool": { + "id": "/subscriptions/--subscription--/resourceGroups/--resource-group--/providers/Microsoft.Network/applicationGateways/--app-gw-name--/backendAddressPools/pool---namespace---hello-world-80-bp-80" + }, + "backendHttpSettings": { + "id": "/subscriptions/--subscription--/resourceGroups/--resource-group--/providers/Microsoft.Network/applicationGateways/--app-gw-name--/backendHttpSettingsCollection/bp---namespace---hello-world-80-80---name--" + }, + "httpListener": { + "id": "/subscriptions/--subscription--/resourceGroups/--resource-group--/providers/Microsoft.Network/applicationGateways/--app-gw-name--/httpListeners/fl-foo.baz-443" + }, + "ruleType": "Basic" + } + }, + { + "etag": "*", + "id": "/subscriptions/--subscription--/resourceGroups/--resource-group--/providers/Microsoft.Network/applicationGateways/--app-gw-name--/requestRoutingRules/rr-foo.baz-80", + "name": "rr-foo.baz-80", + "properties": { + "httpListener": { + "id": "/subscriptions/--subscription--/resourceGroups/--resource-group--/providers/Microsoft.Network/applicationGateways/--app-gw-name--/httpListeners/fl-foo.baz-80" + }, + "redirectConfiguration": { + "id": "/subscriptions/--subscription--/resourceGroups/--resource-group--/providers/Microsoft.Network/applicationGateways/--app-gw-name--/redirectConfigurations/sslr-fl-foo.baz-443" + }, + "ruleType": "Basic" + } + } + ], + "sku": { + "capacity": 3, + "name": "Standard_v2", + "tier": "Standard_v2" + }, + "sslCertificates": [ + { + "etag": "*", + "id": "/subscriptions/--subscription--/resourceGroups/--resource-group--/providers/Microsoft.Network/applicationGateways/--app-gw-name--/sslCertificates/--namespace-----the-name-of-the-secret--", + "name": "--namespace-----the-name-of-the-secret--", + "properties": { + "data": "xx", + "password": "msazure" + } + } + ], + "urlPathMaps": null + }, + "tags": { + "ingress-for-aks-cluster-id": "/subscriptions/subid/resourcegroups/aksresgp/providers/Microsoft.ContainerService/managedClusters/aksname", + "last-updated-by-k8s-ingress": "2009-11-17 20:34:58.651387237 +0000 UTC", + "managed-by-k8s-ingress": "a/b/c" + } +} \ No newline at end of file diff --git a/functional_tests/functional_test.go b/functional_tests/functional_test.go index b31f18951..c669f2733 100644 --- a/functional_tests/functional_test.go +++ b/functional_tests/functional_test.go @@ -32,6 +32,7 @@ import ( "github.com/Azure/application-gateway-kubernetes-ingress/pkg/tests/mocks" "github.com/Azure/application-gateway-kubernetes-ingress/pkg/utils" "github.com/Azure/application-gateway-kubernetes-ingress/pkg/version" + "github.com/Azure/application-gateway-kubernetes-ingress/pkg/tests/fixtures" ) func TestFunctional(t *testing.T) { @@ -522,5 +523,23 @@ var _ = ginkgo.Describe("Tests `appgw.ConfigBuilder`", func() { check(cbCtx, "two_ingresses_same_domain_tls_notls.json", stopChannel, ctxt, configBuilder) }) + ginkgo.It("Preexisting port w/ same port number", func() { + + cbCtx := &ConfigBuilderContext{ + IngressList: []*v1beta1.Ingress{ + ingress, + }, + ServiceList: serviceList, + EnvVariables: environment.GetFakeEnv(), + DefaultAddressPoolID: to.StringPtr("xx"), + DefaultHTTPSettingsID: to.StringPtr("yyxx"), + ExistingPortsByNumber: map[Port]n.ApplicationGatewayFrontendPort{ + Port(80): fixtures.GetDefaultPort(), + Port(8989): fixtures.GetPort(8989), + }, + } + check(cbCtx, "duplicate_ports.json", stopChannel, ctxt, configBuilder) + }) + }) }) diff --git a/pkg/appgw/configbuilder.go b/pkg/appgw/configbuilder.go index 0e6363eb5..4973cd566 100644 --- a/pkg/appgw/configbuilder.go +++ b/pkg/appgw/configbuilder.go @@ -26,7 +26,6 @@ import ( // Clock is an interface, which allows you to implement your own Time. type Clock interface { Now() time.Time - } // ConfigBuilder is a builder for application gateway configuration diff --git a/pkg/appgw/frontend_listeners.go b/pkg/appgw/frontend_listeners.go index 1e3bd0dcc..df81fbba3 100644 --- a/pkg/appgw/frontend_listeners.go +++ b/pkg/appgw/frontend_listeners.go @@ -23,16 +23,19 @@ func (c *appGwConfigBuilder) getListeners(cbCtx *ConfigBuilderContext) (*[]n.App } publIPPorts := make(map[string]string) - portSet := make(map[string]interface{}) + portsByNumber := cbCtx.ExistingPortsByNumber var listeners []n.ApplicationGatewayHTTPListener - var ports []n.ApplicationGatewayFrontendPort + + if portsByNumber == nil { + portsByNumber = make(map[Port]n.ApplicationGatewayFrontendPort) + } if cbCtx.EnvVariables.EnableIstioIntegration { - listeners, ports, portSet, publIPPorts = c.getIstioListenersPorts(cbCtx) + listeners, portsByNumber, publIPPorts = c.getIstioListenersPorts(cbCtx) } for listenerID, config := range c.getListenerConfigs(cbCtx) { - listener, port, err := c.newListener(cbCtx, listenerID, config.Protocol) + listener, port, err := c.newListener(cbCtx, listenerID, config.Protocol, portsByNumber) if err != nil { glog.Errorf("Failed creating listener %+v: %s", listenerID, err) continue @@ -47,15 +50,16 @@ func (c *appGwConfigBuilder) getListeners(cbCtx *ConfigBuilderContext) (*[]n.App publIPPorts[*port.Name] = *listener.Name } + // newlistener created a new port; Add it to the set + if _, exists := portsByNumber[Port(*port.Port)]; !exists { + portsByNumber[Port(*port.Port)] = *port + } + if config.Protocol == n.HTTPS { sslCertificateID := c.appGwIdentifier.sslCertificateID(config.Secret.secretFullName()) listener.SslCertificate = resourceRef(sslCertificateID) } listeners = append(listeners, *listener) - if _, exists := portSet[*port.Name]; !exists { - portSet[*port.Name] = nil - ports = append(ports, *port) - } } if cbCtx.EnvVariables.EnableBrownfieldDeployment { @@ -71,11 +75,19 @@ func (c *appGwConfigBuilder) getListeners(cbCtx *ConfigBuilderContext) (*[]n.App listeners = brownfield.MergeListeners(existingBlacklisted, listeners) } - if cbCtx.EnvVariables.EnableBrownfieldDeployment { - er := brownfield.NewExistingResources(c.appGw, cbCtx.ProhibitedTargets, nil) - existingBlacklisted, existingNonBlacklisted := er.GetBlacklistedPorts() - brownfield.LogPorts(existingBlacklisted, existingNonBlacklisted, ports) - ports = brownfield.MergePorts(existingBlacklisted, ports) + portIDs := make(map[string]interface{}) + // Cleanup unused ports + for _, listener := range listeners { + if listener.FrontendPort != nil && listener.FrontendPort.ID != nil { + portIDs[*listener.FrontendPort.ID] = nil + } + } + + var ports []n.ApplicationGatewayFrontendPort + for _, port := range portsByNumber { + if _, exists := portIDs[*port.ID]; exists { + ports = append(ports, port) + } } sort.Sort(sorter.ByListenerName(listeners)) @@ -116,17 +128,21 @@ func (c *appGwConfigBuilder) getListenerConfigs(cbCtx *ConfigBuilderContext) map return allListeners } -func (c *appGwConfigBuilder) newListener(cbCtx *ConfigBuilderContext, listenerID listenerIdentifier, protocol n.ApplicationGatewayProtocol) (*n.ApplicationGatewayHTTPListener, *n.ApplicationGatewayFrontendPort, error) { +func (c *appGwConfigBuilder) newListener(cbCtx *ConfigBuilderContext, listenerID listenerIdentifier, protocol n.ApplicationGatewayProtocol, portsByNumber map[Port]n.ApplicationGatewayFrontendPort) (*n.ApplicationGatewayHTTPListener, *n.ApplicationGatewayFrontendPort, error) { frontIPConfiguration := *LookupIPConfigurationByType(c.appGw.FrontendIPConfigurations, listenerID.UsePrivateIP) - - portName := generateFrontendPortName(listenerID.FrontendPort) - frontendPort := n.ApplicationGatewayFrontendPort{ - Etag: to.StringPtr("*"), - Name: &portName, - ID: to.StringPtr(c.appGwIdentifier.frontendPortID(portName)), - ApplicationGatewayFrontendPortPropertiesFormat: &n.ApplicationGatewayFrontendPortPropertiesFormat{ - Port: to.Int32Ptr(int32(listenerID.FrontendPort)), - }, + portNumber := listenerID.FrontendPort + var frontendPort n.ApplicationGatewayFrontendPort + var exists bool + if frontendPort, exists = portsByNumber[portNumber]; !exists { + portName := generateFrontendPortName(listenerID.FrontendPort) + frontendPort = n.ApplicationGatewayFrontendPort{ + Etag: to.StringPtr("*"), + Name: &portName, + ID: to.StringPtr(c.appGwIdentifier.frontendPortID(portName)), + ApplicationGatewayFrontendPortPropertiesFormat: &n.ApplicationGatewayFrontendPortPropertiesFormat{ + Port: to.Int32Ptr(int32(portNumber)), + }, + } } listenerName := generateListenerName(listenerID) @@ -164,12 +180,19 @@ func (c *appGwConfigBuilder) groupListenersByListenerIdentifier(cbCtx *ConfigBui listenersByID := make(map[listenerIdentifier]*n.ApplicationGatewayHTTPListener) // Update the listenerMap with the final listener lists for idx, listener := range *listeners { - port := portsByID[*listener.FrontendPort.ID] + port, portExists := portsByID[*listener.FrontendPort.ID] + listenerID := listenerIdentifier{ - HostName: *listener.HostName, - FrontendPort: Port(*port.Port), UsePrivateIP: IsPrivateIPConfiguration(LookupIPConfigurationByID(c.appGw.FrontendIPConfigurations, listener.FrontendIPConfiguration.ID)), } + if listener.HostName != nil { + listenerID.HostName = *listener.HostName + } + if portExists && port.Port != nil { + listenerID.FrontendPort = Port(*port.Port) + } else { + glog.Errorf("Failed to find port '%s' referenced by listener '%s'", *listener.FrontendPort.ID, *listener.Name) + } listenersByID[listenerID] = &((*listeners)[idx]) } diff --git a/pkg/appgw/frontend_listeners_istio.go b/pkg/appgw/frontend_listeners_istio.go index aa51dcdb6..ddb46f5bc 100644 --- a/pkg/appgw/frontend_listeners_istio.go +++ b/pkg/appgw/frontend_listeners_istio.go @@ -10,15 +10,14 @@ import ( "github.com/golang/glog" ) -func (c *appGwConfigBuilder) getIstioListenersPorts(cbCtx *ConfigBuilderContext) ([]n.ApplicationGatewayHTTPListener, []n.ApplicationGatewayFrontendPort, map[string]interface{}, map[string]string) { +func (c *appGwConfigBuilder) getIstioListenersPorts(cbCtx *ConfigBuilderContext) ([]n.ApplicationGatewayHTTPListener, map[Port]n.ApplicationGatewayFrontendPort, map[string]string) { publIPPorts := make(map[string]string) - portSet := make(map[string]interface{}) + portsByNumber := cbCtx.ExistingPortsByNumber var listeners []n.ApplicationGatewayHTTPListener - var ports []n.ApplicationGatewayFrontendPort if cbCtx.EnvVariables.EnableIstioIntegration { for listenerID, config := range c.getListenerConfigsFromIstio(cbCtx.IstioGateways, cbCtx.IstioVirtualServices) { - listener, port, err := c.newListener(cbCtx, listenerID, config.Protocol) + listener, port, err := c.newListener(cbCtx, listenerID, config.Protocol, portsByNumber) if err != nil { glog.Errorf("Failed creating listener %+v: %s", listenerID, err) continue @@ -33,11 +32,10 @@ func (c *appGwConfigBuilder) getIstioListenersPorts(cbCtx *ConfigBuilderContext) } listeners = append(listeners, *listener) - if _, exists := portSet[*port.Name]; !exists { - portSet[*port.Name] = nil - ports = append(ports, *port) + if _, exists := portsByNumber[Port(*port.Port)]; !exists { + portsByNumber[Port(*port.Port)] = *port } } } - return listeners, ports, portSet, publIPPorts + return listeners, portsByNumber, publIPPorts } diff --git a/pkg/appgw/frontend_listeners_test.go b/pkg/appgw/frontend_listeners_test.go index 7f2520ce3..8574eae62 100644 --- a/pkg/appgw/frontend_listeners_test.go +++ b/pkg/appgw/frontend_listeners_test.go @@ -181,7 +181,8 @@ var _ = Describe("MutateAppGateway ingress rules and parse frontend listener con DefaultHTTPSettingsID: to.StringPtr("yy"), } - listener, port, err := cb.newListener(cbCtx, listenerID80, n.ApplicationGatewayProtocol("Https")) + ports := make(map[Port]n.ApplicationGatewayFrontendPort) + listener, port, err := cb.newListener(cbCtx, listenerID80, n.ApplicationGatewayProtocol("Https"), ports) Expect(err).ToNot(HaveOccurred()) expectedListener80.ApplicationGatewayHTTPListenerPropertiesFormat.Protocol = n.ApplicationGatewayProtocol("Https") @@ -216,7 +217,8 @@ var _ = Describe("MutateAppGateway ingress rules and parse frontend listener con listenerID := listenerIdentifier{80, "bye.com", true} listenerAzConfig, exists := listenerConfigs[listenerID] Expect(exists).To(BeTrue()) - listener, port, err := cb.newListener(cbCtx, listenerID, listenerAzConfig.Protocol) + ports := make(map[Port]n.ApplicationGatewayFrontendPort) + listener, port, err := cb.newListener(cbCtx, listenerID, listenerAzConfig.Protocol, ports) Expect(err).ToNot(HaveOccurred()) Expect(*listener.FrontendIPConfiguration.ID).To(Equal(tests.PrivateIPID)) Expect(*port).To(Equal(expectedPort80)) @@ -226,7 +228,8 @@ var _ = Describe("MutateAppGateway ingress rules and parse frontend listener con listenerID := listenerIdentifier{443, "bye.com", true} listenerAzConfig, exists := listenerConfigs[listenerID] Expect(exists).To(BeTrue()) - listener, port, err := cb.newListener(cbCtx, listenerID, listenerAzConfig.Protocol) + ports := make(map[Port]n.ApplicationGatewayFrontendPort) + listener, port, err := cb.newListener(cbCtx, listenerID, listenerAzConfig.Protocol, ports) Expect(err).ToNot(HaveOccurred()) Expect(*listener.FrontendIPConfiguration.ID).To(Equal(tests.PrivateIPID)) Expect(*port).To(Equal(expectedPort443)) @@ -250,8 +253,8 @@ var _ = Describe("MutateAppGateway ingress rules and parse frontend listener con listeners, ports := cb.getListeners(cbCtx) Expect(len(*listeners)).To(Equal(2)) Expect(len(*ports)).To(Equal(2)) - - listener, port, err := cb.newListener(cbCtx, listenerID80Priv, n.ApplicationGatewayProtocol("Http")) + portsByNumber := make(map[Port]n.ApplicationGatewayFrontendPort) + listener, port, err := cb.newListener(cbCtx, listenerID80Priv, n.ApplicationGatewayProtocol("Http"), portsByNumber) Expect(err).ToNot(HaveOccurred()) Expect(*listener).To(Equal(expectedListener80Priv)) Expect(*port).To(Equal(expectedPort80)) @@ -350,23 +353,27 @@ var _ = Describe("MutateAppGateway ingress rules and parse frontend listener con } It("should create listener with RequireServerNameIndication when (https, hostname) listener", func() { - listener, _, _ := cb.newListener(cbCtx, listenerID80, n.ApplicationGatewayProtocol("Https")) + ports := make(map[Port]n.ApplicationGatewayFrontendPort) + listener, _, _ := cb.newListener(cbCtx, listenerID80, n.ApplicationGatewayProtocol("Https"), ports) Expect(*listener.RequireServerNameIndication).To(BeTrue()) }) It("should not create listener with RequireServerNameIndication when (https, no hostname) listener", func() { - listener, _, _ := cb.newListener(cbCtx, listenerID80WithoutHostname, n.ApplicationGatewayProtocol("Https")) + ports := make(map[Port]n.ApplicationGatewayFrontendPort) + listener, _, _ := cb.newListener(cbCtx, listenerID80WithoutHostname, n.ApplicationGatewayProtocol("Https"), ports) Expect(len(*listener.HostName)).To(Equal(0)) Expect(listener.RequireServerNameIndication).To(BeNil()) }) It("should not create listener with RequireServerNameIndication when (http, hostname) listener", func() { - listener, _, _ := cb.newListener(cbCtx, listenerID80, n.ApplicationGatewayProtocol("Http")) + ports := make(map[Port]n.ApplicationGatewayFrontendPort) + listener, _, _ := cb.newListener(cbCtx, listenerID80, n.ApplicationGatewayProtocol("Http"), ports) Expect(listener.RequireServerNameIndication).To(BeNil()) }) It("should not create listener with RequireServerNameIndication when (http, no hostname) listener", func() { - listener, _, _ := cb.newListener(cbCtx, listenerID80WithoutHostname, n.ApplicationGatewayProtocol("Http")) + ports := make(map[Port]n.ApplicationGatewayFrontendPort) + listener, _, _ := cb.newListener(cbCtx, listenerID80WithoutHostname, n.ApplicationGatewayProtocol("Http"), ports) Expect(len(*listener.HostName)).To(Equal(0)) Expect(listener.RequireServerNameIndication).To(BeNil()) }) diff --git a/pkg/appgw/types.go b/pkg/appgw/types.go index 765ec0021..8c5dc52ab 100644 --- a/pkg/appgw/types.go +++ b/pkg/appgw/types.go @@ -7,6 +7,7 @@ package appgw import ( "github.com/Azure/application-gateway-kubernetes-ingress/pkg/environment" + n "github.com/Azure/azure-sdk-for-go/services/network/mgmt/2019-06-01/network" "github.com/knative/pkg/apis/istio/v1alpha3" v1 "k8s.io/api/core/v1" "k8s.io/api/extensions/v1beta1" @@ -26,6 +27,8 @@ type ConfigBuilderContext struct { DefaultAddressPoolID *string DefaultHTTPSettingsID *string + + ExistingPortsByNumber map[Port]n.ApplicationGatewayFrontendPort } // InIngressList returns true if an ingress is in the ingress list diff --git a/pkg/brownfield/listeners.go b/pkg/brownfield/listeners.go index d699dc44d..c77a6bce5 100644 --- a/pkg/brownfield/listeners.go +++ b/pkg/brownfield/listeners.go @@ -46,12 +46,17 @@ func MergeListeners(listenerBuckets ...[]n.ApplicationGatewayHTTPListener) []n.A for _, bucket := range listenerBuckets { for _, listener := range bucket { listenerConfig := uniqueListenerConfig{ - HostName: *listener.HostName, - Protocol: listener.Protocol, - FrontendIPConfiguration: *listener.FrontendIPConfiguration.ID, - FrontendPortID: *listener.FrontendPort.ID, + Protocol: listener.Protocol, + } + if listener.HostName != nil { + listenerConfig.HostName = *listener.HostName + } + if listener.FrontendIPConfiguration != nil && listener.FrontendIPConfiguration.ID != nil { + listenerConfig.FrontendIPConfiguration = *listener.FrontendIPConfiguration.ID + } + if listener.FrontendPort != nil && listener.FrontendPort.ID != nil { + listenerConfig.FrontendPortID = *listener.FrontendPort.ID } - if _, exists := uniq[listenerConfig]; !exists { uniq[listenerConfig] = listener } diff --git a/pkg/brownfield/ports.go b/pkg/brownfield/ports.go deleted file mode 100644 index e3bf372ae..000000000 --- a/pkg/brownfield/ports.go +++ /dev/null @@ -1,109 +0,0 @@ -// ------------------------------------------------------------------------------------------- -// Copyright (c) Microsoft Corporation. All rights reserved. -// Licensed under the MIT License. See License.txt in the project root for license information. -// -------------------------------------------------------------------------------------------- - -package brownfield - -import ( - "strings" - - "github.com/Azure/application-gateway-kubernetes-ingress/pkg/utils" - - n "github.com/Azure/azure-sdk-for-go/services/network/mgmt/2019-06-01/network" - "github.com/golang/glog" -) - -type portName string -type portsByPortNumber map[int32]n.ApplicationGatewayFrontendPort - -// GetBlacklistedPorts filters the given list of ports to the list of ports that AGIC is allowed to manage. -func (er ExistingResources) GetBlacklistedPorts() ([]n.ApplicationGatewayFrontendPort, []n.ApplicationGatewayFrontendPort) { - - blacklistedPortSet := er.getBlacklistedPortsSet() - - var nonBlacklistedPorts []n.ApplicationGatewayFrontendPort - var blacklistedPorts []n.ApplicationGatewayFrontendPort - for _, port := range er.Ports { - portJSON, _ := port.MarshalJSON() - // Is the port associated with a blacklisted listener? - if _, isBlacklisted := blacklistedPortSet[portName(*port.Name)]; isBlacklisted { - glog.V(5).Infof("[brownfield] Port %s is blacklisted: %s", portName(*port.Name), portJSON) - blacklistedPorts = append(blacklistedPorts, port) - continue - } - glog.V(5).Infof("[brownfield] Port %s is not blacklisted: %s", portName(*port.Name), portJSON) - nonBlacklistedPorts = append(nonBlacklistedPorts, port) - } - return blacklistedPorts, nonBlacklistedPorts -} - -// MergePorts merges list of lists of ports into a single list, maintaining uniqueness. -func MergePorts(portBuckets ...[]n.ApplicationGatewayFrontendPort) []n.ApplicationGatewayFrontendPort { - uniq := make(portsByPortNumber) - for _, bucket := range portBuckets { - for _, port := range bucket { - // Add the port from the list only when it is missing, otherwise use the existing one. - if _, exists := uniq[*port.Port]; !exists { - uniq[*port.Port] = port - } - } - } - var merged []n.ApplicationGatewayFrontendPort - for _, port := range uniq { - merged = append(merged, port) - } - return merged -} - -// LogPorts emits a few log lines detailing what Ports are created, blacklisted, and removed from ARM. -func LogPorts(existingBlacklisted []n.ApplicationGatewayFrontendPort, existingNonBlacklisted []n.ApplicationGatewayFrontendPort, managedPorts []n.ApplicationGatewayFrontendPort) { - var garbage []n.ApplicationGatewayFrontendPort - - blacklistedSet := indexPortsByPortNumber(existingBlacklisted) - managedSet := indexPortsByPortNumber(managedPorts) - - for portNumber, port := range indexPortsByPortNumber(existingNonBlacklisted) { - _, existsInBlacklist := blacklistedSet[portNumber] - _, existsInNewPorts := managedSet[portNumber] - if !existsInBlacklist && !existsInNewPorts { - garbage = append(garbage, port) - } - } - - glog.V(3).Info("[brownfield] Ports AGIC created: ", getPortNames(managedPorts)) - glog.V(3).Info("[brownfield] Existing Blacklisted Ports AGIC will retain: ", getPortNames(existingBlacklisted)) - glog.V(3).Info("[brownfield] Existing Ports AGIC will remove: ", getPortNames(garbage)) -} - -func getPortNames(port []n.ApplicationGatewayFrontendPort) string { - var names []string - for _, p := range port { - names = append(names, *p.Name) - } - if len(names) == 0 { - return "n/a" - } - return strings.Join(names, ", ") -} - -func indexPortsByPortNumber(ports []n.ApplicationGatewayFrontendPort) portsByPortNumber { - indexed := make(portsByPortNumber) - for _, port := range ports { - indexed[*port.Port] = port - } - return indexed -} - -func (er ExistingResources) getBlacklistedPortsSet() map[portName]interface{} { - // Get the list of blacklisted listeners, from which we can determine what ports should be blacklisted. - existingBlacklistedListeners, _ := er.GetBlacklistedListeners() - blacklistedPortSet := make(map[portName]interface{}) - for _, listener := range existingBlacklistedListeners { - if listener.FrontendPort != nil && listener.FrontendPort.ID != nil { - portName := portName(utils.GetLastChunkOfSlashed(*listener.FrontendPort.ID)) - blacklistedPortSet[portName] = nil - } - } - return blacklistedPortSet -} diff --git a/pkg/brownfield/ports_test.go b/pkg/brownfield/ports_test.go deleted file mode 100644 index 6a9649d93..000000000 --- a/pkg/brownfield/ports_test.go +++ /dev/null @@ -1,27 +0,0 @@ -// ------------------------------------------------------------------------------------------- -// Copyright (c) Microsoft Corporation. All rights reserved. -// Licensed under the MIT License. See License.txt in the project root for license information. -// -------------------------------------------------------------------------------------------- - -package brownfield - -import ( - . "github.com/onsi/ginkgo" - . "github.com/onsi/gomega" - - "github.com/Azure/application-gateway-kubernetes-ingress/pkg/tests/fixtures" -) - -var _ = Describe("Test blacklisting ports", func() { - - appGw := fixtures.GetAppGateway() - - Context("Test getBlacklistedPortsSet()", func() { - It("should create a set of blacklisted ports", func() { - prohibitedTargets := fixtures.GetAzureIngressProhibitedTargets() - er := NewExistingResources(appGw, prohibitedTargets, nil) - set := er.getBlacklistedPortsSet() - Expect(len(set)).To(Equal(1)) - }) - }) -}) diff --git a/pkg/controller/mutate_app_gateway.go b/pkg/controller/mutate_app_gateway.go index 1f642b9ec..69d46be6f 100644 --- a/pkg/controller/mutate_app_gateway.go +++ b/pkg/controller/mutate_app_gateway.go @@ -45,6 +45,12 @@ func (c AppGwIngressController) getAppGw() (*n.ApplicationGateway, *appgw.Config DefaultAddressPoolID: to.StringPtr(c.appGwIdentifier.AddressPoolID(appgw.DefaultBackendAddressPoolName)), DefaultHTTPSettingsID: to.StringPtr(c.appGwIdentifier.HTTPSettingsID(appgw.DefaultBackendHTTPSettingsName)), + + ExistingPortsByNumber: make(map[appgw.Port]n.ApplicationGatewayFrontendPort), + } + + for _, port := range *appGw.FrontendPorts { + cbCtx.ExistingPortsByNumber[appgw.Port(*port.Port)] = port } return &appGw, cbCtx, nil diff --git a/pkg/tests/fixtures/ports.go b/pkg/tests/fixtures/ports.go index d273ea172..b7b69d3b4 100644 --- a/pkg/tests/fixtures/ports.go +++ b/pkg/tests/fixtures/ports.go @@ -1,6 +1,8 @@ package fixtures import ( + "fmt" + n "github.com/Azure/azure-sdk-for-go/services/network/mgmt/2019-06-01/network" "github.com/Azure/go-autorest/autorest/to" ) @@ -13,6 +15,23 @@ const ( // GetDefaultPort creates a struct used for unit testing. func GetDefaultPort() n.ApplicationGatewayFrontendPort { return n.ApplicationGatewayFrontendPort{ + Etag: to.StringPtr("*"), Name: to.StringPtr(DefaultPortName), + ID: to.StringPtr("/subscriptions/--subscription--/resourceGroups/--resource-group--/providers/Microsoft.Network/applicationGateways/--app-gw-name--/frontendPorts/fp-80"), + ApplicationGatewayFrontendPortPropertiesFormat: &n.ApplicationGatewayFrontendPortPropertiesFormat{ + Port: to.Int32Ptr(int32(80)), + }, + } +} + +// GetPort creates a struct used for unit testing. +func GetPort(portNo int32) n.ApplicationGatewayFrontendPort { + return n.ApplicationGatewayFrontendPort{ + Etag: to.StringPtr("*"), + Name: to.StringPtr(fmt.Sprintf("fp-%d", portNo)), + ID: to.StringPtr(fmt.Sprintf("/subscriptions/--subscription--/resourceGroups/--resource-group--/providers/Microsoft.Network/applicationGateways/--app-gw-name--/frontendPorts/fp-%d", portNo)), + ApplicationGatewayFrontendPortPropertiesFormat: &n.ApplicationGatewayFrontendPortPropertiesFormat{ + Port: to.Int32Ptr(portNo), + }, } }