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

confd config prefix vs resource prefix #109

Open
vincentgna opened this issue Aug 20, 2022 · 2 comments
Open

confd config prefix vs resource prefix #109

vincentgna opened this issue Aug 20, 2022 · 2 comments
Labels
enhancement New feature or request

Comments

@vincentgna
Copy link

vincentgna commented Aug 20, 2022

Thank you so much for maintaining this fork!

Currently confd config prefix overrides the template resource prefix, which seems counter-intuitive:

tr.Prefix = config.Prefix

Scenario:

  1. Set prefix in /etc/confd/confd.toml to production
  2. Set prefix in /etc/confd/conf.d/myapp.toml to myapp
  3. Set prefix in /etc/confd/conf.d/yourapp.toml to yourapp

Current behaviour:

Debug logs:

DEBUG Loading template resource from /etc/confd/conf.d/myapp.toml
DEBUG Retrieving keys from store
DEBUG Key prefix set to /production
DEBUG Processing key=/production/foo
DEBUG Loading template resource from /etc/confd/conf.d/yourapp.toml
DEBUG Retrieving keys from store
DEBUG Key prefix set to /production
DEBUG Processing key=/production/foo

Expected behaviour:

Debug logs:

DEBUG Loading template resource from /etc/confd/conf.d/myapp.toml
DEBUG Retrieving keys from store
DEBUG Key prefix set to /production/myapp
DEBUG Processing key=/production/myapp/foo
DEBUG Loading template resource from /etc/confd/conf.d/yourapp.toml
DEBUG Retrieving keys from store
DEBUG Key prefix set to /production/yourapp
DEBUG Processing key=/production/yourapp/foo

Does this make sense? is there a reason/use-case I'm missing?

I realize changing this would be a breaking change, so I'm not sure if you would consider this?

Suggestion

if tr.Prefix is set, concat with config.Prefix

@abtreece
Copy link
Owner

abtreece commented Sep 8, 2022

🤔

@abtreece
Copy link
Owner

abtreece commented Sep 9, 2022

It appears the behavior was modified in this PR. I think this is the appropriate behavior for the CLI, but I'm not sure there is a reason why "nested" prefixes of the .toml configs couldn't be concatenated.

@abtreece abtreece added the enhancement New feature or request label Sep 9, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants