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

Server-side validation - envvar interpolation #156

Closed
roshan-gh opened this issue Oct 31, 2023 · 3 comments · Fixed by #180
Closed

Server-side validation - envvar interpolation #156

roshan-gh opened this issue Oct 31, 2023 · 3 comments · Fixed by #180
Assignees

Comments

@roshan-gh
Copy link
Contributor

Support envvar interpolation for the server-side validation

@roshan-gh
Copy link
Contributor Author

@mmanciop opened the issue

@mmanciop
Copy link
Member

mmanciop commented Nov 1, 2023

We should detect browser-side env vars to be interpolated and, before validating backend-side, ask for test values to be used in the backend validation.

@mmanciop
Copy link
Member

mmanciop commented Nov 8, 2023

Based on open-telemetry/opentelemetry-collector#4742, when validating backend-side, in the frontend we need to:

  1. Detect \${([a-zA-Z_]+:)?([^}]+)} and \$[a-zA-Z_][a-zA-Z0-9_]* sequences.

  2. Check whether the first matching group in \${([a-zA-Z_]+:)?([^}]+)} is not the empty string, and therefore it is a configuration resolver prefix. If we do find a configuration resolver prefix, and it is env:, the second capture group denotes an environment variable name. Other prefixes, like file:, need to raise an error that disables backend validation, as we won't be able to resolve the values server-side.

Given the list of environment variable names found with the steps above, we need to prompt the user for input before any backend validation can occur. I do not see a meaningful and practical way of proposing default values at this stage.

As a note about other configuration providers (e.g., file:): while they will prevent any validation from succeeding, by disabling validation, we are giving up on catch other errors before we get stuck on the unresolvable config. However, the UX would be consistent, and it seems like the better trade-off.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants