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

Jump to bottom

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

Merged
merged 6 commits into from
Oct 22, 2024
Merged

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

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
3732a3b
Enable alternative module reading by its external ID
dsa0x Oct 17, 2024
ed36a7a
add test
dsa0x Oct 17, 2024
c3e48c4
update changelog, reverse conditional order
dsa0x Oct 17, 2024
06bf105
fix bug in bug fix
dsa0x Oct 17, 2024
414e429
remove external_id, spurious fix
dsa0x Oct 17, 2024
7649437
Merge remote-tracking branch 'origin/main' into sams/TFECO-6399-read-…
dsa0x Oct 17, 2024
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
Expand Up @@ -4,6 +4,8 @@

* Add support for enabling Stacks on an organization by @brandonc [#987](https://github.com/hashicorp/go-tfe/pull/987)
* Add support for filtering by key/value tags by @brandonc [#987](https://github.com/hashicorp/go-tfe/pull/987)
* 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

Expand Down
44 changes: 28 additions & 16 deletions registry_module.go
View file
Open in desktop
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 ID of the module. If given, the other fields are ignored.
ID string
// 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.ID == "" {
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.ID))
}

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.ID) && validStringID(&o.ID) {
return nil
}

if !validStringID(&o.Organization) {
return ErrInvalidOrg
}
Expand Down
21 changes: 21 additions & 0 deletions registry_module_integration_test.go
View file
Open in desktop
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 unique ID field for private module", func(t *testing.T) {
rm, err := client.RegistryModules.Read(ctx, RegistryModuleID{
ID: 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
Loading