Skip to content

Commit b275e44

Browse files
ahrakosahrakos
committed
added validation for dummy ports introduced in the aws-load-balancer-ssl-ports annotation
1 parent edb5e99 commit b275e44

File tree

3 files changed

+167
-12
lines changed

3 files changed

+167
-12
lines changed

‎pkg/service/model_build_listener.go‎

Lines changed: 49 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -14,9 +14,13 @@ import (
1414
)
1515

1616
func (t *defaultModelBuildTask) buildListeners(ctx context.Context, scheme elbv2model.LoadBalancerScheme) error {
17-
cfg := t.buildListenerConfig(ctx)
17+
cfg, err := t.buildListenerConfig(ctx)
18+
if err != nil {
19+
return err
20+
}
21+
1822
for _, port := range t.service.Spec.Ports {
19-
_, err := t.buildListener(ctx, port, cfg, scheme)
23+
_, err := t.buildListener(ctx, port, *cfg, scheme)
2024
if err != nil {
2125
return err
2226
}
@@ -115,10 +119,43 @@ func (t *defaultModelBuildTask) buildListenerCertificates(_ context.Context) []e
115119
return certificates
116120
}
117121

118-
func (t *defaultModelBuildTask) buildTLSPortsSet(_ context.Context) sets.String {
122+
func validateTLSPortsSet(rawTLSPorts []string, ports []corev1.ServicePort) error {
123+
unusedPorts := make([]string, 0)
124+
125+
for _, tlsPort := range rawTLSPorts {
126+
isPortUsed := false
127+
for _, portObj := range ports {
128+
if portObj.Name == tlsPort || strconv.Itoa(int(portObj.Port)) == tlsPort {
129+
isPortUsed = true
130+
break
131+
}
132+
}
133+
134+
if !isPortUsed {
135+
unusedPorts = append(unusedPorts, tlsPort)
136+
}
137+
}
138+
139+
if len(unusedPorts) > 0 {
140+
unusedPortErr := errors.Errorf("Unused port in ssl-ports annotation %v", unusedPorts)
141+
return unusedPortErr
142+
}
143+
144+
return nil
145+
}
146+
147+
func (t *defaultModelBuildTask) buildTLSPortsSet(_ context.Context) (sets.String, error) {
119148
var rawTLSPorts []string
149+
120150
_ = t.annotationParser.ParseStringSliceAnnotation(annotations.SvcLBSuffixSSLPorts, &rawTLSPorts, t.service.Annotations)
121-
return sets.NewString(rawTLSPorts...)
151+
152+
err := validateTLSPortsSet(rawTLSPorts, t.service.Spec.Ports)
153+
154+
if err != nil {
155+
return nil, err
156+
}
157+
158+
return sets.NewString(rawTLSPorts...), nil
122159
}
123160

124161
func (t *defaultModelBuildTask) buildBackendProtocol(_ context.Context) string {
@@ -154,18 +191,22 @@ type listenerConfig struct {
154191
backendProtocol string
155192
}
156193

157-
func (t *defaultModelBuildTask) buildListenerConfig(ctx context.Context) listenerConfig {
194+
func (t *defaultModelBuildTask) buildListenerConfig(ctx context.Context) (*listenerConfig, error) {
158195
certificates := t.buildListenerCertificates(ctx)
159-
tlsPortsSet := t.buildTLSPortsSet(ctx)
196+
tlsPortsSet, err := t.buildTLSPortsSet(ctx)
197+
if err != nil {
198+
return nil, err
199+
}
200+
160201
backendProtocol := t.buildBackendProtocol(ctx)
161202
sslPolicy := t.buildSSLNegotiationPolicy(ctx)
162203

163-
return listenerConfig{
204+
return &listenerConfig{
164205
certificates: certificates,
165206
tlsPortsSet: tlsPortsSet,
166207
sslPolicy: sslPolicy,
167208
backendProtocol: backendProtocol,
168-
}
209+
}, nil
169210
}
170211

171212
func (t *defaultModelBuildTask) buildListenerTags(ctx context.Context) (map[string]string, error) {

‎pkg/service/model_build_listener_test.go‎

Lines changed: 94 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,9 +4,12 @@ import (
44
"context"
55
"testing"
66

7+
"github.com/pkg/errors"
78
"github.com/stretchr/testify/assert"
89
corev1 "k8s.io/api/core/v1"
910
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
11+
"k8s.io/apimachinery/pkg/util/intstr"
12+
"k8s.io/apimachinery/pkg/util/sets"
1013
"sigs.k8s.io/aws-load-balancer-controller/pkg/annotations"
1114
elbv2model "sigs.k8s.io/aws-load-balancer-controller/pkg/model/elbv2"
1215
)
@@ -88,3 +91,94 @@ func Test_defaultModelBuilderTask_buildListenerALPNPolicy(t *testing.T) {
8891
})
8992
}
9093
}
94+
95+
func Test_defaultModelBuilderTask_buildListenerConfig(t *testing.T) {
96+
tests := []struct {
97+
name string
98+
svc *corev1.Service
99+
wantErr error
100+
want *listenerConfig
101+
}{
102+
{
103+
name: "Service with unused ports in the ssl-ports annotation, Unused ports provided",
104+
svc: &corev1.Service{
105+
ObjectMeta: metav1.ObjectMeta{
106+
Annotations: map[string]string{
107+
"service.beta.kubernetes.io/aws-load-balancer-ssl-ports": "80, 85, 90, arbitrary-name",
108+
},
109+
},
110+
Spec: corev1.ServiceSpec{
111+
Type: corev1.ServiceTypeLoadBalancer,
112+
Ports: []corev1.ServicePort{
113+
{
114+
Name: "http",
115+
Port: 80,
116+
TargetPort: intstr.FromInt(80),
117+
Protocol: corev1.ProtocolTCP,
118+
NodePort: 31223,
119+
},
120+
{
121+
Name: "alt2",
122+
Port: 83,
123+
TargetPort: intstr.FromInt(8883),
124+
Protocol: corev1.ProtocolTCP,
125+
NodePort: 32323,
126+
},
127+
},
128+
},
129+
},
130+
wantErr: errors.New("Unused port in ssl-ports annotation [85 90 arbitrary-name]"),
131+
},
132+
{
133+
name: "Service with unused ports in the ssl-ports annotation, No unused ports",
134+
svc: &corev1.Service{
135+
ObjectMeta: metav1.ObjectMeta{
136+
Annotations: map[string]string{
137+
"service.beta.kubernetes.io/aws-load-balancer-ssl-ports": "83",
138+
},
139+
},
140+
Spec: corev1.ServiceSpec{
141+
Type: corev1.ServiceTypeLoadBalancer,
142+
Ports: []corev1.ServicePort{
143+
{
144+
Name: "http",
145+
Port: 80,
146+
TargetPort: intstr.FromInt(80),
147+
Protocol: corev1.ProtocolTCP,
148+
NodePort: 31223,
149+
},
150+
{
151+
Name: "alt2",
152+
Port: 83,
153+
TargetPort: intstr.FromInt(8883),
154+
Protocol: corev1.ProtocolTCP,
155+
NodePort: 32323,
156+
},
157+
},
158+
},
159+
},
160+
want: &listenerConfig{
161+
certificates: ([]elbv2model.Certificate)(nil),
162+
tlsPortsSet: sets.NewString("83"),
163+
sslPolicy: new(string),
164+
backendProtocol: "",
165+
},
166+
},
167+
}
168+
169+
for _, tt := range tests {
170+
t.Run(tt.name, func(t *testing.T) {
171+
parser := annotations.NewSuffixAnnotationParser("service.beta.kubernetes.io")
172+
builder := &defaultModelBuildTask{
173+
annotationParser: parser,
174+
service: tt.svc,
175+
}
176+
got, err := builder.buildListenerConfig(context.Background())
177+
if tt.wantErr != nil {
178+
assert.EqualError(t, err, tt.wantErr.Error())
179+
} else {
180+
assert.Equal(t, tt.want, got)
181+
}
182+
})
183+
}
184+
}

‎test/e2e/service/nlb_ip_target_test.go‎

Lines changed: 24 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -217,6 +217,18 @@ var _ = Describe("k8s service reconciled by the aws load balancer", func() {
217217
TargetPort: intstr.FromInt(80),
218218
Protocol: corev1.ProtocolTCP,
219219
},
220+
{
221+
Port: 443,
222+
Name: "https",
223+
TargetPort: intstr.FromInt(443),
224+
Protocol: corev1.ProtocolTCP,
225+
},
226+
{
227+
Port: 333,
228+
Name: "arbitrary-port",
229+
TargetPort: intstr.FromInt(333),
230+
Protocol: corev1.ProtocolTCP,
231+
},
220232
},
221233
},
222234
}
@@ -245,10 +257,14 @@ var _ = Describe("k8s service reconciled by the aws load balancer", func() {
245257
Scheme: "internet-facing",
246258
TargetType: "ip",
247259
Listeners: map[string]string{
248-
"80": "TLS",
260+
"80": "TLS",
261+
"443": "TLS",
262+
"333": "TLS",
249263
},
250264
TargetGroups: map[string]string{
251-
"80": "TCP",
265+
"80": "TCP",
266+
"443": "TCP",
267+
"333": "TCP",
252268
},
253269
NumTargets: int(numReplicas),
254270
})
@@ -272,10 +288,14 @@ var _ = Describe("k8s service reconciled by the aws load balancer", func() {
272288
Scheme: "internet-facing",
273289
TargetType: "ip",
274290
Listeners: map[string]string{
275-
"80": "TCP",
291+
"80": "TCP",
292+
"443": "TLS",
293+
"333": "TLS",
276294
},
277295
TargetGroups: map[string]string{
278-
"80": "TCP",
296+
"80": "TCP",
297+
"443": "TCP",
298+
"333": "TCP",
279299
},
280300
NumTargets: int(numReplicas),
281301
})

0 commit comments

Comments
 (0)