Skip to content

Commit b944df0

Browse files
theobarberbanytheobarberbany
committed
CAPI MachineSet VAP
1 parent 844e790 commit b944df0

File tree

4 files changed

+345
-3
lines changed

4 files changed

+345
-3
lines changed

‎manifests/0000_30_cluster-api_09_admission-policies.yaml‎

Lines changed: 108 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -277,7 +277,7 @@ data:
277277
variables.newLabels[?k].orValue(null) == variables.oldLabels[?k].orValue(null) ||
278278
variables.newLabels[?k].orValue(null) == variables.paramLabels[k]
279279
)
280-
message: "Cannot modify a Cluster API controlled label except to match the Cluster API mirrored machine. This is because status.authoritativeAPI is set to Cluster API."
280+
message: "Cannot modify a Cluster API controlled label except to match the Cluster API mirrored MachineSet. This is because status.authoritativeAPI is set to Cluster API."
281281
---
282282
apiVersion: admissionregistration.k8s.io/v1
283283
kind: ValidatingAdmissionPolicyBinding
@@ -391,6 +391,113 @@ data:
391391
message: "Setting the 'machine-template-hash' label is forbidden.'"
392392
---
393393
apiVersion: admissionregistration.k8s.io/v1
394+
kind: ValidatingAdmissionPolicyBinding
395+
metadata:
396+
name: cluster-api-machine-set-vap
397+
spec:
398+
matchResources:
399+
namespaceSelector:
400+
matchLabels:
401+
kubernetes.io/metadata.name: openshift-cluster-api
402+
paramRef:
403+
namespace: openshift-machine-api
404+
# We 'Allow' here as we don't want to block CAPI Machine functionality
405+
# when no MAPI machine (param) exists. This might happen when a user
406+
# wants to not use MAPI, or is migrating.
407+
parameterNotFoundAction: Allow
408+
selector: {}
409+
policyName: cluster-api-machine-set-vap
410+
validationActions: [Deny]
411+
---
412+
apiVersion: admissionregistration.k8s.io/v1
413+
kind: ValidatingAdmissionPolicy
414+
metadata:
415+
name: cluster-api-machine-set-vap
416+
spec:
417+
failurePolicy: Fail
418+
419+
paramKind:
420+
apiVersion: machine.openshift.io/v1beta1
421+
kind: MachineSet
422+
423+
matchConstraints:
424+
resourceRules:
425+
- apiGroups: ["cluster.x-k8s.io"]
426+
apiVersions: ["v1beta2"]
427+
operations: ["UPDATE"]
428+
resources: ["machinesets"]
429+
430+
# Requests must satisfy every matchCondition to reach the validations
431+
matchConditions:
432+
- name: check-only-non-service-account-requests
433+
expression: >-
434+
!(request.userInfo.username in [
435+
"system:serviceaccount:openshift-machine-api:machine-api-controllers",
436+
"system:serviceaccount:openshift-cluster-api:cluster-capi-operator"
437+
])
438+
- name: check-param-match
439+
expression: 'object.metadata.name == params.metadata.name'
440+
- name: check-authoritativeAPI-machineapi
441+
expression: "params.?status.?authoritativeAPI.orValue(\"\") == \"MachineAPI\""
442+
variables:
443+
# label maps
444+
- name: newLabels
445+
expression: "object.metadata.?labels.orValue({})"
446+
- name: oldLabels
447+
expression: "oldObject.metadata.?labels.orValue({})"
448+
- name: paramLabels
449+
expression: "params.metadata.?labels.orValue({})"
450+
451+
# annotation maps
452+
- name: newAnn
453+
expression: "object.metadata.?annotations.orValue({})"
454+
- name: oldAnn
455+
expression: "oldObject.metadata.?annotations.orValue({})"
456+
457+
# All validations must evaluate to TRUE
458+
validations:
459+
# Only spec.authoritativeAPI may change
460+
- expression: "object.spec == oldObject.spec"
461+
message: "Changing .spec is not allowed. This is because status.authoritativeAPI is set to Machine API."
462+
463+
# Guard machine.openshift.io/* and kubernetes.io/* and cluster.x-k8s.io/* labels
464+
- expression: >
465+
!(
466+
variables.newLabels.exists(k,
467+
(k.startsWith('machine.openshift.io') || k.startsWith('kubernetes.io') || k.contains('cluster.x-k8s.io/')) &&
468+
(variables.oldLabels[?k].orValue(null) != variables.newLabels[k])
469+
) ||
470+
variables.oldLabels.exists(k,
471+
(k.startsWith('machine.openshift.io') || k.startsWith('kubernetes.io') || k.contains('cluster.x-k8s.io/')) &&
472+
!(k in variables.newLabels)
473+
)
474+
)
475+
message: "Cannot add, modify or delete any machine.openshift.io/*, kubernetes.io/* or cluster.x-k8s.io/* label. This is because status.authoritativeAPI is set to Machine API."
476+
477+
# Guard machine.openshift.io/* and cluster.x-k8s.io/* and clusters.x-k8s.io/* annotations
478+
- expression: >
479+
!(
480+
variables.newAnn.exists(k,
481+
(k.startsWith('machine.openshift.io') || k.contains('cluster.x-k8s.io') || k.contains('clusters.x-k8s.io')) &&
482+
(variables.oldAnn[?k].orValue(null) != variables.newAnn[k])
483+
) ||
484+
variables.oldAnn.exists(k,
485+
(k.startsWith('machine.openshift.io') || k.contains('cluster.x-k8s.io') || k.contains('clusters.x-k8s.io')) &&
486+
!(k in variables.newAnn)
487+
)
488+
)
489+
message: "Cannot add, modify or delete any machine.openshift.io/* or cluster.x-k8s.io or clusters.x-k8s.io annotation. This is because status.authoritativeAPI is set to Machine API."
490+
491+
# Param-controlled labels (labels on the MAPI machine) may change only to match the value on the MAPI Machine
492+
- expression: >
493+
variables.paramLabels.all(
494+
k,
495+
variables.newLabels[?k].orValue(null) == variables.oldLabels[?k].orValue(null) ||
496+
variables.newLabels[?k].orValue(null) == variables.paramLabels[k]
497+
)
498+
message: "Cannot modify a Machine API controlled label except to match the Machine API mirrored MachineSet. This is because status.authoritativeAPI is set to Machine API."
499+
---
500+
apiVersion: admissionregistration.k8s.io/v1
394501
kind: ValidatingAdmissionPolicy
395502
metadata:
396503
name: openshift-cluster-api-prevent-setting-of-capi-fields-unsupported-by-mapi

‎pkg/controllers/machinesetsync/machineset_vap_test.go‎

Lines changed: 237 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -417,7 +417,7 @@ var _ = Describe("MachineSet VAP Tests", func() {
417417

418418
Eventually(k.Get(capiSentinelMachineSet)).Should(Succeed())
419419

420-
admissiontestutils.VerifySentinelValidation(k, sentinelMachineSet, 2*timeout)
420+
admissiontestutils.VerifySentinelValidation(k, sentinelMachineSet, timeout)
421421

422422
By("Creating a shared machineset pair to be used across the tests")
423423
mapiMachineSetBuilder = machinev1resourcebuilder.MachineSet().
@@ -585,7 +585,7 @@ var _ = Describe("MachineSet VAP Tests", func() {
585585
It("rejects changing a label to differ from the param machine", func() {
586586
Eventually(k.Update(mapiMachineSet, func() {
587587
mapiMachineSet.Labels["capi-param-controlled-label"] = "foo"
588-
}), timeout).Should(MatchError(ContainSubstring("Cannot modify a Cluster API controlled label except to match the Cluster API mirrored machine")))
588+
}), timeout).Should(MatchError(ContainSubstring("Cannot modify a Cluster API controlled label except to match the Cluster API mirrored MachineSet")))
589589
})
590590
})
591591

@@ -599,4 +599,239 @@ var _ = Describe("MachineSet VAP Tests", func() {
599599

600600
})
601601
})
602+
603+
Context("Prevent changes to non-authoritative CAPI MachineSets except from sync controller", func() {
604+
var mapiMachineSetBuilder machinev1resourcebuilder.MachineSetBuilder
605+
var mapiMachineSet *mapiv1beta1.MachineSet
606+
607+
const (
608+
vapName string = "cluster-api-machine-set-vap"
609+
testLabelValue string = "test-value"
610+
)
611+
612+
BeforeEach(func() {
613+
By("Waiting for VAP to be ready")
614+
machineSetVap = &admissionregistrationv1.ValidatingAdmissionPolicy{}
615+
Eventually(k8sClient.Get(ctx, client.ObjectKey{Name: vapName}, machineSetVap), timeout).Should(Succeed())
616+
617+
Eventually(k.Update(machineSetVap, func() {
618+
admissiontestutils.AddSentinelValidation(machineSetVap)
619+
})).Should(Succeed())
620+
621+
Eventually(k.Object(machineSetVap), timeout).Should(
622+
HaveField("Status.ObservedGeneration", BeNumerically(">=", 2)),
623+
)
624+
625+
By("Updating the VAP binding")
626+
policyBinding = &admissionregistrationv1.ValidatingAdmissionPolicyBinding{}
627+
Eventually(k8sClient.Get(ctx, client.ObjectKey{
628+
Name: vapName}, policyBinding), timeout).Should(Succeed())
629+
630+
Eventually(k.Update(policyBinding, func() {
631+
// paramNamespace=mapiNamespace (MAPI resources are params)
632+
// targetNamespace=capiNamespace (CAPI resources are validated)
633+
admissiontestutils.UpdateVAPBindingNamespaces(policyBinding, mapiNamespace.GetName(), capiNamespace.GetName())
634+
}), timeout).Should(Succeed())
635+
636+
// Wait until the binding shows the patched values
637+
Eventually(k.Object(policyBinding), timeout).Should(
638+
SatisfyAll(
639+
HaveField("Spec.MatchResources.NamespaceSelector.MatchLabels",
640+
HaveKeyWithValue("kubernetes.io/metadata.name",
641+
capiNamespace.GetName())),
642+
),
643+
)
644+
645+
By("Creating throwaway MachineSet pair for sentinel validation")
646+
sentinelMachineSet := machinev1resourcebuilder.MachineSet().
647+
WithNamespace(mapiNamespace.Name).
648+
WithName("sentinel-machineset").
649+
WithAuthoritativeAPI(mapiv1beta1.MachineAuthorityMachineAPI).
650+
Build()
651+
Eventually(k8sClient.Create(ctx, sentinelMachineSet), timeout).Should(Succeed())
652+
653+
Eventually(k.UpdateStatus(sentinelMachineSet, func() {
654+
sentinelMachineSet.Status.AuthoritativeAPI = mapiv1beta1.MachineAuthorityMachineAPI
655+
})).Should(Succeed())
656+
657+
Eventually(k.Object(sentinelMachineSet), timeout).Should(
658+
HaveField("Status.AuthoritativeAPI", Equal(mapiv1beta1.MachineAuthorityMachineAPI)))
659+
660+
capiSentinelMachineSet := clusterv1resourcebuilder.MachineSet().
661+
WithName("sentinel-machineset").
662+
WithNamespace(capiNamespace.Name).
663+
WithTemplate(clusterv1.MachineTemplateSpec{
664+
Spec: clusterv1.MachineSpec{
665+
ProviderID: "force-having-a-spec",
666+
},
667+
}).
668+
Build()
669+
Eventually(k8sClient.Create(ctx, capiSentinelMachineSet)).Should(Succeed())
670+
671+
Eventually(k.Get(capiSentinelMachineSet)).Should(Succeed())
672+
673+
admissiontestutils.VerifySentinelValidation(k, capiSentinelMachineSet, timeout)
674+
675+
By("Creating a shared machineset pair to be used across the tests")
676+
mapiMachineSetBuilder = machinev1resourcebuilder.MachineSet().
677+
WithNamespace(mapiNamespace.Name).
678+
WithName(capiMachineSet.Name).
679+
WithProviderSpecBuilder(machinev1resourcebuilder.AWSProviderSpec().WithLoadBalancers(nil)).
680+
WithLabels(map[string]string{
681+
"machine.openshift.io/cluster-api-cluster": "ci-op-gs2k97d6-c9e33-2smph",
682+
"mapi-param-controlled-label": "param-controlled-key",
683+
}).WithAnnotations(map[string]string{
684+
"capacity.cluster-autoscaler.kubernetes.io/labels": "kubernetes.io/arch=amd64",
685+
"machine.openshift.io/GPU": "0",
686+
"machine.openshift.io/memoryMb": "16384",
687+
"machine.openshift.io/vCPU": "4",
688+
})
689+
mapiMachineSet = mapiMachineSetBuilder.Build()
690+
Eventually(k8sClient.Create(ctx, mapiMachineSet), timeout).Should(Succeed())
691+
692+
capiMachineSet = capiMachineSetBuilder.WithLabels(map[string]string{
693+
"machine.openshift.io/cluster-api-cluster": "ci-op-gs2k97d6-c9e33-2smph",
694+
"cluster.x-k8s.io/cluster-name": "ci-op-gs2k97d6-c9e33-2smph",
695+
696+
"capi-param-controlled-label": "param-controlled-key",
697+
}).WithAnnotations(map[string]string{
698+
"capacity.cluster-autoscaler.kubernetes.io/labels": "kubernetes.io/arch=amd64",
699+
"machine.openshift.io/GPU": "0",
700+
"machine.openshift.io/memoryMb": "16384",
701+
"machine.openshift.io/vCPU": "4",
702+
}).Build()
703+
704+
Eventually(k8sClient.Create(ctx, capiMachineSet), timeout).Should(Succeed())
705+
706+
})
707+
Context("with status.authoritativeAPI: Machine API (on MAPI MachineSet)", func() {
708+
BeforeEach(func() {
709+
By("Setting the MAPI MachineSet AuthoritativeAPI to Machine API")
710+
Eventually(k.UpdateStatus(mapiMachineSet, func() {
711+
mapiMachineSet.Status.AuthoritativeAPI = mapiv1beta1.MachineAuthorityMachineAPI
712+
})).Should(Succeed())
713+
714+
Eventually(k.Object(mapiMachineSet), timeout).Should(
715+
HaveField("Status.AuthoritativeAPI", Equal(mapiv1beta1.MachineAuthorityMachineAPI)))
716+
})
717+
718+
It("updating the spec should be prevented", func() {
719+
Eventually(k.Update(capiMachineSet, func() {
720+
replicas := int32(5)
721+
capiMachineSet.Spec.Replicas = &replicas
722+
}), timeout).Should(MatchError(ContainSubstring("Changing .spec is not allowed")))
723+
})
724+
725+
It("updating the spec.AuthoritativeAPI (on the MAPI MachineSet) should be allowed", func() {
726+
Eventually(k.Update(mapiMachineSet, func() {
727+
mapiMachineSet.Spec.AuthoritativeAPI = mapiv1beta1.MachineAuthorityClusterAPI
728+
}), timeout).Should(Succeed())
729+
})
730+
731+
Context("when trying to update metadata.labels", func() {
732+
It("rejects modification of the protected machine.openshift.io label", func() {
733+
Eventually(k.Update(capiMachineSet, func() {
734+
capiMachineSet.Labels["machine.openshift.io/cluster-api-cluster"] = "different-cluster"
735+
}), timeout).Should(MatchError(ContainSubstring("Cannot add, modify or delete any machine.openshift.io/*, kubernetes.io/* or cluster.x-k8s.io/* label")))
736+
})
737+
738+
It("rejects deletion of the protected machine.openshift.io label", func() {
739+
Eventually(k.Update(capiMachineSet, func() {
740+
delete(capiMachineSet.Labels, "machine.openshift.io/cluster-api-cluster")
741+
}), timeout).Should(MatchError(ContainSubstring("Cannot add, modify or delete any machine.openshift.io/*, kubernetes.io/* or cluster.x-k8s.io/* label")))
742+
})
743+
744+
It("rejects setting of the protected machine.openshift.io label to the empty string ''", func() {
745+
Eventually(k.Update(capiMachineSet, func() {
746+
capiMachineSet.Labels["machine.openshift.io/cluster-api-cluster"] = ""
747+
}), timeout).Should(MatchError(ContainSubstring("Cannot add, modify or delete any machine.openshift.io/*, kubernetes.io/* or cluster.x-k8s.io/* label")))
748+
})
749+
750+
It("rejects adding a new machine.openshift.io label", func() {
751+
Eventually(k.Update(capiMachineSet, func() {
752+
capiMachineSet.Labels["machine.openshift.io/foo"] = testLabelValue
753+
}), timeout).Should(MatchError(ContainSubstring("Cannot add, modify or delete any machine.openshift.io/*, kubernetes.io/* or cluster.x-k8s.io/* label")))
754+
})
755+
756+
It("rejects adding a new machine.openshift.io label with an empty string value", func() {
757+
Eventually(k.Update(capiMachineSet, func() {
758+
capiMachineSet.Labels["machine.openshift.io/foo"] = ""
759+
}), timeout).Should(MatchError(ContainSubstring("Cannot add, modify or delete any machine.openshift.io/*, kubernetes.io/* or cluster.x-k8s.io/* label")))
760+
})
761+
762+
It("rejects modification of the protected cluster.x-k8s.io label", func() {
763+
Eventually(k.Update(capiMachineSet, func() {
764+
capiMachineSet.Labels["cluster.x-k8s.io/cluster-name"] = "different-cluster"
765+
}), timeout).Should(MatchError(ContainSubstring("Cannot add, modify or delete any machine.openshift.io/*, kubernetes.io/* or cluster.x-k8s.io/* label")))
766+
})
767+
768+
It("rejects deletion of the protected cluster.x-k8s.io label", func() {
769+
Eventually(k.Update(capiMachineSet, func() {
770+
delete(capiMachineSet.Labels, "cluster.x-k8s.io/cluster-name")
771+
}), timeout).Should(MatchError(ContainSubstring("Cannot add, modify or delete any machine.openshift.io/*, kubernetes.io/* or cluster.x-k8s.io/* label")))
772+
})
773+
774+
It("allows modification of a non-protected label", func() {
775+
Eventually(k.Update(capiMachineSet, func() {
776+
capiMachineSet.Labels["test"] = "val"
777+
}), timeout).Should(Succeed(), "expected success when modifying unrelated labels")
778+
})
779+
})
780+
781+
Context("when trying to update metadata.Annotations", func() {
782+
It("rejects modification of a protected machine.openshift.io annotation", func() {
783+
Eventually(k.Update(capiMachineSet, func() {
784+
capiMachineSet.Annotations["machine.openshift.io/vCPU"] = "8"
785+
}), timeout).Should(MatchError(ContainSubstring("Cannot add, modify or delete any machine.openshift.io/* or cluster.x-k8s.io or clusters.x-k8s.io annotation")))
786+
})
787+
788+
It("rejects deletion of a protected machine.openshift.io annotation", func() {
789+
Eventually(k.Update(capiMachineSet, func() {
790+
delete(capiMachineSet.Annotations, "machine.openshift.io/vCPU")
791+
}), timeout).Should(MatchError(ContainSubstring("Cannot add, modify or delete any machine.openshift.io/* or cluster.x-k8s.io or clusters.x-k8s.io annotation")))
792+
})
793+
794+
It("rejects modification of a protected machine.openshift.io annotation to the empty string ''", func() {
795+
Eventually(k.Update(capiMachineSet, func() {
796+
capiMachineSet.Annotations["machine.openshift.io/vCPU"] = ""
797+
}), timeout).Should(MatchError(ContainSubstring("Cannot add, modify or delete any machine.openshift.io/* or cluster.x-k8s.io or clusters.x-k8s.io annotation")))
798+
})
799+
800+
It("rejects adding a new protected machine.openshift.io annotation", func() {
801+
Eventually(k.Update(capiMachineSet, func() {
802+
capiMachineSet.Annotations["machine.openshift.io/foo"] = testLabelValue
803+
}), timeout).Should(MatchError(ContainSubstring("Cannot add, modify or delete any machine.openshift.io/* or cluster.x-k8s.io or clusters.x-k8s.io annotation")))
804+
})
805+
806+
It("rejects adding a new protected machine.openshift.io annotation with an empty string value", func() {
807+
Eventually(k.Update(capiMachineSet, func() {
808+
capiMachineSet.Annotations["machine.openshift.io/foo"] = ""
809+
}), timeout).Should(MatchError(ContainSubstring("Cannot add, modify or delete any machine.openshift.io/* or cluster.x-k8s.io or clusters.x-k8s.io annotation")))
810+
})
811+
812+
It("allows modification of a non-protected annotation", func() {
813+
Eventually(k.Update(capiMachineSet, func() {
814+
capiMachineSet.Annotations["bar"] = "baz"
815+
}), timeout).Should(Succeed(), "expected success when modifying unrelated annotations")
816+
})
817+
})
818+
819+
Context("when trying to update Machine API owned metadata.labels", func() {
820+
It("allows changing a metadata label to match the param MachineSet", func() {
821+
Eventually(k.Object(mapiMachineSet), timeout).Should(
822+
HaveField("Labels", HaveKeyWithValue("mapi-param-controlled-label", "param-controlled-key")))
823+
824+
Eventually(k.Update(capiMachineSet, func() {
825+
capiMachineSet.Labels["mapi-param-controlled-label"] = "param-controlled-key"
826+
}), timeout).Should(Succeed(), "expected success when updating label to match MAPI MachineSet")
827+
})
828+
829+
It("rejects changing a label to differ from the param MachineSet", func() {
830+
Eventually(k.Update(capiMachineSet, func() {
831+
capiMachineSet.Labels["mapi-param-controlled-label"] = testLabelValue
832+
}), timeout).Should(MatchError(ContainSubstring("Cannot modify a Machine API controlled label except to match the Machine API mirrored MachineSet")))
833+
})
834+
})
835+
})
836+
})
602837
})

‎pkg/controllers/machinesync/__debug_bin2952830453‎

Whitespace-only changes.

‎pkg/controllers/machinesync/__debug_bin4126191047‎

Whitespace-only changes.

0 commit comments

Comments
 (0)