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

prevent constant diff even after a successful apply of resource_tfe_registry_module resource #1357

Merged
merged 4 commits into from
May 29, 2024

Conversation

Uk1288
Copy link
Contributor

@Uk1288 Uk1288 commented May 13, 2024

Description

This PR prevents constant diff even after a successful apply of resource_tfe_registry_module resource. It also fixes the following cases:

  • when tags is set to true and branch is specified, the API ignores tags value and uses the specified branch. This leads to a constant diff attempt. Solution added is to match the API by verifying that either branch or tags is set but not both
  • previously, the tags value was never sent during the Create request, this PR ensures that the specified tags value is always sent in the request
  • test_config was only updated into state when the test config response is non-empty leading to never unsetting the value in state. Added fix to always update the test_config state to reflect api response.
  • adds some tests to catch previously failing cases
  • fix panic that occurs while logging errors during registry module create

Remember to:

Testing plan

  1. Some configs like below resulted in constant diff even after a successful apply
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-123"
    branch             = "main"
    tags             = false
    # branch             = ""
    # tags               = true
  }
}

resource "tfe_registry_module" "bare-module" {
  vcs_repo {
    oauth_token_id     = "ot-123"
    display_identifier = "Uk1288/terraform-google-cloud-dns"
    identifier         = "Uk1288/terraform-google-cloud-dns"
  }
}

resource "tfe_registry_module" "test-with-tags-and-branch-disabled" {
 vcs_repo {
    display_identifier = "Uk1288/terraform-aws-simple-s3-cdn"
    identifier         = "Uk1288/terraform-aws-simple-s3-cdn"
    oauth_token_id     = "ot-123"
    tags             = false
    branch         = ""
  }
}

  1. Applying tfe_registry_module config combo should no longer result in constant diff,
$ TESTARGS="-run TestAccTFERegistryModule" make testacc

=== RUN   TestAccTFERegistryModule_vcs
--- PASS: TestAccTFERegistryModule_vcs (7.69s)
=== RUN   TestAccTFERegistryModule_GitHubApp
--- PASS: TestAccTFERegistryModule_GitHubApp (4.32s)
=== RUN   TestAccTFERegistryModule_emptyVCSRepo
--- PASS: TestAccTFERegistryModule_emptyVCSRepo (0.19s)
=== RUN   TestAccTFERegistryModule_nonVCSPrivateRegistryModuleWithoutRegistryName
--- PASS: TestAccTFERegistryModule_nonVCSPrivateRegistryModuleWithoutRegistryName (4.29s)
=== RUN   TestAccTFERegistryModule_nonVCSPrivateRegistryModuleWithRegistryName
--- PASS: TestAccTFERegistryModule_nonVCSPrivateRegistryModuleWithRegistryName (4.11s)
=== RUN   TestAccTFERegistryModule_publicRegistryModule
--- PASS: TestAccTFERegistryModule_publicRegistryModule (4.14s)
=== RUN   TestAccTFERegistryModule_vcsRepoWithTagField
--- PASS: TestAccTFERegistryModule_vcsRepoWithTagField (6.30s)
=== RUN   TestAccTFERegistryModuleImport_vcsPrivateRMDeprecatedFormat
--- PASS: TestAccTFERegistryModuleImport_vcsPrivateRMDeprecatedFormat (7.75s)
=== RUN   TestAccTFERegistryModuleImport_vcsPrivateRMRecommendedFormat
--- PASS: TestAccTFERegistryModuleImport_vcsPrivateRMRecommendedFormat (6.87s)
=== RUN   TestAccTFERegistryModuleImport_vcsPublishingMechanismBranchToTagsToBranch
--- PASS: TestAccTFERegistryModuleImport_vcsPublishingMechanismBranchToTagsToBranch (16.40s)
=== RUN   TestAccTFERegistryModuleImport_vcsPublishingMechanismBranchToTagsToBranchWithTests
--- PASS: TestAccTFERegistryModuleImport_vcsPublishingMechanismBranchToTagsToBranchWithTests (11.21s)
=== RUN   TestAccTFERegistryModuleImport_vcsPublishingMechanismTagsToBranchToTags
--- PASS: TestAccTFERegistryModuleImport_vcsPublishingMechanismTagsToBranchToTags (13.98s)
=== RUN   TestAccTFERegistryModule_invalidTestConfigOnCreate
--- PASS: TestAccTFERegistryModule_invalidTestConfigOnCreate (3.28s)
=== RUN   TestAccTFERegistryModule_invalidTestConfigOnUpdate
--- PASS: TestAccTFERegistryModule_invalidTestConfigOnUpdate (7.02s)
=== RUN   TestAccTFERegistryModule_branchAndInvalidTagsOnCreate
--- PASS: TestAccTFERegistryModule_branchAndInvalidTagsOnCreate (2.94s)
=== RUN   TestAccTFERegistryModule_branchAndTagsEnabledOnCreate
--- PASS: TestAccTFERegistryModule_branchAndTagsEnabledOnCreate (2.76s)
=== RUN   TestAccTFERegistryModule_branchAndTagsDisabledOnCreate
--- PASS: TestAccTFERegistryModule_branchAndTagsDisabledOnCreate (2.81s)
=== RUN   TestAccTFERegistryModule_branchAndTagsEnabledOnUpdate
--- PASS: TestAccTFERegistryModule_branchAndTagsEnabledOnUpdate (7.02s)
=== RUN   TestAccTFERegistryModule_branchAndTagsDisabledOnUpdate
--- PASS: TestAccTFERegistryModule_branchAndTagsDisabledOnUpdate (7.31s)
=== RUN   TestAccTFERegistryModuleImport_nonVCSPrivateRM
--- PASS: TestAccTFERegistryModuleImport_nonVCSPrivateRM (4.23s)
=== RUN   TestAccTFERegistryModuleImport_publicRM
--- PASS: TestAccTFERegistryModuleImport_publicRM (4.33s)
=== RUN   TestAccTFERegistryModule_invalidWithBothVCSRepoAndModuleProvider
--- PASS: TestAccTFERegistryModule_invalidWithBothVCSRepoAndModuleProvider (0.15s)
=== RUN   TestAccTFERegistryModule_invalidRegistryName
--- PASS: TestAccTFERegistryModule_invalidRegistryName (0.32s)
=== RUN   TestAccTFERegistryModule_invalidWithModuleProviderAndNoName
--- PASS: TestAccTFERegistryModule_invalidWithModuleProviderAndNoName (0.13s)
=== RUN   TestAccTFERegistryModule_invalidWithModuleProviderAndNoOrganization
--- PASS: TestAccTFERegistryModule_invalidWithModuleProviderAndNoOrganization (0.13s)
=== RUN   TestAccTFERegistryModule_invalidWithNamespaceAndNoRegistryName
--- PASS: TestAccTFERegistryModule_invalidWithNamespaceAndNoRegistryName (0.12s)
=== RUN   TestAccTFERegistryModule_invalidWithRegistryNameAndNoModuleProvider
--- PASS: TestAccTFERegistryModule_invalidWithRegistryNameAndNoModuleProvider (0.12s)

unrelated failure for nocode

=== RUN TestAccTFERegistryModule_noCodeModule
resource_tfe_registry_module_test.go:343: Step 1/1 error: Error running apply: exit status 1

    Error: Error creating registry module vpc: resource not found
    
      with tfe_registry_module.foobar,
      on terraform_plugin_test.tf line 8, in resource "tfe_registry_module" "foobar":
       8: resource "tfe_registry_module" "foobar" {

--- FAIL: TestAccTFERegistryModule_noCodeModule (2.23s)

@Uk1288 Uk1288 requested a review from a team as a code owner May 13, 2024 17:37
@brandonc brandonc linked an issue May 13, 2024 that may be closed by this pull request
Copy link
Collaborator

@brandonc brandonc left a comment

Choose a reason for hiding this comment

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

It works! Under what conditions do our normal test cases catch this behavior?

@Uk1288 Uk1288 force-pushed the TF-14822-fix-constant-diff-on-tfe_registry_module branch from a88e7a3 to 0a88c2d Compare May 22, 2024 22:17
@Uk1288 Uk1288 force-pushed the TF-14822-fix-constant-diff-on-tfe_registry_module branch from 0a88c2d to 4ef15dd Compare May 22, 2024 22:39
@Uk1288 Uk1288 merged commit 933f36f into main May 29, 2024
9 checks passed
@Uk1288 Uk1288 deleted the TF-14822-fix-constant-diff-on-tfe_registry_module branch May 29, 2024 15:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Constant diff on tfe_registry_module with v0.51.1
3 participants