Add make target to remove labels and annotations from manifests

[bnallapeta]

Fixes #621

[bnallapeta]

@fdberlking PTAL.

[github-actions]

@bnallapeta Images are available for testing. docker pull ghcr.io/stakater/reloader:SNAPSHOT-PR-625-221369bd\ndocker pull ghcr.io/stakater/reloader:SNAPSHOT-PR-625-UBI-221369bd

[github-actions]

@bnallapeta Yikes! You better fix it before anyone else finds out! Build has Failed!

[fdberlking]

@fdberlking PTAL.

Sure, I have a look around the weekend, thanks :)

[fdberlking]

@fdberlking PTAL.

@bnallapeta LGTM, I'd only change the yq update statement to the following:

remove-labels-annotations: yq-install
	@for file in $$(find deployments/kubernetes/manifests -type f -name '*.yaml'); do \
		echo "Processing $$file"; \
		$(YQ_BIN) eval 'del(.metadata.annotations | with_entries(select(.key | startswith("meta.helm.sh/"))), .metadata.labels | with_entries(select(.value == "Helm")), .spec.template.metadata.labels | with_entries(select(.value == "Helm")))' -i "$$file"; \
	done

I think my initial issue statement was not precise enough, sorry for that. We should only remove annotations and labels from the .yaml files within deployments/kubernetes/manifests/* which are Helm related. E.g.

metadata:
  annotations:
    meta.helm.sh/release-namespace: "default"
    meta.helm.sh/release-name: "reloader"
  labels:
    heritage: "Helm"
    app.kubernetes.io/managed-by: "Helm"

The rest needs to stay, otherwise e.g. the deployment.yaml does not specify which pods the controller should manage or control.

With the statement above, we would only delete annotations starting with meta.helm.sh/ or labels with the value Helm.

Unfortunately, I was not able to push my draft to your PR due to missing access rights, but maybe you could have another check and verify, if my proposal makes sense or not. Thank you 🚀

[bnallapeta]

@fdberlking

• As far as the workflow is concerned and the initial files that are present in our repo, we will only have the helm related labels and annotations and nothing else.
• Also, the condition you have for labels does not remove all helm related labels.
  Here's what the final set of labels are:

  labels:
    app: reloader-reloader
    chart: "reloader-1.0.69"
    release: "reloader"
    heritage: "Helm"
    app.kubernetes.io/managed-by: "Helm"

I think from the workflow's perspective, its fine to remove all labels and annotations as they are the only ones present.

[fdberlking]

@fdberlking

• As far as the workflow is concerned and the initial files that are present in our repo, we will only have the helm related labels and annotations and nothing else.

• Also, the condition you have for labels does not remove all helm related labels.

Here's what the final set of labels are:

  labels:

    app: reloader-reloader

    chart: "reloader-1.0.69"

    release: "reloader"

    heritage: "Helm"

    app.kubernetes.io/managed-by: "Helm"

I think from the workflow's perspective, its fine to remove all labels and annotations as they are the only ones present.

@bnallapeta thanks for coming back to me so quickly. I need to admit that I wasn't executing the make command as implemented by you, but from my understanding it

• Loops through the deployments/kubernetes/manifests folder initially created with the 4 resource files (deployments, sa, crb, cr)
• Remove all annotations and labels from the files

From my understanding, we could leave all annotations except the ones related to Helm. If my deletion proposal for labels is not correct (as pointed out by you above), please feel free to adjust it.

Additionally, we would need to take care as well on the labels within the spec.templates section (so the ones outside metadata). We do only have this with the deployment.yaml, but otherwise, the clean up proposal wouldn't be consistent (and again, only delete labels associated with Helm and keep the others, the installation via Vanilla Kustomize would fail).

[bnallapeta]

@fdberlking Ah you're right. I didn't account for the fact that users can provide their custom labels from values.yaml and the ones that are being applied from _helpers.tpl.

I will work on this one and the deployment change as well.

[fdberlking]

@fdberlking Ah you're right. I didn't account for the fact that users can provide their custom labels from values.yaml and the ones that are being applied from _helpers.tpl.

I will work on this one and the deployment change as well.

@bnallapeta perfect, thank you very much. Overall, I hope my comments made sense 🙂 please let me know, if I should do another check whenever you have found the time to push your changes.

[bnallapeta]

@fdberlking
Let me try to explain this once again. I think the changes I have made at the moment are right because:

• the final files that are placed in the deployments/manifests folder should be free from a tool dependent labels/annotations (in this case, helm)
• Users can apply using
  kubectl
  kubectl apply -f release.yaml
  or
  kustomize
  kubectl apply -k deployments/kubernetes

So, any labels/annotations coming from either _helpers.tpl or values.yaml should be customer specific and should not be part of the main manifests folder.

Hope this makes sense. Let me know your thoughts.

P.S.: I have made the deployment change.

[github-actions]

@bnallapeta Images are available for testing. docker pull ghcr.io/stakater/reloader:SNAPSHOT-PR-625-08b02e07\ndocker pull ghcr.io/stakater/reloader:SNAPSHOT-PR-625-UBI-08b02e07

[fdberlking]

@fdberlking Let me try to explain this once again. I think the changes I have made at the moment are right because:

* the final files that are placed in the deployments/manifests folder should be free from a tool dependent labels/annotations (in this case, helm)

* Users can apply using
  [kubectl](https://github.com/stakater/Reloader/tree/master?tab=readme-ov-file#vanilla-manifests)
  `kubectl apply -f release.yaml`
  or
  [kustomize](https://github.com/stakater/Reloader/tree/master?tab=readme-ov-file#vanilla-kustomize)
  `kubectl apply -k deployments/kubernetes`

So, any labels/annotations coming from either _helpers.tpl or values.yaml should be customer specific and should not be part of the main manifests folder.

Hope this makes sense. Let me know your thoughts.

P.S.: I have made the deployment change.

@bnallapeta but files from this folder are used by Kustomize afterwards and if someone uses the reference, vanilla installation kubectl apply -k https://github.com/stakater/Reloader/deployments/kubernetes, my gut feeling is that the installation would fail after publishing the change as it is (see reference issue #618).

serviceaccount/reloader-reloader created
clusterrole.rbac.authorization.k8s.io/reloader-reloader-role created
clusterrolebinding.rbac.authorization.k8s.io/reloader-reloader-role-binding created
The Deployment "reloader-reloader" is invalid: spec.template.metadata.labels: Invalid value: map[string]string(nil): `selector` does not match template `labels`

Erasing the complete spec.template.metadata.labels section

$(YQ_BIN) eval 'del(.spec.template.metadata.labels)' -i deployments/kubernetes/manifests/deployment.yaml

would cause the selectors not being able to find the set labels anymore. Thus, I'd not recommend it as a good idea to delete everything below this labels block, but worst case, we need to create another PR here.

[bnallapeta]

@fdberlking Just pushed a commit with the changes. PTAL.

[github-actions]

@bnallapeta Images are available for testing. docker pull ghcr.io/stakater/reloader:SNAPSHOT-PR-625-352f6ff2\ndocker pull ghcr.io/stakater/reloader:SNAPSHOT-PR-625-UBI-352f6ff2

[fdberlking]

@fdberlking Just pushed a commit with the changes. PTAL.

LGTM, this should work out 😃 Thanks for making the changes, @bnallapeta!

[github-actions]

@bnallapeta Images are available for testing. docker pull ghcr.io/stakater/reloader:SNAPSHOT-PR-625-ea12d912\ndocker pull ghcr.io/stakater/reloader:SNAPSHOT-PR-625-UBI-ea12d912

[github-actions]

@bnallapeta Images are available for testing. docker pull ghcr.io/stakater/reloader:SNAPSHOT-PR-625-e6154787\ndocker pull ghcr.io/stakater/reloader:SNAPSHOT-PR-625-UBI-e6154787