Skip to content
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

Replace NLBs on subnets change #610

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

Replace NLBs on subnets change #610

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
46b80d9
Replace NLBs on subnets change
mikkeloscar Jun 9, 2023
bc8d081
use slices.Equal for subnet comparison
mikkeloscar Jun 12, 2023
fceb0ed
Check if LB has a stack
mikkeloscar Jun 14, 2023
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
7 changes: 7 additions & 0 deletions aws/cf.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ type Stack struct {
WAFWebACLID string
CertificateARNs map[string]time.Time
tags map[string]string
Subnets []string
}

// IsComplete returns true if the stack status is a complete state.
Expand Down Expand Up @@ -480,6 +481,11 @@ func mapToManagedStack(stack *cloudformation.Stack) *Stack {
http2 = false
}

var subnets []string
if parameters[parameterLoadBalancerSubnetsParameter] != "" {
subnets = strings.Split(parameters[parameterLoadBalancerSubnetsParameter], ",")
}

return &Stack{
Name: aws.StringValue(stack.StackName),
DNSName: outputs.dnsName(),
Expand All @@ -497,6 +503,7 @@ func mapToManagedStack(stack *cloudformation.Stack) *Stack {
statusReason: aws.StringValue(stack.StackStatusReason),
CWAlarmConfigHash: tags[cwAlarmConfigHashTag],
WAFWebACLID: parameters[parameterLoadBalancerWAFWebACLIDParameter],
Subnets: subnets,
}
}

Expand Down
3 changes: 1 addition & 2 deletions go.mod
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ require (
github.com/sirupsen/logrus v1.9.3
github.com/stretchr/testify v1.9.0
github.com/zalando/skipper v0.21.54
golang.org/x/exp v0.0.0-20240416160154-fe59bbe5cc7f
k8s.io/api v0.28.9
k8s.io/apimachinery v0.28.9
k8s.io/client-go v0.28.9
Expand Down Expand Up @@ -70,14 +71,12 @@ require (
go.uber.org/atomic v1.11.0 // indirect
go4.org/netipx v0.0.0-20231129151722-fdeea329fbba // indirect
golang.org/x/crypto v0.22.0 // indirect
golang.org/x/exp v0.0.0-20231006140011-7918f672742d // indirect
golang.org/x/net v0.24.0 // indirect
golang.org/x/oauth2 v0.19.0 // indirect
golang.org/x/sys v0.19.0 // indirect
golang.org/x/term v0.19.0 // indirect
golang.org/x/text v0.14.0 // indirect
golang.org/x/time v0.5.0 // indirect
golang.org/x/tools v0.20.0 // indirect
google.golang.org/genproto/googleapis/api v0.0.0-20240415180920-8c6c420018be // indirect
google.golang.org/genproto/googleapis/rpc v0.0.0-20240415180920-8c6c420018be // indirect
google.golang.org/grpc v1.63.2 // indirect
Expand Down
4 changes: 2 additions & 2 deletions go.sum
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -276,8 +276,8 @@ golang.org/x/crypto v0.0.0-20191011191535-87dc89f01550/go.mod h1:yigFU9vqHzYiE8U
golang.org/x/crypto v0.0.0-20200622213623-75b288015ac9/go.mod h1:LzIPMQfyMNhhGPhUkYOs5KpL4U8rLKemX1yGLhDgUto=
golang.org/x/crypto v0.22.0 h1:g1v0xeRhjcugydODzvb3mEM9SQ0HGp9s/nh3COQ/C30=
golang.org/x/crypto v0.22.0/go.mod h1:vr6Su+7cTlO45qkww3VDJlzDn0ctJvRgYbC2NvXHt+M=
golang.org/x/exp v0.0.0-20231006140011-7918f672742d h1:jtJma62tbqLibJ5sFQz8bKtEM8rJBtfilJ2qTU199MI=
golang.org/x/exp v0.0.0-20231006140011-7918f672742d/go.mod h1:ldy0pHrwJyGW56pPQzzkH36rKxoZW1tw7ZJpeKx+hdo=
golang.org/x/exp v0.0.0-20240416160154-fe59bbe5cc7f h1:99ci1mjWVBWwJiEKYY6jWa4d2nTQVIEhZIptnrVb1XY=
golang.org/x/exp v0.0.0-20240416160154-fe59bbe5cc7f/go.mod h1:/lliqkxwWAhPjf5oSOIJup2XcqJaw8RGS6k3TGEc7GI=
golang.org/x/mod v0.2.0/go.mod h1:s0Qsj1ACt9ePp/hMypM3fl4fZqREWJwdYDEqhRiZZUA=
golang.org/x/mod v0.3.0/go.mod h1:s0Qsj1ACt9ePp/hMypM3fl4fZqREWJwdYDEqhRiZZUA=
golang.org/x/mod v0.17.0 h1:zY54UmvipHiNd+pm+m0x9KhZ9hl1/7QNMyxXbc6ICqA=
Expand Down
18 changes: 16 additions & 2 deletions worker.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ import (
"github.com/zalando-incubator/kube-ingress-aws-controller/certs"
"github.com/zalando-incubator/kube-ingress-aws-controller/kubernetes"
"github.com/zalando-incubator/kube-ingress-aws-controller/problem"
"golang.org/x/exp/slices"
)

type loadBalancer struct {
Expand Down Expand Up @@ -333,7 +334,7 @@ func doWork(
awsAdapter.UpdateTargetGroupsAndAutoScalingGroups(stacks, problems)

certs := NewCertificates(certificateSummaries)
model := buildManagedModel(certs, certsPerALB, certTTL, ingresses, stacks, cwAlarms, globalWAFACL)
model := buildManagedModel(certs, certsPerALB, certTTL, ingresses, stacks, cwAlarms, globalWAFACL, awsAdapter.FindLBSubnets)
log.Debugf("Have %d model(s)", len(model))
for _, loadBalancer := range model {
switch loadBalancer.Status() {
Expand Down Expand Up @@ -408,6 +409,7 @@ func matchIngressesToLoadBalancers(
certs CertificatesFinder,
certsPerALB int,
ingresses []*kubernetes.Ingress,
subnetsByScheme func(scheme string) []string,
) []*loadBalancer {
clusterLocalLB := &loadBalancer{
clusterLocal: true,
Expand Down Expand Up @@ -451,6 +453,17 @@ func matchIngressesToLoadBalancers(
continue
}

// Ignore NLBs with a wrong set of subnets
if lb.loadBalancerType == aws.LoadBalancerTypeNetwork && lb.stack != nil {
subnets := subnetsByScheme(lb.scheme)
sort.Strings(subnets)
sort.Strings(lb.stack.Subnets)

if !slices.Equal[[]string](lb.stack.Subnets, subnets) {
continue
}
}

if lb.addIngress(certificateARNs, ingress, certsPerALB) {
added = true
break
Expand Down Expand Up @@ -516,11 +529,12 @@ func buildManagedModel(
stacks []*aws.Stack,
cwAlarms aws.CloudWatchAlarmList,
globalWAFACL string,
subnetsByScheme func(scheme string) []string,
) []*loadBalancer {
sortStacks(stacks)
attachGlobalWAFACL(ingresses, globalWAFACL)
model := getAllLoadBalancers(certs, certTTL, stacks)
model = matchIngressesToLoadBalancers(model, certs, certsPerALB, ingresses)
model = matchIngressesToLoadBalancers(model, certs, certsPerALB, ingresses, subnetsByScheme)
attachCloudWatchAlarms(model, cwAlarms)

return model
Expand Down
46 changes: 45 additions & 1 deletion worker_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,10 +18,12 @@ import (
log "github.com/sirupsen/logrus"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
"github.com/zalando-incubator/kube-ingress-aws-controller/aws"
awsAdapter "github.com/zalando-incubator/kube-ingress-aws-controller/aws"
"github.com/zalando-incubator/kube-ingress-aws-controller/certs"
"github.com/zalando-incubator/kube-ingress-aws-controller/kubernetes"
"github.com/zalando/skipper/dataclients/kubernetes/kubernetestest"
"golang.org/x/exp/slices"
"k8s.io/apimachinery/pkg/util/wait"

"github.com/zalando-incubator/kube-ingress-aws-controller/aws/fake"
Expand Down Expand Up @@ -1179,6 +1181,7 @@ func TestMatchIngressesToLoadbalancers(t *testing.T) {
maxCertsPerLB int
lbs []*loadBalancer
ingresses []*kubernetes.Ingress
subnets []string
validate func(*testing.T, []*loadBalancer)
}{{
title: "only cluster local",
Expand Down Expand Up @@ -1361,6 +1364,37 @@ func TestMatchIngressesToLoadbalancers(t *testing.T) {
require.Equal(t, 1, len(lb.ingresses["foo"]))
}
},
}, {
title: "load balancer with invalid subnets",
ingresses: []*kubernetes.Ingress{{
Name: "foo-ingress",
Hostnames: []string{
"foo.org",
"bar.org",
},
LoadBalancerType: awsAdapter.LoadBalancerTypeNetwork,
Shared: true,
}},
lbs: []*loadBalancer{{
loadBalancerType: awsAdapter.LoadBalancerTypeNetwork,
ingresses: make(map[string][]*kubernetes.Ingress),
stack: &aws.Stack{Subnets: []string{"a", "b", "c"}},
}},
validate: func(t *testing.T, lbs []*loadBalancer) {
require.Equal(t, 3, len(lbs))
for _, lb := range lbs {
if lb.clusterLocal {
continue
}

if lb.stack != nil && slices.Equal[[]string](lb.stack.Subnets, []string{"a", "b", "c"}) {
require.Len(t, lb.ingresses, 0)
} else {
require.Len(t, lb.ingresses, 1)
}
}
},
subnets: []string{"x", "y", "z"},
}} {
t.Run(test.title, func(t *testing.T) {
var certs CertificatesFinder = defaultCerts
Expand All @@ -1373,7 +1407,11 @@ func TestMatchIngressesToLoadbalancers(t *testing.T) {
maxCertsPerLB = test.maxCertsPerLB
}

lbs := matchIngressesToLoadBalancers(test.lbs, certs, maxCertsPerLB, test.ingresses)
subnetsByScheme := func(scheme string) []string {
return test.subnets
}

lbs := matchIngressesToLoadBalancers(test.lbs, certs, maxCertsPerLB, test.ingresses, subnetsByScheme)
test.validate(t, lbs)
})
}
Expand Down Expand Up @@ -1403,6 +1441,7 @@ func TestBuildModel(t *testing.T) {
stacks []*awsAdapter.Stack
alarms awsAdapter.CloudWatchAlarmList
globalWAFACL string
subnets []string
validate func(*testing.T, []*loadBalancer)
}{{
title: "no alarm, no waf",
Expand Down Expand Up @@ -1549,6 +1588,10 @@ func TestBuildModel(t *testing.T) {
maxCertsPerLB = test.maxCertsPerLB
}

subnetsByScheme := func(scheme string) []string {
return test.subnets
}

m := buildManagedModel(
certs,
maxCertsPerLB,
Expand All @@ -1557,6 +1600,7 @@ func TestBuildModel(t *testing.T) {
test.stacks,
test.alarms,
test.globalWAFACL,
subnetsByScheme,
)

test.validate(t, m)
Expand Down
Loading