Skip to content

Commit 5b491e0

Browse files
dilyar85dilyar85
authored
Update Policy Evaluation Controller to add compute policies correctly (#1284)
This PR updates the policy evaluation controller to apply compute polices as follows: - Automatically apply **mandatory** polices if matched (optional polices are added explicitly) - For explicit polices defined in `PolicyEvaluation.Spec.Policies` (populated from `VM.Spec.Polices`), add them only if matched (regardless of mandatory or optional mode) Also, this PR renames the existing `reconcileMatchingPolicies` function to `reconcileMandatoryPolicies` so it aligns better the purpose and the other `reconcileExplicitPolicies` function called together by the Reconciler. While here, adding the missing `patch` and `update` controller RBAC permission for `policyevaluations/status`.
1 parent e223a37 commit 5b491e0

File tree

3 files changed

+97
-23
lines changed

3 files changed

+97
-23
lines changed

‎config/rbac/role.yaml‎

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -327,7 +327,6 @@ rules:
327327
- vsphere.policy.vmware.com
328328
resources:
329329
- computepolicies/status
330-
- policyevaluations/status
331330
- tagpolicies/status
332331
verbs:
333332
- get
@@ -342,3 +341,11 @@ rules:
342341
- patch
343342
- update
344343
- watch
344+
- apiGroups:
345+
- vsphere.policy.vmware.com
346+
resources:
347+
- policyevaluations/status
348+
verbs:
349+
- get
350+
- patch
351+
- update

‎controllers/vspherepolicy/policyevaluation/policyevaluation_controller.go‎

Lines changed: 23 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -86,7 +86,7 @@ type Reconciler struct {
8686
}
8787

8888
// +kubebuilder:rbac:groups=vsphere.policy.vmware.com,resources=policyevaluations,verbs=create;get;list;watch;update;patch
89-
// +kubebuilder:rbac:groups=vsphere.policy.vmware.com,resources=policyevaluations/status,verbs=get
89+
// +kubebuilder:rbac:groups=vsphere.policy.vmware.com,resources=policyevaluations/status,verbs=get;update;patch
9090
// +kubebuilder:rbac:groups=vsphere.policy.vmware.com,resources=computepolicies,verbs=get;list;watch
9191
// +kubebuilder:rbac:groups=vsphere.policy.vmware.com,resources=computepolicies/status,verbs=get
9292
// +kubebuilder:rbac:groups=vsphere.policy.vmware.com,resources=tagpolicies,verbs=get;list;watch
@@ -134,8 +134,8 @@ func (r *Reconciler) ReconcileDelete(
134134
}
135135

136136
const (
137-
failMatchingPolicies = "failed to reconcile matching policies"
138-
failExplicitPolicies = "failed to reconcile explicit policies"
137+
failMandatoryPolicies = "failed to reconcile mandatory policies"
138+
failExplicitPolicies = "failed to reconcile explicit policies"
139139
)
140140

141141
func (r *Reconciler) ReconcileNormal(
@@ -150,14 +150,14 @@ func (r *Reconciler) ReconcileNormal(
150150
// Clear existing policies before reconciliation.
151151
obj.Status.Policies = nil
152152

153-
if err := r.reconcileMatchingPolicies(ctx, obj); err != nil {
153+
if err := r.reconcileMandatoryPolicies(ctx, obj); err != nil {
154154
conditions.MarkError(
155155
obj,
156156
vspherepolv1.ReadyConditionType,
157-
failMatchingPolicies,
157+
failMandatoryPolicies,
158158
err)
159159
return ctrl.Result{}, fmt.Errorf(
160-
failMatchingPolicies+": %w", err)
160+
failMandatoryPolicies+": %w", err)
161161
}
162162

163163
if err := r.reconcileExplicitPolicies(ctx, obj); err != nil {
@@ -179,19 +179,19 @@ func (r *Reconciler) ReconcileNormal(
179179
return ctrl.Result{}, nil
180180
}
181181

182-
func (r *Reconciler) reconcileMatchingPolicies(
182+
func (r *Reconciler) reconcileMandatoryPolicies(
183183
ctx context.Context,
184184
obj *vspherepolv1.PolicyEvaluation) error {
185185

186-
if err := r.reconcileMatchingComputePolicies(ctx, obj); err != nil {
186+
if err := r.reconcileMandatoryComputePolicies(ctx, obj); err != nil {
187187
return fmt.Errorf(
188-
"failed to reconcile matching compute policies: %w", err)
188+
"failed to reconcile mandatory compute policies: %w", err)
189189
}
190190

191191
return nil
192192
}
193193

194-
func (r *Reconciler) reconcileMatchingComputePolicies(
194+
func (r *Reconciler) reconcileMandatoryComputePolicies(
195195
ctx context.Context,
196196
obj *vspherepolv1.PolicyEvaluation) error {
197197

@@ -205,6 +205,11 @@ func (r *Reconciler) reconcileMatchingComputePolicies(
205205
}
206206

207207
for _, p := range list.Items {
208+
// Only mandatory policies should be automatically applied.
209+
if p.Spec.EnforcementMode != vspherepolv1.PolicyEnforcementModeMandatory {
210+
continue
211+
}
212+
208213
matches, err := matchesPolicy(obj, p)
209214
if err != nil {
210215
return err
@@ -540,6 +545,14 @@ func (r *Reconciler) addComputePolicyRef(
540545
return fmt.Errorf("failed to get compute policy: %w", err)
541546
}
542547

548+
matches, err := matchesPolicy(obj, pol)
549+
if err != nil {
550+
return err
551+
}
552+
if !matches {
553+
return fmt.Errorf("compute policy %q does not match", pol.Name)
554+
}
555+
543556
return r.addComputePolicy(ctx, obj, pol)
544557
}
545558

‎controllers/vspherepolicy/policyevaluation/policyevaluation_controller_test.go‎

Lines changed: 66 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -160,7 +160,7 @@ var _ = Describe("Reconcile", func() {
160160
Expect(updated.Status.Policies).To(BeEmpty())
161161
})
162162

163-
Context("with matching compute policy", func() {
163+
Context("with mandatory compute policy", func() {
164164
var computePolicy *vspherepolv1.ComputePolicy
165165

166166
BeforeEach(func() {
@@ -676,7 +676,7 @@ var _ = Describe("Reconcile", func() {
676676
Expect(policy.APIVersion).To(Equal(vspherepolv1.GroupVersion.String()))
677677
Expect(policy.Kind).To(Equal("ComputePolicy"))
678678
}
679-
Expect(policyNames).To(ConsistOf("policy-1", "policy-2"))
679+
Expect(policyNames).To(ConsistOf("policy-1"))
680680
})
681681
})
682682

@@ -1036,7 +1036,7 @@ var _ = Describe("Reconcile", func() {
10361036
}
10371037
})
10381038

1039-
It("should include explicitly referenced policies", func() {
1039+
It("should return an error if the explicit policy does not match", func() {
10401040
req := ctrl.Request{
10411041
NamespacedName: types.NamespacedName{
10421042
Name: obj.Name,
@@ -1045,15 +1045,10 @@ var _ = Describe("Reconcile", func() {
10451045
}
10461046

10471047
result, err := reconciler.Reconcile(ctx, req)
1048-
Expect(err).ToNot(HaveOccurred())
1048+
Expect(err).To(HaveOccurred())
1049+
Expect(err.Error()).To(ContainSubstring("compute policy \"explicit-compute-policy\" does not match"))
10491050
Expect(result).To(Equal(ctrl.Result{}))
10501051

1051-
var updated vspherepolv1.PolicyEvaluation
1052-
Expect(client.Get(ctx, ctrlclient.ObjectKeyFromObject(obj), &updated)).To(Succeed())
1053-
Expect(updated.Status.Policies).To(HaveLen(1))
1054-
Expect(updated.Status.Policies[0].APIVersion).To(Equal(vspherepolv1.GroupVersion.String()))
1055-
Expect(updated.Status.Policies[0].Kind).To(Equal("ComputePolicy"))
1056-
Expect(updated.Status.Policies[0].Generation).To(Equal(computePolicy.Generation))
10571052
})
10581053

10591054
Context("when explicit policy does not exist", func() {
@@ -1106,6 +1101,65 @@ var _ = Describe("Reconcile", func() {
11061101
Expect(updated.Status.Policies).To(BeEmpty())
11071102
})
11081103
})
1104+
1105+
Context("when explicit policy matches", func() {
1106+
BeforeEach(func() {
1107+
computePolicy = &vspherepolv1.ComputePolicy{
1108+
ObjectMeta: metav1.ObjectMeta{
1109+
Name: "explicit-matching-compute-policy",
1110+
Namespace: namespace,
1111+
},
1112+
Spec: vspherepolv1.ComputePolicySpec{
1113+
EnforcementMode: vspherepolv1.PolicyEnforcementModeMandatory,
1114+
Match: &vspherepolv1.MatchSpec{
1115+
Workload: &vspherepolv1.MatchWorkloadSpec{
1116+
Labels: []metav1.LabelSelectorRequirement{
1117+
{
1118+
Key: "env",
1119+
Operator: metav1.LabelSelectorOpIn,
1120+
Values: []string{"prod"},
1121+
},
1122+
},
1123+
},
1124+
},
1125+
},
1126+
}
1127+
withObjs = append(withObjs, computePolicy)
1128+
1129+
obj.Spec.Policies = []vspherepolv1.LocalObjectRef{
1130+
{
1131+
APIVersion: vspherepolv1.GroupVersion.String(),
1132+
Kind: "ComputePolicy",
1133+
Name: "explicit-matching-compute-policy",
1134+
},
1135+
}
1136+
obj.Spec.Workload = &vspherepolv1.PolicyEvaluationWorkloadSpec{
1137+
Labels: map[string]string{
1138+
"env": "prod",
1139+
},
1140+
}
1141+
})
1142+
1143+
It("should add the policy to the status", func() {
1144+
req := ctrl.Request{
1145+
NamespacedName: types.NamespacedName{
1146+
Name: obj.Name,
1147+
Namespace: obj.Namespace,
1148+
},
1149+
}
1150+
result, err := reconciler.Reconcile(ctx, req)
1151+
Expect(err).ToNot(HaveOccurred())
1152+
Expect(result).To(Equal(ctrl.Result{}))
1153+
1154+
var updated vspherepolv1.PolicyEvaluation
1155+
Expect(client.Get(ctx, ctrlclient.ObjectKeyFromObject(obj), &updated)).To(Succeed())
1156+
Expect(updated.Status.Policies).To(HaveLen(1))
1157+
Expect(updated.Status.Policies[0].Name).To(Equal(computePolicy.Name))
1158+
Expect(updated.Status.Policies[0].APIVersion).To(Equal(vspherepolv1.GroupVersion.String()))
1159+
Expect(updated.Status.Policies[0].Kind).To(Equal("ComputePolicy"))
1160+
Expect(updated.Status.Policies[0].Generation).To(Equal(computePolicy.Generation))
1161+
})
1162+
})
11091163
})
11101164

11111165
Context("with duplicate policies", func() {
@@ -1940,7 +1994,7 @@ var _ = Describe("Reconcile", func() {
19401994

19411995
result, err := reconciler.Reconcile(ctx, req)
19421996
Expect(err).To(HaveOccurred())
1943-
Expect(err.Error()).To(ContainSubstring("failed to reconcile matching policies"))
1997+
Expect(err.Error()).To(ContainSubstring("failed to reconcile mandatory policies"))
19441998
Expect(result).To(Equal(ctrl.Result{}))
19451999
})
19462000
})
@@ -2024,7 +2078,7 @@ var _ = Describe("Reconcile", func() {
20242078

20252079
result, err := reconciler.Reconcile(ctx, req)
20262080
Expect(err).To(HaveOccurred())
2027-
Expect(err.Error()).To(ContainSubstring("failed to reconcile matching policies"))
2081+
Expect(err.Error()).To(ContainSubstring("failed to reconcile mandatory policies"))
20282082
Expect(err.Error()).To(ContainSubstring("failed to list compute policies"))
20292083
Expect(result).To(Equal(ctrl.Result{}))
20302084
})

0 commit comments

Comments
 (0)