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

Command-line args instead of env vars #363

Closed
guicassolato opened this issue Nov 17, 2022 · 2 comments · Fixed by #366
Closed

Command-line args instead of env vars #363

guicassolato opened this issue Nov 17, 2022 · 2 comments · Fixed by #366

Comments

@guicassolato
Copy link
Collaborator

Authorino reads configuration for the executable from environment variables. This is not ideal as env vars usually belong to the global state of the operating system and, most importantly, are mutable. We should follow a better practice and read configuration options from command-line arguments instead.

This will require changes in https://github.com/kuadrant/authorino-operator as well.

@gsaslis
Copy link
Collaborator

gsaslis commented Nov 17, 2022

@guicassolato this caught my eye - just wondering if this implies that authorino can never pick up configuration changes at runtime ?

i.e. would I always need to restart pods for config changes to be picked up ? are there cases where the application logic would need to handle a certain config param's value transition ?

@guicassolato
Copy link
Collaborator Author

guicassolato commented Nov 17, 2022

just wondering if this implies that authorino can never pick up configuration changes at runtime

@gsaslis, this is true for things as they are today already. Authorino reads env vars at boot-time and then works with those values all along.

So what we gain here with this issue is not really predictability about the execution itself, but rather the ability to read the state of the application as of when it was booted in the first place. By reading the env vars on the other hand, you know the current values, but not the original ones (at least not from within the environment).

One could argue that in a containerised world, env vars are (should be) immutable and you should always redeploy the application when you want to modify the environment, like for example by editing a Deployment resource in Kubernetes. However, if that is true, then so is your original statement all the same. Authorino never picks up configuration changes at runtime.

That all been said, what I did not say about this issue before is that it also has to do with:

  1. improving the command-line for the application – something we did for Limitador already (CLI limitador#92);
  2. standardising the way boot options are passed to the application – today there's a mix of env vars and command-line args.

As final thoughts, I would add that, for inter-process communication, there are better ways to exchange data than by modifying the global state of environment; while for intra-process using env vars is an indication of bad design IMO.

All the freedom we deserve, but just as much so we don't hurt ourselves! 🙂

@guicassolato guicassolato mentioned this issue Nov 21, 2022
Merged
Repository owner moved this from In Progress to Done in Kuadrant Service Protection Nov 30, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
No open projects
Archived in project
Development

Successfully merging a pull request may close this issue.

2 participants