Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Helmraiser creates naming conflicts. #361

Closed
captncraig opened this issue Aug 25, 2020 · 5 comments · Fixed by #381
Closed

Helmraiser creates naming conflicts. #361

captncraig opened this issue Aug 25, 2020 · 5 comments · Fixed by #381
Labels
kind/bug Something isn't working

Comments

@captncraig
Copy link
Contributor

I am playing with helmraiser with the flux chart which creates a role named flux in each namespace I ask it to run flux against. When I run tk eval I see that there is only a single object named flux_role in the evaluated json.

This is because resources get added to the json with only $NAME_$KIND as the key. Code here.

This leads to a few competing concerns:

  • The keys need to be unique so as to not cause conflicts / overwriting keys.
  • The keys need to be usable and consistent to make later jsonnet overrides easier.

Possible solutions:

  1. Always name things $NAME_$KIND_$NAMESPACE.
    • fully explicit
    • we might not know the namespace until later (although I think we should set the namespace in the helm command. separate issue).
    • still a pain to access by a dynamic json key.
  2. Keep naming as is unless the namespace differs from the spec namespace, then append it.
    • will usually act as it does now.
    • usually easier to consume / override.
  3. Somehow make things a map by namespace, either at the top level, or inside child levels
    • still kinda hard to pull things out with jsonnet by dynamic namespace.

In any case, I'd vote that an error would be preferable to silently overwriting objects.

@issue-label-bot
Copy link

Issue-Label Bot is automatically applying the label kind/bug to this issue, with a confidence of 0.72. Please mark this comment with 👍 or 👎 to give our bot feedback!

Links: app homepage, dashboard and code for this bot.

@issue-label-bot issue-label-bot bot added the kind/bug Something isn't working label Aug 25, 2020
@captncraig
Copy link
Contributor Author

I can also work around this particular case by postprocessing the jsonnet and making copies of the object in question, but that isn't a great solution long term.

@sh0rez
Copy link
Member

sh0rez commented Aug 25, 2020

@captncraig A thought I had while reviewing Helmraiser was that it should not hardcode the name format at all, but accept it from Jsonnet, possibly as a Go template the same way the export filename works.

helm-util could pick a sensible default here, e.g. {{.metadata.name}}_{{.kind}}. This should work in many cases.

In special ones like yours, you can use a different template. You then also know exactly how things will turn out, which makes it clear how to mixin.

wdyt?

@captncraig
Copy link
Contributor Author

I like that on the surface. But my concern is that this is a particularly well hidden kind of bug. I didn't think about json keys until like 4 levels of debugging why my objects were not getting created in the cluster. The json keys are pretty low level, and you aren't going to know you need to change them until you have a problem that is frustrating you and you dig down into tk eval and the source of the plugin.

The json keys are an intermediate step between executing a helm template and creating k8s objects.

Perhaps your solution would be feasable, but we should certainly show a super helpful error message if we still encounter a conflict.

@sh0rez
Copy link
Member

sh0rez commented Aug 25, 2020

Shouldn't be too hard to just abort the helmraiser code in case this naming conflict manifests

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants