Skip to content

feat: implement dynamic parameter validation #18482

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.

Sign up for GitHub

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

Jump to bottom
Merged
Emyrk merged 1 commit into from
Jun 23, 2025
Merged

feat: implement dynamic parameter validation #18482

Emyrk merged 1 commit into from
Jun 23, 2025

Conversation

@Emyrk
Copy link
Member

@Emyrk Emyrk commented Jun 20, 2025 •
edited
Loading

What does this do?

This does parameter validation for dynamic parameters in wsbuilder. All input parameters are validated in coder/coder before being sent to terraform.

The heart of this PR is ResolveParameters.

What else changes?

wsbuilder now needs to load the terraform files into memory to succeed. This does add a larger memory requirement to workspace builds.

Future work

  • Sort autostart handling workspaces by template version id. So workspaces with the same template version only load the terraform files once from the db, and store them in the cache.

@Emyrk Graphite App
Copy link
Member Author

Emyrk commented Jun 20, 2025 •
edited
Loading

This stack of pull requests is managed by Graphite. Learn more about stacking.

@Emyrk Emyrk mentioned this pull request Jun 20, 2025
Merged
@Emyrk Emyrk force-pushed the stevenmasley/dynamic_validate branch from ab677c2 to 6a480d0 Compare June 20, 2025 20:25
@Emyrk Emyrk force-pushed the stevenmasley/parameters_to_preview branch from 46d2751 to 635c56b Compare June 22, 2025 01:03
@Emyrk Emyrk force-pushed the stevenmasley/dynamic_validate branch 7 times, most recently from 3522bb0 to 24b830b Compare June 22, 2025 22:47
@Emyrk Emyrk mentioned this pull request Jun 23, 2025
Merged
@Emyrk Emyrk changed the base branch from stevenmasley/parameters_to_preview to graphite-base/18482 June 23, 2025 01:39
@Emyrk Emyrk force-pushed the stevenmasley/dynamic_validate branch from 24b830b to 1991e91 Compare June 23, 2025 01:40
@Emyrk Emyrk changed the base branch from graphite-base/18482 to stevenmasley/sequential_query June 23, 2025 01:40
@Emyrk Emyrk changed the base branch from stevenmasley/sequential_query to graphite-base/18482 June 23, 2025 02:47
@Emyrk Emyrk force-pushed the stevenmasley/dynamic_validate branch from 1991e91 to 95aacbf Compare June 23, 2025 02:47
@Emyrk Emyrk force-pushed the graphite-base/18482 branch from 077e337 to d7398d9 Compare June 23, 2025 02:47
@Emyrk Emyrk changed the base branch from graphite-base/18482 to stevenmasley/deadlock_fetch June 23, 2025 02:47
@Emyrk Emyrk mentioned this pull request Jun 23, 2025
Merged
@Emyrk Emyrk force-pushed the stevenmasley/dynamic_validate branch 4 times, most recently from 2889dac to 0a26d90 Compare June 23, 2025 03:27
@Emyrk Emyrk force-pushed the stevenmasley/deadlock_fetch branch from d7398d9 to 353f193 Compare June 23, 2025 03:27
@Emyrk Emyrk force-pushed the stevenmasley/dynamic_validate branch from 0a26d90 to ff6d2de Compare June 23, 2025 03:38
@Emyrk Emyrk force-pushed the stevenmasley/deadlock_fetch branch from 353f193 to 2a37b9b Compare June 23, 2025 03:38
@Emyrk Emyrk force-pushed the stevenmasley/dynamic_validate branch 2 times, most recently from 81d451c to 9ae5ec8 Compare June 23, 2025 04:50
@Emyrk Emyrk force-pushed the stevenmasley/dynamic_validate branch 2 times, most recently from 9bc40dd to 0e3dc00 Compare June 23, 2025 14:23
@Emyrk Emyrk mentioned this pull request Jun 23, 2025
Merged
@Emyrk Emyrk force-pushed the stevenmasley/dynamic_validate branch 2 times, most recently from a51570a to 4f52076 Compare June 23, 2025 15:20
This was referenced Jun 23, 2025
Merged
Merged
johnstcn
johnstcn reviewed Jun 23, 2025
View reviewed changes
coderd/autobuild/lifecycle_executor.go
ctx context.Context
db database.Store
ps pubsub.Pubsub
fileCache *files.Cache
Copy link
Member

@johnstcn johnstcn Jun 23, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

TIL this is a thing!

coderd/dynamicparameters/resolver.go
Comment on lines +146 to +149
// TODO: Fix the `hcl.Diagnostics(...)` type casting. It should not be needed.
if hcl.Diagnostics(parameter.Diagnostics).HasErrors() {
// All validation errors are raised here.
diags = diags.Extend(hcl.Diagnostics(parameter.Diagnostics))
}
Copy link
Member

@johnstcn johnstcn Jun 23, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

follow-up issue?

Copy link
Member Author

@Emyrk Emyrk Jun 23, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll do you one better: #18501

Just trying to break out some of the diffs

coderd/dynamicparameters/resolver.go
Severity: hcl.DiagError,
Summary: "Immutable parameter changed",
Detail: fmt.Sprintf("Parameter %q is not mutable, so it can't be updated after creating a workspace.", parameter.Name),
Subject: src,
Copy link
Member

@johnstcn johnstcn Jun 23, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice 👍

@Emyrk Emyrk force-pushed the stevenmasley/dynamic_validate branch from 4f52076 to ff96e32 Compare June 23, 2025 16:11
@Emyrk Emyrk force-pushed the stevenmasley/sequential_query branch from 1d27539 to ac9d0e6 Compare June 23, 2025 16:32
@Emyrk Emyrk force-pushed the stevenmasley/dynamic_validate branch 2 times, most recently from ba8cf59 to c1d05a0 Compare June 23, 2025 16:45
@Emyrk Emyrk force-pushed the stevenmasley/sequential_query branch 3 times, most recently from 1e1cb20 to cb43418 Compare June 23, 2025 16:59
@Emyrk Emyrk force-pushed the stevenmasley/dynamic_validate branch from c1d05a0 to 129235b Compare June 23, 2025 16:59
@Emyrk Emyrk force-pushed the stevenmasley/sequential_query branch from cb43418 to a908ce6 Compare June 23, 2025 17:04
@Emyrk Emyrk force-pushed the stevenmasley/dynamic_validate branch from 129235b to 6a5ee9e Compare June 23, 2025 17:04
@Emyrk Emyrk changed the base branch from stevenmasley/sequential_query to graphite-base/18482 June 23, 2025 17:17
@Emyrk
feat: implement dynamic parameter validation
0523a2f 
Validation to occur in coder/coder inside wsbuilder for all dynamic
parameters.
@Emyrk Emyrk force-pushed the graphite-base/18482 branch from a908ce6 to 7254c08 Compare June 23, 2025 17:18
@Emyrk Emyrk force-pushed the stevenmasley/dynamic_validate branch from 6a5ee9e to 0523a2f Compare June 23, 2025 17:18
@graphite-app graphite-app bot changed the base branch from graphite-base/18482 to main June 23, 2025 17:18
@Emyrk Emyrk force-pushed the stevenmasley/dynamic_validate branch 2 times, most recently from f4e5548 to 0523a2f Compare June 23, 2025 17:18
@Emyrk Emyrk merged commit 82af2e0 into main Jun 23, 2025
40 of 80 checks passed
@Emyrk Graphite App
Copy link
Member Author

Emyrk commented Jun 23, 2025

Merge activity

@Emyrk Emyrk deleted the stevenmasley/dynamic_validate branch June 23, 2025 17:35
@github-actions github-actions bot locked and limited conversation to collaborators Jun 23, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

Copilot code review Copilot Copilot left review comments

@johnstcn johnstcn johnstcn approved these changes

Assignees

@Emyrk Emyrk

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@Emyrk @johnstcn