add support for both public and private module creation without VCS #460

Uk1288 · 2022-07-11T15:28:30Z

Description

Currently, private registry modules are created with an option containing: Name and Provider.
This PR adds the ability to create both public/private registry modules without VCS with options containing: Name, Provider, RegistryName and Namespace.

The existing behaviour of creating a private module by specifying just the Name and Provider still works.

Testing plan

External links

Related PR This PR must be merged and released before the related PR is merged

Output from tests

Including output from tests may require access to a TFE instance. Ignore this section if you have no environment to test against.

$ TFE_ADDRESS="https://example" TFE_TOKEN="example" TF_ACC="1" go test ./... -v -tags=integration -run TestRegistryModulesCreate

=== RUN   TestRegistryModulesCreate
=== RUN   TestRegistryModulesCreate/with_valid_options
=== RUN   TestRegistryModulesCreate/with_valid_options/without_RegistryName
=== RUN   TestRegistryModulesCreate/with_valid_options/without_RegistryName/permissions_are_properly_decoded
=== RUN   TestRegistryModulesCreate/with_valid_options/without_RegistryName/relationships_are_properly_decoded
=== RUN   TestRegistryModulesCreate/with_valid_options/without_RegistryName/timestamps_are_properly_decoded
=== RUN   TestRegistryModulesCreate/with_valid_options/with_private_RegistryName
=== RUN   TestRegistryModulesCreate/with_valid_options/with_private_RegistryName/permissions_are_properly_decoded
=== RUN   TestRegistryModulesCreate/with_valid_options/with_private_RegistryName/relationships_are_properly_decoded
=== RUN   TestRegistryModulesCreate/with_valid_options/with_private_RegistryName/timestamps_are_properly_decoded
=== RUN   TestRegistryModulesCreate/with_valid_options/with_public_RegistryName
=== RUN   TestRegistryModulesCreate/with_valid_options/with_public_RegistryName/permissions_are_properly_decoded
=== RUN   TestRegistryModulesCreate/with_valid_options/with_public_RegistryName/relationships_are_properly_decoded
=== RUN   TestRegistryModulesCreate/with_valid_options/with_public_RegistryName/timestamps_are_properly_decoded
=== RUN   TestRegistryModulesCreate/with_invalid_options
=== RUN   TestRegistryModulesCreate/with_invalid_options/without_a_name
=== RUN   TestRegistryModulesCreate/with_invalid_options/with_an_invalid_name
=== RUN   TestRegistryModulesCreate/with_invalid_options/without_a_provider
=== RUN   TestRegistryModulesCreate/with_invalid_options/with_an_invalid_provider
=== RUN   TestRegistryModulesCreate/with_invalid_options/with_an_invalid_registry_name
=== RUN   TestRegistryModulesCreate/with_invalid_options/without_a_namespace_for_public_registry_name
=== RUN   TestRegistryModulesCreate/with_invalid_options/with_a_namespace_for_private_registry_name
=== RUN   TestRegistryModulesCreate/without_a_valid_organization
--- PASS: TestRegistryModulesCreate (1.21s)
    --- PASS: TestRegistryModulesCreate/with_valid_options (0.44s)
        --- PASS: TestRegistryModulesCreate/with_valid_options/without_RegistryName (0.13s)
            --- PASS: TestRegistryModulesCreate/with_valid_options/without_RegistryName/permissions_are_properly_decoded (0.00s)
            --- PASS: TestRegistryModulesCreate/with_valid_options/without_RegistryName/relationships_are_properly_decoded (0.00s)
            --- PASS: TestRegistryModulesCreate/with_valid_options/without_RegistryName/timestamps_are_properly_decoded (0.00s)
        --- PASS: TestRegistryModulesCreate/with_valid_options/with_private_RegistryName (0.13s)
            --- PASS: TestRegistryModulesCreate/with_valid_options/with_private_RegistryName/permissions_are_properly_decoded (0.00s)
            --- PASS: TestRegistryModulesCreate/with_valid_options/with_private_RegistryName/relationships_are_properly_decoded (0.00s)
            --- PASS: TestRegistryModulesCreate/with_valid_options/with_private_RegistryName/timestamps_are_properly_decoded (0.00s)
        --- PASS: TestRegistryModulesCreate/with_valid_options/with_public_RegistryName (0.18s)
            --- PASS: TestRegistryModulesCreate/with_valid_options/with_public_RegistryName/permissions_are_properly_decoded (0.00s)
            --- PASS: TestRegistryModulesCreate/with_valid_options/with_public_RegistryName/relationships_are_properly_decoded (0.00s)
            --- PASS: TestRegistryModulesCreate/with_valid_options/with_public_RegistryName/timestamps_are_properly_decoded (0.00s)
    --- PASS: TestRegistryModulesCreate/with_invalid_options (0.00s)
        --- PASS: TestRegistryModulesCreate/with_invalid_options/without_a_name (0.00s)
        --- PASS: TestRegistryModulesCreate/with_invalid_options/with_an_invalid_name (0.00s)
        --- PASS: TestRegistryModulesCreate/with_invalid_options/without_a_provider (0.00s)
        --- PASS: TestRegistryModulesCreate/with_invalid_options/with_an_invalid_provider (0.00s)
        --- PASS: TestRegistryModulesCreate/with_invalid_options/with_an_invalid_registry_name (0.00s)
        --- PASS: TestRegistryModulesCreate/with_invalid_options/without_a_namespace_for_public_registry_name (0.00s)
        --- PASS: TestRegistryModulesCreate/with_invalid_options/with_a_namespace_for_private_registry_name (0.00s)
    --- PASS: TestRegistryModulesCreate/without_a_valid_organization (0.00s)

sebasslash

Overall code looks 🔥 Some things down below ⬇️

sebasslash · 2022-07-11T20:51:59Z

registry_module_integration_test.go

-			Provider: String("provider"),
+		assertRegistryModuleAttributes := func(t *testing.T, registryModule *RegistryModule) {
+			t.Run("permissions are properly decoded", func(t *testing.T) {
+				assert.True(t, registryModule.Permissions.CanDelete)


You'll want to require Permissions to not be nil, otherwise this will panic if Permissions was not decoded properly.

I'm wondering if my changes in #458 will be overwritten somehow.

yes, I will fix the conflicts once you merge the changes.

sebasslash · 2022-07-11T20:54:02Z

registry_module_integration_test.go

+			})
+
+			t.Run("relationships are properly decoded", func(t *testing.T) {
+				assert.Equal(t, orgTest.Name, registryModule.Organization.Name)


Same here for Organization

sebasslash · 2022-07-13T21:45:19Z

registry_module.go

@@ -436,6 +443,22 @@ func (o RegistryModuleCreateOptions) valid() error {
 	if !validStringID(o.Provider) {
 		return ErrInvalidProvider
 	}
+
+	// RegistryName is optional, only validate if specified
+	if validString((*string)(&o.RegistryName)) {


[For discussion] We can potentially simplify this logic:

We don't need to check if RegistryName is a valid string as there are only two possible values we would care about:

switch(&o.RegistryName) { case PublicRegistry: if !validString(&o.Namespace) { // return error } case PrivateRegistry: if validString(&o.Namespace) { // return other error } // for all other strings or nil: default: return ErrInvalidRegistryName }

…RS file

brandonc

woop

CHANGELOG.md

github-actions · 2022-07-20T18:25:23Z

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.

Uk1288 mentioned this pull request Jul 11, 2022

add support for registry modules without vcs hashicorp/terraform-provider-tfe#546

Merged

sebasslash reviewed Jul 13, 2022

View reviewed changes

Uk1288 force-pushed the uk1288-support-public-modules branch from bf74088 to 59db300 Compare July 14, 2022 13:41

Uk1288 mentioned this pull request Jul 14, 2022

Replace deprecated GET api for registry modules #464

Merged

Uk1288 force-pushed the uk1288-support-public-modules branch from 2cfb379 to a8e68ec Compare July 15, 2022 13:18

Uk1288 requested a review from a team as a code owner July 18, 2022 17:32

Modify codeowners to have wildcard access, removed duplicate CODEOWNE…

e142f48

…RS file

Uk1288 force-pushed the uk1288-support-public-modules branch from 357e7af to 9b83a37 Compare July 18, 2022 17:36

brandonc previously approved these changes Jul 18, 2022

View reviewed changes

CHANGELOG.md Outdated Show resolved Hide resolved

Uk1288 added 2 commits July 19, 2022 10:37

add support for both public and private module creation without VCS

4350dd8

update changelog

599ef7c

Uk1288 dismissed brandonc’s stale review via 599ef7c July 19, 2022 14:37

Uk1288 force-pushed the uk1288-support-public-modules branch from 9b83a37 to 599ef7c Compare July 19, 2022 14:37

brandonc previously approved these changes Jul 19, 2022

View reviewed changes

Merge branch 'main' into uk1288-support-public-modules

ddfffc7

Uk1288 dismissed brandonc’s stale review via ddfffc7 July 19, 2022 16:06

brandonc approved these changes Jul 20, 2022

View reviewed changes

Uk1288 merged commit 392b639 into main Jul 20, 2022

Uk1288 deleted the uk1288-support-public-modules branch July 20, 2022 18:25

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

add support for both public and private module creation without VCS #460

add support for both public and private module creation without VCS #460

Uk1288 commented Jul 11, 2022 •

edited

sebasslash left a comment

sebasslash Jul 11, 2022

sebasslash Jul 13, 2022

Uk1288 Jul 14, 2022

sebasslash Jul 11, 2022

sebasslash Jul 13, 2022

brandonc left a comment

github-actions bot commented Jul 20, 2022

add support for both public and private module creation without VCS #460

add support for both public and private module creation without VCS #460

Conversation

Uk1288 commented Jul 11, 2022 • edited

Description

Testing plan

External links

Output from tests

sebasslash left a comment

Choose a reason for hiding this comment

sebasslash Jul 11, 2022

Choose a reason for hiding this comment

sebasslash Jul 13, 2022

Choose a reason for hiding this comment

Uk1288 Jul 14, 2022

Choose a reason for hiding this comment

sebasslash Jul 11, 2022

Choose a reason for hiding this comment

sebasslash Jul 13, 2022

Choose a reason for hiding this comment

brandonc left a comment

Choose a reason for hiding this comment

github-actions bot commented Jul 20, 2022

Uk1288 commented Jul 11, 2022 •

edited