-
Notifications
You must be signed in to change notification settings - Fork 22
feat: validate user input values the same way as "default" #381
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
base: main
Are you sure you want to change the base?
Conversation
@johnstcn @matifali should this pass a template import? Before this PR, this would pass a data "coder_parameter" "region" {
name = "region"
description = "Which region would you like to deploy to?"
type = "string"
order = 1
option {
name = "Europe"
value = "eu"
}
option {
name = "United States"
value = "us"
}
} This PR sets the error to:
To fix this, either the value must be passed via an env var:
Or with a default:
I feel like we should reject it? But that is a breaking change. |
There are use cases where an admin may not want to set a default to force the user to choose an option. At least it was possible before this PR. I am not sure how many users were actually using it. But I have seen requests to support |
This behaviour is problematic because you could end up with a plan that does not correspond to the apply, depending on how the value of the parameter is used. Would marking the parameter as EDIT: marking a parameter as ephemeral requires it to be mutable and to also have a default set. This may not suit certain use-cases. |
Do we require a user input if the parameters are ephemeral? Do we not allow setting a default value for such parameters? If the answers are yes, then we just need better docs on how to fulfill this use case. So that people don't try to use the existing non ephemeral parameters. |
How I see it is the relaxed validation on template import is incorrect. If a parameter has 0 values, but will have some value at workspace create, then the However, I understand the use case. The provider SDK we are using does not indicate anywhere if My gut thought is that we need to effectively have 2 modes of validation. 1 for template import, 1 for workspace create, where the latter is strict. The strict validation is the most correct. What if we pass through an env var a relaxed validation mode? |
I think this approach could work, but we'd want to perhaps tie its behaviour specifically to what "operation" we're doing in Coder. We might also want to be wary of folks just setting this env var on the provisioner to work around null values entirely. |
No description provided.