feat: add archive_path attribute to coderd_template versions#344
feat: add archive_path attribute to coderd_template versions#344jatcod3r wants to merge 3 commits intocoder:mainfrom
Conversation
Implements coder#340. Adds archive_path as an alternative to directory in the versions block of coderd_template, supporting .tar and .zip files. Changes: - Add archive_path (Optional) and archive_hash (Computed) attributes - Validator ensures exactly one of directory/archive_path is set - Plan modifier computes archive_hash from file contents (SHA-256) - normalizeZip adds missing directory entries for archives produced by Terraform's archive_file data source - uploadArchive sends the archive to Coder with correct content type - Warn when switching between archive_path and directory about hidden file inclusion differences - Document 100 MiB server upload limit in archive_path description Tests: - TestComputeArchiveHash (4 cases) - TestArchiveContentType (5 cases) - TestNormalizeZip (2 cases) - TestReconcileVersionIDs extended with ArchiveHashMatching/Changed
|
@codex review |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1f8930debf
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| dirSet := !version.Directory.IsNull() | ||
| archiveSet := !version.ArchivePath.IsNull() | ||
| if !dirSet && !archiveSet { |
There was a problem hiding this comment.
The new directory/archive_path XOR check treats any non-null value as "set", but unknown values are non-null in the framework, so a configuration where these attributes are conditionally computed can fail validation early with a false "mutually exclusive" or "must be specified" error. Terraform’s framework guidance is to avoid emitting validation errors for unknown non-computed values and wait for the later known pass; otherwise valid plans can be blocked before dependencies are resolved.
Useful? React with 👍 / 👎.
Per Terraform framework guidance, validators should not emit errors for unknown values. When either directory or archive_path is unknown (e.g., referencing a computed value from another resource), the XOR check is skipped. Terraform will re-run validators once values resolve during apply.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a48a37f432
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| tflog.Info(ctx, "skipping vars file discovery for archive upload, use tf_vars to provide variables") | ||
| } else { |
There was a problem hiding this comment.
The archive_path branch skips vars-file discovery entirely, so UserVariableValues are only populated from explicit tf_vars. In contrast, the directory flow still parses terraform.tfvars/*.auto.tfvars before creating the template version. This means switching an existing template from directory to archive_path can silently drop variable values and cause import/job failures when required variables were previously sourced from vars files.
Useful? React with 👍 / 👎.
| dir := filepath.Dir(f.Name) | ||
| for dir != "." && dir != "/" && dir != "" { | ||
| dirEntry := dir + "/" | ||
| if existingDirs[dirEntry] || missingDirs[dirEntry] { |
There was a problem hiding this comment.
normalizeZip derives directory entries with filepath.Dir, which is OS-specific. On Windows, this can produce backslash-based path segments while ZIP entry names are forward-slash based, leading to synthesized directory names that do not match archive paths and leaving nested entries effectively unnormalized.
Useful? React with 👍 / 👎.
- Use stdpath.Dir (stdlib path package) instead of filepath.Dir when traversing ZIP entry names. ZIP entries always use forward slashes per spec, and filepath.Dir is OS-specific (would produce backslashes on Windows). - Enhance the directory-to-archive_path switch warning to also mention that automatic tfvars file discovery is not performed for archive uploads, directing users to use tf_vars explicitly.
|
@codex review |
|
Codex Review: Didn't find any major issues. More of your lovely PRs please. ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
3 participants
Implements #340. Adds archive_path as an alternative to directory in the versions block of coderd_template, supporting .tar and .zip files.
Changes:
Tests: