Skip to content

Commit 0eb4a91

Browse files
k8s-ci-robotk8s-ci-robot
authored
Merge pull request #4382 from zac-nixon/main
fix sorting when comparing listener rule differences
2 parents 595d1b5 + d61f8b8 commit 0eb4a91

File tree

6 files changed

+107
-7
lines changed

6 files changed

+107
-7
lines changed

‎pkg/deploy/elbv2/listener_manager.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -296,7 +296,7 @@ func (m *defaultListenerManager) isSDKListenerSettingsDrifted(lsSpec elbv2model.
296296
if string(lsSpec.Protocol) != string(sdkLS.Listener.Protocol) {
297297
return true
298298
}
299-
if !cmp.Equal(desiredDefaultActions, sdkLS.Listener.DefaultActions, elbv2equality.CompareOptionForActions()) {
299+
if !cmp.Equal(desiredDefaultActions, sdkLS.Listener.DefaultActions, elbv2equality.CompareOptionForActions(desiredDefaultActions, sdkLS.Listener.DefaultActions)) {
300300
return true
301301
}
302302
if !cmp.Equal(desiredDefaultCerts, sdkLS.Listener.Certificates, elbv2equality.CompareOptionForCertificates()) {

‎pkg/deploy/elbv2/listener_rule_synthesizer.go‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -167,8 +167,10 @@ func (s *listenerRuleSynthesizer) matchResAndSDKListenerRules(resLRs []*elbv2mod
167167
found := false
168168
for i := 0; i < len(unmatchedSDKLRs); i++ {
169169
sdkLR := unmatchedSDKLRs[i]
170-
if cmp.Equal(resLRDesiredActionsAndConditionsPair.desiredActions, sdkLR.ListenerRule.Actions, elbv2equality.CompareOptionForActions()) &&
171-
cmp.Equal(resLRDesiredActionsAndConditionsPair.desiredConditions, sdkLR.ListenerRule.Conditions, elbv2equality.CompareOptionForRuleConditions()) {
170+
171+
actionsEqual := cmp.Equal(resLRDesiredActionsAndConditionsPair.desiredActions, sdkLR.ListenerRule.Actions, elbv2equality.CompareOptionForActions(resLRDesiredActionsAndConditionsPair.desiredActions, sdkLR.ListenerRule.Actions))
172+
conditionsEqual := cmp.Equal(resLRDesiredActionsAndConditionsPair.desiredConditions, sdkLR.ListenerRule.Conditions, elbv2equality.CompareOptionForRuleConditions(resLRDesiredActionsAndConditionsPair.desiredConditions, sdkLR.ListenerRule.Conditions))
173+
if actionsEqual && conditionsEqual {
172174
sdkLRPriority, _ := strconv.ParseInt(awssdk.ToString(sdkLR.ListenerRule.Priority), 10, 64)
173175
if resLR.Spec.Priority != int32(sdkLRPriority) {
174176
matchedResAndSDKLRsBySettings = append(matchedResAndSDKLRsBySettings, resAndSDKListenerRulePair{

‎pkg/equality/elbv2/compare_option_for_actions.go‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -87,7 +87,10 @@ func CompareOptionForAction() cmp.Option {
8787
}
8888

8989
// CompareOptionForActions returns the compare option for action slice.
90-
func CompareOptionForActions() cmp.Option {
90+
// IMPORTANT:
91+
// When changing the types compared (e.g. the input to the function)
92+
// ensure to update cmpopts.SortSlices to reflect the new type, otherwise sorting silently doesn't work.
93+
func CompareOptionForActions(_, _ []elbv2types.Action) cmp.Option {
9194
return cmp.Options{
9295
cmpopts.EquateEmpty(),
9396
cmpopts.IgnoreUnexported(elbv2types.AuthenticateCognitoActionConfig{}),

‎pkg/equality/elbv2/compare_option_for_actions_test.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1667,7 +1667,7 @@ func TestCompareOptionForActions(t *testing.T) {
16671667
}
16681668
for _, tt := range tests {
16691669
t.Run(tt.name, func(t *testing.T) {
1670-
got := cmp.Equal(tt.args.lhs, tt.args.rhs, CompareOptionForActions())
1670+
got := cmp.Equal(tt.args.lhs, tt.args.rhs, CompareOptionForActions(nil, nil))
16711671
assert.Equal(t, tt.want, got)
16721672
})
16731673
}

‎pkg/equality/elbv2/compare_option_for_rule_conditions.go‎

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -21,11 +21,16 @@ func CompareOptionForRuleCondition() cmp.Option {
2121
}
2222
}
2323

24+
// https://github.com/google/go-cmp/issues/273
25+
2426
// CompareOptionForRuleConditions returns the compare option for rule conditions slice.
25-
func CompareOptionForRuleConditions() cmp.Option {
27+
// IMPORTANT:
28+
// When changing the types compared (e.g. the input to the function)
29+
// ensure to update cmpopts.SortSlices to reflect the new type, otherwise sorting silently doesn't work.
30+
func CompareOptionForRuleConditions(_, _ []elbv2types.RuleCondition) cmp.Option {
2631
return cmp.Options{
2732
cmpopts.EquateEmpty(),
28-
cmpopts.SortSlices(func(lhs *elbv2types.RuleCondition, rhs *elbv2types.RuleCondition) bool {
33+
cmpopts.SortSlices(func(lhs elbv2types.RuleCondition, rhs elbv2types.RuleCondition) bool {
2934
return awssdk.ToString(lhs.Field) < awssdk.ToString(rhs.Field)
3035
}),
3136
CompareOptionForRuleCondition(),

‎pkg/equality/elbv2/compare_option_for_rule_conditions_test.go‎

Lines changed: 90 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,90 @@
1+
package elbv2
2+
3+
import (
4+
awssdk "github.com/aws/aws-sdk-go-v2/aws"
5+
"github.com/aws/aws-sdk-go-v2/service/elasticloadbalancingv2/types"
6+
"github.com/google/go-cmp/cmp"
7+
"github.com/stretchr/testify/assert"
8+
"testing"
9+
)
10+
11+
func Test_CompareOptionForRuleConditions(t *testing.T) {
12+
testCase := []struct {
13+
name string
14+
desiredRuleCondition []types.RuleCondition
15+
actualRuleCondition []types.RuleCondition
16+
expected bool
17+
}{
18+
{
19+
name: "equal",
20+
desiredRuleCondition: []types.RuleCondition{
21+
{
22+
Field: awssdk.String("host-header"),
23+
HostHeaderConfig: &types.HostHeaderConditionConfig{
24+
Values: []string{"h1"},
25+
},
26+
},
27+
{
28+
Field: awssdk.String("path-pattern"),
29+
PathPatternConfig: &types.PathPatternConditionConfig{
30+
Values: []string{"path-pattern"},
31+
},
32+
},
33+
},
34+
actualRuleCondition: []types.RuleCondition{
35+
{
36+
Field: awssdk.String("host-header"),
37+
HostHeaderConfig: &types.HostHeaderConditionConfig{
38+
Values: []string{"h1"},
39+
},
40+
},
41+
{
42+
Field: awssdk.String("path-pattern"),
43+
PathPatternConfig: &types.PathPatternConditionConfig{
44+
Values: []string{"path-pattern"},
45+
},
46+
},
47+
},
48+
expected: true,
49+
},
50+
{
51+
name: "equal - different order",
52+
desiredRuleCondition: []types.RuleCondition{
53+
{
54+
Field: awssdk.String("host-header"),
55+
HostHeaderConfig: &types.HostHeaderConditionConfig{
56+
Values: []string{"h1"},
57+
},
58+
},
59+
{
60+
Field: awssdk.String("path-pattern"),
61+
PathPatternConfig: &types.PathPatternConditionConfig{
62+
Values: []string{"path-pattern"},
63+
},
64+
},
65+
},
66+
actualRuleCondition: []types.RuleCondition{
67+
{
68+
Field: awssdk.String("path-pattern"),
69+
PathPatternConfig: &types.PathPatternConditionConfig{
70+
Values: []string{"path-pattern"},
71+
},
72+
},
73+
{
74+
Field: awssdk.String("host-header"),
75+
HostHeaderConfig: &types.HostHeaderConditionConfig{
76+
Values: []string{"h1"},
77+
},
78+
},
79+
},
80+
expected: true,
81+
},
82+
}
83+
84+
for _, tc := range testCase {
85+
t.Run(tc.name, func(t *testing.T) {
86+
conditionsEqual := cmp.Equal(tc.desiredRuleCondition, tc.actualRuleCondition, CompareOptionForRuleConditions(nil, nil))
87+
assert.Equal(t, tc.expected, conditionsEqual)
88+
})
89+
}
90+
}

0 commit comments

Comments
 (0)