Update Policy Evaluation Controller to add compute policies correctly

Author: dilyar85

config/rbac/role.yaml

@@ -327,7 +327,6 @@ rules:
   - vsphere.policy.vmware.com
   resources:
   - computepolicies/status
-  - policyevaluations/status
   - tagpolicies/status
   verbs:
   - get
@@ -342,3 +341,11 @@ rules:
   - patch
   - update
   - watch
+- apiGroups:
+  - vsphere.policy.vmware.com
+  resources:
+  - policyevaluations/status
+  verbs:
+  - get
+  - patch
+  - update

controllers/vspherepolicy/policyevaluation/policyevaluation_controller.go

@@ -86,7 +86,7 @@ type Reconciler struct {
 }
 
 // +kubebuilder:rbac:groups=vsphere.policy.vmware.com,resources=policyevaluations,verbs=create;get;list;watch;update;patch
-// +kubebuilder:rbac:groups=vsphere.policy.vmware.com,resources=policyevaluations/status,verbs=get
+// +kubebuilder:rbac:groups=vsphere.policy.vmware.com,resources=policyevaluations/status,verbs=get;update;patch
 // +kubebuilder:rbac:groups=vsphere.policy.vmware.com,resources=computepolicies,verbs=get;list;watch
 // +kubebuilder:rbac:groups=vsphere.policy.vmware.com,resources=computepolicies/status,verbs=get
 // +kubebuilder:rbac:groups=vsphere.policy.vmware.com,resources=tagpolicies,verbs=get;list;watch
@@ -134,8 +134,8 @@ func (r *Reconciler) ReconcileDelete(
 }
 
 const (
-	failMatchingPolicies = "failed to reconcile matching policies"
-	failExplicitPolicies = "failed to reconcile explicit policies"
+	failMandatoryPolicies = "failed to reconcile mandatory policies"
+	failExplicitPolicies  = "failed to reconcile explicit policies"
 )
 
 func (r *Reconciler) ReconcileNormal(
@@ -150,14 +150,14 @@ func (r *Reconciler) ReconcileNormal(
 	// Clear existing policies before reconciliation.
 	obj.Status.Policies = nil
 
-	if err := r.reconcileMatchingPolicies(ctx, obj); err != nil {
+	if err := r.reconcileMandatoryPolicies(ctx, obj); err != nil {
 		conditions.MarkError(
 			obj,
 			vspherepolv1.ReadyConditionType,
-			failMatchingPolicies,
+			failMandatoryPolicies,
 			err)
 		return ctrl.Result{}, fmt.Errorf(
-			failMatchingPolicies+": %w", err)
+			failMandatoryPolicies+": %w", err)
 	}
 
 	if err := r.reconcileExplicitPolicies(ctx, obj); err != nil {
@@ -179,19 +179,19 @@ func (r *Reconciler) ReconcileNormal(
 	return ctrl.Result{}, nil
 }
 
-func (r *Reconciler) reconcileMatchingPolicies(
+func (r *Reconciler) reconcileMandatoryPolicies(
 	ctx context.Context,
 	obj *vspherepolv1.PolicyEvaluation) error {
 
-	if err := r.reconcileMatchingComputePolicies(ctx, obj); err != nil {
+	if err := r.reconcileMandatoryComputePolicies(ctx, obj); err != nil {
 		return fmt.Errorf(
-			"failed to reconcile matching compute policies: %w", err)
+			"failed to reconcile mandatory compute policies: %w", err)
 	}
 
 	return nil
 }
 
-func (r *Reconciler) reconcileMatchingComputePolicies(
+func (r *Reconciler) reconcileMandatoryComputePolicies(
 	ctx context.Context,
 	obj *vspherepolv1.PolicyEvaluation) error {
 
@@ -205,6 +205,11 @@ func (r *Reconciler) reconcileMatchingComputePolicies(
 	}
 
 	for _, p := range list.Items {
+		// Only mandatory policies should be automatically applied.
+		if p.Spec.EnforcementMode != vspherepolv1.PolicyEnforcementModeMandatory {
+			continue
+		}
+
 		matches, err := matchesPolicy(obj, p)
 		if err != nil {
 			return err
@@ -540,6 +545,14 @@ func (r *Reconciler) addComputePolicyRef(
 		return fmt.Errorf("failed to get compute policy: %w", err)
 	}
 
+	matches, err := matchesPolicy(obj, pol)
+	if err != nil {
+		return err
+	}
+	if !matches {
+		return fmt.Errorf("compute policy %q does not match", pol.Name)
+	}
+
 	return r.addComputePolicy(ctx, obj, pol)
 }
 

controllers/vspherepolicy/policyevaluation/policyevaluation_controller_test.go

@@ -160,7 +160,7 @@ var _ = Describe("Reconcile", func() {
 				Expect(updated.Status.Policies).To(BeEmpty())
 			})
 
-			Context("with matching compute policy", func() {
+			Context("with mandatory compute policy", func() {
 				var computePolicy *vspherepolv1.ComputePolicy
 
 				BeforeEach(func() {
@@ -676,7 +676,7 @@ var _ = Describe("Reconcile", func() {
 						Expect(policy.APIVersion).To(Equal(vspherepolv1.GroupVersion.String()))
 						Expect(policy.Kind).To(Equal("ComputePolicy"))
 					}
-					Expect(policyNames).To(ConsistOf("policy-1", "policy-2"))
+					Expect(policyNames).To(ConsistOf("policy-1"))
 				})
 			})
 
@@ -1036,7 +1036,7 @@ var _ = Describe("Reconcile", func() {
 					}
 				})
 
-				It("should include explicitly referenced policies", func() {
+				It("should return an error if the explicit policy does not match", func() {
 					req := ctrl.Request{
 						NamespacedName: types.NamespacedName{
 							Name:      obj.Name,
@@ -1045,15 +1045,10 @@ var _ = Describe("Reconcile", func() {
 					}
 
 					result, err := reconciler.Reconcile(ctx, req)
-					Expect(err).ToNot(HaveOccurred())
+					Expect(err).To(HaveOccurred())
+					Expect(err.Error()).To(ContainSubstring("compute policy \"explicit-compute-policy\" does not match"))
 					Expect(result).To(Equal(ctrl.Result{}))
 
-					var updated vspherepolv1.PolicyEvaluation
-					Expect(client.Get(ctx, ctrlclient.ObjectKeyFromObject(obj), &updated)).To(Succeed())
-					Expect(updated.Status.Policies).To(HaveLen(1))
-					Expect(updated.Status.Policies[0].APIVersion).To(Equal(vspherepolv1.GroupVersion.String()))
-					Expect(updated.Status.Policies[0].Kind).To(Equal("ComputePolicy"))
-					Expect(updated.Status.Policies[0].Generation).To(Equal(computePolicy.Generation))
 				})
 
 				Context("when explicit policy does not exist", func() {
@@ -1106,6 +1101,65 @@ var _ = Describe("Reconcile", func() {
 						Expect(updated.Status.Policies).To(BeEmpty())
 					})
 				})
+
+				Context("when explicit policy matches", func() {
+					BeforeEach(func() {
+						computePolicy = &vspherepolv1.ComputePolicy{
+							ObjectMeta: metav1.ObjectMeta{
+								Name:      "explicit-matching-compute-policy",
+								Namespace: namespace,
+							},
+							Spec: vspherepolv1.ComputePolicySpec{
+								EnforcementMode: vspherepolv1.PolicyEnforcementModeMandatory,
+								Match: &vspherepolv1.MatchSpec{
+									Workload: &vspherepolv1.MatchWorkloadSpec{
+										Labels: []metav1.LabelSelectorRequirement{
+											{
+												Key:      "env",
+												Operator: metav1.LabelSelectorOpIn,
+												Values:   []string{"prod"},
+											},
+										},
+									},
+								},
+							},
+						}
+						withObjs = append(withObjs, computePolicy)
+
+						obj.Spec.Policies = []vspherepolv1.LocalObjectRef{
+							{
+								APIVersion: vspherepolv1.GroupVersion.String(),
+								Kind:       "ComputePolicy",
+								Name:       "explicit-matching-compute-policy",
+							},
+						}
+						obj.Spec.Workload = &vspherepolv1.PolicyEvaluationWorkloadSpec{
+							Labels: map[string]string{
+								"env": "prod",
+							},
+						}
+					})
+
+					It("should add the policy to the status", func() {
+						req := ctrl.Request{
+							NamespacedName: types.NamespacedName{
+								Name:      obj.Name,
+								Namespace: obj.Namespace,
+							},
+						}
+						result, err := reconciler.Reconcile(ctx, req)
+						Expect(err).ToNot(HaveOccurred())
+						Expect(result).To(Equal(ctrl.Result{}))
+
+						var updated vspherepolv1.PolicyEvaluation
+						Expect(client.Get(ctx, ctrlclient.ObjectKeyFromObject(obj), &updated)).To(Succeed())
+						Expect(updated.Status.Policies).To(HaveLen(1))
+						Expect(updated.Status.Policies[0].Name).To(Equal(computePolicy.Name))
+						Expect(updated.Status.Policies[0].APIVersion).To(Equal(vspherepolv1.GroupVersion.String()))
+						Expect(updated.Status.Policies[0].Kind).To(Equal("ComputePolicy"))
+						Expect(updated.Status.Policies[0].Generation).To(Equal(computePolicy.Generation))
+					})
+				})
 			})
 
 			Context("with duplicate policies", func() {
@@ -1940,7 +1994,7 @@ var _ = Describe("Reconcile", func() {
 
 						result, err := reconciler.Reconcile(ctx, req)
 						Expect(err).To(HaveOccurred())
-						Expect(err.Error()).To(ContainSubstring("failed to reconcile matching policies"))
+						Expect(err.Error()).To(ContainSubstring("failed to reconcile mandatory policies"))
 						Expect(result).To(Equal(ctrl.Result{}))
 					})
 				})
@@ -2024,7 +2078,7 @@ var _ = Describe("Reconcile", func() {
 
 				result, err := reconciler.Reconcile(ctx, req)
 				Expect(err).To(HaveOccurred())
-				Expect(err.Error()).To(ContainSubstring("failed to reconcile matching policies"))
+				Expect(err.Error()).To(ContainSubstring("failed to reconcile mandatory policies"))
 				Expect(err.Error()).To(ContainSubstring("failed to list compute policies"))
 				Expect(result).To(Equal(ctrl.Result{}))
 			})