Skip to content

Commit 779825a

Browse files
abeowluabeowlu
authored
Merge branch 'kubernetes-sigs:main' into auto-discovery-cert-list
2 parents 7829ff3 + 5cfeef8 commit 779825a

19 files changed

+1075
-321
lines changed

‎controllers/gateway/gateway_controller.go‎

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -231,12 +231,16 @@ func (r *gatewayReconciler) reconcileHelper(ctx context.Context, req reconcile.R
231231

232232
loaderResults, err := r.gatewayLoader.LoadRoutesForGateway(ctx, *gw, r.routeFilter, r.controllerName, resolvedDefaultTGC)
233233

234-
if err != nil || loaderResults.ValidationResults.HasErrors {
234+
validationHasErrors := false
235+
if loaderResults != nil {
236+
validationHasErrors = loaderResults.ValidationResults.HasErrors()
237+
}
238+
if err != nil || validationHasErrors {
235239
var loaderErr routeutils.LoaderError
236-
if errors.As(err, &loaderErr) || loaderResults.ValidationResults.HasErrors {
240+
if errors.As(err, &loaderErr) || validationHasErrors {
237241
var gatewayReason gwv1.GatewayConditionReason
238242
var gatewayMessage string
239-
if loaderErr == nil && loaderResults.ValidationResults.HasErrors {
243+
if loaderErr == nil && validationHasErrors {
240244
gatewayReason = gwv1.GatewayReasonAccepted
241245
gatewayMessage = gateway_constants.GatewayAcceptedFalseMessage
242246
} else {

‎controllers/gateway/listener_status_utils.go‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,8 +11,9 @@ import (
1111
gwv1 "sigs.k8s.io/gateway-api/apis/v1"
1212
)
1313

14-
func buildListenerStatus(gateway gwv1.Gateway, listeners []gwv1.Listener, attachedRoutesMap map[gwv1.SectionName]int32, validateListenerResults routeutils.ListenerValidationResults, isProgrammed bool) []gwv1.ListenerStatus {
14+
func buildListenerStatus(gateway gwv1.Gateway, listeners []gwv1.Listener, attachedRoutesMap map[gwv1.SectionName]int32, validatedListeners routeutils.ValidatedGatewayListeners, isProgrammed bool) []gwv1.ListenerStatus {
1515
var listenerStatuses []gwv1.ListenerStatus
16+
validateListenerResults := validatedListeners.GatewayListenerValidation
1617

1718
for _, listener := range listeners {
1819
listenerValidationResult := validateListenerResults.Results[listener.Name]

‎controllers/gateway/listener_status_utils_test.go‎

Lines changed: 9 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@ func Test_buildListenerStatus(t *testing.T) {
1616
name string
1717
gateway gwv1.Gateway
1818
attachedRoutesMap map[gwv1.SectionName]int32
19-
validateListenerResults routeutils.ListenerValidationResults
19+
validateListenerResults routeutils.ValidatedGatewayListeners
2020
supportedKinds []gwv1.RouteGroupKind
2121
isProgrammed bool
2222
expectedListenerCount int
@@ -37,11 +37,13 @@ func Test_buildListenerStatus(t *testing.T) {
3737
},
3838
},
3939
attachedRoutesMap: map[gwv1.SectionName]int32{"listener1": 1},
40-
validateListenerResults: routeutils.ListenerValidationResults{
41-
Results: map[gwv1.SectionName]routeutils.ListenerValidationResult{
42-
"listener1": {Reason: gwv1.ListenerReasonAccepted, Message: "accepted", SupportedKinds: []gwv1.RouteGroupKind{{
43-
Kind: "HTTPRoute",
44-
}}},
40+
validateListenerResults: routeutils.ValidatedGatewayListeners{
41+
GatewayListenerValidation: routeutils.ListenerValidationResults{
42+
Results: map[gwv1.SectionName]routeutils.ListenerValidationResult{
43+
"listener1": {Reason: gwv1.ListenerReasonAccepted, Message: "accepted", SupportedKinds: []gwv1.RouteGroupKind{{
44+
Kind: "HTTPRoute",
45+
}}},
46+
},
4547
},
4648
},
4749
supportedKinds: []gwv1.RouteGroupKind{{
@@ -59,7 +61,7 @@ func Test_buildListenerStatus(t *testing.T) {
5961
},
6062
},
6163
attachedRoutesMap: map[gwv1.SectionName]int32{},
62-
validateListenerResults: routeutils.ListenerValidationResults{},
64+
validateListenerResults: routeutils.ValidatedGatewayListeners{},
6365
isProgrammed: true,
6466
expectedListenerCount: 0,
6567
},

‎pkg/algorithm/maps.go‎

Lines changed: 23 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,14 @@
11
package algorithm
22

33
import (
4-
"k8s.io/apimachinery/pkg/util/sets"
54
"strings"
5+
6+
"k8s.io/apimachinery/pkg/util/sets"
67
)
78

9+
// awsSystemTagPrefix is the prefix for AWS-reserved system tags, which are immutable.
10+
const awsSystemTagPrefix = "aws:"
11+
812
// MapFindFirst get from list of maps until first found.
913
func MapFindFirst(key string, maps ...map[string]string) (string, bool) {
1014
for _, m := range maps {
@@ -53,6 +57,24 @@ func DiffStringMap(desired map[string]string, current map[string]string) (map[st
5357
return modify, remove
5458
}
5559

60+
// DiffStringMapIgnoreAWSTags is like DiffStringMap but filters out AWS-reserved system tags
61+
// (prefixed with "aws:") from both results. These tags are immutable.
62+
func DiffStringMapIgnoreAWSTags(desired map[string]string, current map[string]string) (map[string]string, map[string]string) {
63+
modify, remove := DiffStringMap(desired, current)
64+
RemoveKeysByPrefix(modify, awsSystemTagPrefix)
65+
RemoveKeysByPrefix(remove, awsSystemTagPrefix)
66+
return modify, remove
67+
}
68+
69+
// RemoveKeysByPrefix removes all entries from the map whose key starts with the given prefix.
70+
func RemoveKeysByPrefix(m map[string]string, prefix string) {
71+
for key := range m {
72+
if strings.HasPrefix(key, prefix) {
73+
delete(m, key)
74+
}
75+
}
76+
}
77+
5678
func CSVToStringSet(csv string) sets.Set[string] {
5779
s := sets.Set[string]{}
5880

‎pkg/algorithm/maps_test.go‎

Lines changed: 113 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,10 @@
11
package algorithm
22

33
import (
4+
"testing"
5+
46
"github.com/stretchr/testify/assert"
57
"k8s.io/apimachinery/pkg/util/sets"
6-
"testing"
78
)
89

910
func TestMapFindFirst(t *testing.T) {
@@ -358,3 +359,114 @@ func TestStringSetToCSV(t *testing.T) {
358359
})
359360
}
360361
}
362+
363+
func TestRemoveKeysByPrefix(t *testing.T) {
364+
tests := []struct {
365+
name string
366+
input map[string]string
367+
prefix string
368+
want map[string]string
369+
}{
370+
{
371+
name: "removes aws: prefixed keys",
372+
input: map[string]string{"aws:cloudformation:stack-name": "my-stack", "elbv2.k8s.aws/cluster": "my-cluster", "aws:cloudformation:logical-id": "NLB"},
373+
prefix: "aws:",
374+
want: map[string]string{"elbv2.k8s.aws/cluster": "my-cluster"},
375+
},
376+
{
377+
name: "no matching prefix",
378+
input: map[string]string{"elbv2.k8s.aws/cluster": "my-cluster", "service.k8s.aws/stack": "default/svc"},
379+
prefix: "aws:",
380+
want: map[string]string{"elbv2.k8s.aws/cluster": "my-cluster", "service.k8s.aws/stack": "default/svc"},
381+
},
382+
{
383+
name: "all keys match prefix",
384+
input: map[string]string{"aws:cloudformation:stack-name": "s", "aws:cloudformation:stack-id": "id"},
385+
prefix: "aws:",
386+
want: map[string]string{},
387+
},
388+
{
389+
name: "empty map",
390+
input: map[string]string{},
391+
prefix: "aws:",
392+
want: map[string]string{},
393+
},
394+
}
395+
for _, tt := range tests {
396+
t.Run(tt.name, func(t *testing.T) {
397+
RemoveKeysByPrefix(tt.input, tt.prefix)
398+
assert.Equal(t, tt.want, tt.input)
399+
})
400+
}
401+
}
402+
403+
func TestDiffStringMapIgnoreAWSTags(t *testing.T) {
404+
tests := []struct {
405+
name string
406+
desired map[string]string
407+
current map[string]string
408+
wantUpdate map[string]string
409+
wantRemove map[string]string
410+
}{
411+
{
412+
name: "aws: tags in current are not removed",
413+
desired: map[string]string{
414+
"elbv2.k8s.aws/cluster": "my-cluster",
415+
"service.k8s.aws/stack": "default/svc",
416+
},
417+
current: map[string]string{
418+
"elbv2.k8s.aws/cluster": "my-cluster",
419+
"aws:cloudformation:stack-name": "my-stack",
420+
"aws:cloudformation:stack-id": "arn:aws:cloudformation:us-east-1:123:stack/my-stack/abc",
421+
"aws:cloudformation:logical-id": "NLB",
422+
},
423+
wantUpdate: map[string]string{
424+
"service.k8s.aws/stack": "default/svc",
425+
},
426+
wantRemove: map[string]string{},
427+
},
428+
{
429+
name: "aws: tags in desired are not added",
430+
desired: map[string]string{
431+
"elbv2.k8s.aws/cluster": "my-cluster",
432+
"aws:createdBy": "should-be-ignored",
433+
},
434+
current: map[string]string{
435+
"elbv2.k8s.aws/cluster": "my-cluster",
436+
},
437+
wantUpdate: map[string]string{},
438+
wantRemove: map[string]string{},
439+
},
440+
{
441+
name: "no aws: tags behaves like DiffStringMap",
442+
desired: map[string]string{
443+
"a": "1",
444+
"b": "2",
445+
},
446+
current: map[string]string{
447+
"a": "1",
448+
"c": "3",
449+
},
450+
wantUpdate: map[string]string{
451+
"b": "2",
452+
},
453+
wantRemove: map[string]string{
454+
"c": "3",
455+
},
456+
},
457+
{
458+
name: "both empty",
459+
desired: map[string]string{},
460+
current: map[string]string{},
461+
wantUpdate: map[string]string{},
462+
wantRemove: map[string]string{},
463+
},
464+
}
465+
for _, tt := range tests {
466+
t.Run(tt.name, func(t *testing.T) {
467+
gotUpdate, gotRemove := DiffStringMapIgnoreAWSTags(tt.desired, tt.current)
468+
assert.Equal(t, tt.wantUpdate, gotUpdate)
469+
assert.Equal(t, tt.wantRemove, gotRemove)
470+
})
471+
}
472+
}

‎pkg/deploy/aga/tagging_manager.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -103,7 +103,7 @@ func (m *defaultTaggingManager) ReconcileTags(ctx context.Context, arn string, d
103103
}
104104
}
105105

106-
tagsToUpdate, tagsToRemove := algorithm.DiffStringMap(desiredTags, currentTags)
106+
tagsToUpdate, tagsToRemove := algorithm.DiffStringMapIgnoreAWSTags(desiredTags, currentTags)
107107
for _, ignoredTagKey := range reconcileOpts.IgnoredTagKeys {
108108
delete(tagsToUpdate, ignoredTagKey)
109109
delete(tagsToRemove, ignoredTagKey)

‎pkg/deploy/aga/tagging_manager_test.go‎

Lines changed: 135 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,8 @@ import (
66
"testing"
77

88
awssdk "github.com/aws/aws-sdk-go-v2/aws"
9+
"github.com/aws/aws-sdk-go-v2/service/globalaccelerator"
10+
agatypes "github.com/aws/aws-sdk-go-v2/service/globalaccelerator/types"
911
rgtsdk "github.com/aws/aws-sdk-go-v2/service/resourcegroupstaggingapi"
1012
rgttypes "github.com/aws/aws-sdk-go-v2/service/resourcegroupstaggingapi/types"
1113
"github.com/golang/mock/gomock"
@@ -15,6 +17,139 @@ import (
1517
"sigs.k8s.io/controller-runtime/pkg/log/zap"
1618
)
1719

20+
func Test_defaultTaggingManager_ReconcileTags(t *testing.T) {
21+
type tagResourceCall struct {
22+
req *globalaccelerator.TagResourceInput
23+
resp *globalaccelerator.TagResourceOutput
24+
err error
25+
}
26+
type untagResourceCall struct {
27+
req *globalaccelerator.UntagResourceInput
28+
resp *globalaccelerator.UntagResourceOutput
29+
err error
30+
}
31+
type fields struct {
32+
tagResourceCalls []tagResourceCall
33+
untagResourceCalls []untagResourceCall
34+
}
35+
type args struct {
36+
arn string
37+
desiredTags map[string]string
38+
opts []ReconcileTagsOption
39+
}
40+
tests := []struct {
41+
name string
42+
fields fields
43+
args args
44+
wantErr error
45+
}{
46+
{
47+
name: "standard case - add and remove tags",
48+
fields: fields{
49+
tagResourceCalls: []tagResourceCall{
50+
{
51+
req: &globalaccelerator.TagResourceInput{
52+
ResourceArn: awssdk.String("my-arn"),
53+
Tags: []agatypes.Tag{
54+
{
55+
Key: awssdk.String("keyB"),
56+
Value: awssdk.String("valueB2"),
57+
},
58+
{
59+
Key: awssdk.String("keyD"),
60+
Value: awssdk.String("valueD"),
61+
},
62+
},
63+
},
64+
},
65+
},
66+
untagResourceCalls: []untagResourceCall{
67+
{
68+
req: &globalaccelerator.UntagResourceInput{
69+
ResourceArn: awssdk.String("my-arn"),
70+
TagKeys: []string{"keyC"},
71+
},
72+
},
73+
},
74+
},
75+
args: args{
76+
arn: "my-arn",
77+
desiredTags: map[string]string{
78+
"keyA": "valueA",
79+
"keyB": "valueB2",
80+
"keyD": "valueD",
81+
},
82+
opts: []ReconcileTagsOption{
83+
WithCurrentTags(map[string]string{
84+
"keyA": "valueA",
85+
"keyB": "valueB",
86+
"keyC": "valueC",
87+
}),
88+
},
89+
},
90+
},
91+
{
92+
name: "aws: prefixed tags on current resource are not removed",
93+
fields: fields{
94+
tagResourceCalls: []tagResourceCall{
95+
{
96+
req: &globalaccelerator.TagResourceInput{
97+
ResourceArn: awssdk.String("my-arn"),
98+
Tags: []agatypes.Tag{
99+
{
100+
Key: awssdk.String("aga.k8s.aws/stack"),
101+
Value: awssdk.String("default/my-accelerator"),
102+
},
103+
},
104+
},
105+
},
106+
},
107+
untagResourceCalls: nil,
108+
},
109+
args: args{
110+
arn: "my-arn",
111+
desiredTags: map[string]string{
112+
"elbv2.k8s.aws/cluster": "my-cluster",
113+
"aga.k8s.aws/stack": "default/my-accelerator",
114+
},
115+
opts: []ReconcileTagsOption{
116+
WithCurrentTags(map[string]string{
117+
"elbv2.k8s.aws/cluster": "my-cluster",
118+
"aws:cloudformation:stack-name": "my-stack",
119+
"aws:cloudformation:stack-id": "arn:aws:cloudformation:us-east-1:123:stack/my-stack/abc",
120+
"aws:cloudformation:logical-id": "Accelerator",
121+
}),
122+
},
123+
},
124+
},
125+
}
126+
for _, tt := range tests {
127+
t.Run(tt.name, func(t *testing.T) {
128+
ctrl := gomock.NewController(t)
129+
defer ctrl.Finish()
130+
gaService := services.NewMockGlobalAccelerator(ctrl)
131+
for _, call := range tt.fields.tagResourceCalls {
132+
gaService.EXPECT().TagResourceWithContext(gomock.Any(), call.req).Return(call.resp, call.err)
133+
}
134+
for _, call := range tt.fields.untagResourceCalls {
135+
gaService.EXPECT().UntagResourceWithContext(gomock.Any(), call.req).Return(call.resp, call.err)
136+
}
137+
138+
m := &defaultTaggingManager{
139+
gaService: gaService,
140+
logger: zap.New(),
141+
resourceTagsCache: cache.NewExpiring(),
142+
}
143+
err := m.ReconcileTags(context.Background(), tt.args.arn, tt.args.desiredTags, tt.args.opts...)
144+
if tt.wantErr != nil {
145+
assert.EqualError(t, err, tt.wantErr.Error())
146+
} else {
147+
assert.NoError(t, err)
148+
}
149+
})
150+
}
151+
}
152+
18153
func Test_defaultTaggingManager_describeResourceTagsFromRGT(t *testing.T) {
19154
ctrl := gomock.NewController(t)
20155
defer ctrl.Finish()

0 commit comments

Comments
 (0)