-
Notifications
You must be signed in to change notification settings - Fork 97
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
[WAYP-2819] Support creating no-code workspaces #927
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👋 I know this PR is still a draft, but figured I'd leave some general pointers below.
registry_no_code_module.go
Outdated
Name *string `jsonapi:"attr,name,omitempty"` | ||
|
||
// Description is a description for the workspace. | ||
Description *string `jsonapi:"attr,description,omitempty"` | ||
|
||
AutoApply *bool `jsonapi:"attr,auto-apply,omitempty"` | ||
|
||
// Project is the associated project with the workspace. If not provided, | ||
// default project of the organization will be assigned to the workspace. | ||
Project *Project `jsonapi:"relation,project,omitempty"` | ||
|
||
// Variables is the slice of variables to be configured for the no-code | ||
// workspace. | ||
Variables []*Variable `jsonapi:"relation,vars,omitempty"` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As a convention, we don't express required fields as pointers or with omitempty
.
registry_no_code_module.go
Outdated
if !validStringID(o.Name) { | ||
return ErrInvalidName | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
validStringID ensures the field follows the resource external ID format used by our API (e.g for a workspace it would be ws-1234abcde
).
So either this field accepts a workspace's external ID or a org/workspace name pair to identify a workspace, but not both.
However since this creates a workspace using a no-code module, my guess would be to provide the name not an external ID? In which case we only need to call validString(o.Name)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@sebasslash thanks for the feedback! I've addressed this and your other comment in this commit.
The registry no-code module resource is updated with this commit to include an additional method which can be used to create workspaces using the registry no-code module.
Also fix validation of the no-code workspace name.
0174a85
to
12402eb
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These changes look good! Please update the changelog.md to make a note of the support you are adding.
@@ -95,6 +95,7 @@ runs: | |||
TFC_RUN_TASK_URL: "http://testing-mocks.tfe:22180/runtasks/pass" | |||
GITHUB_POLICY_SET_IDENTIFIER: "hashicorp/test-policy-set" | |||
GITHUB_REGISTRY_MODULE_IDENTIFIER: "hashicorp/terraform-random-module" | |||
GITHUB_REGISTRY_NO_CODE_MODULE_IDENTIFIER: "hashicorp/terraform-random-no-code-module" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could my team be provided with an admin permissions to this repo? I'm just thinking of the future scenario where we may want to add an gh action secret or adjust the settings.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I added your team as admin!
@@ -20,7 +20,7 @@ There are instances where several new resources being added (i.e Workspace Run T | |||
|
|||
After opening a PR, our CI system will perform a series of code checks, one of which is linting. Linting is not strictly required for a change to be merged, but it helps smooth the review process and catch common mistakes early. If you'd like to run the linters manually, follow these steps: | |||
|
|||
1. Ensure you have [installed golangci-lint](https://golangci-lint.run/usage/install/#local-installation) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
oh, "page not found". Thanks for this update!
CHANGELOG.md
Outdated
@@ -1,5 +1,11 @@ | |||
# UNRELEASED | |||
|
|||
# v1.61.0 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you remove this # v1.61.0
line please? My team likes to add this line right when we are about to release to signify that we are ready to release a new version. It is okay for your changelog note to be right under the # UNRELEASED
line
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Gotcha - will fix this!
Reminder to the contributor that merged this PR: if your changes have added important functionality or fixed a relevant bug, open a follow-up PR to update CHANGELOG.md with a note on your changes. |
The registry no-code module resource is updated with this commit to include an additional method which can be used to create workspaces using the registry no-code module.
Description
Testing plan
Run
go-tfe go test -run TestRegistryNoCodeModule -v ./...
with the necessary environment variables.