Skip to content
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

[TF-18527] Add Archs to AdminTerraformVersionCreateOptions #1022

Merged
merged 5 commits into from
Jan 3, 2025
Merged

[TF-18527] Add Archs to AdminTerraformVersionCreateOptions #1022

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
8724bc7
[TF-18527] Add Archs to AdminTerraformVersionCreateOptions
natalie-todd Dec 20, 2024
b3dc95d
Add CHANGELOG entry, omitempty, and remove Archs from structs not use…
natalie-todd Dec 31, 2024
62b5fa7
merge conflicts
natalie-todd Dec 31, 2024
b850462
Add Archs to tests
natalie-todd Dec 31, 2024
56eeb74
Add validation enforcing at least one valid arch or valid URL and SHA…
natalie-todd Jan 3, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,9 @@
# Unreleased

## Enhancements

* Add support for project level auto destroy settings @simonxmh [#1011](https://github.com/hashicorp/go-tfe/pull/1011)
* Add `Archs` field to `AdminTerraformVersionCreateOptions` by @natalie-todd [#1022](https://github.com/hashicorp/go-tfe/pull/1022)
Copy link
Member

Choose a reason for hiding this comment

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

Question for core cloud folks: do we need to note that this is in beta to give any TFE users a heads up that they won't be able to use that field yet?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Thank you, yes! In the past we have used words like: Adds BETA support for [such and such feature], which is EXPERIMENTAL, SUBJECT TO CHANGE, and may not be available to all users.
Those words could be applied here.


# v1.71.0

Expand Down
29 changes: 19 additions & 10 deletions admin_terraform_version.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import (
"context"
"fmt"
"net/url"
"reflect"
"time"
)

Expand Down Expand Up @@ -55,6 +56,13 @@ type AdminTerraformVersion struct {
CreatedAt time.Time `jsonapi:"attr,created-at,iso8601"`
}

type ToolVersionArchitecture struct {
URL string `jsonapi:"attr,url"`
Sha string `jsonapi:"attr,sha"`
OS string `jsonapi:"attr,os"`
Arch string `jsonapi:"attr,arch"`
}

// AdminTerraformVersionsListOptions represents the options for listing
// terraform versions.
type AdminTerraformVersionsListOptions struct {
Expand All @@ -70,15 +78,16 @@ type AdminTerraformVersionsListOptions struct {
// AdminTerraformVersionCreateOptions for creating a terraform version.
// https://developer.hashicorp.com/terraform/enterprise/api-docs/admin/terraform-versions#request-body
type AdminTerraformVersionCreateOptions struct {
Type string `jsonapi:"primary,terraform-versions"`
Version *string `jsonapi:"attr,version"` // Required
URL *string `jsonapi:"attr,url"` // Required
Sha *string `jsonapi:"attr,sha"` // Required
Official *bool `jsonapi:"attr,official,omitempty"`
Deprecated *bool `jsonapi:"attr,deprecated,omitempty"`
DeprecatedReason *string `jsonapi:"attr,deprecated-reason,omitempty"`
Enabled *bool `jsonapi:"attr,enabled,omitempty"`
Beta *bool `jsonapi:"attr,beta,omitempty"`
Type string `jsonapi:"primary,terraform-versions"`
Version *string `jsonapi:"attr,version"` // Required
URL *string `jsonapi:"attr,url"` // Required
Sha *string `jsonapi:"attr,sha"` // Required
Official *bool `jsonapi:"attr,official,omitempty"`
Deprecated *bool `jsonapi:"attr,deprecated,omitempty"`
DeprecatedReason *string `jsonapi:"attr,deprecated-reason,omitempty"`
Enabled *bool `jsonapi:"attr,enabled,omitempty"`
Beta *bool `jsonapi:"attr,beta,omitempty"`
Archs []*ToolVersionArchitecture `jsonapi:"attr,archs,omitempty"`
}

// AdminTerraformVersionUpdateOptions for updating terraform version.
Expand Down Expand Up @@ -194,7 +203,7 @@ func (a *adminTerraformVersions) Delete(ctx context.Context, id string) error {
}

func (o AdminTerraformVersionCreateOptions) valid() error {
Copy link
Member

Choose a reason for hiding this comment

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

Do we want to add a validation that checks all fields are provided for architectures, if present? I could also make a case for waiting to add that handling along with update support once the feature is released and the API is fully stable!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I just added a validation to check whether at least one valid arch was provided or a valid URL and SHA. Let me know if this seems good or if you were thinking of something else.

Copy link
Member

Choose a reason for hiding this comment

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

That's perfect! Just a little nicer to skip making the request if we already know it's going to fail.

if (o == AdminTerraformVersionCreateOptions{}) {
if (reflect.DeepEqual(o, AdminTerraformVersionCreateOptions{})) {
return ErrRequiredTFVerCreateOps
}
if !validString(o.Version) {
Expand Down
16 changes: 15 additions & 1 deletion admin_terraform_version_integration_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -103,15 +103,22 @@ func TestAdminTerraformVersions_CreateDelete(t *testing.T) {
version := genSafeRandomTerraformVersion()

t.Run("with valid options", func(t *testing.T) {
sha := String(genSha(t))
opts := AdminTerraformVersionCreateOptions{
Version: String(version),
URL: String("https://www.hashicorp.com"),
Sha: String(genSha(t)),
Sha: sha,
Deprecated: Bool(true),
DeprecatedReason: String("Test Reason"),
Official: Bool(false),
Enabled: Bool(false),
Beta: Bool(false),
Archs: []*ToolVersionArchitecture{{
URL: "https://www.hashicorp.com",
Sha: *sha,
OS: "linux",
Arch: "amd64",
}},
}
tfv, err := client.Admin.TerraformVersions.Create(ctx, opts)
require.NoError(t, err)
Expand Down Expand Up @@ -170,6 +177,7 @@ func TestAdminTerraformVersions_ReadUpdate(t *testing.T) {

t.Run("reads and updates", func(t *testing.T) {
version := genSafeRandomTerraformVersion()
sha := String(genSha(t))
opts := AdminTerraformVersionCreateOptions{
Version: String(version),
URL: String("https://www.hashicorp.com"),
Expand All @@ -179,6 +187,12 @@ func TestAdminTerraformVersions_ReadUpdate(t *testing.T) {
DeprecatedReason: String("Test Reason"),
Enabled: Bool(false),
Beta: Bool(false),
Archs: []*ToolVersionArchitecture{{
URL: "https://www.hashicorp.com",
Sha: *sha,
OS: "linux",
Arch: "amd64",
}},
}
tfv, err := client.Admin.TerraformVersions.Create(ctx, opts)
require.NoError(t, err)
Expand Down
Loading