WIP: CAPI Machine creation validation

Author: theobarberbany

manifests/0000_30_cluster-api_09_admission-policies.yaml

@@ -400,6 +400,46 @@ data:
           - Deny
     ---
     apiVersion: admissionregistration.k8s.io/v1
+    kind: ValidatingAdmissionPolicyBinding
+    metadata:
+      name: openshift-validate-capi-machine-creation
+    spec:
+      matchResources:
+        namespaceSelector:
+          matchLabels:
+            kubernetes.io/metadata.name: openshift-cluster-api
+      paramRef:
+        namespace: openshift-machine-api
+        parameterNotFoundAction: Allow
+        selector: {}
+      policyName: openshift-validate-capi-machine-creation
+      validationActions:
+          - Deny
+    ---
+    apiVersion: admissionregistration.k8s.io/v1
+    kind: ValidatingAdmissionPolicy
+    metadata:
+      name: openshift-validate-capi-machine-creation 
+    spec:
+      failurePolicy: Fail
+      paramKind:
+        apiVersion: machine.openshift.io/v1beta1
+        kind: Machine
+      matchConstraints:
+        resourceRules:
+        - apiGroups:   ["cluster.x-k8s.io"]
+          apiVersions: ["v1beta1"]
+          operations:  ["UPDATE"]
+          resources:   ["machines"]
+      # Requests must satisfy every matchCondition to reach the validations
+      matchConditions:
+        - name: check-param-match
+          expression: 'object.metadata.name == params.metadata.name'
+      # All validations must evaluate to true
+      validations:
+      - expression: 'true'
+    ---
+    apiVersion: admissionregistration.k8s.io/v1
     kind: ValidatingAdmissionPolicy
     metadata:
       name: openshift-prevent-migration-when-machine-updating

pkg/controllers/machinesync/machine_sync_controller_test.go

@@ -1876,6 +1876,115 @@ var _ = Describe("With a running MachineSync Reconciler", func() {
 			})
 		})
 
+		FContext("cluster api machine validation", func() {
+			var vapName = "openshift-validate-capi-machine-creation"
+
+			BeforeEach(func() {
+				By("Waiting for VAP to be ready")
+				machineVap = &admissionregistrationv1.ValidatingAdmissionPolicy{}
+				Eventually(k8sClient.Get(ctx, client.ObjectKey{Name: vapName}, machineVap), timeout).Should(Succeed())
+				Eventually(k.Update(machineVap, func() {
+					admissiontestutils.AddSentinelValidation(machineVap)
+				})).Should(Succeed())
+
+				Eventually(k.Object(machineVap), timeout).Should(
+					HaveField("Status.ObservedGeneration", BeNumerically(">=", 2)),
+				)
+
+				By("Updating the VAP binding")
+				policyBinding = &admissionregistrationv1.ValidatingAdmissionPolicyBinding{}
+				Eventually(k8sClient.Get(ctx, client.ObjectKey{
+					Name: vapName}, policyBinding), timeout).Should(Succeed())
+
+				Eventually(k.Update(policyBinding, func() {
+					admissiontestutils.UpdateVAPBindingNamespaces(policyBinding, mapiNamespace.GetName(), capiNamespace.GetName())
+				}), timeout).Should(Succeed())
+
+				// Wait until the binding shows the patched values
+				Eventually(k.Object(policyBinding), timeout).Should(
+					SatisfyAll(
+						HaveField("Spec.ParamRef.Namespace",
+							Equal(mapiNamespace.GetName())),
+
+						HaveField("Spec.MatchResources.NamespaceSelector.MatchLabels",
+							HaveKeyWithValue("kubernetes.io/metadata.name",
+								capiNamespace.GetName())),
+					),
+				)
+
+				By("Creating the sentinel CAPI infra machine")
+				capaMachine = capaMachineBuilder.WithName("sentinel-machine").Build()
+				Eventually(k8sClient.Create(ctx, capaMachine)).Should(Succeed(), "capa machine should be able to be created")
+
+				capaMachineRef := corev1.ObjectReference{
+					Kind:      capaMachine.Kind,
+					Name:      capaMachine.GetName(),
+					Namespace: capaMachine.GetNamespace(),
+				}
+
+				By("Creating a sentinel CAPI machine")
+				testMachine := capiMachineBuilder.WithName("sentinel-machine").WithInfrastructureRef(capaMachineRef).Build()
+				Eventually(k8sClient.Create(ctx, testMachine), timeout).Should(Succeed())
+
+				By("setting the owner reference on the sentinel CAPI infra machine")
+				Eventually(k.Update(capaMachine, func() {
+					capaMachine.SetOwnerReferences([]metav1.OwnerReference{
+						{
+							Kind:               machineKind,
+							APIVersion:         clusterv1.GroupVersion.String(),
+							Name:               testMachine.Name,
+							UID:                testMachine.UID,
+							BlockOwnerDeletion: ptr.To(true),
+							Controller:         ptr.To(false),
+						},
+					})
+				})).Should(Succeed())
+
+				By("Creating a sentinel MAPI Machine")
+				testMapiMachine := mapiMachineBuilder.WithName(testMachine.Name).Build()
+				Eventually(k8sClient.Create(ctx, testMapiMachine), timeout).Should(Succeed())
+
+				By("Setting the sentinel MAPI machine AuthoritativeAPI to Machine API")
+				Eventually(k.UpdateStatus(testMapiMachine, func() {
+					testMapiMachine.Status.AuthoritativeAPI = mapiv1beta1.MachineAuthorityMachineAPI
+				})).Should(Succeed())
+
+				Eventually(k.Object(testMapiMachine), timeout).Should(
+					HaveField("Status.AuthoritativeAPI", Equal(mapiv1beta1.MachineAuthorityMachineAPI)))
+
+				// The sync controller copies labels MAPI → CAPI. Labels under
+				// machine.openshift.io/* and cluster.x-k8s.io/* are controller-managed and
+				// write-protected by the labels rule. If we update before these labels are
+				// populated, our change can drop them and be rejected. Wait for sync, then add
+				// the test sentinel.
+
+				Eventually(k.Object(testMachine), timeout).Should(
+					HaveField("ObjectMeta.Labels", Not(BeNil())),
+				)
+
+				Eventually(k.Update(testMachine, func() {
+					testMachine.ObjectMeta.Labels["test-sentinel"] = "fubar"
+				}), timeout).Should(MatchError(ContainSubstring("policy in place")))
+			})
+
+			Context("with status.authoritativeAPI: Machine API", func() {
+				BeforeEach(func() {
+					By("Setting the MAPI machine AuthoritativeAPI to Machine API")
+					Eventually(k.UpdateStatus(mapiMachine, func() {
+						mapiMachine.Status.AuthoritativeAPI = mapiv1beta1.MachineAuthorityMachineAPI
+					})).Should(Succeed())
+
+					Eventually(k.Object(mapiMachine), timeout).Should(
+						HaveField("Status.AuthoritativeAPI", Equal(mapiv1beta1.MachineAuthorityMachineAPI)))
+				})
+
+				It("updating the spec should be prevented", func() {
+					Eventually(k.Update(capiMachine, func() {
+					}), timeout).Should(MatchError(ContainSubstring("Changing .spec is not allowed")))
+				})
+			})
+		})
+
 	})
 })