From 9803840b5fd3affce2b54b86ae8e15aee0fda9f5 Mon Sep 17 00:00:00 2001 From: Justin Santa Barbara Date: Mon, 12 Jun 2017 23:30:53 -0400 Subject: [PATCH] AWS: Perform ELB listener comparison in case-insensitive manner Fix #47067 --- pkg/cloudprovider/providers/aws/BUILD | 1 + .../providers/aws/aws_loadbalancer.go | 24 +++- .../providers/aws/aws_loadbalancer_test.go | 127 ++++++++++++++++++ 3 files changed, 149 insertions(+), 3 deletions(-) create mode 100644 pkg/cloudprovider/providers/aws/aws_loadbalancer_test.go diff --git a/pkg/cloudprovider/providers/aws/BUILD b/pkg/cloudprovider/providers/aws/BUILD index eba94c8da41..c9a580329bf 100644 --- a/pkg/cloudprovider/providers/aws/BUILD +++ b/pkg/cloudprovider/providers/aws/BUILD @@ -56,6 +56,7 @@ go_library( go_test( name = "go_default_test", srcs = [ + "aws_loadbalancer_test.go", "aws_test.go", "device_allocator_test.go", "regions_test.go", diff --git a/pkg/cloudprovider/providers/aws/aws_loadbalancer.go b/pkg/cloudprovider/providers/aws/aws_loadbalancer.go index c4409740f82..46e1a05cf21 100644 --- a/pkg/cloudprovider/providers/aws/aws_loadbalancer.go +++ b/pkg/cloudprovider/providers/aws/aws_loadbalancer.go @@ -190,10 +190,10 @@ func (c *Cloud) ensureLoadBalancer(namespacedName types.NamespacedName, loadBala found := -1 for i, expected := range listeners { - if orEmpty(actual.Protocol) != orEmpty(expected.Protocol) { + if elbProtocolsAreEqual(actual.Protocol, expected.Protocol) { continue } - if orEmpty(actual.InstanceProtocol) != orEmpty(expected.InstanceProtocol) { + if elbProtocolsAreEqual(actual.InstanceProtocol, expected.InstanceProtocol) { continue } if orZero(actual.InstancePort) != orZero(expected.InstancePort) { @@ -202,7 +202,7 @@ func (c *Cloud) ensureLoadBalancer(namespacedName types.NamespacedName, loadBala if orZero(actual.LoadBalancerPort) != orZero(expected.LoadBalancerPort) { continue } - if orEmpty(actual.SSLCertificateId) != orEmpty(expected.SSLCertificateId) { + if awsArnEquals(actual.SSLCertificateId, expected.SSLCertificateId) { continue } found = i @@ -355,6 +355,24 @@ func (c *Cloud) ensureLoadBalancer(namespacedName types.NamespacedName, loadBala return loadBalancer, nil } +// elbProtocolsAreEqual checks if two ELB protocol strings are considered the same +// Comparison is case insensitive +func elbProtocolsAreEqual(l, r *string) bool { + if l == nil || r == nil { + return l == r + } + return strings.EqualFold(aws.StringValue(l), aws.StringValue(r)) +} + +// awsArnEquals checks if two ARN strings are considered the same +// Comparison is case insensitive +func awsArnEquals(l, r *string) bool { + if l == nil || r == nil { + return l == r + } + return strings.EqualFold(aws.StringValue(l), aws.StringValue(r)) +} + // Makes sure that the health check for an ELB matches the configured health check node port func (c *Cloud) ensureLoadBalancerHealthCheck(loadBalancer *elb.LoadBalancerDescription, protocol string, port int32, path string) error { name := aws.StringValue(loadBalancer.LoadBalancerName) diff --git a/pkg/cloudprovider/providers/aws/aws_loadbalancer_test.go b/pkg/cloudprovider/providers/aws/aws_loadbalancer_test.go new file mode 100644 index 00000000000..fb1cb12fbdc --- /dev/null +++ b/pkg/cloudprovider/providers/aws/aws_loadbalancer_test.go @@ -0,0 +1,127 @@ +/* +Copyright 2017 The Kubernetes Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package aws + +import ( + "github.com/aws/aws-sdk-go/aws" + "testing" +) + +func TestElbProtocolsAreEqual(t *testing.T) { + grid := []struct { + L *string + R *string + Expected bool + }{ + { + L: aws.String("http"), + R: aws.String("http"), + Expected: true, + }, + { + L: aws.String("HTTP"), + R: aws.String("http"), + Expected: true, + }, + { + L: aws.String("HTTP"), + R: aws.String("TCP"), + Expected: false, + }, + { + L: aws.String(""), + R: aws.String("TCP"), + Expected: false, + }, + { + L: aws.String(""), + R: aws.String(""), + Expected: true, + }, + { + L: nil, + R: aws.String(""), + Expected: false, + }, + { + L: aws.String(""), + R: nil, + Expected: false, + }, + { + L: nil, + R: nil, + Expected: true, + }, + } + for _, g := range grid { + actual := elbProtocolsAreEqual(g.L, g.R) + if actual != g.Expected { + t.Errorf("unexpected result from protocolsEquals(%v, %v)", g.L, g.R) + } + } +} + +func TestAWSARNEquals(t *testing.T) { + grid := []struct { + L *string + R *string + Expected bool + }{ + { + L: aws.String("arn:aws:acm:us-east-1:123456789012:certificate/12345678-1234-1234-1234-123456789012"), + R: aws.String("arn:aws:acm:us-east-1:123456789012:certificate/12345678-1234-1234-1234-123456789012"), + Expected: true, + }, + { + L: aws.String("ARN:AWS:ACM:US-EAST-1:123456789012:CERTIFICATE/12345678-1234-1234-1234-123456789012"), + R: aws.String("arn:aws:acm:us-east-1:123456789012:certificate/12345678-1234-1234-1234-123456789012"), + Expected: true, + }, + { + L: aws.String("arn:aws:acm:us-east-1:123456789012:certificate/12345678-1234-1234-1234-123456789012"), + R: aws.String(""), + Expected: false, + }, + { + L: aws.String(""), + R: aws.String(""), + Expected: true, + }, + { + L: nil, + R: aws.String(""), + Expected: false, + }, + { + L: aws.String(""), + R: nil, + Expected: false, + }, + { + L: nil, + R: nil, + Expected: true, + }, + } + for _, g := range grid { + actual := awsArnEquals(g.L, g.R) + if actual != g.Expected { + t.Errorf("unexpected result from awsArnEquals(%v, %v)", g.L, g.R) + } + } +}