Skip to content

Commit 2ae6096

Browse files
lambertpandianlambertpandian
authored
feat: Track SSL policy source to prevent LB recreation on CLI changes (#774)
- Add HasSSLPolicyAnnotation field to distinguish annotation-based SSL policy from CLI defaults - Only enforce SSL policy matching for shared LBs when ingress has explicit annotation - Add SSL policy to inSync() check to trigger updates instead of new LB creation - Update load balancer SSL policy when adding ingress to allow CLI changes to propagate - Add comprehensive unit tests for SSL policy annotation detection and matching logic Fixes issue where changing --ssl-policy command-line flag would create new load balancers instead of updating existing ones. Now only ingresses with explicit SSL policy annotations will create separate load balancers, while CLI changes will update existing LBs. Fixes #772" Signed-off-by: Lambert Pandian <lambert_pandian@trimble.com>
1 parent 956e5d2 commit 2ae6096

File tree

5 files changed

+355
-79
lines changed

5 files changed

+355
-79
lines changed

‎kubernetes/adapter.go‎

Lines changed: 28 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -93,12 +93,13 @@ type Ingress struct {
9393
Name string
9494
Shared bool
9595
HTTP2 bool
96-
ClusterLocal bool
97-
CertificateARN string
98-
Hostname string
99-
Scheme string
100-
SecurityGroup string
101-
SSLPolicy string
96+
ClusterLocal bool
97+
CertificateARN string
98+
Hostname string
99+
Scheme string
100+
SecurityGroup string
101+
SSLPolicy string
102+
HasSSLPolicyAnnotation bool
102103
IPAddressType string
103104
LoadBalancerType string
104105
WAFWebACLID string
@@ -205,8 +206,13 @@ func (a *Adapter) newIngress(typ IngressType, metadata kubeItemMetadata, host st
205206
ipAddressType := getAnnotationsString(annotations, ingressALBIPAddressType, a.ingressIpAddressType)
206207

207208
sslPolicy := getAnnotationsString(annotations, ingressSSLPolicyAnnotation, a.ingressDefaultSSLPolicy)
209+
hasSSLPolicyAnnotation := false
210+
if _, ok := annotations[ingressSSLPolicyAnnotation]; ok {
211+
hasSSLPolicyAnnotation = true
212+
}
208213
if _, ok := aws.SSLPolicies[sslPolicy]; !ok {
209214
sslPolicy = a.ingressDefaultSSLPolicy
215+
hasSSLPolicyAnnotation = false
210216
}
211217

212218
loadBalancerType, hasLB := annotations[ingressLoadBalancerTypeAnnotation]
@@ -248,21 +254,22 @@ func (a *Adapter) newIngress(typ IngressType, metadata kubeItemMetadata, host st
248254
}
249255

250256
return &Ingress{
251-
ResourceType: typ,
252-
Namespace: metadata.Namespace,
253-
Name: metadata.Name,
254-
Hostname: host,
255-
Hostnames: hostnames,
256-
ClusterLocal: len(hostnames) < 1,
257-
CertificateARN: getAnnotationsString(annotations, ingressCertificateARNAnnotation, ""),
258-
Scheme: string(scheme),
259-
Shared: shared,
260-
SecurityGroup: securityGroup,
261-
SSLPolicy: sslPolicy,
262-
IPAddressType: ipAddressType,
263-
LoadBalancerType: loadBalancerType,
264-
WAFWebACLID: wafWebAclId,
265-
HTTP2: http2,
257+
ResourceType: typ,
258+
Namespace: metadata.Namespace,
259+
Name: metadata.Name,
260+
Hostname: host,
261+
Hostnames: hostnames,
262+
ClusterLocal: len(hostnames) < 1,
263+
CertificateARN: getAnnotationsString(annotations, ingressCertificateARNAnnotation, ""),
264+
Scheme: string(scheme),
265+
Shared: shared,
266+
SecurityGroup: securityGroup,
267+
SSLPolicy: sslPolicy,
268+
HasSSLPolicyAnnotation: hasSSLPolicyAnnotation,
269+
IPAddressType: ipAddressType,
270+
LoadBalancerType: loadBalancerType,
271+
WAFWebACLID: wafWebAclId,
272+
HTTP2: http2,
266273
}, nil
267274
}
268275

‎kubernetes/adapter_ssl_policy_test.go‎

Lines changed: 86 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,86 @@
1+
package kubernetes
2+
3+
import (
4+
"testing"
5+
6+
"github.com/stretchr/testify/assert"
7+
"github.com/zalando-incubator/kube-ingress-aws-controller/aws"
8+
)
9+
10+
func TestHasSSLPolicyAnnotation(t *testing.T) {
11+
defaultSSLPolicy := "ELBSecurityPolicy-2016-08"
12+
customSSLPolicy := "ELBSecurityPolicy-TLS-1-2-2017-01"
13+
14+
tests := []struct {
15+
name string
16+
annotations map[string]string
17+
expectedSSLPolicy string
18+
expectedHasAnnotation bool
19+
}{
20+
{
21+
name: "no SSL policy annotation - uses default",
22+
annotations: map[string]string{},
23+
expectedSSLPolicy: defaultSSLPolicy,
24+
expectedHasAnnotation: false,
25+
},
26+
{
27+
name: "explicit SSL policy annotation",
28+
annotations: map[string]string{
29+
ingressSSLPolicyAnnotation: customSSLPolicy,
30+
},
31+
expectedSSLPolicy: customSSLPolicy,
32+
expectedHasAnnotation: true,
33+
},
34+
{
35+
name: "invalid SSL policy annotation - falls back to default",
36+
annotations: map[string]string{
37+
ingressSSLPolicyAnnotation: "InvalidPolicy",
38+
},
39+
expectedSSLPolicy: defaultSSLPolicy,
40+
expectedHasAnnotation: false,
41+
},
42+
{
43+
name: "valid SSL policy same as default - still marked as annotation",
44+
annotations: map[string]string{
45+
ingressSSLPolicyAnnotation: defaultSSLPolicy,
46+
},
47+
expectedSSLPolicy: defaultSSLPolicy,
48+
expectedHasAnnotation: true,
49+
},
50+
}
51+
52+
for _, tt := range tests {
53+
t.Run(tt.name, func(t *testing.T) {
54+
adapter, err := NewAdapter(
55+
testConfig,
56+
IngressAPIVersionNetworking,
57+
[]string{},
58+
"sg-123",
59+
defaultSSLPolicy,
60+
aws.LoadBalancerTypeApplication,
61+
"",
62+
aws.IPAddressTypeIPV4,
63+
false,
64+
)
65+
assert.NoError(t, err)
66+
67+
kubeIngress := &ingress{
68+
Metadata: kubeItemMetadata{
69+
Namespace: "default",
70+
Name: "test-ingress",
71+
Annotations: tt.annotations,
72+
},
73+
Spec: ingressSpec{
74+
Rules: []ingressItemRule{
75+
{Host: "example.com"},
76+
},
77+
},
78+
}
79+
80+
result, err := adapter.newIngressFromKube(kubeIngress)
81+
assert.NoError(t, err)
82+
assert.Equal(t, tt.expectedSSLPolicy, result.SSLPolicy, "SSL policy mismatch")
83+
assert.Equal(t, tt.expectedHasAnnotation, result.HasSSLPolicyAnnotation, "HasSSLPolicyAnnotation mismatch")
84+
})
85+
}
86+
}

‎kubernetes/adapter_test.go‎

Lines changed: 60 additions & 56 deletions
Original file line numberDiff line numberDiff line change
@@ -38,20 +38,21 @@ func TestNewIngressFromKube(tt *testing.T) {
3838
msg: "test parsing a simple ingress object",
3939
defaultLoadBalancerType: aws.LoadBalancerTypeApplication,
4040
ingress: &Ingress{
41-
Namespace: "default",
42-
Name: "foo",
43-
Hostname: "bar",
44-
Scheme: "internal",
45-
CertificateARN: "zbr",
46-
Shared: true,
47-
HTTP2: true,
48-
Hostnames: []string{"domain.example.org"},
49-
SecurityGroup: testSecurityGroup,
50-
SSLPolicy: testSSLPolicy,
51-
IPAddressType: testIPAddressTypeDefault,
52-
LoadBalancerType: aws.LoadBalancerTypeApplication,
53-
ResourceType: TypeIngress,
54-
WAFWebACLID: testWAFWebACLID,
41+
Namespace: "default",
42+
Name: "foo",
43+
Hostname: "bar",
44+
Scheme: "internal",
45+
CertificateARN: "zbr",
46+
Shared: true,
47+
HTTP2: true,
48+
Hostnames: []string{"domain.example.org"},
49+
SecurityGroup: testSecurityGroup,
50+
SSLPolicy: testSSLPolicy,
51+
HasSSLPolicyAnnotation: true,
52+
IPAddressType: testIPAddressTypeDefault,
53+
LoadBalancerType: aws.LoadBalancerTypeApplication,
54+
ResourceType: TypeIngress,
55+
WAFWebACLID: testWAFWebACLID,
5556
},
5657
kubeIngress: &ingress{
5758
Metadata: kubeItemMetadata{
@@ -90,20 +91,21 @@ func TestNewIngressFromKube(tt *testing.T) {
9091
msg: "test parsing an ingress object with cluster.local domain",
9192
defaultLoadBalancerType: aws.LoadBalancerTypeApplication,
9293
ingress: &Ingress{
93-
Namespace: "default",
94-
Name: "foo",
95-
Hostname: "bar",
96-
Scheme: "internal",
97-
CertificateARN: "zbr",
98-
Shared: true,
99-
HTTP2: true,
100-
ClusterLocal: true,
101-
SecurityGroup: testSecurityGroup,
102-
SSLPolicy: testSSLPolicy,
103-
IPAddressType: testIPAddressTypeDefault,
104-
LoadBalancerType: aws.LoadBalancerTypeApplication,
105-
ResourceType: TypeIngress,
106-
WAFWebACLID: testWAFWebACLID,
94+
Namespace: "default",
95+
Name: "foo",
96+
Hostname: "bar",
97+
Scheme: "internal",
98+
CertificateARN: "zbr",
99+
Shared: true,
100+
HTTP2: true,
101+
ClusterLocal: true,
102+
SecurityGroup: testSecurityGroup,
103+
SSLPolicy: testSSLPolicy,
104+
HasSSLPolicyAnnotation: true,
105+
IPAddressType: testIPAddressTypeDefault,
106+
LoadBalancerType: aws.LoadBalancerTypeApplication,
107+
ResourceType: TypeIngress,
108+
WAFWebACLID: testWAFWebACLID,
107109
},
108110
kubeIngress: &ingress{
109111
Metadata: kubeItemMetadata{
@@ -142,20 +144,21 @@ func TestNewIngressFromKube(tt *testing.T) {
142144
msg: "test parsing an ingress object with shared=false,h2-enabled=false annotations",
143145
defaultLoadBalancerType: aws.LoadBalancerTypeApplication,
144146
ingress: &Ingress{
145-
Namespace: "default",
146-
Name: "foo",
147-
Hostname: "bar",
148-
Scheme: "internal",
149-
CertificateARN: "zbr",
150-
Shared: false,
151-
HTTP2: false,
152-
ClusterLocal: true,
153-
SecurityGroup: testSecurityGroup,
154-
SSLPolicy: testSSLPolicy,
155-
IPAddressType: testIPAddressTypeDefault,
156-
LoadBalancerType: aws.LoadBalancerTypeApplication,
157-
ResourceType: TypeIngress,
158-
WAFWebACLID: testWAFWebACLID,
147+
Namespace: "default",
148+
Name: "foo",
149+
Hostname: "bar",
150+
Scheme: "internal",
151+
CertificateARN: "zbr",
152+
Shared: false,
153+
HTTP2: false,
154+
ClusterLocal: true,
155+
SecurityGroup: testSecurityGroup,
156+
SSLPolicy: testSSLPolicy,
157+
HasSSLPolicyAnnotation: true,
158+
IPAddressType: testIPAddressTypeDefault,
159+
LoadBalancerType: aws.LoadBalancerTypeApplication,
160+
ResourceType: TypeIngress,
161+
WAFWebACLID: testWAFWebACLID,
159162
},
160163
kubeIngress: &ingress{
161164
Metadata: kubeItemMetadata{
@@ -187,20 +190,21 @@ func TestNewIngressFromKube(tt *testing.T) {
187190
msg: "test parsing an ingress object with dualstack annotation",
188191
defaultLoadBalancerType: aws.LoadBalancerTypeApplication,
189192
ingress: &Ingress{
190-
Namespace: "default",
191-
Name: "foo",
192-
Hostname: "bar",
193-
Scheme: "internal",
194-
CertificateARN: "zbr",
195-
Shared: true,
196-
HTTP2: true,
197-
ClusterLocal: true,
198-
SecurityGroup: testSecurityGroup,
199-
SSLPolicy: testSSLPolicy,
200-
IPAddressType: testIPAddressTypeDualStack,
201-
LoadBalancerType: aws.LoadBalancerTypeApplication,
202-
ResourceType: TypeIngress,
203-
WAFWebACLID: testWAFWebACLID,
193+
Namespace: "default",
194+
Name: "foo",
195+
Hostname: "bar",
196+
Scheme: "internal",
197+
CertificateARN: "zbr",
198+
Shared: true,
199+
HTTP2: true,
200+
ClusterLocal: true,
201+
SecurityGroup: testSecurityGroup,
202+
SSLPolicy: testSSLPolicy,
203+
HasSSLPolicyAnnotation: true,
204+
IPAddressType: testIPAddressTypeDualStack,
205+
LoadBalancerType: aws.LoadBalancerTypeApplication,
206+
ResourceType: TypeIngress,
207+
WAFWebACLID: testWAFWebACLID,
204208
},
205209
kubeIngress: &ingress{
206210
Metadata: kubeItemMetadata{

‎worker.go‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -88,7 +88,8 @@ func (l *loadBalancer) Status() int {
8888
func (l *loadBalancer) inSync() bool {
8989
return reflect.DeepEqual(l.CertificateARNs(), l.stack.CertificateARNs) &&
9090
l.stack.CWAlarmConfigHash == l.cwAlarms.Hash() &&
91-
l.wafWebACLID == l.stack.WAFWebACLID
91+
l.wafWebACLID == l.stack.WAFWebACLID &&
92+
l.sslPolicy == l.stack.SSLPolicy
9293
}
9394

9495
// addIngress adds an ingress object to the load balancer.
@@ -132,7 +133,7 @@ func (l *loadBalancer) addIngress(certificateARNs []string, ingress *kubernetes.
132133
// settings that can be changed on an existing load balancer if it's
133134
// NOT shared.
134135
if ingress.Shared && (l.securityGroup != ingress.SecurityGroup ||
135-
l.sslPolicy != ingress.SSLPolicy ||
136+
(ingress.HasSSLPolicyAnnotation && l.sslPolicy != ingress.SSLPolicy) ||
136137
l.wafWebACLID != ingress.WAFWebACLID) {
137138
return false
138139
}
@@ -158,6 +159,7 @@ func (l *loadBalancer) addIngress(certificateARNs []string, ingress *kubernetes.
158159
}
159160

160161
l.shared = ingress.Shared
162+
l.sslPolicy = ingress.SSLPolicy
161163
return true
162164
}
163165

0 commit comments

Comments
 (0)