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

Prefix CMP env var names #8986

Closed
crenshaw-dev opened this issue Apr 4, 2022 · 3 comments · Fixed by #9319
Closed

Prefix CMP env var names #8986

crenshaw-dev opened this issue Apr 4, 2022 · 3 comments · Fixed by #9319
Labels
enhancement New feature or request security Security related

Comments

@crenshaw-dev
Copy link
Collaborator

Summary

When a user sets an env var in a plugin-type Application spec, that env var is set for CMP manifest generation. We should prefix those env vars to limit what the user can change.

Motivation

CMPs involve plugin-author-defined tools which may be affected in various ways by env vars. By adding a prefix, we can make sure Application authors can't negatively impact the behavior of the CMP tools.

Proposal

We should prefix env vars with ARGOCD_USER_. This will be a breaking change, so we should do it on a major version and communicate clearly that plugin authors will have to update their plugins if it expects certain env vars.

@crenshaw-dev crenshaw-dev added enhancement New feature or request security Security related labels Apr 4, 2022
@crenshaw-dev crenshaw-dev changed the title We should prefix CMP env vars Prefix CMP env var names Apr 4, 2022
@thatsmydoing
Copy link
Contributor

Is there a way for plugins to check which version of argocd is running to decide what env var to use? Or is it expected to just try both variable names?

@crenshaw-dev
Copy link
Collaborator Author

@thatsmydoing it's currently expected to try both. If that's causing trouble, lmk, and we can try to figure something else out!

@thatsmydoing
Copy link
Contributor

Got it, the changelog suggested updating plugins but not how to be compatible with previous versions so just wanted to confirm what the suggested approach is.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request security Security related
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants