arttor / helmify

Creates Helm chart from Kubernetes yaml
MIT License
1.48k stars 136 forks source link

fix: inject-ca-from is not correctly generated #42

Closed holyspectral closed 2 years ago

holyspectral commented 2 years ago

Hi there,

Thanks for creating this fantastic project! This will definitely enable more usage when users want to use both kustomize and helm charts but also want to maintain single version of truth.

When I tried to use helmify with kubebuilder for my admission controller, I noticed that inject-ca-from field is not correctly converted into helm charts.

Expected:

    cert-manager.io/inject-ca-from: {{ .Release.Namespace }}/{{ include "chart.fullname" . }}-serving-cert

Actual:

    cert-manager.io/inject-ca-from: {{ .Release.Namespace }}/{{ include "chart.fullname" . }}-admission-controller-system/admission-controller-serving-cert

Looks like its namespace and prefix is not correctly trimmed. Somehow appMeta.ChartName() is always chart, so it fails to remove those prefix.

This PR fixes the issue by using appMeta.TrimName() to trim, like other processors.

Let me know if you have any question or comment.

Thanks a lot! Sam

arttor commented 2 years ago

Sorry, i can see that you've updated the source test data. Please also run helmify against the test data to update generated examples by executing cat test_data/k8s-operator-kustomize.output | go run ./cmd/helmify examples/operator in the project root.

holyspectral commented 2 years ago

Hi @arttor , sorry for confusion. So it looks like there is some data inconsistency in the current k8s-operator-kustomize.output.

As you can see here, the one in ValidatingWebhookConfiguration doesn't have the my-operator prefix, which is usually generated by kubebuilder, like Certificate does.

operator-serving-cert https://github.com/arttor/helmify/blob/main/test_data/k8s-operator-kustomize.output#L679

apiVersion: admissionregistration.k8s.io/v1
kind: ValidatingWebhookConfiguration
metadata:
  annotations:
    cert-manager.io/inject-ca-from: my-operator-system/operator-serving-cert

v.s. my-operator-serving-cert https://github.com/arttor/helmify/blob/main/test_data/k8s-operator-kustomize.output#L656

apiVersion: cert-manager.io/v1
kind: Certificate
metadata:
  name: my-operator-serving-cert
  namespace: my-operator-system

That makes me change test_data instead of examples/. After I change the test_data and run the command you suggested, there is no change in examples/

Does this make sense to you? Let me know if there is any chance to improve this PR.

Thanks, Sam

arttor commented 2 years ago

Sorry, you are absolutely right.

arttor commented 2 years ago

Thank you for your contribution. Your changes are available in v0.3.13 release