[TF-9605] Add Private Registry Module Testing #1096

hashimoon · 2023-10-09T22:56:44Z

Description

Add the following attributes to RegistryModule to support testing

publishing_mechanism
vcs_repo.branch
vcs_repo.tags
test_config.tests_enabled

Remember to:

Update the Change Log
Update the Documentation

Testing plan

Create a RegistryModule
Confirm the publishing_mechanism is git_tag
Set vcs_repo.branch to a branch on the git repository
Set vcs_repo.tags to false
Set test_config.tests_enabled to true
Perform changes
Confirm the publishing_mechanism is branch
Enjoy your registry module with tests enabled

External links

Include any links here that might be helpful for people reviewing your PR. If there are none, feel free to delete this section.

Output from acceptance tests

Please run applicable acceptance tests locally and include the output here. See testing.md to learn how to run acceptance tests.

If you are an external contributor, your contribution(s) will first be reviewed before running them against the project's CI pipeline.

$ TESTARGS="-run TestAccTFERegistryModuleImport" make testacc

?   	github.com/hashicorp/terraform-provider-tfe	[no test files]
testing: warning: no tests to run
PASS
ok  	github.com/hashicorp/terraform-provider-tfe/internal/client	0.137s [no tests to run]
testing: warning: no tests to run
PASS
ok  	github.com/hashicorp/terraform-provider-tfe/internal/logging	0.181s [no tests to run]
?   	github.com/hashicorp/terraform-provider-tfe/version	[no test files]
=== RUN   TestAccTFERegistryModule_vcs
--- PASS: TestAccTFERegistryModule_vcs (7.89s)
=== RUN   TestAccTFERegistryModule_GitHubApp
    provider_test.go:230: Please set GITHUB_APP_INSTALLATION_ID to run this test
--- SKIP: TestAccTFERegistryModule_GitHubApp (0.00s)
=== RUN   TestAccTFERegistryModule_emptyVCSRepo
--- PASS: TestAccTFERegistryModule_emptyVCSRepo (0.13s)
=== RUN   TestAccTFERegistryModule_nonVCSPrivateRegistryModuleWithoutRegistryName
--- PASS: TestAccTFERegistryModule_nonVCSPrivateRegistryModuleWithoutRegistryName (4.48s)
=== RUN   TestAccTFERegistryModule_nonVCSPrivateRegistryModuleWithRegistryName
--- PASS: TestAccTFERegistryModule_nonVCSPrivateRegistryModuleWithRegistryName (4.35s)
=== RUN   TestAccTFERegistryModule_publicRegistryModule
--- PASS: TestAccTFERegistryModule_publicRegistryModule (4.92s)
=== RUN   TestAccTFERegistryModule_noCodeModule
--- PASS: TestAccTFERegistryModule_noCodeModule (4.95s)
=== RUN   TestAccTFERegistryModuleImport_vcsPrivateRMDeprecatedFormat
--- PASS: TestAccTFERegistryModuleImport_vcsPrivateRMDeprecatedFormat (8.42s)
=== RUN   TestAccTFERegistryModuleImport_vcsPrivateRMRecommendedFormat
--- PASS: TestAccTFERegistryModuleImport_vcsPrivateRMRecommendedFormat (8.14s)
=== RUN   TestAccTFERegistryModuleImport_vcsPublishingMechanismBranchToTagsToBranch
--- PASS: TestAccTFERegistryModuleImport_vcsPublishingMechanismBranchToTagsToBranch (19.21s)
=== RUN   TestAccTFERegistryModuleImport_vcsPublishingMechanismBranchToTagsToBranchWithTests
--- PASS: TestAccTFERegistryModuleImport_vcsPublishingMechanismBranchToTagsToBranchWithTests (9.25s)
=== RUN   TestAccTFERegistryModuleImport_vcsPublishingMechanismTagsToBranchToTags
--- PASS: TestAccTFERegistryModuleImport_vcsPublishingMechanismTagsToBranchToTags (14.34s)
=== RUN   TestAccTFERegistryModuleImport_nonVCSPrivateRM
--- PASS: TestAccTFERegistryModuleImport_nonVCSPrivateRM (4.95s)
=== RUN   TestAccTFERegistryModuleImport_publicRM
--- PASS: TestAccTFERegistryModuleImport_publicRM (4.49s)
=== RUN   TestAccTFERegistryModule_invalidWithBothVCSRepoAndModuleProvider
--- PASS: TestAccTFERegistryModule_invalidWithBothVCSRepoAndModuleProvider (0.14s)
=== RUN   TestAccTFERegistryModule_invalidRegistryName
--- PASS: TestAccTFERegistryModule_invalidRegistryName (0.27s)
=== RUN   TestAccTFERegistryModule_invalidWithModuleProviderAndNoName
--- PASS: TestAccTFERegistryModule_invalidWithModuleProviderAndNoName (0.11s)
=== RUN   TestAccTFERegistryModule_invalidWithModuleProviderAndNoOrganization
--- PASS: TestAccTFERegistryModule_invalidWithModuleProviderAndNoOrganization (0.11s)
=== RUN   TestAccTFERegistryModule_invalidWithNamespaceAndNoRegistryName
--- PASS: TestAccTFERegistryModule_invalidWithNamespaceAndNoRegistryName (0.11s)
=== RUN   TestAccTFERegistryModule_invalidWithRegistryNameAndNoModuleProvider
--- PASS: TestAccTFERegistryModule_invalidWithRegistryNameAndNoModuleProvider (0.11s)
PASS
ok  	github.com/hashicorp/terraform-provider-tfe/internal/provider	96.758s

...

internal/provider/resource_tfe_registry_module.go

internal/provider/resource_tfe_registry_module_test.go

brandonc

No smoke test yet. Just some initial quick observations and questions so far.

brandonc · 2023-10-13T22:04:28Z

CHANGELOG.md

+## UNRELEASED
+
+Features
+* `d/tfe_registry_module`: Add `test_config` and `vcs_repo.tags` attributes, by @hashimoon [1096](https://github.com/hashicorp/terraform-provider-tfe/pull/1096)


Also, publishing_mechanism and vcs_repo.branch -- It might be worth quickly explaining that these are related in the sense that they allow module developers more release flexibility but I don't have the words

internal/provider/resource_tfe_registry_module.go

Uk1288

Smoke test feedback:

creating RM with tags set to true creates RM with tag => false, config used:

resource "tfe_registry_module" "test-with-tags-only-true" {
vcs_repo {
 display_identifier = ""
   identifier         = ""
   oauth_token_id     = "id"
  tags             = true
}
}

Should be able to create tags with true value

creating RM with test_config.tests_enabled set to true creates RM with tests_enabled => false, config used:

resource "tfe_registry_module" "test-with-test-config-set-to-true" {
test_config {
  tests_enabled = true
}
vcs_repo {
 display_identifier = ""
   identifier         = ""
   oauth_token_id     = "id"
  tags             = true
}
}

Should be able to create tests_enabled with true value

creating a RM without adding the test_config block is successful. Attempting a new apply without any config changes, tries to unsets the test_config block:

 # tfe_registry_module.test-with-branch-only will be updated in-place
 ~ resource "tfe_registry_module" "test-with-branch-only" {
       id                   = "mod-1"
       name                 = "tfe-module"
       # (6 unchanged attributes hidden)

     - test_config {
         - tests_enabled = false -> null
       }

       # (1 unchanged block hidden)
   }

internal/provider/resource_tfe_registry_module.go

hashimoon · 2023-10-24T00:16:59Z

@Uk1288

creating RM with tags set to true creates RM with tag => false, config used:

Do you have any more context on this? Can you see share what value is return for the publishingMechanism ? Creating a VCS-based Registry Module defaults to tag based publishing currently. With this PR, we are returning the tags attribute back to the client.

Should be able to create tags with true value

- creating RM with test_config.tests_enabled set to true creates RM with tests_enabled => false, config used:

This is expected behavior. Tests are only supported for modules with branch-based publishing.

Should be able to create tests_enabled with true value

- creating a RM without adding the test_config block is successful. Attempting a new apply without any config changes, tries to unsets the test_config block:

This is expected behavior. Tests are only supported for Registry Modules with branch-based publishing at the moment.

website/docs/r/registry_module.html.markdown

Uk1288

Good progress, see our slack chat for other comments with usecase examples.

Uk1288 · 2023-10-31T05:35:42Z

website/docs/r/registry_module.html.markdown

@@ -125,6 +155,9 @@ The following arguments are supported:
 * `namespace` - (Optional) The namespace of a public registry module. It can be used if `module_provider` is set and `registry_name` is public.
 * `registry_name` - (Optional) Whether the registry module is private or public. It can be used if `module_provider` is set.

+The `test_config` block supports
+* `tests_enabled` - (Required) Specifies whether tests run for the registry module. Tests are only supported for branch-based publishing.


the implementation sets both test_config and tests_enabled as optional attributes, but here it is state as required...update to reflect same requirement.

website/docs/r/registry_module.html.markdown

Uk1288

Smoke testing feedback:
Most of the config is working as expected 🎉

Found 2 config with issues below
1.
Actual: successful apply..s of the config below always ends up in a state with changes to update. A second apply seems to get stuck in an infinite state trying to modify. I ctrl c to stop the process.
Expected: after an apply the resource state should reflect created resource

Config:

resource "tfe_registry_module" "test-with-branch-tags-and-test-config" {
 test_config {
   tests_enabled = true
 }
 vcs_repo {
    display_identifier = "Uk1288/terraform-google-cloud-dns"
    identifier         = "Uk1288/terraform-google-cloud-dns"
    oauth_token_id     = "ot-ID..."
    branch             = "main"
    tags             = true
 }
}

Results in changes to update:

      ~ vcs_repo {
          ~ tags               = false -> true
            # (4 unchanged attributes hidden)
        }

Actual: provider crashes.
Expected: should successfully apply or return an error

Config:

resource "tfe_registry_module" "test-with-branch-tags-and-test-config" {
 test_config {
 }
 vcs_repo {
    display_identifier = "Uk1288/terraform-google-cloud-dns"
    identifier         = "Uk1288/terraform-google-cloud-dns"
    oauth_token_id     = "ot-ID..."
    branch             = "main"
 }
}

CHANGELOG.md

internal/provider/resource_tfe_registry_module_test.go

Uk1288

Gave this another go, working as expected. Approving because functionality is working as expected.

🍹 One of my configs seems to be reset even after a successful apply. Expected behaviour is for state changes to reflect the resources after a successful apply.
Config:

resource "tfe_registry_module" "test-without-test-config-block" {
 vcs_repo {
    display_identifier = "Uk1288/terraform-aws-simple-s3-cdn"
    identifier         = "Uk1288/terraform-aws-simple-s3-cdn"
    oauth_token_id     = "ot-id..."
   branch             = "main"
   tags             = false
 }
}

After a successful apply still results in

# tfe_registry_module.test-without-test-config-block will be updated in-place
  ~ resource "tfe_registry_module" "test-without-test-config-block" {
        id                   = "mod-..."
        name                 = "simple-s3-cdn"
        # (6 unchanged attributes hidden)

      - test_config {
          - tests_enabled = false -> null
        }

        # (1 unchanged block hidden)
    }

Plan: 0 to add, 1 to change, 0 to destroy.

Expected behaviour to result in no changes to apply.

hashimoon changed the title ~~WIP~~ Add Private Registry Module Testing Oct 9, 2023

hashimoon force-pushed the hashimoon/pmr-testing branch 6 times, most recently from e31531e to 8ab253d Compare October 13, 2023 15:27

hashimoon changed the title ~~Add Private Registry Module Testing~~ [TF-9605] Add Private Registry Module Testing Oct 13, 2023

hashimoon marked this pull request as ready for review October 13, 2023 15:52

hashimoon requested a review from a team as a code owner October 13, 2023 15:52

hashimoon requested a review from aaabdelgany October 13, 2023 15:54

hashimoon force-pushed the hashimoon/pmr-testing branch 6 times, most recently from 8a3824c to d9da171 Compare October 13, 2023 17:44

Uk1288 reviewed Oct 13, 2023

View reviewed changes

brandonc reviewed Oct 13, 2023

View reviewed changes

Uk1288 requested changes Oct 13, 2023

View reviewed changes

internal/provider/resource_tfe_registry_module.go Show resolved Hide resolved

hashimoon force-pushed the hashimoon/pmr-testing branch 2 times, most recently from 13f2812 to 8c76c27 Compare October 23, 2023 23:56

hashimoon force-pushed the hashimoon/pmr-testing branch 2 times, most recently from e7f9532 to 00b2886 Compare October 24, 2023 00:17

hashimoon requested review from brandonc and Uk1288 October 24, 2023 00:17

hashimoon force-pushed the hashimoon/pmr-testing branch from 00b2886 to 9f6fc4a Compare October 24, 2023 00:20

aaabdelgany reviewed Oct 25, 2023

View reviewed changes

website/docs/r/registry_module.html.markdown Show resolved Hide resolved

hashimoon force-pushed the hashimoon/pmr-testing branch from 9f6fc4a to 0005396 Compare October 27, 2023 02:34

hashimoon force-pushed the hashimoon/pmr-testing branch from 0005396 to 8417e8f Compare October 27, 2023 02:34

hashimoon requested a review from aaabdelgany October 27, 2023 16:41

Uk1288 reviewed Oct 31, 2023

View reviewed changes

hashimoon requested a review from Uk1288 November 8, 2023 03:06

hashimoon force-pushed the hashimoon/pmr-testing branch 2 times, most recently from 1ce75a4 to 1585890 Compare November 8, 2023 18:23

Uk1288 reviewed Nov 10, 2023

View reviewed changes

CHANGELOG.md Outdated Show resolved Hide resolved

internal/provider/resource_tfe_registry_module_test.go Show resolved Hide resolved

hashimoon force-pushed the hashimoon/pmr-testing branch from bef01d0 to 7a1e4d5 Compare November 15, 2023 01:29

hashimoon requested a review from Uk1288 November 15, 2023 01:29

[TF-9605] Add Private Registry Module Testing

73f87a6

hashimoon force-pushed the hashimoon/pmr-testing branch from 7a1e4d5 to 73f87a6 Compare November 15, 2023 19:09

Uk1288 approved these changes Nov 16, 2023

View reviewed changes

hashimoon merged commit ce83189 into main Nov 16, 2023
9 checks passed

hashimoon deleted the hashimoon/pmr-testing branch November 16, 2023 19:09

jamesmbourne mentioned this pull request Jan 16, 2024

Constant diff on tfe_registry_module with v0.51.1 #1216

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[TF-9605] Add Private Registry Module Testing #1096

[TF-9605] Add Private Registry Module Testing #1096

hashimoon commented Oct 9, 2023 •

edited

Loading

brandonc left a comment

brandonc Oct 13, 2023

Uk1288 left a comment

hashimoon commented Oct 24, 2023

Uk1288 left a comment

Uk1288 Oct 31, 2023

hashimoon Nov 8, 2023

Uk1288 left a comment

Uk1288 left a comment

[TF-9605] Add Private Registry Module Testing #1096

[TF-9605] Add Private Registry Module Testing #1096

Conversation

hashimoon commented Oct 9, 2023 • edited Loading

Description

Testing plan

External links

Output from acceptance tests

brandonc left a comment

Choose a reason for hiding this comment

brandonc Oct 13, 2023

Choose a reason for hiding this comment

Uk1288 left a comment

Choose a reason for hiding this comment

hashimoon commented Oct 24, 2023

Uk1288 left a comment

Choose a reason for hiding this comment

Uk1288 Oct 31, 2023

Choose a reason for hiding this comment

hashimoon Nov 8, 2023

Choose a reason for hiding this comment

Uk1288 left a comment

Choose a reason for hiding this comment

Uk1288 left a comment

Choose a reason for hiding this comment

hashimoon commented Oct 9, 2023 •

edited

Loading