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

[bartoszmajsak]

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.

[KevFan]

Thanks for your contribution @bartoszmajsak. Nice work 👍

I've left a comment on some further consideration around updates and upgrades. Let me know your thoughts 🧑‍💻

[KevFan]

This works well for the inital creation of an Authorino CR but there's a few things missing that require some further consideration for this to work for if an existing Authorino CR labels is updated also:

1. The comparison logic for labels to determine if the Authorino CR has changed is currently missing and needs to be added. Currently, the labels is not compared so it will not be updated if the labels are changed.

• authorino-operator/controllers/authorino_controller.go

  Line 134 in a92b462

  if changed := r.authorinoDeploymentChanges(existingDeployment, desiredDeployment); changed {

• authorino-operator/controllers/authorino_controller.go

  Line 552 in a92b462

  func (r *AuthorinoReconciler) authorinoDeploymentChanges(existingDeployment, desiredDeployment *k8sapps.Deployment) bool {

2. Once 1) is addressed and updating labels is allowed, the current logic will also update the deployment.spec.matchLabels which are immutable once set. Because these labels are immutable, the update will never succeed with an error from the API server

• authorino-operator/pkg/resources/k8s_deployment.go

  Lines 17 to 19 in a92b462

  	Selector: &v1.LabelSelector{
  	MatchLabels: authorinoLabels,
  	},

• Not sure do we want a separate podLabels map of labels that contains the hardcoded authorinoLabels and the Authorino CR labels and use this in the deployment pod template. The matchLabel should always just use the hardcoded labels 🤔

3. There is a possibilty that the labels defined in the Authorino CR can overwrite the hardcoded labels in labelsForAuthorino function, which then hit into 2) where the we attempt to update the deployment.spec.matchLabels. The hardcoded labels should take precedence in this case the prevent this 🤔

• authorino-operator/pkg/resources/k8s_util.go

  Lines 13 to 18 in a92b462

  	func labelsForAuthorino(name string) map[string]string {
  	return map[string]string{
  	"control-plane": "controller-manager",
  	"authorino-resource": name,
  	}
  	}

Other considerations around upgrade are:

1. This currently do not respect any user added labels to deployment.spec.template.labels 🤔. In this case, if the deployment is upgraded, any user added labels will be lost since only the labels currently present on the Authorino CR and the hardcoded labels is kept. We should do a "smart" merge of labels where:

• Known hardcoded labels take precedence
• Merge Authorino CR labels
• Keep any other user added labels already added to the deployment pod template.

This will however mean there's a wider scope of change as the desired deployment will need awareness of the existing deployment pod template labels

@eguzki Let me know if I've missed anything or if you have any thoughts on these suggestions.

@bartoszmajsak What are your thought on the above?

[bartoszmajsak]

These are all valid points. Thanks for your thorough analysis.

Do you want me to expand this PR based on this discussion or is something to be done as a follow-up? Happy to work on it either way.

[KevFan]

Personally, I think it would be nicer to exapnd this PR based on this discussion, especially for points 1 - 3 for accounting for updates.

However, If you feel the PR change would get too big though, I think the smart merge bit for upgrades can be done as follow-up/seperate PR. Let me know if you prefer this. Thanks for taking on this work 👍