Skip to content

Commit 225bcd3

Browse files
authored
Merge pull request #1724 from viccuad/feat/keep-policies-policyserver
feat!: Don't delete policies when deleting their PolicyServer
2 parents 04c1329 + af78e9e commit 225bcd3

9 files changed

Lines changed: 342 additions & 113 deletions

api/policies/v1/policyserver_types.go

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -227,15 +227,21 @@ const (
227227
// PolicyServerPodDisruptionBudgetReconciled represents the condition of the
228228
// Policy Server PodDisruptionBudget reconciliation.
229229
PolicyServerPodDisruptionBudgetReconciled PolicyServerConditionType = "PodDisruptionBudgetReconciled"
230+
// PolicyServerPolicyWebhooksCleanedUp represents the condition signalling
231+
// whether the webhook configurations of the policies bound to a
232+
// PolicyServer being deleted have been cleaned up. The PolicyServer's
233+
// finalizer is only removed once this condition becomes True.
234+
PolicyServerPolicyWebhooksCleanedUp PolicyServerConditionType = "PolicyWebhooksCleanedUp"
230235
)
231236

232237
// PolicyServerStatus defines the observed state of PolicyServer.
233238
type PolicyServerStatus struct {
234239
// Conditions represent the observed conditions of the
235-
// PolicyServer resource. Known .status.conditions.types
236-
// are: "PolicyServerSecretReconciled",
237-
// "PolicyServerDeploymentReconciled" and
238-
// "PolicyServerServiceReconciled"
240+
// PolicyServer resource. Known .status.conditions.types are:
241+
// "CertSecretReconciled", "CARootSecretReconciled",
242+
// "ConfigMapReconciled", "DeploymentReconciled",
243+
// "ServiceReconciled", "PodDisruptionBudgetReconciled" and
244+
// "PolicyWebhooksCleanedUp"
239245
// +patchMergeKey=type
240246
// +patchStrategy=merge
241247
// +listType=map

charts/kubewarden-crds/templates/crds/policies.kubewarden.io_policyservers.yaml

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1782,10 +1782,11 @@ spec:
17821782
conditions:
17831783
description: |-
17841784
Conditions represent the observed conditions of the
1785-
PolicyServer resource. Known .status.conditions.types
1786-
are: "PolicyServerSecretReconciled",
1787-
"PolicyServerDeploymentReconciled" and
1788-
"PolicyServerServiceReconciled"
1785+
PolicyServer resource. Known .status.conditions.types are:
1786+
"CertSecretReconciled", "CARootSecretReconciled",
1787+
"ConfigMapReconciled", "DeploymentReconciled",
1788+
"ServiceReconciled", "PodDisruptionBudgetReconciled" and
1789+
"PolicyWebhooksCleanedUp"
17891790
items:
17901791
description: Condition contains details for one aspect of the current
17911792
state of this API Resource.

internal/controller/admissionpolicy_controller.go

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -86,6 +86,10 @@ func (r *AdmissionPolicyReconciler) SetupWithManager(mgr ctrl.Manager) error {
8686
&corev1.Pod{},
8787
handler.EnqueueRequestsFromMapFunc(r.findAdmissionPoliciesForPod),
8888
).
89+
Watches(
90+
&policiesv1.PolicyServer{},
91+
handler.EnqueueRequestsFromMapFunc(r.findAdmissionPoliciesForPolicyServer),
92+
).
8993
Watches(
9094
&admissionregistrationv1.ValidatingWebhookConfiguration{},
9195
handler.EnqueueRequestsFromMapFunc(r.findAdmissionPolicyForWebhookConfiguration),
@@ -106,6 +110,10 @@ func (r *AdmissionPolicyReconciler) findAdmissionPoliciesForPod(ctx context.Cont
106110
return findPoliciesForPod(ctx, r.Client, object)
107111
}
108112

113+
func (r *AdmissionPolicyReconciler) findAdmissionPoliciesForPolicyServer(ctx context.Context, object client.Object) []reconcile.Request {
114+
return findAdmissionPoliciesForPolicyServer(ctx, r.Client, object)
115+
}
116+
109117
func (r *AdmissionPolicyReconciler) findAdmissionPolicyForWebhookConfiguration(_ context.Context, webhookConfiguration client.Object) []reconcile.Request {
110118
if !hasKubewardenLabel(webhookConfiguration.GetLabels()) {
111119
return []reconcile.Request{}

internal/controller/admissionpolicygroup_controller.go

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -86,6 +86,10 @@ func (r *AdmissionPolicyGroupReconciler) SetupWithManager(mgr ctrl.Manager) erro
8686
&corev1.Pod{},
8787
handler.EnqueueRequestsFromMapFunc(r.findAdmissionPoliciesForPod),
8888
).
89+
Watches(
90+
&policiesv1.PolicyServer{},
91+
handler.EnqueueRequestsFromMapFunc(r.findAdmissionPolicyGroupsForPolicyServer),
92+
).
8993
Watches(
9094
&admissionregistrationv1.ValidatingWebhookConfiguration{},
9195
handler.EnqueueRequestsFromMapFunc(r.findAdmissionPolicyForWebhookConfiguration),
@@ -102,6 +106,10 @@ func (r *AdmissionPolicyGroupReconciler) findAdmissionPoliciesForPod(ctx context
102106
return findPoliciesForPod(ctx, r.Client, object)
103107
}
104108

109+
func (r *AdmissionPolicyGroupReconciler) findAdmissionPolicyGroupsForPolicyServer(ctx context.Context, object client.Object) []reconcile.Request {
110+
return findAdmissionPolicyGroupsForPolicyServer(ctx, r.Client, object)
111+
}
112+
105113
func (r *AdmissionPolicyGroupReconciler) findAdmissionPolicyForWebhookConfiguration(_ context.Context, webhookConfiguration client.Object) []reconcile.Request {
106114
if !hasKubewardenLabel(webhookConfiguration.GetLabels()) {
107115
return []reconcile.Request{}

internal/controller/clusteradmissionpolicy_controller.go

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -86,6 +86,10 @@ func (r *ClusterAdmissionPolicyReconciler) SetupWithManager(mgr ctrl.Manager) er
8686
&corev1.Pod{},
8787
handler.EnqueueRequestsFromMapFunc(r.findClusterAdmissionPoliciesForPod),
8888
).
89+
Watches(
90+
&policiesv1.PolicyServer{},
91+
handler.EnqueueRequestsFromMapFunc(r.findClusterAdmissionPoliciesForPolicyServer),
92+
).
8993
Watches(
9094
&admissionregistrationv1.ValidatingWebhookConfiguration{},
9195
handler.EnqueueRequestsFromMapFunc(r.findClusterAdmissionPolicyForWebhookConfiguration),
@@ -105,6 +109,10 @@ func (r *ClusterAdmissionPolicyReconciler) findClusterAdmissionPoliciesForPod(ct
105109
return findClusterPoliciesForPod(ctx, r.Client, object)
106110
}
107111

112+
func (r *ClusterAdmissionPolicyReconciler) findClusterAdmissionPoliciesForPolicyServer(ctx context.Context, object client.Object) []reconcile.Request {
113+
return findClusterAdmissionPoliciesForPolicyServer(ctx, r.Client, object)
114+
}
115+
108116
func (r *ClusterAdmissionPolicyReconciler) findClusterAdmissionPolicyForWebhookConfiguration(_ context.Context, webhookConfiguration client.Object) []reconcile.Request {
109117
if !hasKubewardenLabel(webhookConfiguration.GetLabels()) {
110118
return []reconcile.Request{}

internal/controller/clusteradmissionpolicygroup_controller.go

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -86,6 +86,10 @@ func (r *ClusterAdmissionPolicyGroupReconciler) SetupWithManager(mgr ctrl.Manage
8686
&corev1.Pod{},
8787
handler.EnqueueRequestsFromMapFunc(r.findClusterAdmissionPoliciesForPod),
8888
).
89+
Watches(
90+
&policiesv1.PolicyServer{},
91+
handler.EnqueueRequestsFromMapFunc(r.findClusterAdmissionPolicyGroupsForPolicyServer),
92+
).
8993
Watches(
9094
&admissionregistrationv1.ValidatingWebhookConfiguration{},
9195
handler.EnqueueRequestsFromMapFunc(r.findClusterAdmissionPolicyForWebhookConfiguration),
@@ -101,6 +105,10 @@ func (r *ClusterAdmissionPolicyGroupReconciler) findClusterAdmissionPoliciesForP
101105
return findClusterPoliciesForPod(ctx, r.Client, object)
102106
}
103107

108+
func (r *ClusterAdmissionPolicyGroupReconciler) findClusterAdmissionPolicyGroupsForPolicyServer(ctx context.Context, object client.Object) []reconcile.Request {
109+
return findClusterAdmissionPolicyGroupsForPolicyServer(ctx, r.Client, object)
110+
}
111+
104112
func (r *ClusterAdmissionPolicyGroupReconciler) findClusterAdmissionPolicyForWebhookConfiguration(_ context.Context, webhookConfiguration client.Object) []reconcile.Request {
105113
if !hasKubewardenLabel(webhookConfiguration.GetLabels()) {
106114
return []reconcile.Request{}

internal/controller/policy_subreconciler.go

Lines changed: 144 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -86,10 +86,12 @@ func (r *policySubReconciler) reconcilePolicy(ctx context.Context, policy polici
8686

8787
policyServer, err := r.getPolicyServer(ctx, policy)
8888
if err != nil {
89-
policy.SetStatus(policiesv1.PolicyStatusScheduled)
90-
//nolint:nilerr // set status to scheduled if policyServer can't be retrieved, and stop reconciling
91-
return ctrl.Result{}, nil
89+
if errors.Is(err, errPolicyServerNotAvailable) {
90+
return r.reconcilePolicyServerUnavailable(ctx, policy)
91+
}
92+
return ctrl.Result{}, err
9293
}
94+
9395
if policy.GetStatus().PolicyStatus != policiesv1.PolicyStatusActive {
9496
policy.SetStatus(policiesv1.PolicyStatusPending)
9597
}
@@ -130,29 +132,53 @@ func (r *policySubReconciler) reconcilePolicy(ctx context.Context, policy polici
130132
return ctrl.Result{}, errors.Join(errors.New("cannot find policy server secret"), err)
131133
}
132134

133-
if policy.IsMutating() {
134-
if err = r.reconcileMutatingWebhookConfiguration(ctx, policy, &secret, policyServer.NameWithPrefix()); err != nil {
135-
return ctrl.Result{}, errors.Join(errors.New("error reconciling mutating webhook"), err)
136-
}
137-
} else {
138-
if err = r.reconcileValidatingWebhookConfiguration(ctx, policy, &secret, policyServer.NameWithPrefix()); err != nil {
139-
return ctrl.Result{}, errors.Join(errors.New("error reconciling validating webhook"), err)
140-
}
135+
if err = r.reconcileWebhookConfiguration(ctx, policy, &secret, policyServer.NameWithPrefix()); err != nil {
136+
return ctrl.Result{}, err
141137
}
142138
setPolicyAsActive(policy)
143139

144140
return ctrl.Result{}, nil
145141
}
146142

147-
func (r *policySubReconciler) reconcilePolicyDeletion(ctx context.Context, policy policiesv1.Policy) (ctrl.Result, error) {
143+
// reconcilePolicyServerUnavailable handles the case where the PolicyServer is
144+
// confirmed missing or being deleted.
145+
// It removes the webhook configuration to prevent orphaned webhooks from blocking
146+
// admission requests, then sets the policy status as scheduled.
147+
func (r *policySubReconciler) reconcilePolicyServerUnavailable(ctx context.Context, policy policiesv1.Policy) (ctrl.Result, error) {
148+
if err := r.deleteWebhookConfiguration(ctx, policy); err != nil {
149+
return ctrl.Result{}, err
150+
}
151+
policy.SetStatus(policiesv1.PolicyStatusScheduled)
152+
return ctrl.Result{}, nil
153+
}
154+
155+
// reconcileWebhookConfiguration creates or patches the webhook configuration
156+
// for the policy, dispatching to the mutating or validating variant as needed.
157+
func (r *policySubReconciler) reconcileWebhookConfiguration(ctx context.Context, policy policiesv1.Policy, secret *corev1.Secret, policyServerName string) error {
148158
if policy.IsMutating() {
149-
if err := r.reconcileMutatingWebhookConfigurationDeletion(ctx, policy); err != nil {
150-
return ctrl.Result{}, err
151-
}
152-
} else {
153-
if err := r.reconcileValidatingWebhookConfigurationDeletion(ctx, policy); err != nil {
154-
return ctrl.Result{}, err
159+
if err := r.reconcileMutatingWebhookConfiguration(ctx, policy, secret, policyServerName); err != nil {
160+
return errors.Join(errors.New("error reconciling mutating webhook"), err)
155161
}
162+
return nil
163+
}
164+
if err := r.reconcileValidatingWebhookConfiguration(ctx, policy, secret, policyServerName); err != nil {
165+
return errors.Join(errors.New("error reconciling validating webhook"), err)
166+
}
167+
return nil
168+
}
169+
170+
// deleteWebhookConfiguration removes the webhook configuration for the policy,
171+
// dispatching to the mutating or validating variant as needed.
172+
func (r *policySubReconciler) deleteWebhookConfiguration(ctx context.Context, policy policiesv1.Policy) error {
173+
if policy.IsMutating() {
174+
return r.reconcileMutatingWebhookConfigurationDeletion(ctx, policy)
175+
}
176+
return r.reconcileValidatingWebhookConfigurationDeletion(ctx, policy)
177+
}
178+
179+
func (r *policySubReconciler) reconcilePolicyDeletion(ctx context.Context, policy policiesv1.Policy) (ctrl.Result, error) {
180+
if err := r.deleteWebhookConfiguration(ctx, policy); err != nil {
181+
return ctrl.Result{}, err
156182
}
157183
// Remove the old finalizer used to ensure that the policy server created
158184
// before this controller version is delete as well. As the upgrade path
@@ -202,11 +228,23 @@ func (r *policySubReconciler) setPolicyModeStatus(ctx context.Context, policy po
202228
return nil
203229
}
204230

231+
// errPolicyServerNotAvailable is returned by getPolicyServer when the
232+
// PolicyServer CR is confirmed to not exist or is being deleted. It is
233+
// distinct from transient API errors so callers can decide whether to clean up
234+
// webhook configurations or simply requeue.
235+
var errPolicyServerNotAvailable = errors.New("policy server not available")
236+
205237
func (r *policySubReconciler) getPolicyServer(ctx context.Context, policy policiesv1.Policy) (*policiesv1.PolicyServer, error) {
206238
policyServer := policiesv1.PolicyServer{}
207239
if err := r.Get(ctx, types.NamespacedName{Name: policy.GetPolicyServer()}, &policyServer); err != nil {
240+
if apierrors.IsNotFound(err) {
241+
return nil, errPolicyServerNotAvailable
242+
}
208243
return nil, errors.Join(errors.New("could not get policy server"), err)
209244
}
245+
if policyServer.DeletionTimestamp != nil {
246+
return nil, errPolicyServerNotAvailable
247+
}
210248
return &policyServer, nil
211249
}
212250

@@ -353,6 +391,94 @@ func findClusterPoliciesForPod(ctx context.Context, k8sClient client.Client, obj
353391
return findClusterPoliciesForConfigMap(&configMap)
354392
}
355393

394+
// findAdmissionPoliciesForPolicyServer maps a PolicyServer event to reconcile
395+
// requests for AdmissionPolicies that reference it by name.
396+
func findAdmissionPoliciesForPolicyServer(ctx context.Context, k8sClient client.Client, object client.Object) []reconcile.Request {
397+
policyServer, ok := object.(*policiesv1.PolicyServer)
398+
if !ok {
399+
return []reconcile.Request{}
400+
}
401+
402+
admissionPolicies := policiesv1.AdmissionPolicyList{}
403+
if err := k8sClient.List(ctx, &admissionPolicies, client.MatchingFields{constants.PolicyServerIndexKey: policyServer.Name}); err != nil {
404+
return []reconcile.Request{}
405+
}
406+
407+
requests := make([]reconcile.Request, 0, len(admissionPolicies.Items))
408+
for _, policy := range admissionPolicies.Items {
409+
requests = append(requests, reconcile.Request{
410+
NamespacedName: client.ObjectKey{Name: policy.Name, Namespace: policy.Namespace},
411+
})
412+
}
413+
return requests
414+
}
415+
416+
// findAdmissionPolicyGroupsForPolicyServer maps a PolicyServer event to
417+
// reconcile requests for all AdmissionPolicyGroups that reference it by name.
418+
func findAdmissionPolicyGroupsForPolicyServer(ctx context.Context, k8sClient client.Client, object client.Object) []reconcile.Request {
419+
policyServer, ok := object.(*policiesv1.PolicyServer)
420+
if !ok {
421+
return []reconcile.Request{}
422+
}
423+
424+
admissionPolicyGroups := policiesv1.AdmissionPolicyGroupList{}
425+
if err := k8sClient.List(ctx, &admissionPolicyGroups, client.MatchingFields{constants.PolicyServerIndexKey: policyServer.Name}); err != nil {
426+
return []reconcile.Request{}
427+
}
428+
429+
requests := make([]reconcile.Request, 0, len(admissionPolicyGroups.Items))
430+
for _, policy := range admissionPolicyGroups.Items {
431+
requests = append(requests, reconcile.Request{
432+
NamespacedName: client.ObjectKey{Name: policy.Name, Namespace: policy.Namespace},
433+
})
434+
}
435+
return requests
436+
}
437+
438+
// findClusterAdmissionPoliciesForPolicyServer maps a PolicyServer event to reconcile
439+
// requests for all ClusterAdmissionPolicies that reference it by name.
440+
func findClusterAdmissionPoliciesForPolicyServer(ctx context.Context, k8sClient client.Client, object client.Object) []reconcile.Request {
441+
policyServer, ok := object.(*policiesv1.PolicyServer)
442+
if !ok {
443+
return []reconcile.Request{}
444+
}
445+
446+
clusterAdmissionPolicies := policiesv1.ClusterAdmissionPolicyList{}
447+
if err := k8sClient.List(ctx, &clusterAdmissionPolicies, client.MatchingFields{constants.PolicyServerIndexKey: policyServer.Name}); err != nil {
448+
return []reconcile.Request{}
449+
}
450+
451+
requests := make([]reconcile.Request, 0, len(clusterAdmissionPolicies.Items))
452+
for _, policy := range clusterAdmissionPolicies.Items {
453+
requests = append(requests, reconcile.Request{
454+
NamespacedName: client.ObjectKey{Name: policy.Name},
455+
})
456+
}
457+
return requests
458+
}
459+
460+
// findClusterAdmissionPolicyGroupsForPolicyServer maps a PolicyServer event to reconcile
461+
// requests for all ClusterAdmissionPolicyGroups that reference it by name.
462+
func findClusterAdmissionPolicyGroupsForPolicyServer(ctx context.Context, k8sClient client.Client, object client.Object) []reconcile.Request {
463+
policyServer, ok := object.(*policiesv1.PolicyServer)
464+
if !ok {
465+
return []reconcile.Request{}
466+
}
467+
468+
clusterAdmissionPolicyGroups := policiesv1.ClusterAdmissionPolicyGroupList{}
469+
if err := k8sClient.List(ctx, &clusterAdmissionPolicyGroups, client.MatchingFields{constants.PolicyServerIndexKey: policyServer.Name}); err != nil {
470+
return []reconcile.Request{}
471+
}
472+
473+
requests := make([]reconcile.Request, 0, len(clusterAdmissionPolicyGroups.Items))
474+
for _, policy := range clusterAdmissionPolicyGroups.Items {
475+
requests = append(requests, reconcile.Request{
476+
NamespacedName: client.ObjectKey{Name: policy.Name},
477+
})
478+
}
479+
return requests
480+
}
481+
356482
func hasKubewardenLabel(labels map[string]string) bool {
357483
// Pre v1.16.0
358484
kubewardenLabel := labels["kubewarden"]

0 commit comments

Comments
 (0)