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.

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

Enable alternative reading of a registry module by its ID #988

Merged
merged 6 commits into from
Oct 22, 2024
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
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
5 changes: 5 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,10 @@
# Unreleased

## Enhancements

* Add support for reading a registry module by its unique identifier by @dsa0x
[#988](https://github.com/hashicorp/go-tfe/pull/988)

# v1.68.0

## Enhancements
Expand Down
44 changes: 28 additions & 16 deletions registry_module.go
Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,10 @@ const (

// RegistryModuleID represents the set of IDs that identify a RegistryModule
// Use NewPublicRegistryModuleID or NewPrivateRegistryModuleID to build one

type RegistryModuleID struct {
// The unique external ID of the organization. If given, the other fields are ignored.
ExternalID string
dsa0x marked this conversation as resolved.
Show resolved Hide resolved
// The organization the module belongs to, see RegistryModule.Organization.Name
Organization string
// The name of the module, see RegistryModule.Name
Expand Down Expand Up @@ -523,24 +526,29 @@ 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
}
var u string
if moduleID.ExternalID == "" {
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.")
Copy link
Collaborator

Choose a reason for hiding this comment

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

I know you didn't add this, but registry_module.go seems to be the only file in go-tfe which is using the log package, and, as a library, probably shouldn't be logging to stderr. It would be nice if you could go ahead and remove all these log.Print calls from this file, but I'll take care of it if you don't feel like waiting for another review!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thank you. I will go ahead and merge this. Is there a convention employed in go-tfe for including deprecated messages?. Perhaps we could just remove them entirely, and have the comments indicate the deprecated attributes.

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
}
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(
"organizations/%s/registry-modules/%s/%s/%s/%s",
url.PathEscape(moduleID.Organization),
url.PathEscape(string(moduleID.RegistryName)),
url.PathEscape(moduleID.Namespace),
url.PathEscape(moduleID.Name),
url.PathEscape(moduleID.Provider),
)
u = fmt.Sprintf(
"organizations/%s/registry-modules/%s/%s/%s/%s",
url.PathEscape(moduleID.Organization),
url.PathEscape(string(moduleID.RegistryName)),
url.PathEscape(moduleID.Namespace),
url.PathEscape(moduleID.Name),
url.PathEscape(moduleID.Provider),
)
} else {
u = fmt.Sprintf("registry-modules/%s", url.PathEscape(moduleID.ExternalID))
}

req, err := r.client.NewRequest("GET", u, nil)
if err != nil {
Expand Down Expand Up @@ -690,6 +698,10 @@ func (r *registryModules) DeleteVersion(ctx context.Context, moduleID RegistryMo
}

func (o RegistryModuleID) valid() error {
if validString(&o.ExternalID) && validStringID(&o.ExternalID) {
return nil
}

if !validStringID(&o.Organization) {
return ErrInvalidOrg
}
Expand Down
21 changes: 21 additions & 0 deletions registry_module_integration_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1193,6 +1193,27 @@ func TestRegistryModulesRead(t *testing.T) {
})
})

t.Run("with a external ID field for private module", func(t *testing.T) {
rm, err := client.RegistryModules.Read(ctx, RegistryModuleID{
ExternalID: registryModuleTest.ID,
})
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)
})

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,
Expand Down
7 changes: 6 additions & 1 deletion tfe.go
Original file line number Diff line number Diff line change
Expand Up @@ -319,7 +319,12 @@ func (c *Client) NewRequestWithAdditionalQueryParams(method, path string, reqBod
q[k] = v
}

u.RawQuery = encodeQueryParams(q)
encodedParams := encodeQueryParams(q)
if u.RawQuery == "" {
u.RawQuery = encodedParams
} else if encodedParams != "" {
u.RawQuery = strings.Join([]string{u.RawQuery, encodedParams}, "&")
}
dsa0x marked this conversation as resolved.
Show resolved Hide resolved

req, err := retryablehttp.NewRequest(method, u.String(), body)
if err != nil {
Expand Down
Loading