Skip to content

Commit

Permalink
Fix ELB cleanup failure due to resource name end with a hyphen
Browse files Browse the repository at this point in the history
Fix the incorrect resource name as mentioned in the issue #7028
Refector the getting resource name logic from ELB hostname with unit test
cases
  • Loading branch information
0xlen committed Sep 11, 2023
1 parent 0c2ced9 commit 5bcf25a
Show file tree
Hide file tree
Showing 3 changed files with 83 additions and 11 deletions.
32 changes: 21 additions & 11 deletions pkg/elb/cleanup.go
Original file line number Diff line number Diff line change
Expand Up @@ -199,6 +199,26 @@ func getServiceLoadBalancer(ctx context.Context, ec2API awsapi.EC2, elbAPI Descr
return &lb, nil
}

func getIngressELBName(ctx context.Context, metadataName string, hosts []string) string {
// Expected e.g. bf647c9e-default-appingres-350b-1622159649.eu-central-1.elb.amazonaws.com where AWS ALB name is
// bf647c9e-default-appingres-350b (cannot be longer than 32 characters).
hostNameParts := strings.Split(hosts[0], ".")
if len(hostNameParts[0]) == 0 {
logger.Debug("%s is ALB Ingress, but probably not provisioned or something other unexpected, skip", metadataName)
return ""
}
name := strings.TrimPrefix(hostNameParts[0], "internal-") // Trim 'internal-' prefix for ALB DNS name which is not a part of name.
idIdx := strings.LastIndex(name, "-")
if (idIdx) != -1 {
name = name[:idIdx] // Remove the ELB ID and last hyphen at the end of the hostname (ELB name cannot end with a hyphen)
}
if len(name) > 32 {
name = name[:32]
}
logger.Debug("ALB resource name: %s", name)
return name
}

func getIngressLoadBalancer(ctx context.Context, ec2API awsapi.EC2, elbAPI DescribeLoadBalancersAPI, elbv2API DescribeLoadBalancersAPIV2,
clusterName string, ingress Ingress) (*loadBalancer, error) {
metadata := ingress.GetMetadata()
Expand All @@ -215,17 +235,7 @@ func getIngressLoadBalancer(ctx context.Context, ec2API awsapi.EC2, elbAPI Descr
logger.Debug("%s is ALB Ingress, but probably not provisioned, skip", metadata.Name)
return nil, nil
}
// Expected e.g. bf647c9e-default-appingres-350b-1622159649.eu-central-1.elb.amazonaws.com where AWS ALB name is
// bf647c9e-default-appingres-350b (cannot be longer than 32 characters).
hostNameParts := strings.Split(hosts[0], ".")
if len(hostNameParts[0]) == 0 {
logger.Debug("%s is ALB Ingress, but probably not provisioned or something other unexpected, skip", metadata.Name)
return nil, nil
}
name := strings.TrimPrefix(hostNameParts[0], "internal-") // Trim 'internal-' prefix for ALB DNS name which is not a part of name.
if len(name) > 32 {
name = name[:32]
}
name := getIngressELBName(ctx, metadata.Name, hosts)
ctx, cleanup := context.WithTimeout(ctx, 30*time.Second)
defer cleanup()
securityGroupIDs, err := getSecurityGroupsOwnedByLoadBalancer(ctx, ec2API, elbAPI, elbv2API, clusterName, name, application)
Expand Down
49 changes: 49 additions & 0 deletions pkg/elb/cleanup_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,49 @@
package elb

import (
"context"

. "github.com/onsi/ginkgo/v2"
. "github.com/onsi/gomega"
)

var _ = Describe("ELB Cleanup", func() {
When("Getting ingress load balancers to clean up", func() {
It("should get the right ELB hostname", func() {

testELBDNSnames := [][]string{
{"bf647c9e-default-appingres-350b-1622159649.eu-central-1.elb.amazonaws.com", ""},
{"internal-k8s-default-testcase-93d8419948-541000771.us-west-2.elb.amazonaws.com", ""},
{"abcdefgh-default-bugfixtest-001-1356525548.us-west-2.elb.amazonaws.com", ""},
{"internal-abcdefgh-default-bugfixtest-002-541910110.us-west-2.elb.amazonaws.com", ""},
{"abcdefghijklmnopqrstuvwxyz012345-2118752702.us-west-2.elb.amazonaws.com", ""},
{"internal-abcdefghijklmnopqrstuvwxyz999999-67491582.us-west-2.elb.amazonaws.com", ""},
{"k8s-default-testcase-98cdbf582b-1474733506.us-west-2.elb.amazonaws.com", ""},
{"internal-k8s-default-testcase-fb10378931-824853021.us-west-2.elb.amazonaws.com", ""},
{"abcdefghijklmnopqrstuvw000-1623371943.us-west-2.elb.amazonaws.com", ""},
{"internal-abcdefghijklmnopqrstuvw001-774959707.us-west-2.elb.amazonaws.com", ""},
{"myloadbalancer-1234567890.us-west-2.elb.amazonaws.com", ""},
{"my-loadbalancer-1234567890.us-west-2.elb.amazonaws.com", ""},
}
expectELBName := []string{
"bf647c9e-default-appingres-350b",
"k8s-default-testcase-93d8419948",
"abcdefgh-default-bugfixtest-001",
"abcdefgh-default-bugfixtest-002",
"abcdefghijklmnopqrstuvwxyz012345",
"abcdefghijklmnopqrstuvwxyz999999",
"k8s-default-testcase-98cdbf582b",
"k8s-default-testcase-fb10378931",
"abcdefghijklmnopqrstuvw000",
"abcdefghijklmnopqrstuvw001",
"myloadbalancer",
"my-loadbalancer",
}

for i, elbDNSName := range testELBDNSnames {
name := getIngressELBName(context.TODO(), "", elbDNSName)
Expect(name).To(Equal(expectELBName[i]))
}
})
})
})
13 changes: 13 additions & 0 deletions pkg/elb/elb_suite_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
package elb_test

import (
"testing"

. "github.com/onsi/ginkgo/v2"
. "github.com/onsi/gomega"
)

func TestElb(t *testing.T) {
RegisterFailHandler(Fail)
RunSpecs(t, "Elb Suite")
}

0 comments on commit 5bcf25a

Please sign in to comment.