Skip to content

Commit

Permalink
fixup! fix: Ensure patch is idempotent
Browse files Browse the repository at this point in the history
  • Loading branch information
@jimmidyson
jimmidyson committed Oct 5, 2023
1 parent 2b18b09 commit 4569a22
Show file tree
Hide file tree
Showing 3 changed files with 208 additions and 8 deletions.
24 changes: 18 additions & 6 deletions common/pkg/testutils/capitest/request/items.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -113,14 +113,26 @@ func NewKubeadmControlPlaneTemplateRequestItem(

func NewAWSClusterTemplateRequestItem(
uid types.UID,
existingSpec ...capav1.AWSClusterTemplateSpec,
) runtimehooksv1.GeneratePatchesRequestItem {
return NewRequestItem(
&capav1.AWSClusterTemplate{
TypeMeta: metav1.TypeMeta{
APIVersion: capav1.GroupVersion.String(),
Kind: "AWSClusterTemplate",
},
awsClusterTemplate := &capav1.AWSClusterTemplate{
TypeMeta: metav1.TypeMeta{
APIVersion: capav1.GroupVersion.String(),
Kind: "AWSClusterTemplate",
},
}

switch len(existingSpec) {
case 0:
// Do nothing.
case 1:
awsClusterTemplate.Spec = existingSpec[0]
default:
panic("can only take at most one existing spec")
}

return NewRequestItem(
awsClusterTemplate,
&runtimehooksv1.HolderReference{
Kind: "Cluster",
FieldPath: "spec.infrastructureRef",
Expand Down
18 changes: 17 additions & 1 deletion pkg/handlers/aws/mutation/cni/calico/inject.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ package calico
import (
"context"
_ "embed"
"slices"

"github.com/go-logr/logr"
apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1"
Expand Down Expand Up @@ -149,8 +150,9 @@ func mutateAWSClusterTemplateFunc(log logr.Logger) func(obj *capav1.AWSClusterTe
if obj.Spec.Template.Spec.NetworkSpec.CNI == nil {
obj.Spec.Template.Spec.NetworkSpec.CNI = &capav1.CNISpec{}
}
obj.Spec.Template.Spec.NetworkSpec.CNI.CNIIngressRules = append(
obj.Spec.Template.Spec.NetworkSpec.CNI.CNIIngressRules = addOrUpdateCNIIngressRules(
obj.Spec.Template.Spec.NetworkSpec.CNI.CNIIngressRules,

capav1.CNIIngressRule{
Description: "typha (calico)",
Protocol: capav1.SecurityGroupProtocolTCP,
Expand Down Expand Up @@ -186,3 +188,17 @@ func mutateAWSClusterTemplateFunc(log logr.Logger) func(obj *capav1.AWSClusterTe
return nil
}
}

func addOrUpdateCNIIngressRules(
rules []capav1.CNIIngressRule, newRules ...capav1.CNIIngressRule,
) []capav1.CNIIngressRule {
clonedRules := slices.Clone(rules)

for _, newRule := range newRules {
if !slices.Contains(clonedRules, newRule) {
clonedRules = append(clonedRules, newRule)
}
}

return clonedRules
}
174 changes: 173 additions & 1 deletion pkg/handlers/aws/mutation/cni/calico/tests/generate_patches.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ func TestGeneratePatches(
Path: "/spec/template/spec/network/cni",
ValueMatcher: gomega.HaveKeyWithValue(
"cniIngressRules",
gomega.ContainElements(
gomega.ConsistOf(
gomega.SatisfyAll(
gomega.HaveKeyWithValue("description", "typha (calico)"),
gomega.HaveKeyWithValue(
Expand Down Expand Up @@ -101,5 +101,177 @@ func TestGeneratePatches(
),
}},
},
capitest.PatchTestDef{
Name: "provider set with AWSClusterTemplate pre-existing rules",
Vars: []runtimehooksv1.Variable{
capitest.VariableWithValue(
variableName,
v1alpha1.CNI{
Provider: v1alpha1.CNIProviderCalico,
},
variablePath...,
),
},
RequestItem: request.NewAWSClusterTemplateRequestItem(
"1234",
capav1.AWSClusterTemplateSpec{
Template: capav1.AWSClusterTemplateResource{
Spec: capav1.AWSClusterSpec{
NetworkSpec: capav1.NetworkSpec{
CNI: &capav1.CNISpec{
CNIIngressRules: []capav1.CNIIngressRule{{
Description: "test",
Protocol: capav1.SecurityGroupProtocolAll,
FromPort: 1234,
ToPort: 12345,
}},
},
},
},
},
},
),
ExpectedPatchMatchers: []capitest.JSONPatchMatcher{{
Operation: "add",
Path: "/spec/template/spec/network/cni/cniIngressRules/1",
ValueMatcher: gomega.SatisfyAll(
gomega.HaveKeyWithValue("description", "typha (calico)"),
gomega.HaveKeyWithValue(
"protocol",
gomega.BeEquivalentTo(capav1.SecurityGroupProtocolTCP),
),
gomega.HaveKeyWithValue("fromPort", gomega.BeEquivalentTo(5473)),
gomega.HaveKeyWithValue("toPort", gomega.BeEquivalentTo(5473)),
),
}, {
Operation: "add",
Path: "/spec/template/spec/network/cni/cniIngressRules/2",
ValueMatcher: gomega.SatisfyAll(
gomega.HaveKeyWithValue("description", "bgp (calico)"),
gomega.HaveKeyWithValue(
"protocol",
gomega.BeEquivalentTo(capav1.SecurityGroupProtocolTCP),
),
gomega.HaveKeyWithValue("fromPort", gomega.BeEquivalentTo(179)),
gomega.HaveKeyWithValue("toPort", gomega.BeEquivalentTo(179)),
),
}, {
Operation: "add",
Path: "/spec/template/spec/network/cni/cniIngressRules/3",
ValueMatcher: gomega.SatisfyAll(
gomega.HaveKeyWithValue("description", "IP-in-IP (calico)"),
gomega.HaveKeyWithValue(
"protocol",
gomega.BeEquivalentTo(capav1.SecurityGroupProtocolIPinIP),
),
gomega.HaveKeyWithValue("fromPort", gomega.BeEquivalentTo(-1)),
gomega.HaveKeyWithValue("toPort", gomega.BeEquivalentTo(65535)),
),
}, {
Operation: "add",
Path: "/spec/template/spec/network/cni/cniIngressRules/4",
ValueMatcher: gomega.SatisfyAll(
gomega.HaveKeyWithValue("description", "node metrics (calico)"),
gomega.HaveKeyWithValue(
"protocol",
gomega.BeEquivalentTo(capav1.SecurityGroupProtocolTCP),
),
gomega.HaveKeyWithValue("fromPort", gomega.BeEquivalentTo(9091)),
gomega.HaveKeyWithValue("toPort", gomega.BeEquivalentTo(9091)),
),
}, {
Operation: "add",
Path: "/spec/template/spec/network/cni/cniIngressRules/5",
ValueMatcher: gomega.SatisfyAll(
gomega.HaveKeyWithValue("description", "typha metrics (calico)"),
gomega.HaveKeyWithValue(
"protocol",
gomega.BeEquivalentTo(capav1.SecurityGroupProtocolTCP),
),
gomega.HaveKeyWithValue("fromPort", gomega.BeEquivalentTo(9093)),
gomega.HaveKeyWithValue("toPort", gomega.BeEquivalentTo(9093)),
),
}},
},
capitest.PatchTestDef{
Name: "provider set with AWSClusterTemplate conflicting pre-existing rules",
Vars: []runtimehooksv1.Variable{
capitest.VariableWithValue(
variableName,
v1alpha1.CNI{
Provider: v1alpha1.CNIProviderCalico,
},
variablePath...,
),
},
RequestItem: request.NewAWSClusterTemplateRequestItem(
"1234",
capav1.AWSClusterTemplateSpec{
Template: capav1.AWSClusterTemplateResource{
Spec: capav1.AWSClusterSpec{
NetworkSpec: capav1.NetworkSpec{
CNI: &capav1.CNISpec{
CNIIngressRules: []capav1.CNIIngressRule{{
Description: "typha (calico)",
Protocol: capav1.SecurityGroupProtocolTCP,
FromPort: 5473,
ToPort: 5473,
}},
},
},
},
},
},
),
ExpectedPatchMatchers: []capitest.JSONPatchMatcher{{
Operation: "add",
Path: "/spec/template/spec/network/cni/cniIngressRules/1",
ValueMatcher: gomega.SatisfyAll(
gomega.HaveKeyWithValue("description", "bgp (calico)"),
gomega.HaveKeyWithValue(
"protocol",
gomega.BeEquivalentTo(capav1.SecurityGroupProtocolTCP),
),
gomega.HaveKeyWithValue("fromPort", gomega.BeEquivalentTo(179)),
gomega.HaveKeyWithValue("toPort", gomega.BeEquivalentTo(179)),
),
}, {
Operation: "add",
Path: "/spec/template/spec/network/cni/cniIngressRules/2",
ValueMatcher: gomega.SatisfyAll(
gomega.HaveKeyWithValue("description", "IP-in-IP (calico)"),
gomega.HaveKeyWithValue(
"protocol",
gomega.BeEquivalentTo(capav1.SecurityGroupProtocolIPinIP),
),
gomega.HaveKeyWithValue("fromPort", gomega.BeEquivalentTo(-1)),
gomega.HaveKeyWithValue("toPort", gomega.BeEquivalentTo(65535)),
),
}, {
Operation: "add",
Path: "/spec/template/spec/network/cni/cniIngressRules/3",
ValueMatcher: gomega.SatisfyAll(
gomega.HaveKeyWithValue("description", "node metrics (calico)"),
gomega.HaveKeyWithValue(
"protocol",
gomega.BeEquivalentTo(capav1.SecurityGroupProtocolTCP),
),
gomega.HaveKeyWithValue("fromPort", gomega.BeEquivalentTo(9091)),
gomega.HaveKeyWithValue("toPort", gomega.BeEquivalentTo(9091)),
),
}, {
Operation: "add",
Path: "/spec/template/spec/network/cni/cniIngressRules/4",
ValueMatcher: gomega.SatisfyAll(
gomega.HaveKeyWithValue("description", "typha metrics (calico)"),
gomega.HaveKeyWithValue(
"protocol",
gomega.BeEquivalentTo(capav1.SecurityGroupProtocolTCP),
),
gomega.HaveKeyWithValue("fromPort", gomega.BeEquivalentTo(9093)),
gomega.HaveKeyWithValue("toPort", gomega.BeEquivalentTo(9093)),
),
}},
},
)
}

0 comments on commit 4569a22

Please sign in to comment.