Add ability to specify annotations in helm chart CRDs manifests

[minhthong582000]

What would you like added?

Currently in the actions-runner helm chart, we don't have any way to specify a custom annotations inside CRDs manifests. So I would like to add this feature by template it and move all of the CRDs manifests to templates folder.

For example:

# ./templates/crds/actions.summerwind.dev_horizontalrunnerautoscalers.yaml
{{- if .Values.crds.install }}
apiVersion: apiextensions.k8s.io/v1
kind: CustomResourceDefinition
metadata:
  annotations:
    controller-gen.kubebuilder.io/version: v0.7.0
    {{- if .Values.crds.keep }}
    helm.sh/resource-policy: keep
    {{- end }}
    {{- with .Values.crds.annotations }}
      {{- toYaml . | nindent 4 }}
    {{- end }}
  name: horizontalrunnerautoscalers.actions.summerwind.dev
...
{{- end }}

# ./values.yaml
# Custom resource configuration
crds:
  # Install and upgrade CRDs
  install: true
  # Keep CRDs on chart uninstall
  keep: true
  # Annotations to be added to all CRDs
  annotations: {}

Why is this needed?

I'm facing an issue with annotations size limitation (metadata.annotations: Too long: must have at most 262144 characters) when deploying the actions-runner-controller helm chart using ArgoCD. One way to fix this is using server side apply. And I know that I can specify this annotation in the ArgoCD Aplication. But I think it would be cleaner to specify argocd.argoproj.io/sync-options: ServerSideApply=true in the CRDs annotation.

[github-actions[bot]]

Hello! Thank you for filing an issue.

The maintainers will triage your issue shortly.

In the meantime, please take a look at the troubleshooting guide for bug reports.

If this is a feature request, please review our contribution guidelines.

[rufreakde]

This is super helpful in many ways. I would suggest to make this feature generic though:

Having a generic way to apply annotations to any specified "kind:" maybe would also solve this issue and allow for more flexibility as to when you want to use it. For example:

helm install ... --annotate-kind="CustomResourceDefinition argocd.argoproj.io/sync-options: ServerSideApply=true" --annotate-kind="CustomResourceDefinition argocd.argoproj.io/sync-options: Replace=true"

Alternatively if wished (since templating can be used for others) it can be limited to CRDs:

helm install ... --annotate-crds="argocd.argoproj.io/sync-options: ServerSideApply=true" --annotate-crds="argocd.argoproj.io/sync-options: Replace=true"

This would solve a lot of cases where declarative setups would need to use Kustomise...

PS: There are many issue threads I link this one here: https://github.com/prometheus-community/helm-charts/issues/1500

[mumoshu]

Hey! Thanks for your feedback. I understand your motivation. However, doing this in a generic way means CRDs need to be rendered as Helm templates, which makes the chart unconventional as I've commented in #2126. Maybe a more feasible approach would be to hard-code the well-known and useful annotations to CRDs, until we come up with use-cases that actually require the generic feature. WDYT?

[minhthong582000]

Hey! Thanks for your feedback. I understand your motivation. However, doing this in a generic way means CRDs need to be rendered as Helm templates, which makes the chart unconventional as I've commented in #2126. Maybe a more feasible approach would be to add the hard-code the well-known and useful annotations to CRDs, until we come up with use-cases that actually require the generic feature. WDYT?

Thank you for the response in #2126. Beside of the solutions in the issue from @rufreakde comment and mine, we can handle the the lifecycle of the CRDs in a separate helm chart (while keeping the crd folder in the current helm chart) and let the users choose to install one of them. This is a recommended way from official helm documentation.

[rufreakde]

I comented in #2126 that these issues could be fixed in the most clean way if helm itself would provide CRD labeling and Annotations or general templating.

[mumoshu]

@minhthong582000 Ah, good point! Yeah, I absolutely agree that having a separate helm chart for CRDs only would be the best option today.

Would you mind reforming your pull request (or submitting another one) to add the separate chart?

[mumoshu]

@rufreakde Thank you very much for sharing! I've never realized there was momentum to improve the situation upstream- good to know!

Probably we might be adding a separate chart for now. However once the upstream added the CRD templating facility, we will be able to omit the separate chart. Does this plan sound good to you overall?

[minhthong582000]

@minhthong582000 Ah, good point! Yeah, I absolutely agree that having a separate helm chart for CRDs only would be the best option today.

Would you mind reforming your pull request (or submitting another one) to add the separate chart?

Sure, I'll create a new PR for this and notify you when it's done.