fix(labels): propagates labels to Deployment's spec.template

PR #91 brought the ability to propagate labels defined for Authorino CR
down to Deployment. However, more often than not, it's more desired to
have those labels propagated to template and therefore pods which are
part of the deployment. This can simplify e.g. making Authorino part of
service mesh, as injection label can be consistently added to relevant
resources.

When creating the deployment, instead of having fixed set of labels for
Deployment's `spec.template` this change appends Authorino CR labels as
well.

Signed-off-by: bartoszmajsak <[email protected]>

bundle/manifests/authorino.kuadrant.io_authconfigs.yaml

@@ -72,7 +72,7 @@ spec:
             type: object
           spec:
             description: Specifies the desired state of the AuthConfig resource, i.e.
-              the authencation/authorization scheme to be applied to protect the matching
+              the authentication/authorization scheme to be applied to protect the matching
               service hosts.
             properties:
               authentication:
@@ -2606,7 +2606,7 @@ spec:
             type: object
           spec:
             description: Specifies the desired state of the AuthConfig resource, i.e.
-              the authencation/authorization scheme to be applied to protect the matching
+              the authentication/authorization scheme to be applied to protect the matching
               service hosts.
             properties:
               authentication:

charts/authorino-operator/templates/manifests.yaml

@@ -71,7 +71,7 @@ spec:
             type: object
           spec:
             description: Specifies the desired state of the AuthConfig resource, i.e.
-              the authencation/authorization scheme to be applied to protect the matching
+              the authentication/authorization scheme to be applied to protect the matching
               service hosts.
             properties:
               authentication:
@@ -2605,7 +2605,7 @@ spec:
             type: object
           spec:
             description: Specifies the desired state of the AuthConfig resource, i.e.
-              the authencation/authorization scheme to be applied to protect the matching
+              the authentication/authorization scheme to be applied to protect the matching
               service hosts.
             properties:
               authentication:

config/deploy/manifests.yaml

@@ -78,7 +78,7 @@ spec:
             type: object
           spec:
             description: Specifies the desired state of the AuthConfig resource, i.e.
-              the authencation/authorization scheme to be applied to protect the matching
+              the authentication/authorization scheme to be applied to protect the matching
               service hosts.
             properties:
               authentication:
@@ -2612,7 +2612,7 @@ spec:
             type: object
           spec:
             description: Specifies the desired state of the AuthConfig resource, i.e.
-              the authencation/authorization scheme to be applied to protect the matching
+              the authentication/authorization scheme to be applied to protect the matching
               service hosts.
             properties:
               authentication:

controllers/authorino_controller.go

@@ -66,13 +66,12 @@ type AuthorinoReconciler struct {
 // +kubebuilder:rbac:groups="",resources=events,verbs=create;patch;
 // +kubebuilder:rbac:groups="",resources=secrets,verbs=get;list;watch
 // +kubebuilder:rbac:groups="authorino.kuadrant.io",resources=authconfigs,verbs=create;delete;get;list;patch;update;watch
-// +kubebuilder:rbac:groups="authorino.kuadrant.io",resources=authconfigs,verbs=create;delete;get;list;patch;update;watch
 // +kubebuilder:rbac:groups="authorino.kuadrant.io",resources=authconfigs/status,verbs=get;patch;update
 // +kubebuilder:rbac:groups="coordination.k8s.io",resources=leases,verbs=get;list;create;update;
 
 // Reconcile deploys an instance of authorino depending on the settings
-// defined in the API, any change applied to the existings CRs will trigger
-// a new reconcilation to apply the required changes
+// defined in the API, any change applied to the existing CRs will trigger
+// a new reconciliation to apply the required changes
 func (r *AuthorinoReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Result, error) {
 	logger := r.Log.WithValues("authorino", req.NamespacedName)
 
@@ -106,14 +105,21 @@ func (r *AuthorinoReconciler) Reconcile(ctx context.Context, req ctrl.Request) (
 	}
 
 	// Gets Deployment resource for the authorino instance
-	if existingDeployment, err := r.getAuthorinoDeployment(authorinoInstance); err != nil {
+	existingDeployment, err := r.getAuthorinoDeployment(authorinoInstance)
+	if err != nil {
 		return ctrl.Result{}, r.wrapErrorWithStatusUpdate(logger, authorinoInstance, r.setStatusFailed(statusUnableToGetDeployment),
 			fmt.Errorf("failed to get %s Deployment resource, err: %v", authorinoInstance.Name, err),
 		)
-	} else if existingDeployment == nil {
+	}
+	if existingDeployment == nil {
 		// Creates a new deployment resource to deploy the new authorino instance
-		newDeployment := r.buildAuthorinoDeployment(authorinoInstance)
-		if err := r.Client.Create(context.TODO(), newDeployment); err != nil {
+		newDeployment, err := r.buildAuthorinoDeployment(authorinoInstance, logger)
+		if err != nil {
+			return ctrl.Result{}, r.wrapErrorWithStatusUpdate(logger, authorinoInstance, r.setStatusFailed(statusUnableToBuildDeploymentObject),
+				fmt.Errorf("failed to build %s Deployment resource for creation, err: %v", authorinoInstance.Name, err),
+			)
+		}
+		if err := r.Client.Create(ctx, newDeployment); err != nil {
 			return ctrl.Result{}, r.wrapErrorWithStatusUpdate(
 				logger, authorinoInstance, r.setStatusFailed(statusUnableToCreateDeployment),
 				fmt.Errorf("failed to create %s Deployment resource, err: %v", newDeployment.Name, err),
@@ -126,11 +132,23 @@ func (r *AuthorinoReconciler) Reconcile(ctx context.Context, req ctrl.Request) (
 		// Deployment created successfully - return and requeue
 		return ctrl.Result{Requeue: true}, nil
 	} else {
+
+		if !deploymentAvailable(existingDeployment) {
+			// Deployment not ready – return and requeue
+			err = updateStatusConditions(logger, authorinoInstance, r.Client, statusNotReady(statusDeploymentNotReady, "Authorino Deployment resource not ready"))
+			return ctrl.Result{RequeueAfter: time.Second}, err
+		}
+
 		// deployment already exists, then build a new resource with the desired changes
 		// and compare them, if changes are encountered apply the desired changes
-		desiredDeployment := r.buildAuthorinoDeployment(authorinoInstance)
-		logger.Info("desiredDeployment", "deployment", desiredDeployment)
-		logger.Info("existingDeployment", "deployment", existingDeployment)
+		desiredDeployment, err := r.buildAuthorinoDeployment(authorinoInstance, logger)
+		if err != nil {
+			return ctrl.Result{}, r.wrapErrorWithStatusUpdate(logger, authorinoInstance, r.setStatusFailed(statusUnableToBuildDeploymentObject),
+				fmt.Errorf("failed to build %s Deployment resource for updating, err: %v", authorinoInstance.Name, err),
+			)
+		}
+		logger.V(1).Info("desiredDeployment", "deployment", desiredDeployment)
+		logger.V(1).Info("existingDeployment", "deployment", existingDeployment)
 		if changed := r.authorinoDeploymentChanges(existingDeployment, desiredDeployment); changed {
 			if err := r.Update(ctx, desiredDeployment); err != nil {
 				return ctrl.Result{}, r.wrapErrorWithStatusUpdate(
@@ -142,12 +160,6 @@ func (r *AuthorinoReconciler) Reconcile(ctx context.Context, req ctrl.Request) (
 			err = updateStatusConditions(logger, authorinoInstance, r.Client, statusNotReady(statusUpdated, "Authorino Deployment resource updated"))
 			return ctrl.Result{RequeueAfter: time.Second}, err
 		}
-
-		if !deploymentAvailable(existingDeployment) {
-			// Deployment not ready – return and requeue
-			err = updateStatusConditions(logger, authorinoInstance, r.Client, statusNotReady(statusDeploymentNotReady, "Authorino Deployment resource not ready"))
-			return ctrl.Result{RequeueAfter: time.Second}, err
-		}
 	}
 
 	// Updates the status conditions to provisioned
@@ -188,7 +200,7 @@ func (r *AuthorinoReconciler) getAuthorinoDeployment(authorino *api.Authorino) (
 	return deployment, nil
 }
 
-func (r *AuthorinoReconciler) buildAuthorinoDeployment(authorino *api.Authorino) *k8sapps.Deployment {
+func (r *AuthorinoReconciler) buildAuthorinoDeployment(authorino *api.Authorino, logger logr.Logger) (*k8sapps.Deployment, error) {
 	var containers []k8score.Container
 	var saName = authorino.Name + "-authorino"
 
@@ -259,7 +271,7 @@ func (r *AuthorinoReconciler) buildAuthorinoDeployment(authorino *api.Authorino)
 		volumes = append(volumes, authorinoResources.GetTlsVolume(authorinoTlsCertVolumeName, secretName))
 	}
 
-	// if an external OIDC server is enable mounts a volume to the container
+	// if an external OIDC server is enabled mounts a volume to the container
 	// by using the secret with the certs
 	if enabled := authorino.Spec.OIDCServer.Tls.Enabled; enabled == nil || *enabled {
 		secretName := authorino.Spec.OIDCServer.Tls.CertSecret.Name
@@ -270,7 +282,7 @@ func (r *AuthorinoReconciler) buildAuthorinoDeployment(authorino *api.Authorino)
 	args := r.buildAuthorinoArgs(authorino)
 	var envs []k8score.EnvVar
 
-	// [DEPRECATED] configure authorino using env vars (only for old Authorino versions)
+	// Deprecated: configure authorino using env vars (only for old Authorino versions)
 	authorinoVersion := authorinoVersionFromImageTag(image)
 	if detectEnvVarAuthorinoVersion(authorinoVersion) {
 		envs = r.buildAuthorinoEnv(authorino)
@@ -306,10 +318,11 @@ func (r *AuthorinoReconciler) buildAuthorinoDeployment(authorino *api.Authorino)
 		containers,
 		volumes,
 		authorino.Labels,
+		logger,
 	)
 
-	_ = ctrl.SetControllerReference(authorino, deployment, r.Scheme)
-	return deployment
+	err := ctrl.SetControllerReference(authorino, deployment, r.Scheme)
+	return deployment, err
 }
 
 func (r *AuthorinoReconciler) buildAuthorinoArgs(authorino *api.Authorino) []string {
@@ -425,7 +438,7 @@ func (r *AuthorinoReconciler) buildAuthorinoArgs(authorino *api.Authorino) []str
 	return args
 }
 
-// [DEPRECATED] Configures Authorino by defining environment variables (instead of command-line args)
+// Deprecated: Configures Authorino by defining environment variables (instead of command-line args)
 // Kept for backward compatibility with older versions of Authorino (<= v0.10.x)
 func (r *AuthorinoReconciler) buildAuthorinoEnv(authorino *api.Authorino) []k8score.EnvVar {
 	envVar := []k8score.EnvVar{}
@@ -554,6 +567,16 @@ func (r *AuthorinoReconciler) authorinoDeploymentChanges(existingDeployment, des
 		return true
 	}
 
+	// check the labels here to see if there needs to be a change
+	// selector labels are immutable so don't need to check those.
+	if authorinoResources.MapUpdateNeeded(existingDeployment.Labels, desiredDeployment.Labels) {
+		return true
+	}
+
+	if authorinoResources.MapUpdateNeeded(existingDeployment.Spec.Template.Labels, desiredDeployment.Spec.Template.Labels) {
+		return true
+	}
+
 	existingContainer := existingDeployment.Spec.Template.Spec.Containers[0]
 	desiredContainer := desiredDeployment.Spec.Template.Spec.Containers[0]
 

controllers/authorino_controller_test.go

@@ -119,8 +119,19 @@ var _ = Describe("Authorino controller", func() {
 			image := DefaultAuthorinoImage
 			existContainer := false
 
-			Expect(deployment.Spec.Replicas).Should(Equal(&replicas))
-			Expect(deployment.Labels).Should(Equal(map[string]string{"thisLabel": "willPropagate"}))
+			Expect(deployment.Spec.Replicas).To(Equal(&replicas))
+
+			Expect(deployment.Labels).Should(HaveKeyWithValue("thisLabel", "willPropagate"))
+			Expect(deployment.Spec.Selector.MatchLabels).ShouldNot(HaveKeyWithValue("thisLabel", "willPropagate"))
+			Expect(deployment.Spec.Template.Labels).Should(HaveKeyWithValue("thisLabel", "willPropagate"))
+
+			Expect(deployment.Labels).Should(HaveKeyWithValue("control-plane", "controller-manager"))
+			Expect(deployment.Spec.Selector.MatchLabels).Should(HaveKeyWithValue("control-plane", "controller-manager"))
+			Expect(deployment.Spec.Template.Labels).Should(HaveKeyWithValue("control-plane", "controller-manager"))
+
+			Expect(deployment.Labels).Should(HaveKeyWithValue("authorino-resource", authorinoInstance.Name))
+			Expect(deployment.Spec.Selector.MatchLabels).Should(HaveKeyWithValue("authorino-resource", authorinoInstance.Name))
+			Expect(deployment.Spec.Template.Labels).Should(HaveKeyWithValue("authorino-resource", authorinoInstance.Name))
 			for _, container := range deployment.Spec.Template.Spec.Containers {
 				if container.Name == authorinoContainerName {
 					Expect(container.Image).Should(Equal(image))

controllers/constants.go

@@ -89,6 +89,7 @@ const (
 	statusTlsSecretNotProvided                    = "TlsSecretNotProvided"
 	statusUnableToUpdateDeployment                = "UnableToUpdateDeployment"
 	statusDeploymentNotReady                      = "DeploymentNotReady"
+	statusUnableToBuildDeploymentObject           = "UnableToBuildDeployementObject"
 )
 
 // ldflags

pkg/resources/k8s_deployment.go

@@ -1,25 +1,36 @@
 package resources
 
 import (
+	"github.com/go-logr/logr"
 	k8sapps "k8s.io/api/apps/v1"
 	k8score "k8s.io/api/core/v1"
 	v1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 )
 
-func GetDeployment(name, namespace, saName string, replicas *int32, containers []k8score.Container, vol []k8score.Volume, labels map[string]string) *k8sapps.Deployment {
-	objMeta := getObjectMeta(namespace, name, labels)
-	authorinoLabels := labelsForAuthorino(name)
+func GetDeployment(name, namespace, saName string, replicas *int32, containers []k8score.Container, vol []k8score.Volume, labelsFromAuthorinoCR map[string]string, logger logr.Logger) *k8sapps.Deployment {
+	mutableLabels := defaultAuthorinoLabels(name)
+	if labelsFromAuthorinoCR != nil {
+		for key, value := range labelsFromAuthorinoCR {
+			if key == "limitador-resource" || key == "app" {
+				logger.V(1).Info("skipping authorino labels with keys \"control-plane\" and \"authorino-resource\" as these are reserved for use by the operator")
+				continue
+			}
+			mutableLabels[key] = value
+		}
+	}
+	immutableLabels := defaultAuthorinoLabels(name)
+	objMeta := getObjectMeta(namespace, name, mutableLabels)
 
 	return &k8sapps.Deployment{
 		ObjectMeta: objMeta,
 		Spec: k8sapps.DeploymentSpec{
 			Replicas: replicas,
 			Selector: &v1.LabelSelector{
-				MatchLabels: authorinoLabels,
+				MatchLabels: immutableLabels,
 			},
 			Template: k8score.PodTemplateSpec{
 				ObjectMeta: v1.ObjectMeta{
-					Labels: authorinoLabels,
+					Labels: mutableLabels,
 				},
 				Spec: k8score.PodSpec{
 					ServiceAccountName: saName,
@@ -75,3 +86,16 @@ func GetTlsVolume(certName, secretName string) k8score.Volume {
 		},
 	}
 }
+
+func MapUpdateNeeded(existing map[string]string, desired map[string]string) bool {
+	if existing == nil {
+		existing = map[string]string{}
+	}
+
+	for k, v := range desired {
+		if existingVal, exists := (existing)[k]; !exists || v != existingVal {
+			return true
+		}
+	}
+	return false
+}

pkg/resources/k8s_services.go

@@ -57,7 +57,7 @@ func newService(serviceName, serviceNamespace, authorinoName string, labels map[
 	objMeta := getObjectMeta(serviceNamespace, authorinoName+"-"+serviceName, labels)
 	return &k8score.Service{
 		ObjectMeta: objMeta,
-		Spec:       newServiceSpec(labelsForAuthorino(authorinoName), servicePorts...),
+		Spec:       newServiceSpec(defaultAuthorinoLabels(authorinoName), servicePorts...),
 	}
 }
 

pkg/resources/k8s_util.go

@@ -10,7 +10,7 @@ func getObjectMeta(namespace, name string, labels map[string]string) v1.ObjectMe
 	return v1.ObjectMeta{Name: name, Namespace: namespace, Labels: labels}
 }
 
-func labelsForAuthorino(name string) map[string]string {
+func defaultAuthorinoLabels(name string) map[string]string {
 	return map[string]string{
 		"control-plane":      "controller-manager",
 		"authorino-resource": name,