diff --git a/api/policies/v1/policyserver_types.go b/api/policies/v1/policyserver_types.go index 9f51172e8..51e17840f 100644 --- a/api/policies/v1/policyserver_types.go +++ b/api/policies/v1/policyserver_types.go @@ -58,6 +58,13 @@ type PolicyServerSpec struct { // +optional Annotations map[string]string `json:"annotations,omitempty"` + // Labels is a map of custom labels to be applied to the Deployment created by the + // PolicyServer and to the Pods managed by that Deployment. System labels set by + // the controller always take precedence over user-defined labels with the same key. + // More info: https://kubernetes.io/docs/concepts/overview/working-with-objects/labels/ + // +optional + Labels map[string]string `json:"labels,omitempty"` + // List of environment variables to set in the container. // +optional Env []corev1.EnvVar `json:"env,omitempty"` diff --git a/api/policies/v1/zz_generated.deepcopy.go b/api/policies/v1/zz_generated.deepcopy.go index 121639f0f..60f9eb901 100644 --- a/api/policies/v1/zz_generated.deepcopy.go +++ b/api/policies/v1/zz_generated.deepcopy.go @@ -814,6 +814,13 @@ func (in *PolicyServerSpec) DeepCopyInto(out *PolicyServerSpec) { (*out)[key] = val } } + if in.Labels != nil { + in, out := &in.Labels, &out.Labels + *out = make(map[string]string, len(*in)) + for key, val := range *in { + (*out)[key] = val + } + } if in.Env != nil { in, out := &in.Env, &out.Env *out = make([]corev1.EnvVar, len(*in)) diff --git a/charts/kubewarden-crds/templates/policies.kubewarden.io_policyservers.yaml b/charts/kubewarden-crds/templates/policies.kubewarden.io_policyservers.yaml index d81067277..5929af542 100644 --- a/charts/kubewarden-crds/templates/policies.kubewarden.io_policyservers.yaml +++ b/charts/kubewarden-crds/templates/policies.kubewarden.io_policyservers.yaml @@ -1145,6 +1145,15 @@ spec: items: type: string type: array + labels: + additionalProperties: + type: string + description: |- + Labels is a map of custom labels to be applied to the Deployment created by the + PolicyServer and to the Pods managed by that Deployment. System labels set by + the controller always take precedence over user-defined labels with the same key. + More info: https://kubernetes.io/docs/concepts/overview/working-with-objects/labels/ + type: object limits: additionalProperties: anyOf: diff --git a/config/crd/bases/policies.kubewarden.io_policyservers.yaml b/config/crd/bases/policies.kubewarden.io_policyservers.yaml index d62c51179..5929af542 100644 --- a/config/crd/bases/policies.kubewarden.io_policyservers.yaml +++ b/config/crd/bases/policies.kubewarden.io_policyservers.yaml @@ -1145,6 +1145,15 @@ spec: items: type: string type: array + labels: + additionalProperties: + type: string + description: |- + Labels is a map of custom labels to be applied to the Deployment created by the + PolicyServer and to the Pods managed by that Deployment. System labels set by + the controller always take precedence over user-defined labels with the same key. + More info: https://kubernetes.io/docs/concepts/overview/working-with-objects/labels/ + type: object limits: additionalProperties: anyOf: @@ -1721,7 +1730,7 @@ spec: Name of VerificationConfig configmap in the kubewarden namespace (same namespace as the controller deployment), containing Sigstore verification configuration. The configuration must be under a key named - verification-config in the Configmap. + verification-config in the ConfigMap. type: string required: - image diff --git a/docs/crds/CRD-docs-for-docs-repo.adoc b/docs/crds/CRD-docs-for-docs-repo.adoc index e082e0ec4..a5c53570f 100644 --- a/docs/crds/CRD-docs-for-docs-repo.adoc +++ b/docs/crds/CRD-docs-for-docs-repo.adoc @@ -876,6 +876,11 @@ set by external tools to store and retrieve arbitrary metadata. They are not + queryable and should be preserved when modifying objects. + More info: https://kubernetes.io/docs/concepts/overview/working-with-objects/annotations/ + | | Optional: \{} + +| *`labels`* __object (keys:string, values:string)__ | Labels is a map of custom labels to be applied to the Deployment created by the + +PolicyServer and to the Pods managed by that Deployment. System labels set by + +the controller always take precedence over user-defined labels with the same key. + +More info: https://kubernetes.io/docs/concepts/overview/working-with-objects/labels/ + | | Optional: \{} + + | *`env`* __link:https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.31/#envvar-v1-core[$$EnvVar$$] array__ | List of environment variables to set in the container. + | | Optional: \{} + | *`serviceAccountName`* __string__ | Name of the service account associated with the policy server. + diff --git a/docs/crds/CRD-docs-for-docs-repo.md b/docs/crds/CRD-docs-for-docs-repo.md index 8fb8eeaaf..1f4f2670e 100644 --- a/docs/crds/CRD-docs-for-docs-repo.md +++ b/docs/crds/CRD-docs-for-docs-repo.md @@ -525,6 +525,7 @@ _Appears in:_ | `minAvailable` _[IntOrString](https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.31/#intorstring-intstr-util)_ | Number of policy server replicas that must be still available after the
eviction. The value can be an absolute number or a percentage. Only one of
MinAvailable or Max MaxUnavailable can be set. | | | | `maxUnavailable` _[IntOrString](https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.31/#intorstring-intstr-util)_ | Number of policy server replicas that can be unavailable after the
eviction. The value can be an absolute number or a percentage. Only one of
MinAvailable or Max MaxUnavailable can be set. | | | | `annotations` _object (keys:string, values:string)_ | Annotations is an unstructured key value map stored with a resource that may be
set by external tools to store and retrieve arbitrary metadata. They are not
queryable and should be preserved when modifying objects.
More info: https://kubernetes.io/docs/concepts/overview/working-with-objects/annotations/ | | Optional: \{\}
| +| `labels` _object (keys:string, values:string)_ | Labels is a map of custom labels to be applied to the Deployment created by the
PolicyServer and to the Pods managed by that Deployment. System labels set by
the controller always take precedence over user-defined labels with the same key.
More info: https://kubernetes.io/docs/concepts/overview/working-with-objects/labels/ | | Optional: \{\}
| | `env` _[EnvVar](https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.31/#envvar-v1-core) array_ | List of environment variables to set in the container. | | Optional: \{\}
| | `serviceAccountName` _string_ | Name of the service account associated with the policy server.
Namespace service account will be used if not specified. | | Optional: \{\}
| | `imagePullSecret` _string_ | Name of ImagePullSecret secret in the same namespace, used for pulling
policies from repositories. | | Optional: \{\}
| diff --git a/internal/constants/constants.go b/internal/constants/constants.go index 61b67190e..586e3857d 100644 --- a/internal/constants/constants.go +++ b/internal/constants/constants.go @@ -55,6 +55,20 @@ const ( OptelInjectAnnotation = "sidecar.opentelemetry.io/inject" + // PolicyServerDeploymentManagedAnnotationKeysAnnotation is the annotation used to track + // which annotation keys on a Deployment/Pod template are managed by the controller (i.e., + // came from spec.annotations). On each reconcile the controller removes keys that were + // previously managed but are no longer present in the spec, without touching annotations + // set by Kubernetes itself or other tooling. + PolicyServerDeploymentManagedAnnotationKeysAnnotation = "kubewarden.io/managed-annotation-keys" + + // PolicyServerDeploymentManagedLabelKeysAnnotation is the annotation used to track + // which label keys on a Deployment/Pod template are managed by the controller (i.e., + // came from spec.labels). On each reconcile the controller removes keys that were + // previously managed but are no longer present in the spec, without touching labels + // set by Kubernetes itself or other tooling. + PolicyServerDeploymentManagedLabelKeysAnnotation = "kubewarden.io/managed-label-keys" + WebhookConfigurationPolicyNameAnnotationKey = "kubewardenPolicyName" WebhookConfigurationPolicyNamespaceAnnotationKey = "kubewardenPolicyNamespace" diff --git a/internal/controller/policyserver_controller_deployment.go b/internal/controller/policyserver_controller_deployment.go index 946e12261..0b9eaa53a 100644 --- a/internal/controller/policyserver_controller_deployment.go +++ b/internal/controller/policyserver_controller_deployment.go @@ -4,9 +4,12 @@ import ( "context" "errors" "fmt" + "maps" "os" "path/filepath" + "slices" "strconv" + "strings" appsv1 "k8s.io/api/apps/v1" corev1 "k8s.io/api/core/v1" @@ -326,17 +329,64 @@ func configuresInsecureSources(policyServer *policiesv1.PolicyServer, admissionC } } +// applyManagedKeys removes keys that were previously managed (recorded in the tracking +// annotation/label) but are no longer present in the desired user-defined map, then applies +// the new desired keys and records the current managed set in the tracking annotation. +// +// Parameters: +// - existing: the current annotation/label map on the object (mutated in-place). +// - desired: the user-defined map from spec (may be nil). +// - trackingAnnotations: the annotation map where the tracking key is stored (may differ from +// existing when we track label keys inside the annotation map). +// - trackingKey: the annotation key used to store the comma-separated list of managed keys. +func applyManagedKeys(existing map[string]string, desired map[string]string, trackingAnnotations map[string]string, trackingKey string) { + // Remove keys that were managed last time but are no longer desired. + if prev, found := trackingAnnotations[trackingKey]; found && prev != "" { + for _, key := range strings.Split(prev, ",") { + if _, stillDesired := desired[key]; !stillDesired { + delete(existing, key) + } + } + } + + // Apply desired keys (user-defined values first; system values will overwrite after this call). + for key, value := range desired { + existing[key] = value + } + + // Record the current set of managed keys. + trackingAnnotations[trackingKey] = strings.Join(slices.Collect(maps.Keys(desired)), ",") +} + func configureLabelsAndAnnotations(policyServerDeployment *appsv1.Deployment, policyServer *policiesv1.PolicyServer, configMapVersion string) { + // --- Annotations --- if policyServerDeployment.ObjectMeta.Annotations == nil { policyServerDeployment.ObjectMeta.Annotations = make(map[string]string) } + // The tracking annotation lives in ObjectMeta.Annotations itself. + applyManagedKeys( + policyServerDeployment.ObjectMeta.Annotations, + policyServer.Spec.Annotations, + policyServerDeployment.ObjectMeta.Annotations, + constants.PolicyServerDeploymentManagedAnnotationKeysAnnotation, + ) + // System annotation always wins. policyServerDeployment.ObjectMeta.Annotations[constants.PolicyServerDeploymentConfigVersionAnnotation] = configMapVersion + // --- Labels --- if policyServerDeployment.Labels == nil { policyServerDeployment.Labels = make(map[string]string) } + // The tracking annotation for labels is stored in the annotation map so that we don't + // pollute the labels map with a non-label value. + applyManagedKeys( + policyServerDeployment.Labels, + policyServer.Spec.Labels, + policyServerDeployment.ObjectMeta.Annotations, + constants.PolicyServerDeploymentManagedLabelKeysAnnotation, + ) + // System labels always win. policyServerDeployment.Labels[constants.PolicyServerLabelKey] = policyServer.Name - for key, value := range policyServer.CommonLabels() { policyServerDeployment.Labels[key] = value } @@ -417,12 +467,15 @@ func buildPolicyServerDeploymentSpec( podSecurityContext *corev1.PodSecurityContext, imagePullSecrets []corev1.LocalObjectReference, ) appsv1.DeploymentSpec { - templateLabels := map[string]string{ - //nolint:staticcheck // this label will remove soon when policy lifecycle is revisited - constants.AppLabelKey: policyServer.AppLabel(), - constants.PolicyServerDeploymentPodSpecConfigVersionLabel: configMapVersion, - constants.PolicyServerLabelKey: policyServer.Name, - } + // Apply user-defined labels first, then system labels overwrite any conflicts. + templateLabels := maps.Clone(policyServer.Spec.Labels) + if templateLabels == nil { + templateLabels = make(map[string]string) + } + //nolint:staticcheck // this label will remove soon when policy lifecycle is revisited + templateLabels[constants.AppLabelKey] = policyServer.AppLabel() + templateLabels[constants.PolicyServerDeploymentPodSpecConfigVersionLabel] = configMapVersion + templateLabels[constants.PolicyServerLabelKey] = policyServer.Name for key, value := range policyServer.CommonLabels() { templateLabels[key] = value } diff --git a/internal/controller/policyserver_controller_test.go b/internal/controller/policyserver_controller_test.go index e76591f57..4b9de5f1f 100644 --- a/internal/controller/policyserver_controller_test.go +++ b/internal/controller/policyserver_controller_test.go @@ -888,6 +888,180 @@ var _ = Describe("PolicyServer controller", func() { }))) }) + It("should propagate custom labels from spec.labels to the Deployment and Pod template, with system labels taking precedence", func() { + policyServer := policiesv1.NewPolicyServerFactory().WithName(policyServerName).Build() + policyServer.Spec.Labels = map[string]string{ + "custom-label": "custom-value", + "another-label": "another-value", + constants.PolicyServerLabelKey: "should-be-overridden", + "app.kubernetes.io/managed-by": "should-be-overridden", + } + createPolicyServerAndWaitForItsService(ctx, policyServer) + + Eventually(func() error { + deployment, err := getTestPolicyServerDeployment(ctx, policyServerName) + if err != nil { + return err + } + // Custom labels should appear on Deployment ObjectMeta + Expect(deployment.ObjectMeta.Labels).To(HaveKeyWithValue("custom-label", "custom-value")) + Expect(deployment.ObjectMeta.Labels).To(HaveKeyWithValue("another-label", "another-value")) + // Custom labels should appear on Pod template + Expect(deployment.Spec.Template.ObjectMeta.Labels).To(HaveKeyWithValue("custom-label", "custom-value")) + Expect(deployment.Spec.Template.ObjectMeta.Labels).To(HaveKeyWithValue("another-label", "another-value")) + // System labels must not be overridden by user labels + Expect(deployment.ObjectMeta.Labels).To(HaveKeyWithValue(constants.PolicyServerLabelKey, policyServerName)) + Expect(deployment.ObjectMeta.Labels).To(HaveKeyWithValue("app.kubernetes.io/managed-by", "kubewarden-controller")) + Expect(deployment.Spec.Template.ObjectMeta.Labels).To(HaveKeyWithValue(constants.PolicyServerLabelKey, policyServerName)) + Expect(deployment.Spec.Template.ObjectMeta.Labels).To(HaveKeyWithValue("app.kubernetes.io/managed-by", "kubewarden-controller")) + return nil + }).Should(Succeed()) + }) + + It("should propagate custom annotations from spec.annotations to the Deployment ObjectMeta and Pod template", func() { + policyServer := policiesv1.NewPolicyServerFactory().WithName(policyServerName).Build() + policyServer.Spec.Annotations = map[string]string{ + "custom-annotation": "custom-value", + "another-annotation": "another-value", + } + createPolicyServerAndWaitForItsService(ctx, policyServer) + + Eventually(func() error { + deployment, err := getTestPolicyServerDeployment(ctx, policyServerName) + if err != nil { + return err + } + // Custom annotations should appear on Deployment ObjectMeta + Expect(deployment.ObjectMeta.Annotations).To(HaveKeyWithValue("custom-annotation", "custom-value")) + Expect(deployment.ObjectMeta.Annotations).To(HaveKeyWithValue("another-annotation", "another-value")) + // Custom annotations should appear on Pod template + Expect(deployment.Spec.Template.ObjectMeta.Annotations).To(HaveKeyWithValue("custom-annotation", "custom-value")) + Expect(deployment.Spec.Template.ObjectMeta.Annotations).To(HaveKeyWithValue("another-annotation", "another-value")) + // System annotation must still be present + Expect(deployment.ObjectMeta.Annotations).To(HaveKey(constants.PolicyServerDeploymentConfigVersionAnnotation)) + return nil + }).Should(Succeed()) + }) + + It("should remove stale labels and annotations from the Deployment when they are removed from spec", func() { + policyServer := policiesv1.NewPolicyServerFactory().WithName(policyServerName).Build() + policyServer.Spec.Labels = map[string]string{"stale-label": "value"} + policyServer.Spec.Annotations = map[string]string{"stale-annotation": "value"} + createPolicyServerAndWaitForItsService(ctx, policyServer) + + // Confirm they appear initially on both Deployment ObjectMeta and Pod template + Eventually(func() error { + deployment, err := getTestPolicyServerDeployment(ctx, policyServerName) + if err != nil { + return err + } + if _, ok := deployment.ObjectMeta.Labels["stale-label"]; !ok { + return errors.New("stale-label not yet present on Deployment ObjectMeta") + } + if _, ok := deployment.ObjectMeta.Annotations["stale-annotation"]; !ok { + return errors.New("stale-annotation not yet present on Deployment ObjectMeta") + } + if _, ok := deployment.Spec.Template.ObjectMeta.Labels["stale-label"]; !ok { + return errors.New("stale-label not yet present on Pod template") + } + if _, ok := deployment.Spec.Template.ObjectMeta.Annotations["stale-annotation"]; !ok { + return errors.New("stale-annotation not yet present on Pod template") + } + return nil + }, timeout, pollInterval).Should(Succeed()) + + // Remove both from spec + Eventually(func() error { + ps, err := getTestPolicyServer(ctx, policyServerName) + if err != nil { + return err + } + ps.Spec.Labels = nil + ps.Spec.Annotations = nil + return k8sClient.Update(ctx, ps) + }, timeout, pollInterval).Should(Succeed()) + + // Verify they are gone from both Deployment ObjectMeta and Pod template + Eventually(func() error { + deployment, err := getTestPolicyServerDeployment(ctx, policyServerName) + if err != nil { + return err + } + if _, ok := deployment.ObjectMeta.Labels["stale-label"]; ok { + return errors.New("stale-label still present on Deployment ObjectMeta after removal from spec") + } + if _, ok := deployment.ObjectMeta.Annotations["stale-annotation"]; ok { + return errors.New("stale-annotation still present on Deployment ObjectMeta after removal from spec") + } + if _, ok := deployment.Spec.Template.ObjectMeta.Labels["stale-label"]; ok { + return errors.New("stale-label still present on Pod template after removal from spec") + } + if _, ok := deployment.Spec.Template.ObjectMeta.Annotations["stale-annotation"]; ok { + return errors.New("stale-annotation still present on Pod template after removal from spec") + } + // System labels and annotations must still be present + Expect(deployment.ObjectMeta.Labels).To(HaveKey(constants.PolicyServerLabelKey)) + Expect(deployment.ObjectMeta.Annotations).To(HaveKey(constants.PolicyServerDeploymentConfigVersionAnnotation)) + Expect(deployment.Spec.Template.ObjectMeta.Labels).To(HaveKey(constants.PolicyServerLabelKey)) + return nil + }, timeout, pollInterval).Should(Succeed()) + }) + + It("should preserve non-user-managed annotations on the Deployment when reconciling", func() { + policyServer := policiesv1.NewPolicyServerFactory().WithName(policyServerName).Build() + policyServer.Spec.Annotations = map[string]string{"user-annotation": "user-value"} + createPolicyServerAndWaitForItsService(ctx, policyServer) + + // Wait for the user annotation to appear on the Deployment. + Eventually(func() error { + deployment, err := getTestPolicyServerDeployment(ctx, policyServerName) + if err != nil { + return err + } + if _, ok := deployment.ObjectMeta.Annotations["user-annotation"]; !ok { + return errors.New("user-annotation not yet present on Deployment ObjectMeta") + } + return nil + }, timeout, pollInterval).Should(Succeed()) + + // Simulate an external controller (e.g. Kubernetes deployment controller) adding + // an annotation directly to the Deployment. + const externalAnnotation = "some-external-tool/annotation" + Eventually(func() error { + deployment, err := getTestPolicyServerDeployment(ctx, policyServerName) + if err != nil { + return err + } + deployment.ObjectMeta.Annotations[externalAnnotation] = "external-value" + return k8sClient.Update(ctx, deployment) + }, timeout, pollInterval).Should(Succeed()) + + // Trigger a reconcile by updating the PolicyServer spec (remove the user annotation). + Eventually(func() error { + ps, err := getTestPolicyServer(ctx, policyServerName) + if err != nil { + return err + } + ps.Spec.Annotations = nil + return k8sClient.Update(ctx, ps) + }, timeout, pollInterval).Should(Succeed()) + + // The user annotation must be gone, but the external annotation must survive. + Eventually(func() error { + deployment, err := getTestPolicyServerDeployment(ctx, policyServerName) + if err != nil { + return err + } + if _, ok := deployment.ObjectMeta.Annotations["user-annotation"]; ok { + return errors.New("user-annotation still present after removal from spec") + } + if _, ok := deployment.ObjectMeta.Annotations[externalAnnotation]; !ok { + return errors.New("external annotation was unexpectedly removed by the controller") + } + return nil + }, timeout, pollInterval).Should(Succeed()) + }) + It("should set the configMap version as a deployment annotation", func() { policyServer := policiesv1.NewPolicyServerFactory().WithName(policyServerName).Build() createPolicyServerAndWaitForItsService(ctx, policyServer)