Neither Labels nor Annotations are currently supported for CRDs

[fire-ant]

until the follow are reviewed and merged can we remove the labels and annotations from the CRD template?

kubernetes-sigs/controller-tools#691
kubernetes-sigs/controller-tools#454

It doesnt make sense to include something which will be rejected at a later point by native tooling. Maybe better to use kustomize and point to how that could work if needed as an option

[fire-ant]

#46

[AssafKatz3]

We also need it, we tried to work with Ambassador Edge Stack CRDs but got junk due to this issue

[arttor]

@AssafKatz3 @fire-ant please upgrade to release v0.3.15. All CRD labels and annotations are preserved. Standard labels like:

  labels:
    helm.sh/chart: operator-0.1.0
    app.kubernetes.io/name: operator
    app.kubernetes.io/instance: tst
    app.kubernetes.io/version: "0.1.0"
    app.kubernetes.io/managed-by: Helm

will be generated by Helm.

Thank you for reporting the issue!

[AssafKatz3]

@AssafKatz3 @fire-ant please upgrade to release v0.3.15. All CRD labels and annotations are preserved. Standard labels like:

  labels:
    helm.sh/chart: operator-0.1.0
    app.kubernetes.io/name: operator
    app.kubernetes.io/instance: tst
    app.kubernetes.io/version: "0.1.0"
    app.kubernetes.io/managed-by: Helm

will be generated by Helm.

Thank you for reporting the issue!

Hi,
@arttor I upgraded to release v0.3.16 but I still got:
{{- include "aes-crds.labels" . | nindent 4 }}
In consulresolver-crd.yaml after applying it to Ambassador Edge Stack CRDs. And this is an error since CRDs are not templated.
Thanks

[arttor]

Ok, now i am not sure that it is a good idea to use -cdr-dir. Because it will never work for webhook certificate annotation. I think that we should remove this option.

[AssafKatz3]

@arttor
This is a good option, but it should simply omit the files without any modification in text.
Thanks

[AssafKatz3]

Currently I must use:

helmify -crd-dir -v "${chart_path}" < "$filename"
kubectl-slice --skip-non-k8s --input-file "$filename" --output-dir "${chart_path}/crds" --include-kind CustomResourceDefinition --template '{{.spec.names.singular}}-crd.yaml'

But there is no reason why need two tools for it.

[arttor]

Please don't use -crd-dir option. Since crds does not support templating it won't work for operators using validation webhooks.

[AssafKatz3]

@arttor It still breaks the CRD into another directory so it still useful for it.

[arttor]

I can disable crd templating for -crd-dir to make it work just for your project. But I am afraid that if I do so I we will have another issue: annotation cert-manager.io/inject-ca-from is incorrectly resolved.

So we will always have situation when option -crd-dir not fully supports all project features.

What are benefits of having CRDs in a separate directory?

[AssafKatz3]

make it work just for your project. But I am afraid that if I do so I we will have another issue: annotation cert-manager.io/inject-ca-from is incorrectly resolved.

Hi,
cert-manager.io/inject-ca-from annotation doesn't make sense for CRDs, so we don't need it.

HELM 3 requires the CRDs to be in crds directory, so splitting them is required.
Thanks

[arttor]

For kubebuilder/operator-sdk projecs kustomize config or helm chart does not contain CRD instances. You install operator with one chart and the create CRD instances with a separate one.

[AssafKatz3]

For kubebuilder/operator-sdk projecs kustomize config or helm chart does not contain CRD instances. You install operator with one chart and the create CRD instances with a separate one.

Hi,
There are projects without such clear cut (using method 1 per HELM documentation). I am using this tool for one of them Ambassador Edge Stack CRDs.
Thanks

[arttor]

I fully understand that cert-manager is not relevant for you project. But is required for most of operators which is also using this tool.
I fully agree that current implementations of -crd-dir not makes only sense because it adds templates.
I need to think about on how to warn users to not use this option for kubebuilder projects

[AssafKatz3]

I fully understand that cert-manager is not relevant for you project. But is required for most of operators which is also using this tool. I fully agree that current implementations of -crd-dir not makes only sense because it adds templates. I need to think about on how to warn users to not use this option for kubebuilder projects

Hi,
The point wasn't that cert-manager isn't relevant to CRDs, any CRD, cert-manager won't do anything due to this adding, so you don't need to add it to CRDs, even if your project uses it.
Thanks

[arttor]

It is relevant for kubebuilder projects.
https://book.kubebuilder.io/cronjob-tutorial/running-webhook.html
https://cert-manager.io/docs/contributing/crds/#internal-api-versions

[arttor]

But anyway i will remove CRD templating from -crd-dir option

[arttor]

@AssafKatz3 disabled CRD templating for -crd-dir in v0.3.17.

[fire-ant]

Hi, The point wasn't that cert-manager isn't relevant to CRDs, any CRD, cert-manager won't do anything due to this adding, so you don't need to add it to CRDs, even if your project uses it. Thanks

@arttor thanks for the. fix. I think to satisfy both use cases it might be useful to list out a set of labels which are applied to everything but crd's and filter these out at the point of rendering. we can have our cake and eat it :)