Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

feat: Read params from file for template/workspace creation#1541

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Merged
AbhineetJain merged 13 commits intomainfrom1377-feat-load-parameters-from-file
May 20, 2022

Conversation

AbhineetJain
Copy link
Contributor

@AbhineetJainAbhineetJain commentedMay 18, 2022
edited
Loading

This allows users to use parameter files for variable input during template and workspace creation.

Subtasks

  • read parameters from a yaml file for workspace creation
  • read parameters from a yaml file for template creation
  • added tests for feature

Fixes#1377

@AbhineetJainAbhineetJain changed the titleRead params from file for template/workspace creationfeat: Read params from file for template/workspace creationMay 18, 2022
@@ -18,6 +20,7 @@ func create() *cobra.Command {
var (
workspaceName string
templateName string
parameterFile string
Copy link
Member

Choose a reason for hiding this comment

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

tfvars is the standard way to handle this with Terraform.

I'd prefer to not have our own solution, because that'd remove the ability for users to doterraform apply.

Copy link
Member

Choose a reason for hiding this comment

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

@kylecarbs that assumes we use terraform, but our cli should be terraform agnostic right?

Alsotfvars is just HCL or yaml. So we could just support multiple file types

Copy link
Member

Choose a reason for hiding this comment

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

Developers also won't have thetf file when runningcoder workspace create, only the person who made the template will have thetf file.

Copy link
Member

Choose a reason for hiding this comment

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

We can be Terraform agnostic while still consumingtfvars. Latching onto the existing Terraform ecosystem is a much better user experience than forking it with our own parameter format.

I don't believe it's necessary to have atfvars equivalent when creating a workspace. Or at least, we haven't seen it as a problem yet.

Copy link
ContributorAuthor

Choose a reason for hiding this comment

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

Issue#1377 mentions creating workspaces using parameter files, which is why I went with a non-tfvars approach. However,#1426 does talk about doing this for templates. Would you recommend solving both problems using different approaches or only work on#1426 (template creation via file) viatfvars? Let me quickly check how feasible usingtfvars for template creation is.

Copy link
ContributorAuthor

@AbhineetJainAbhineetJainMay 18, 2022
edited
Loading

Choose a reason for hiding this comment

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

Is there a documented flow of how the customers would useterraform apply when setting up templates? I tried using thetfvars file as follows:

$ terraform init$ terraform apply -var-file="<path_to_tfvars>"...Apply complete! Resources: 3 added, 0 changed, 0 destroyed.

It did not create a template and callingcoder templates create after this still asked me for input, so I am not sure what the actual flow should be.

@AbhineetJainAbhineetJain marked this pull request as ready for reviewMay 20, 2022 04:16
@AbhineetJain
Copy link
ContributorAuthor

Interestingly, the error I am encountering in the Windows tests is a known Go issue:golang/go#52986. Trying a workaround.

@AbhineetJain
Copy link
ContributorAuthor

AbhineetJain commentedMay 20, 2022
edited
Loading

The fix does not help. What is the precedent in these cases? Revert theimplementation fix and skip the tests until Go fixes this?

@AbhineetJainAbhineetJainforce-pushed the1377-feat-load-parameters-from-file branch from86b1028 to1491620CompareMay 20, 2022 14:40
@AbhineetJainAbhineetJain merged commit7c3e1a5 intomainMay 20, 2022
@AbhineetJainAbhineetJain deleted the 1377-feat-load-parameters-from-file branchMay 20, 2022 15:29
kylecarbs pushed a commit that referenced this pull requestJun 10, 2022
* Read params from file for template/workspace creation* Use os.ReadFile* Refactor reading params into a separate module* Add comments and unit tests* Rename variable* Uncomment and fix unit test* Fix comment* Refactor tests* Fix unit tests for windows* Fix unit tests for Windows* Add comments for the hotfix
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment
Reviewers

@kylecarbskylecarbskylecarbs left review comments

@EmyrkEmyrkEmyrk approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Feat: load parameters from file on workspace creation
3 participants
@AbhineetJain@Emyrk@kylecarbs

[8]ページ先頭

©2009-2025 Movatter.jp