From 5ba4056e13225b8cfb7478dcdb8faf922bc14322 Mon Sep 17 00:00:00 2001 From: uk1288 Date: Thu, 14 Jul 2022 08:43:10 -0400 Subject: [PATCH 1/4] replace deprecated GET api for registry modules --- CHANGELOG.md | 2 ++ errors.go | 4 ++- registry_module.go | 35 ++++++++++++++++++++++- registry_module_integration_test.go | 43 +++++++++++++++++++++++++++-- 4 files changed, 80 insertions(+), 4 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 4dc8e2504..b2f6db2a3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,8 +1,10 @@ # Unreleased +## Enhancements * [beta] Add support for triggering Workspace runs through matching Git tags [#434](https://github.com/hashicorp/go-tfe/pull/434) * Add `Query` param field to `AgentPoolListOptions` to allow searching based on agent pool name, by @JarrettSpiker [#417](https://github.com/hashicorp/go-tfe/pull/417) * Add organization scope and allowed workspaces field for scope agents by @Netra2104 [#453](https://github.com/hashicorp/go-tfe/pull/453) +* Adds `Namespace` and `RegistryName` fields to `RegistryModuleID` to allow reading of Public Registry Modules by @Uk1288 [#438](https://github.com/hashicorp/go-tfe/pull/438) ## Bug fixes * Fixed JSON mapping for Configuration Versions failing to properly set the `speculative` property [#459](https://github.com/hashicorp/go-tfe/pull/459) diff --git a/errors.go b/errors.go index 1a8e63aff..7fe3b9157 100644 --- a/errors.go +++ b/errors.go @@ -174,7 +174,7 @@ var ( ErrInvalidArch = errors.New("invalid value for arch") - ErrInvalidRegistryName = errors.New("invalid value for registry-name") + ErrInvalidRegistryName = errors.New(`invalid value for registry-name. It must be either "private" or "public"`) ) // Missing values for required field/option @@ -300,4 +300,6 @@ var ( ErrRequiredFilename = errors.New("filename is required") ErrInvalidAsciiArmor = errors.New("ascii armor is invalid") + + ErrRequiredNamespace = errors.New("namespace is required for public registry") ) diff --git a/registry_module.go b/registry_module.go index 56e753092..93aab9ddb 100644 --- a/registry_module.go +++ b/registry_module.go @@ -3,7 +3,9 @@ package tfe import ( "context" "fmt" + "log" "net/url" + "strings" ) // Compile-time proof of interface implementation. @@ -82,6 +84,11 @@ type RegistryModuleID struct { Name string // The module's provider, see RegistryModule.Provider Provider string + // The namespace of the module. For private modules this is the name of the organization that owns the module + // Required for public modules + Namespace string + // Either public or private. If not provided, defaults to private + RegistryName RegistryName } // RegistryModuleList represents a list of registry modules. @@ -95,6 +102,8 @@ type RegistryModule struct { ID string `jsonapi:"primary,registry-modules"` Name string `jsonapi:"attr,name"` Provider string `jsonapi:"attr,provider"` + RegistryName RegistryName `jsonapi:"attr,registry-name"` + Namespace string `jsonapi:"attr,namespace"` Permissions *RegistryModulePermissions `jsonapi:"attr,permissions"` Status RegistryModuleStatus `jsonapi:"attr,status"` VCSRepo *VCSRepo `jsonapi:"attr,vcs-repo"` @@ -307,12 +316,25 @@ func (r *registryModules) Read(ctx context.Context, moduleID RegistryModuleID) ( return nil, err } + if moduleID.RegistryName == "" { + log.Println("[WARN] Support for using the RegistryModuleID without RegistryName is deprecated as of release 1.5.0 and may be removed in a future version. The preferred method is to include the RegistryName in RegistryModuleID.") + moduleID.RegistryName = PrivateRegistry + } + + if moduleID.RegistryName == PrivateRegistry && strings.TrimSpace(moduleID.Namespace) == "" { + log.Println("[WARN] Support for using the RegistryModuleID without Namespace is deprecated as of release 1.5.0 and may be removed in a future version. The preferred method is to include the Namespace in RegistryModuleID.") + moduleID.Namespace = moduleID.Organization + } + u := fmt.Sprintf( - "registry-modules/show/%s/%s/%s", + "organizations/%s/registry-modules/%s/%s/%s/%s", url.QueryEscape(moduleID.Organization), + url.QueryEscape(string(moduleID.RegistryName)), + url.QueryEscape(moduleID.Namespace), url.QueryEscape(moduleID.Name), url.QueryEscape(moduleID.Provider), ) + req, err := r.client.NewRequest("GET", u, nil) if err != nil { return nil, err @@ -419,6 +441,17 @@ func (o RegistryModuleID) valid() error { if !validStringID(&o.Provider) { return ErrInvalidProvider } + // RegistryName is optional, only validate if specified + if validString((*string)(&o.RegistryName)) { + registryNamesMap := map[RegistryName]RegistryName{PublicRegistry: PublicRegistry, PrivateRegistry: PrivateRegistry} + if _, ok := registryNamesMap[o.RegistryName]; !ok { + return ErrInvalidRegistryName + } + } + + if o.RegistryName == PublicRegistry && !validString(&o.Namespace) { + return ErrRequiredNamespace + } return nil } diff --git a/registry_module_integration_test.go b/registry_module_integration_test.go index 5e4711405..30dbe0abb 100644 --- a/registry_module_integration_test.go +++ b/registry_module_integration_test.go @@ -417,6 +417,29 @@ func TestRegistryModulesRead(t *testing.T) { }) }) + t.Run("with complete registry module ID fields", func(t *testing.T) { + rm, err := client.RegistryModules.Read(ctx, RegistryModuleID{ + Organization: orgTest.Name, + Name: registryModuleTest.Name, + Provider: registryModuleTest.Provider, + Namespace: orgTest.Name, + RegistryName: PrivateRegistry, + }) + require.NoError(t, err) + assert.Equal(t, registryModuleTest.ID, rm.ID) + + t.Run("permissions are properly decoded", func(t *testing.T) { + assert.True(t, rm.Permissions.CanDelete) + assert.True(t, rm.Permissions.CanResync) + assert.True(t, rm.Permissions.CanRetry) + }) + + t.Run("timestamps are properly decoded", func(t *testing.T) { + assert.NotEmpty(t, rm.CreatedAt) + assert.NotEmpty(t, rm.UpdatedAt) + }) + }) + t.Run("without a name", func(t *testing.T) { rm, err := client.RegistryModules.Read(ctx, RegistryModuleID{ Organization: orgTest.Name, @@ -457,6 +480,18 @@ func TestRegistryModulesRead(t *testing.T) { assert.Equal(t, err, ErrInvalidProvider) }) + t.Run("with an invalid registry name", func(t *testing.T) { + rm, err := client.RegistryModules.Read(ctx, RegistryModuleID{ + Organization: orgTest.Name, + Name: registryModuleTest.Name, + Provider: registryModuleTest.Provider, + Namespace: orgTest.Name, + RegistryName: "PRIVATE", + }) + assert.Nil(t, rm) + assert.Equal(t, err, ErrInvalidRegistryName) + }) + t.Run("without a valid organization", func(t *testing.T) { rm, err := client.RegistryModules.Read(ctx, RegistryModuleID{ Organization: badIdentifier, @@ -775,8 +810,10 @@ func TestRegistryModule_Unmarshal(t *testing.T) { "type": "registry-modules", "id": "1", "attributes": map[string]interface{}{ - "name": "module", - "provider": "tfe", + "name": "module", + "provider": "tfe", + "namespace": "org-abc", + "registry-name": "private", "permissions": map[string]interface{}{ "can-delete": true, "can-resync": true, @@ -815,6 +852,8 @@ func TestRegistryModule_Unmarshal(t *testing.T) { assert.Equal(t, rm.ID, "1") assert.Equal(t, rm.Name, "module") assert.Equal(t, rm.Provider, "tfe") + assert.Equal(t, rm.Namespace, "org-abc") + assert.Equal(t, rm.RegistryName, "private") assert.Equal(t, rm.Permissions.CanDelete, true) assert.Equal(t, rm.Permissions.CanRetry, true) assert.Equal(t, rm.Status, RegistryModuleStatusPending) From 6c6a105201af1aa37b8b40d15f94f882045c96dc Mon Sep 17 00:00:00 2001 From: uk1288 Date: Thu, 14 Jul 2022 08:58:02 -0400 Subject: [PATCH 2/4] update PR number --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index b2f6db2a3..0654b77bd 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,7 +4,7 @@ * [beta] Add support for triggering Workspace runs through matching Git tags [#434](https://github.com/hashicorp/go-tfe/pull/434) * Add `Query` param field to `AgentPoolListOptions` to allow searching based on agent pool name, by @JarrettSpiker [#417](https://github.com/hashicorp/go-tfe/pull/417) * Add organization scope and allowed workspaces field for scope agents by @Netra2104 [#453](https://github.com/hashicorp/go-tfe/pull/453) -* Adds `Namespace` and `RegistryName` fields to `RegistryModuleID` to allow reading of Public Registry Modules by @Uk1288 [#438](https://github.com/hashicorp/go-tfe/pull/438) +* Adds `Namespace` and `RegistryName` fields to `RegistryModuleID` to allow reading of Public Registry Modules by @Uk1288 [#464](https://github.com/hashicorp/go-tfe/pull/464) ## Bug fixes * Fixed JSON mapping for Configuration Versions failing to properly set the `speculative` property [#459](https://github.com/hashicorp/go-tfe/pull/459) From 7df1f3412f22bfbacab9242df5225a3c3e3802f0 Mon Sep 17 00:00:00 2001 From: uk1288 Date: Thu, 14 Jul 2022 09:08:32 -0400 Subject: [PATCH 3/4] update test --- registry_module_integration_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/registry_module_integration_test.go b/registry_module_integration_test.go index 30dbe0abb..68c5c5e37 100644 --- a/registry_module_integration_test.go +++ b/registry_module_integration_test.go @@ -853,7 +853,7 @@ func TestRegistryModule_Unmarshal(t *testing.T) { assert.Equal(t, rm.Name, "module") assert.Equal(t, rm.Provider, "tfe") assert.Equal(t, rm.Namespace, "org-abc") - assert.Equal(t, rm.RegistryName, "private") + assert.Equal(t, rm.RegistryName, PrivateRegistry) assert.Equal(t, rm.Permissions.CanDelete, true) assert.Equal(t, rm.Permissions.CanRetry, true) assert.Equal(t, rm.Status, RegistryModuleStatusPending) From 1138e10d22bf8ef835aba10e914058b83126353c Mon Sep 17 00:00:00 2001 From: uk1288 Date: Thu, 14 Jul 2022 09:30:18 -0400 Subject: [PATCH 4/4] update test --- registry_module.go | 20 +++++++++++--------- registry_module_integration_test.go | 2 ++ 2 files changed, 13 insertions(+), 9 deletions(-) diff --git a/registry_module.go b/registry_module.go index 93aab9ddb..08923cdf5 100644 --- a/registry_module.go +++ b/registry_module.go @@ -441,16 +441,18 @@ func (o RegistryModuleID) valid() error { if !validStringID(&o.Provider) { return ErrInvalidProvider } - // RegistryName is optional, only validate if specified - if validString((*string)(&o.RegistryName)) { - registryNamesMap := map[RegistryName]RegistryName{PublicRegistry: PublicRegistry, PrivateRegistry: PrivateRegistry} - if _, ok := registryNamesMap[o.RegistryName]; !ok { - return ErrInvalidRegistryName - } - } - if o.RegistryName == PublicRegistry && !validString(&o.Namespace) { - return ErrRequiredNamespace + switch o.RegistryName { + case PublicRegistry: + if !validString(&o.Namespace) { + return ErrRequiredNamespace + } + case PrivateRegistry: + case "": + // no-op: RegistryName is optional + // for all other string + default: + return ErrInvalidRegistryName } return nil diff --git a/registry_module_integration_test.go b/registry_module_integration_test.go index 68c5c5e37..f38922370 100644 --- a/registry_module_integration_test.go +++ b/registry_module_integration_test.go @@ -426,9 +426,11 @@ func TestRegistryModulesRead(t *testing.T) { RegistryName: PrivateRegistry, }) require.NoError(t, err) + require.NotEmpty(t, rm) assert.Equal(t, registryModuleTest.ID, rm.ID) t.Run("permissions are properly decoded", func(t *testing.T) { + require.NotEmpty(t, rm.Permissions) assert.True(t, rm.Permissions.CanDelete) assert.True(t, rm.Permissions.CanResync) assert.True(t, rm.Permissions.CanRetry)