Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

refactor: simplify wehbook configuration controller handlers #894

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 0 additions & 3 deletions internal/constants/constants.go
Original file line number Diff line number Diff line change
Expand Up @@ -49,11 +49,8 @@ const (
OptelInjectAnnotation = "sidecar.opentelemetry.io/inject"

// Webhook Configurations.
WebhookConfigurationPolicyScopeLabelKey = "kubewardenPolicyScope"
WebhookConfigurationPolicyNameAnnotationKey = "kubewardenPolicyName"
WebhookConfigurationPolicyNamespaceAnnotationKey = "kubewardenPolicyNamespace"
WebhookConfigurationPolicyGroupAnnotationKey = "kubewardenPolicyGroup"
True = "true"

// Scope.
NamespacePolicyScope = "namespace"
Expand Down
24 changes: 23 additions & 1 deletion internal/controller/admissionpolicy_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ import (
"sigs.k8s.io/controller-runtime/pkg/reconcile"

policiesv1 "github.com/kubewarden/kubewarden-controller/api/policies/v1"
"github.com/kubewarden/kubewarden-controller/internal/constants"
)

// Warning: this controller is deployed by a helm chart which has its own
Expand Down Expand Up @@ -102,5 +103,26 @@ func (r *AdmissionPolicyReconciler) findAdmissionPoliciesForPod(ctx context.Cont
}

func (r *AdmissionPolicyReconciler) findAdmissionPolicyForWebhookConfiguration(_ context.Context, webhookConfiguration client.Object) []reconcile.Request {
return findPolicyForWebhookConfiguration(webhookConfiguration, false, r.Log)
if !hasKubewardenLabel(webhookConfiguration.GetLabels()) {
return []reconcile.Request{}
}

policyName := webhookConfiguration.GetAnnotations()[constants.WebhookConfigurationPolicyNameAnnotationKey]
if policyName == "" {
return []reconcile.Request{}
}

policyNamespace := webhookConfiguration.GetAnnotations()[constants.WebhookConfigurationPolicyNamespaceAnnotationKey]
if policyNamespace == "" {
return []reconcile.Request{}
}

return []reconcile.Request{
{
NamespacedName: client.ObjectKey{
Name: policyName,
Namespace: policyNamespace,
},
},
}
}
4 changes: 0 additions & 4 deletions internal/controller/admissionpolicy_controller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,6 @@ var _ = Describe("AdmissionPolicy controller", Label("real-cluster"), func() {
}

Expect(validatingWebhookConfiguration.Labels[constants.PartOfLabelKey]).To(Equal(constants.PartOfLabelValue))
Expect(validatingWebhookConfiguration.Labels[constants.WebhookConfigurationPolicyScopeLabelKey]).To(Equal(constants.NamespacePolicyScope))
Expect(validatingWebhookConfiguration.Annotations[constants.WebhookConfigurationPolicyNameAnnotationKey]).To(Equal(policyName))
Expect(validatingWebhookConfiguration.Annotations[constants.WebhookConfigurationPolicyNamespaceAnnotationKey]).To(Equal(policyNamespace))
Expect(validatingWebhookConfiguration.Webhooks).To(HaveLen(1))
Expand Down Expand Up @@ -114,7 +113,6 @@ var _ = Describe("AdmissionPolicy controller", Label("real-cluster"), func() {

By("changing the ValidatingWebhookConfiguration")
delete(validatingWebhookConfiguration.Labels, constants.PartOfLabelKey)
validatingWebhookConfiguration.Labels[constants.WebhookConfigurationPolicyScopeLabelKey] = newName("scope")
delete(validatingWebhookConfiguration.Annotations, constants.WebhookConfigurationPolicyNameAnnotationKey)
validatingWebhookConfiguration.Annotations[constants.WebhookConfigurationPolicyNamespaceAnnotationKey] = newName("namespace")
validatingWebhookConfiguration.Webhooks[0].ClientConfig.Service.Name = newName("service")
Expand Down Expand Up @@ -210,7 +208,6 @@ var _ = Describe("AdmissionPolicy controller", Label("real-cluster"), func() {
}

Expect(mutatingWebhookConfiguration.Labels[constants.PartOfLabelKey]).To(Equal(constants.PartOfLabelValue))
Expect(mutatingWebhookConfiguration.Labels[constants.WebhookConfigurationPolicyScopeLabelKey]).To(Equal(constants.NamespacePolicyScope))
Expect(mutatingWebhookConfiguration.Annotations[constants.WebhookConfigurationPolicyNameAnnotationKey]).To(Equal(policyName))
Expect(mutatingWebhookConfiguration.Annotations[constants.WebhookConfigurationPolicyNamespaceAnnotationKey]).To(Equal(policyNamespace))
Expect(mutatingWebhookConfiguration.Webhooks).To(HaveLen(1))
Expand Down Expand Up @@ -240,7 +237,6 @@ var _ = Describe("AdmissionPolicy controller", Label("real-cluster"), func() {

By("changing the MutatingWebhookConfiguration")
delete(mutatingWebhookConfiguration.Labels, constants.PartOfLabelKey)
mutatingWebhookConfiguration.Labels[constants.WebhookConfigurationPolicyScopeLabelKey] = newName("scope")
delete(mutatingWebhookConfiguration.Annotations, constants.WebhookConfigurationPolicyNameAnnotationKey)
mutatingWebhookConfiguration.Annotations[constants.WebhookConfigurationPolicyNamespaceAnnotationKey] = newName("namespace")
mutatingWebhookConfiguration.Webhooks[0].ClientConfig.Service.Name = newName("service")
Expand Down
24 changes: 23 additions & 1 deletion internal/controller/admissionpolicygroup_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ import (
"sigs.k8s.io/controller-runtime/pkg/reconcile"

policiesv1 "github.com/kubewarden/kubewarden-controller/api/policies/v1"
"github.com/kubewarden/kubewarden-controller/internal/constants"
)

// Warning: this controller is deployed by a helm chart which has its own
Expand Down Expand Up @@ -98,5 +99,26 @@ func (r *AdmissionPolicyGroupReconciler) findAdmissionPoliciesForPod(ctx context
}

func (r *AdmissionPolicyGroupReconciler) findAdmissionPolicyForWebhookConfiguration(_ context.Context, webhookConfiguration client.Object) []reconcile.Request {
return findPolicyForWebhookConfiguration(webhookConfiguration, true, r.Log)
if !hasKubewardenLabel(webhookConfiguration.GetLabels()) {
return []reconcile.Request{}
}

policyName := webhookConfiguration.GetAnnotations()[constants.WebhookConfigurationPolicyNameAnnotationKey]
if policyName == "" {
return []reconcile.Request{}
}

policyNamespace := webhookConfiguration.GetAnnotations()[constants.WebhookConfigurationPolicyNamespaceAnnotationKey]
if policyNamespace == "" {
return []reconcile.Request{}
}

return []reconcile.Request{
{
NamespacedName: client.ObjectKey{
Name: policyName,
Namespace: policyNamespace,
},
},
}
}
4 changes: 0 additions & 4 deletions internal/controller/admissionpolicygroup_controller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -83,8 +83,6 @@ var _ = Describe("AdmissionPolicyGroup controller", Label("real-cluster"), func(
}

Expect(validatingWebhookConfiguration.Labels[constants.PartOfLabelKey]).To(Equal(constants.PartOfLabelValue))
Expect(validatingWebhookConfiguration.Labels[constants.WebhookConfigurationPolicyScopeLabelKey]).To(Equal(constants.NamespacePolicyScope))
Expect(validatingWebhookConfiguration.Annotations[constants.WebhookConfigurationPolicyGroupAnnotationKey]).To(Equal(constants.True))
Expect(validatingWebhookConfiguration.Annotations[constants.WebhookConfigurationPolicyNameAnnotationKey]).To(Equal(policyName))
Expect(validatingWebhookConfiguration.Annotations[constants.WebhookConfigurationPolicyNamespaceAnnotationKey]).To(Equal(policyNamespace))
Expect(validatingWebhookConfiguration.Webhooks).To(HaveLen(1))
Expand Down Expand Up @@ -114,8 +112,6 @@ var _ = Describe("AdmissionPolicyGroup controller", Label("real-cluster"), func(

By("changing the ValidatingWebhookConfiguration")
delete(validatingWebhookConfiguration.Labels, constants.PartOfLabelKey)
validatingWebhookConfiguration.Labels[constants.WebhookConfigurationPolicyScopeLabelKey] = newName("scope")
validatingWebhookConfiguration.Annotations[constants.WebhookConfigurationPolicyGroupAnnotationKey] = "false"
delete(validatingWebhookConfiguration.Annotations, constants.WebhookConfigurationPolicyNameAnnotationKey)
validatingWebhookConfiguration.Annotations[constants.WebhookConfigurationPolicyNamespaceAnnotationKey] = newName("namespace")
validatingWebhookConfiguration.Webhooks[0].ClientConfig.Service.Name = newName("service")
Expand Down
18 changes: 17 additions & 1 deletion internal/controller/clusteradmissionpolicy_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ import (
"sigs.k8s.io/controller-runtime/pkg/reconcile"

policiesv1 "github.com/kubewarden/kubewarden-controller/api/policies/v1"
"github.com/kubewarden/kubewarden-controller/internal/constants"
)

// Warning: this controller is deployed by a helm chart which has its own
Expand Down Expand Up @@ -101,5 +102,20 @@ func (r *ClusterAdmissionPolicyReconciler) findClusterAdmissionPoliciesForPod(ct
}

func (r *ClusterAdmissionPolicyReconciler) findClusterAdmissionPolicyForWebhookConfiguration(_ context.Context, webhookConfiguration client.Object) []reconcile.Request {
return findClusterPolicyForWebhookConfiguration(webhookConfiguration, false, r.Log)
if !hasKubewardenLabel(webhookConfiguration.GetLabels()) {
return []reconcile.Request{}
}

policyName := webhookConfiguration.GetAnnotations()[constants.WebhookConfigurationPolicyNameAnnotationKey]
if policyName == "" {
return []reconcile.Request{}
}

return []reconcile.Request{
{
NamespacedName: client.ObjectKey{
Name: policyName,
},
},
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,6 @@ var _ = Describe("ClusterAdmissionPolicy controller", Label("real-cluster"), fun
}

Expect(validatingWebhookConfiguration.Labels[constants.PartOfLabelKey]).To(Equal(constants.PartOfLabelValue))
Expect(validatingWebhookConfiguration.Labels[constants.WebhookConfigurationPolicyScopeLabelKey]).To(Equal(constants.ClusterPolicyScope))
Expect(validatingWebhookConfiguration.Annotations[constants.WebhookConfigurationPolicyNameAnnotationKey]).To(Equal(policyName))
Expect(validatingWebhookConfiguration.Annotations[constants.WebhookConfigurationPolicyNamespaceAnnotationKey]).To(BeEmpty())
Expect(validatingWebhookConfiguration.Webhooks).To(HaveLen(1))
Expand Down Expand Up @@ -107,7 +106,6 @@ var _ = Describe("ClusterAdmissionPolicy controller", Label("real-cluster"), fun
}, timeout, pollInterval).Should(Succeed())

delete(validatingWebhookConfiguration.Labels, constants.PartOfLabelKey)
validatingWebhookConfiguration.Labels[constants.WebhookConfigurationPolicyScopeLabelKey] = newName("scope")
delete(validatingWebhookConfiguration.Annotations, constants.WebhookConfigurationPolicyNameAnnotationKey)
validatingWebhookConfiguration.Annotations[constants.WebhookConfigurationPolicyNamespaceAnnotationKey] = newName("namespace")
validatingWebhookConfiguration.Webhooks[0].ClientConfig.Service.Name = newName("service")
Expand Down Expand Up @@ -180,7 +178,6 @@ var _ = Describe("ClusterAdmissionPolicy controller", Label("real-cluster"), fun
return err
}
Expect(mutatingWebhookConfiguration.Labels[constants.PartOfLabelKey]).To(Equal(constants.PartOfLabelValue))
Expect(mutatingWebhookConfiguration.Labels[constants.WebhookConfigurationPolicyScopeLabelKey]).To(Equal(constants.ClusterPolicyScope))
Expect(mutatingWebhookConfiguration.Annotations[constants.WebhookConfigurationPolicyNameAnnotationKey]).To(Equal(policyName))
Expect(mutatingWebhookConfiguration.Annotations[constants.WebhookConfigurationPolicyNamespaceAnnotationKey]).To(BeEmpty())
Expect(mutatingWebhookConfiguration.Webhooks).To(HaveLen(1))
Expand Down Expand Up @@ -216,7 +213,6 @@ var _ = Describe("ClusterAdmissionPolicy controller", Label("real-cluster"), fun
By("changing the MutatingWebhookConfiguration")

delete(mutatingWebhookConfiguration.Labels, constants.PartOfLabelKey)
mutatingWebhookConfiguration.Labels[constants.WebhookConfigurationPolicyScopeLabelKey] = newName("scope")
delete(mutatingWebhookConfiguration.Annotations, constants.WebhookConfigurationPolicyNameAnnotationKey)
mutatingWebhookConfiguration.Annotations[constants.WebhookConfigurationPolicyNamespaceAnnotationKey] = newName("namespace")
mutatingWebhookConfiguration.Webhooks[0].ClientConfig.Service.Name = newName("service")
Expand Down
18 changes: 17 additions & 1 deletion internal/controller/clusteradmissionpolicygroup_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ import (
"sigs.k8s.io/controller-runtime/pkg/reconcile"

policiesv1 "github.com/kubewarden/kubewarden-controller/api/policies/v1"
"github.com/kubewarden/kubewarden-controller/internal/constants"
)

// Warning: this controller is deployed by a helm chart which has its own
Expand Down Expand Up @@ -97,5 +98,20 @@ func (r *ClusterAdmissionPolicyGroupReconciler) findClusterAdmissionPoliciesForP
}

func (r *ClusterAdmissionPolicyGroupReconciler) findClusterAdmissionPolicyForWebhookConfiguration(_ context.Context, webhookConfiguration client.Object) []reconcile.Request {
return findClusterPolicyForWebhookConfiguration(webhookConfiguration, true, r.Log)
if !hasKubewardenLabel(webhookConfiguration.GetLabels()) {
return []reconcile.Request{}
}

policyName := webhookConfiguration.GetAnnotations()[constants.WebhookConfigurationPolicyNameAnnotationKey]
if policyName == "" {
return []reconcile.Request{}
}

return []reconcile.Request{
{
NamespacedName: client.ObjectKey{
Name: policyName,
},
},
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -71,8 +71,6 @@ var _ = Describe("ClusterAdmissionPolicyGroup controller", Label("real-cluster")
}

Expect(validatingWebhookConfiguration.Labels[constants.PartOfLabelKey]).To(Equal(constants.PartOfLabelValue))
Expect(validatingWebhookConfiguration.Labels[constants.WebhookConfigurationPolicyScopeLabelKey]).To(Equal(constants.ClusterPolicyScope))
Expect(validatingWebhookConfiguration.Annotations[constants.WebhookConfigurationPolicyGroupAnnotationKey]).To(Equal(constants.True))
Expect(validatingWebhookConfiguration.Annotations[constants.WebhookConfigurationPolicyNameAnnotationKey]).To(Equal(policyName))
Expect(validatingWebhookConfiguration.Annotations[constants.WebhookConfigurationPolicyNamespaceAnnotationKey]).To(BeEmpty())
Expect(validatingWebhookConfiguration.Webhooks).To(HaveLen(1))
Expand Down Expand Up @@ -108,8 +106,6 @@ var _ = Describe("ClusterAdmissionPolicyGroup controller", Label("real-cluster")
}, timeout, pollInterval).Should(Succeed())

delete(validatingWebhookConfiguration.Labels, constants.PartOfLabelKey)
validatingWebhookConfiguration.Labels[constants.WebhookConfigurationPolicyScopeLabelKey] = newName("scope")
validatingWebhookConfiguration.Annotations[constants.WebhookConfigurationPolicyGroupAnnotationKey] = "false"
delete(validatingWebhookConfiguration.Annotations, constants.WebhookConfigurationPolicyNameAnnotationKey)
validatingWebhookConfiguration.Annotations[constants.WebhookConfigurationPolicyNamespaceAnnotationKey] = newName("namespace")
validatingWebhookConfiguration.Webhooks[0].ClientConfig.Service.Name = newName("service")
Expand Down
86 changes: 2 additions & 84 deletions internal/controller/policy_subreconciler.go
Original file line number Diff line number Diff line change
Expand Up @@ -349,96 +349,14 @@ func findClusterPoliciesForPod(ctx context.Context, k8sClient client.Client, obj
return findClusterPoliciesForConfigMap(&configMap)
}

func findClusterPolicyForWebhookConfiguration(webhookConfiguration client.Object, isGroup bool, log logr.Logger) []reconcile.Request {
if !hasKubewardenLabel(webhookConfiguration.GetLabels()) {
return []reconcile.Request{}
}

if isGroup && !hasGroupAnnotation(webhookConfiguration.GetAnnotations()) {
return []reconcile.Request{}
}

policyScope, found := webhookConfiguration.GetLabels()[constants.WebhookConfigurationPolicyScopeLabelKey]
if !found {
log.Info("Found a webhook configuration without a scope label, reconciling it",
"name", webhookConfiguration.GetName())
return []reconcile.Request{}
}

// Filter out AdmissionPolicies
if policyScope != constants.ClusterPolicyScope {
return []reconcile.Request{}
}

policyName, found := webhookConfiguration.GetAnnotations()[constants.WebhookConfigurationPolicyNameAnnotationKey]
if !found {
log.Info("Found a webhook configuration without a policy name annotation, reconciling it",
"name", webhookConfiguration.GetName())
return []reconcile.Request{}
}

return []reconcile.Request{
{
NamespacedName: client.ObjectKey{
Name: policyName,
},
},
}
}

func findPolicyForWebhookConfiguration(webhookConfiguration client.Object, isGroup bool, log logr.Logger) []reconcile.Request {
if !hasKubewardenLabel(webhookConfiguration.GetLabels()) {
return []reconcile.Request{}
}

if isGroup && !hasGroupAnnotation(webhookConfiguration.GetAnnotations()) {
return []reconcile.Request{}
}

policyScope, found := webhookConfiguration.GetLabels()[constants.WebhookConfigurationPolicyScopeLabelKey]
if !found {
log.Info("Found a webhook configuration without a scope label, reconciling it", "name", webhookConfiguration.GetName())
return []reconcile.Request{}
}

// Filter out ClusterAdmissionPolicies
if policyScope != constants.NamespacePolicyScope {
return []reconcile.Request{}
}

policyNamespace, found := webhookConfiguration.GetAnnotations()[constants.WebhookConfigurationPolicyNamespaceAnnotationKey]
if !found {
log.Info("Found a webhook configuration without a namespace annotation, reconciling it", "name", webhookConfiguration.GetName())
return []reconcile.Request{}
}

policyName, found := webhookConfiguration.GetAnnotations()[constants.WebhookConfigurationPolicyNameAnnotationKey]
if !found {
log.Info("Found a webhook configuration without a policy name annotation, reconciling it", "name", webhookConfiguration.GetName())
return []reconcile.Request{}
}

return []reconcile.Request{
{
NamespacedName: client.ObjectKey{
Name: policyName,
Namespace: policyNamespace,
},
},
}
}

//nolint:goconst // we don't want to use a constant for "true"
func hasKubewardenLabel(labels map[string]string) bool {
// Pre v1.16.0
kubewardenLabel := labels["kubewarden"]
// From v1.16.0 on we are using the recommended label "app.kubernetes.io/part-of"
partOfLabel := labels[constants.PartOfLabelKey]

return kubewardenLabel == constants.True || partOfLabel == constants.PartOfLabelValue
}

func hasGroupAnnotation(annotations map[string]string) bool {
return annotations[constants.WebhookConfigurationPolicyGroupAnnotationKey] == constants.True
return kubewardenLabel == "true" || partOfLabel == constants.PartOfLabelValue
}

func getPolicyMapFromConfigMap(configMap *corev1.ConfigMap) (policyConfigEntryMap, error) {
Expand Down
Loading
Loading