Skip to content

Commit 9ee2310

Browse files
jimmidysonjimmidyson
authored
fix: Ensure CNI ingress rules are added to AWSCluster (#213)
1 parent 03962c2 commit 9ee2310

File tree

6 files changed

+245
-32
lines changed

6 files changed

+245
-32
lines changed

‎common/pkg/testutils/capitest/request/items.go

Lines changed: 18 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -113,14 +113,26 @@ func NewKubeadmControlPlaneTemplateRequestItem(
113113

114114
func NewAWSClusterTemplateRequestItem(
115115
uid types.UID,
116+
existingSpec ...capav1.AWSClusterTemplateSpec,
116117
) runtimehooksv1.GeneratePatchesRequestItem {
117-
return NewRequestItem(
118-
&capav1.AWSClusterTemplate{
119-
TypeMeta: metav1.TypeMeta{
120-
APIVersion: capav1.GroupVersion.String(),
121-
Kind: "AWSClusterTemplate",
122-
},
118+
awsClusterTemplate := &capav1.AWSClusterTemplate{
119+
TypeMeta: metav1.TypeMeta{
120+
APIVersion: capav1.GroupVersion.String(),
121+
Kind: "AWSClusterTemplate",
123122
},
123+
}
124+
125+
switch len(existingSpec) {
126+
case 0:
127+
// Do nothing.
128+
case 1:
129+
awsClusterTemplate.Spec = existingSpec[0]
130+
default:
131+
panic("can only take at most one existing spec")
132+
}
133+
134+
return NewRequestItem(
135+
awsClusterTemplate,
124136
&runtimehooksv1.HolderReference{
125137
Kind: "Cluster",
126138
FieldPath: "spec.infrastructureRef",

‎pkg/handlers/aws/mutation/cni/calico/inject.go

Lines changed: 22 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ package calico
66
import (
77
"context"
88
_ "embed"
9+
"slices"
910

1011
"github.com/go-logr/logr"
1112
apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1"
@@ -48,7 +49,11 @@ func NewPatch() *calicoPatchHandler {
4849
}
4950

5051
func NewMetaPatch() *calicoPatchHandler {
51-
return newCalicoPatchHandler(clusterconfig.MetaVariableName, cni.VariableName)
52+
return newCalicoPatchHandler(
53+
clusterconfig.MetaVariableName,
54+
"addons",
55+
cni.VariableName,
56+
)
5257
}
5358

5459
func newCalicoPatchHandler(
@@ -145,8 +150,9 @@ func mutateAWSClusterTemplateFunc(log logr.Logger) func(obj *capav1.AWSClusterTe
145150
if obj.Spec.Template.Spec.NetworkSpec.CNI == nil {
146151
obj.Spec.Template.Spec.NetworkSpec.CNI = &capav1.CNISpec{}
147152
}
148-
obj.Spec.Template.Spec.NetworkSpec.CNI.CNIIngressRules = append(
153+
obj.Spec.Template.Spec.NetworkSpec.CNI.CNIIngressRules = addOrUpdateCNIIngressRules(
149154
obj.Spec.Template.Spec.NetworkSpec.CNI.CNIIngressRules,
155+
150156
capav1.CNIIngressRule{
151157
Description: "typha (calico)",
152158
Protocol: capav1.SecurityGroupProtocolTCP,
@@ -182,3 +188,17 @@ func mutateAWSClusterTemplateFunc(log logr.Logger) func(obj *capav1.AWSClusterTe
182188
return nil
183189
}
184190
}
191+
192+
func addOrUpdateCNIIngressRules(
193+
rules []capav1.CNIIngressRule, newRules ...capav1.CNIIngressRule,
194+
) []capav1.CNIIngressRule {
195+
clonedRules := slices.Clone(rules)
196+
197+
for _, newRule := range newRules {
198+
if !slices.Contains(clonedRules, newRule) {
199+
clonedRules = append(clonedRules, newRule)
200+
}
201+
}
202+
203+
return clonedRules
204+
}

‎pkg/handlers/aws/mutation/cni/calico/tests/generate_patches.go

Lines changed: 173 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -51,7 +51,7 @@ func TestGeneratePatches(
5151
Path: "/spec/template/spec/network/cni",
5252
ValueMatcher: gomega.HaveKeyWithValue(
5353
"cniIngressRules",
54-
gomega.ContainElements(
54+
gomega.ConsistOf(
5555
gomega.SatisfyAll(
5656
gomega.HaveKeyWithValue("description", "typha (calico)"),
5757
gomega.HaveKeyWithValue(
@@ -101,5 +101,177 @@ func TestGeneratePatches(
101101
),
102102
}},
103103
},
104+
capitest.PatchTestDef{
105+
Name: "provider set with AWSClusterTemplate pre-existing rules",
106+
Vars: []runtimehooksv1.Variable{
107+
capitest.VariableWithValue(
108+
variableName,
109+
v1alpha1.CNI{
110+
Provider: v1alpha1.CNIProviderCalico,
111+
},
112+
variablePath...,
113+
),
114+
},
115+
RequestItem: request.NewAWSClusterTemplateRequestItem(
116+
"1234",
117+
capav1.AWSClusterTemplateSpec{
118+
Template: capav1.AWSClusterTemplateResource{
119+
Spec: capav1.AWSClusterSpec{
120+
NetworkSpec: capav1.NetworkSpec{
121+
CNI: &capav1.CNISpec{
122+
CNIIngressRules: []capav1.CNIIngressRule{{
123+
Description: "test",
124+
Protocol: capav1.SecurityGroupProtocolAll,
125+
FromPort: 1234,
126+
ToPort: 12345,
127+
}},
128+
},
129+
},
130+
},
131+
},
132+
},
133+
),
134+
ExpectedPatchMatchers: []capitest.JSONPatchMatcher{{
135+
Operation: "add",
136+
Path: "/spec/template/spec/network/cni/cniIngressRules/1",
137+
ValueMatcher: gomega.SatisfyAll(
138+
gomega.HaveKeyWithValue("description", "typha (calico)"),
139+
gomega.HaveKeyWithValue(
140+
"protocol",
141+
gomega.BeEquivalentTo(capav1.SecurityGroupProtocolTCP),
142+
),
143+
gomega.HaveKeyWithValue("fromPort", gomega.BeEquivalentTo(5473)),
144+
gomega.HaveKeyWithValue("toPort", gomega.BeEquivalentTo(5473)),
145+
),
146+
}, {
147+
Operation: "add",
148+
Path: "/spec/template/spec/network/cni/cniIngressRules/2",
149+
ValueMatcher: gomega.SatisfyAll(
150+
gomega.HaveKeyWithValue("description", "bgp (calico)"),
151+
gomega.HaveKeyWithValue(
152+
"protocol",
153+
gomega.BeEquivalentTo(capav1.SecurityGroupProtocolTCP),
154+
),
155+
gomega.HaveKeyWithValue("fromPort", gomega.BeEquivalentTo(179)),
156+
gomega.HaveKeyWithValue("toPort", gomega.BeEquivalentTo(179)),
157+
),
158+
}, {
159+
Operation: "add",
160+
Path: "/spec/template/spec/network/cni/cniIngressRules/3",
161+
ValueMatcher: gomega.SatisfyAll(
162+
gomega.HaveKeyWithValue("description", "IP-in-IP (calico)"),
163+
gomega.HaveKeyWithValue(
164+
"protocol",
165+
gomega.BeEquivalentTo(capav1.SecurityGroupProtocolIPinIP),
166+
),
167+
gomega.HaveKeyWithValue("fromPort", gomega.BeEquivalentTo(-1)),
168+
gomega.HaveKeyWithValue("toPort", gomega.BeEquivalentTo(65535)),
169+
),
170+
}, {
171+
Operation: "add",
172+
Path: "/spec/template/spec/network/cni/cniIngressRules/4",
173+
ValueMatcher: gomega.SatisfyAll(
174+
gomega.HaveKeyWithValue("description", "node metrics (calico)"),
175+
gomega.HaveKeyWithValue(
176+
"protocol",
177+
gomega.BeEquivalentTo(capav1.SecurityGroupProtocolTCP),
178+
),
179+
gomega.HaveKeyWithValue("fromPort", gomega.BeEquivalentTo(9091)),
180+
gomega.HaveKeyWithValue("toPort", gomega.BeEquivalentTo(9091)),
181+
),
182+
}, {
183+
Operation: "add",
184+
Path: "/spec/template/spec/network/cni/cniIngressRules/5",
185+
ValueMatcher: gomega.SatisfyAll(
186+
gomega.HaveKeyWithValue("description", "typha metrics (calico)"),
187+
gomega.HaveKeyWithValue(
188+
"protocol",
189+
gomega.BeEquivalentTo(capav1.SecurityGroupProtocolTCP),
190+
),
191+
gomega.HaveKeyWithValue("fromPort", gomega.BeEquivalentTo(9093)),
192+
gomega.HaveKeyWithValue("toPort", gomega.BeEquivalentTo(9093)),
193+
),
194+
}},
195+
},
196+
capitest.PatchTestDef{
197+
Name: "provider set with AWSClusterTemplate conflicting pre-existing rules",
198+
Vars: []runtimehooksv1.Variable{
199+
capitest.VariableWithValue(
200+
variableName,
201+
v1alpha1.CNI{
202+
Provider: v1alpha1.CNIProviderCalico,
203+
},
204+
variablePath...,
205+
),
206+
},
207+
RequestItem: request.NewAWSClusterTemplateRequestItem(
208+
"1234",
209+
capav1.AWSClusterTemplateSpec{
210+
Template: capav1.AWSClusterTemplateResource{
211+
Spec: capav1.AWSClusterSpec{
212+
NetworkSpec: capav1.NetworkSpec{
213+
CNI: &capav1.CNISpec{
214+
CNIIngressRules: []capav1.CNIIngressRule{{
215+
Description: "typha (calico)",
216+
Protocol: capav1.SecurityGroupProtocolTCP,
217+
FromPort: 5473,
218+
ToPort: 5473,
219+
}},
220+
},
221+
},
222+
},
223+
},
224+
},
225+
),
226+
ExpectedPatchMatchers: []capitest.JSONPatchMatcher{{
227+
Operation: "add",
228+
Path: "/spec/template/spec/network/cni/cniIngressRules/1",
229+
ValueMatcher: gomega.SatisfyAll(
230+
gomega.HaveKeyWithValue("description", "bgp (calico)"),
231+
gomega.HaveKeyWithValue(
232+
"protocol",
233+
gomega.BeEquivalentTo(capav1.SecurityGroupProtocolTCP),
234+
),
235+
gomega.HaveKeyWithValue("fromPort", gomega.BeEquivalentTo(179)),
236+
gomega.HaveKeyWithValue("toPort", gomega.BeEquivalentTo(179)),
237+
),
238+
}, {
239+
Operation: "add",
240+
Path: "/spec/template/spec/network/cni/cniIngressRules/2",
241+
ValueMatcher: gomega.SatisfyAll(
242+
gomega.HaveKeyWithValue("description", "IP-in-IP (calico)"),
243+
gomega.HaveKeyWithValue(
244+
"protocol",
245+
gomega.BeEquivalentTo(capav1.SecurityGroupProtocolIPinIP),
246+
),
247+
gomega.HaveKeyWithValue("fromPort", gomega.BeEquivalentTo(-1)),
248+
gomega.HaveKeyWithValue("toPort", gomega.BeEquivalentTo(65535)),
249+
),
250+
}, {
251+
Operation: "add",
252+
Path: "/spec/template/spec/network/cni/cniIngressRules/3",
253+
ValueMatcher: gomega.SatisfyAll(
254+
gomega.HaveKeyWithValue("description", "node metrics (calico)"),
255+
gomega.HaveKeyWithValue(
256+
"protocol",
257+
gomega.BeEquivalentTo(capav1.SecurityGroupProtocolTCP),
258+
),
259+
gomega.HaveKeyWithValue("fromPort", gomega.BeEquivalentTo(9091)),
260+
gomega.HaveKeyWithValue("toPort", gomega.BeEquivalentTo(9091)),
261+
),
262+
}, {
263+
Operation: "add",
264+
Path: "/spec/template/spec/network/cni/cniIngressRules/4",
265+
ValueMatcher: gomega.SatisfyAll(
266+
gomega.HaveKeyWithValue("description", "typha metrics (calico)"),
267+
gomega.HaveKeyWithValue(
268+
"protocol",
269+
gomega.BeEquivalentTo(capav1.SecurityGroupProtocolTCP),
270+
),
271+
gomega.HaveKeyWithValue("fromPort", gomega.BeEquivalentTo(9093)),
272+
gomega.HaveKeyWithValue("toPort", gomega.BeEquivalentTo(9093)),
273+
),
274+
}},
275+
},
104276
)
105277
}

‎pkg/handlers/aws/mutation/metapatch_handler.go

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ import (
99
"github.com/d2iq-labs/capi-runtime-extensions/common/pkg/capi/apis"
1010
"github.com/d2iq-labs/capi-runtime-extensions/common/pkg/capi/clustertopology/handlers"
1111
"github.com/d2iq-labs/capi-runtime-extensions/common/pkg/capi/clustertopology/handlers/mutation"
12+
"github.com/d2iq-labs/capi-runtime-extensions/pkg/handlers/aws/mutation/cni/calico"
1213
"github.com/d2iq-labs/capi-runtime-extensions/pkg/handlers/aws/mutation/region"
1314
genericmutation "github.com/d2iq-labs/capi-runtime-extensions/pkg/handlers/generic/mutation"
1415
)
@@ -18,6 +19,7 @@ func MetaPatchHandler(mgr manager.Manager) handlers.Named {
1819
patchHandlers := append(
1920
[]mutation.MetaMutator{
2021
region.NewMetaPatch(),
22+
calico.NewMetaPatch(),
2123
},
2224
genericmutation.MetaMutators(mgr)...,
2325
)

‎pkg/handlers/aws/mutation/metapatch_handler_test.go

Lines changed: 19 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -12,9 +12,11 @@ import (
1212
"sigs.k8s.io/controller-runtime/pkg/manager"
1313

1414
"github.com/d2iq-labs/capi-runtime-extensions/common/pkg/capi/clustertopology/handlers/mutation"
15+
awsclusterconfig "github.com/d2iq-labs/capi-runtime-extensions/pkg/handlers/aws/clusterconfig"
1516
calicotests "github.com/d2iq-labs/capi-runtime-extensions/pkg/handlers/aws/mutation/cni/calico/tests"
1617
"github.com/d2iq-labs/capi-runtime-extensions/pkg/handlers/aws/mutation/region"
1718
regiontests "github.com/d2iq-labs/capi-runtime-extensions/pkg/handlers/aws/mutation/region/tests"
19+
"github.com/d2iq-labs/capi-runtime-extensions/pkg/handlers/generic/clusterconfig"
1820
auditpolicytests "github.com/d2iq-labs/capi-runtime-extensions/pkg/handlers/generic/mutation/auditpolicy/tests"
1921
"github.com/d2iq-labs/capi-runtime-extensions/pkg/handlers/generic/mutation/cni"
2022
"github.com/d2iq-labs/capi-runtime-extensions/pkg/handlers/generic/mutation/etcd"
@@ -23,6 +25,7 @@ import (
2325
extraapiservercertsanstests "github.com/d2iq-labs/capi-runtime-extensions/pkg/handlers/generic/mutation/extraapiservercertsans/tests"
2426
"github.com/d2iq-labs/capi-runtime-extensions/pkg/handlers/generic/mutation/httpproxy"
2527
httpproxytests "github.com/d2iq-labs/capi-runtime-extensions/pkg/handlers/generic/mutation/httpproxy/tests"
28+
"github.com/d2iq-labs/capi-runtime-extensions/pkg/handlers/generic/mutation/imageregistries"
2629
imageregistrycredentials "github.com/d2iq-labs/capi-runtime-extensions/pkg/handlers/generic/mutation/imageregistries/credentials"
2730
imageregistrycredentialstests "github.com/d2iq-labs/capi-runtime-extensions/pkg/handlers/generic/mutation/imageregistries/credentials/tests"
2831
"github.com/d2iq-labs/capi-runtime-extensions/pkg/handlers/generic/mutation/kubernetesimagerepository"
@@ -50,11 +53,19 @@ func TestGeneratePatches(t *testing.T) {
5053
regiontests.TestGeneratePatches(
5154
t,
5255
metaPatchGeneratorFunc(mgr),
53-
"clusterConfig",
54-
"aws",
56+
clusterconfig.MetaVariableName,
57+
awsclusterconfig.AWSVariableName,
5558
region.VariableName,
5659
)
5760

61+
calicotests.TestGeneratePatches(
62+
t,
63+
metaPatchGeneratorFunc(mgr),
64+
clusterconfig.MetaVariableName,
65+
"addons",
66+
cni.VariableName,
67+
)
68+
5869
auditpolicytests.TestGeneratePatches(
5970
t,
6071
metaPatchGeneratorFunc(mgr),
@@ -63,44 +74,37 @@ func TestGeneratePatches(t *testing.T) {
6374
httpproxytests.TestGeneratePatches(
6475
t,
6576
metaPatchGeneratorFunc(mgr),
66-
"clusterConfig",
77+
clusterconfig.MetaVariableName,
6778
httpproxy.VariableName,
6879
)
6980

7081
etcdtests.TestGeneratePatches(
7182
t,
7283
metaPatchGeneratorFunc(mgr),
73-
"clusterConfig",
84+
clusterconfig.MetaVariableName,
7485
etcd.VariableName,
7586
)
7687

7788
extraapiservercertsanstests.TestGeneratePatches(
7889
t,
7990
metaPatchGeneratorFunc(mgr),
80-
"clusterConfig",
91+
clusterconfig.MetaVariableName,
8192
extraapiservercertsans.VariableName,
8293
)
8394

8495
kubernetesimagerepositorytests.TestGeneratePatches(
8596
t,
8697
metaPatchGeneratorFunc(mgr),
87-
"clusterConfig",
98+
clusterconfig.MetaVariableName,
8899
kubernetesimagerepository.VariableName,
89100
)
90101

91102
imageregistrycredentialstests.TestGeneratePatches(
92103
t,
93104
metaPatchGeneratorFunc(mgr),
94105
mgr.GetClient(),
95-
"clusterConfig",
96-
"imageRegistries",
106+
clusterconfig.MetaVariableName,
107+
imageregistries.VariableName,
97108
imageregistrycredentials.VariableName,
98109
)
99-
100-
calicotests.TestGeneratePatches(
101-
t,
102-
metaPatchGeneratorFunc(mgr),
103-
"clusterConfig",
104-
cni.VariableName,
105-
)
106110
}

0 commit comments

Comments
 (0)