From 415c54b9ad46603850148d684b449c4bc22114bc Mon Sep 17 00:00:00 2001 From: Espen Albert Date: Mon, 27 May 2024 10:30:09 +0100 Subject: [PATCH] fix: Marks `project.region_usage_restrictions` as OptionalComputed to avoid provider upgrade errors (#2291) --- .changelog/2291.txt | 3 ++ .github/workflows/acceptance-tests-runner.yml | 4 ++ .github/workflows/acceptance-tests.yml | 1 + internal/service/project/resource_project.go | 3 +- .../resource_project_migration_test.go | 42 +++++++++++++++++++ .../service/project/resource_project_test.go | 26 ++++++++---- internal/testutil/acc/project.go | 13 ++++-- 7 files changed, 79 insertions(+), 13 deletions(-) create mode 100644 .changelog/2291.txt diff --git a/.changelog/2291.txt b/.changelog/2291.txt new file mode 100644 index 0000000000..523e13cb7e --- /dev/null +++ b/.changelog/2291.txt @@ -0,0 +1,3 @@ +```release-note:bug +resource/mongodbatlas_project: Fixes inconsistent result after apply when region_usage_restrictions are not set in configuration but returned from server +``` diff --git a/.github/workflows/acceptance-tests-runner.yml b/.github/workflows/acceptance-tests-runner.yml index 5e1330d9bb..409fc23f07 100644 --- a/.github/workflows/acceptance-tests-runner.yml +++ b/.github/workflows/acceptance-tests-runner.yml @@ -79,6 +79,9 @@ on: mongodb_atlas_gov_base_url: type: string required: true + mongodb_atlas_gov_project_owner_id: + type: string + required: true mongodb_atlas_federated_settings_associated_domain: type: string required: true @@ -679,6 +682,7 @@ jobs: - name: Acceptance Tests env: MONGODB_ATLAS_PROJECT_OWNER_ID: ${{ inputs.mongodb_atlas_project_owner_id }} + MONGODB_ATLAS_GOV_PROJECT_OWNER_ID: ${{ inputs.mongodb_atlas_gov_project_owner_id }} MONGODB_ATLAS_TEAMS_IDS: ${{ inputs.mongodb_atlas_teams_ids }} AWS_ACCOUNT_ID: ${{ secrets.aws_account_id }} AWS_ACCESS_KEY_ID: ${{ secrets.aws_access_key_id }} diff --git a/.github/workflows/acceptance-tests.yml b/.github/workflows/acceptance-tests.yml index d0b75c9746..0b6e7a703e 100644 --- a/.github/workflows/acceptance-tests.yml +++ b/.github/workflows/acceptance-tests.yml @@ -96,4 +96,5 @@ jobs: mongodb_atlas_federated_org_id: ${{ inputs.atlas_cloud_env == 'qa' && vars.MONGODB_ATLAS_FEDERATED_ORG_ID_QA || vars.MONGODB_ATLAS_FEDERATED_ORG_ID }} mongodb_atlas_gov_base_url: ${{ inputs.atlas_cloud_env == 'qa' && vars.MONGODB_ATLAS_GOV_BASE_URL_QA || vars.MONGODB_ATLAS_GOV_BASE_URL_DEV }} mongodb_atlas_gov_org_id: ${{ inputs.atlas_cloud_env == 'qa' && vars.MONGODB_ATLAS_GOV_ORG_ID_QA || vars.MONGODB_ATLAS_GOV_ORG_ID_DEV }} + mongodb_atlas_gov_project_owner_id: ${{ inputs.atlas_cloud_env == 'qa' && vars.MONGODB_ATLAS_GOV_PROJECT_OWNER_ID_QA || vars.MONGODB_ATLAS_GOV_PROJECT_OWNER_ID_DEV }} mongodb_atlas_federated_settings_associated_domain: ${{ vars.MONGODB_ATLAS_FEDERATED_SETTINGS_ASSOCIATED_DOMAIN }} diff --git a/internal/service/project/resource_project.go b/internal/service/project/resource_project.go index 69d5bd19f0..4a9f0790b1 100644 --- a/internal/service/project/resource_project.go +++ b/internal/service/project/resource_project.go @@ -216,6 +216,7 @@ func (r *projectRS) Schema(ctx context.Context, req resource.SchemaRequest, resp }, }, "region_usage_restrictions": schema.StringAttribute{ + Computed: true, Optional: true, }, "ip_addresses": schema.SingleNestedAttribute{ @@ -311,7 +312,7 @@ func (r *projectRS) Create(ctx context.Context, req resource.CreateRequest, resp OrgId: projectPlan.OrgID.ValueString(), Name: projectPlan.Name.ValueString(), WithDefaultAlertsSettings: projectPlan.WithDefaultAlertsSettings.ValueBoolPointer(), - RegionUsageRestrictions: projectPlan.RegionUsageRestrictions.ValueStringPointer(), + RegionUsageRestrictions: conversion.StringNullIfEmpty(projectPlan.RegionUsageRestrictions.ValueString()).ValueStringPointer(), Tags: &tags, } diff --git a/internal/service/project/resource_project_migration_test.go b/internal/service/project/resource_project_migration_test.go index 5c1f76f602..d55651ae7a 100644 --- a/internal/service/project/resource_project_migration_test.go +++ b/internal/service/project/resource_project_migration_test.go @@ -1,7 +1,9 @@ package project_test import ( + "fmt" "os" + "regexp" "strings" "testing" @@ -139,3 +141,43 @@ func TestMigProject_withLimits(t *testing.T) { }, }) } + +// based on bug report: https://github.com/mongodb/terraform-provider-mongodbatlas/issues/2263 +func TestMigGovProject_regionUsageRestrictionsDefault(t *testing.T) { + var ( + orgID = os.Getenv("MONGODB_ATLAS_GOV_ORG_ID") + projectName = acc.RandomProjectName() + ) + + resource.ParallelTest(t, resource.TestCase{ + PreCheck: func() { acc.PreCheckGovBasic(t) }, + CheckDestroy: acc.CheckDestroyProjectGov, + Steps: []resource.TestStep{ + { + ExternalProviders: acc.ExternalProviders("1.15.3"), + Config: configGovSimple(orgID, projectName), + Check: resource.ComposeTestCheckFunc( + checkExistsGov(resourceName), + ), + }, + { + ExternalProviders: acc.ExternalProviders("1.16.0"), + Config: configGovSimple(orgID, projectName), + Check: resource.ComposeTestCheckFunc( + checkExistsGov(resourceName), + ), + ExpectError: regexp.MustCompile("Provider produced inconsistent result after apply"), + }, + mig.TestStepCheckEmptyPlan(configGovSimple(orgID, projectName)), + }, + }) +} + +func configGovSimple(orgID, projectName string) string { + return acc.ConfigGovProvider() + fmt.Sprintf(` + resource "mongodbatlas_project" "test" { + org_id = %[1]q + name = %[2]q + } + `, orgID, projectName) +} diff --git a/internal/service/project/resource_project_test.go b/internal/service/project/resource_project_test.go index 26755a7d08..a82426f97c 100644 --- a/internal/service/project/resource_project_test.go +++ b/internal/service/project/resource_project_test.go @@ -611,32 +611,32 @@ func TestAccProject_basic(t *testing.T) { }) } -func TestAccProjectGov_withProjectOwner(t *testing.T) { - acc.SkipTestForCI(t) // Gov test config not set - +func TestAccGovProject_withProjectOwner(t *testing.T) { var ( - orgID = os.Getenv("MONGODB_ATLAS_ORG_ID_GOV") - projectOwnerID = os.Getenv("MONGODB_ATLAS_PROJECT_OWNER_ID_GOV") + orgID = os.Getenv("MONGODB_ATLAS_GOV_ORG_ID") + projectOwnerID = os.Getenv("MONGODB_ATLAS_GOV_PROJECT_OWNER_ID") projectName = acc.RandomProjectName() ) resource.ParallelTest(t, resource.TestCase{ PreCheck: func() { acc.PreCheckGovBasic(t) }, ProtoV6ProviderFactories: acc.TestAccProviderV6Factories, - CheckDestroy: acc.CheckDestroyProject, + CheckDestroy: acc.CheckDestroyProjectGov, Steps: []resource.TestStep{ { Config: configGovWithOwner(orgID, projectName, projectOwnerID), Check: resource.ComposeTestCheckFunc( - checkExists(resourceName), + checkExistsGov(resourceName), resource.TestCheckResourceAttr(resourceName, "name", projectName), resource.TestCheckResourceAttr(resourceName, "org_id", orgID), + resource.TestCheckResourceAttr(resourceName, "project_owner_id", projectOwnerID), resource.TestCheckResourceAttr(resourceName, "region_usage_restrictions", "GOV_REGIONS_ONLY"), ), }, }, }) } + func TestAccProject_withFalseDefaultSettings(t *testing.T) { var ( orgID = os.Getenv("MONGODB_ATLAS_ORG_ID") @@ -1076,6 +1076,14 @@ func tagChecks(tags map[string]string, notFoundKeys ...string) resource.TestChec } func checkExists(resourceName string) resource.TestCheckFunc { + return checkExistsWithConn(resourceName, acc.ConnV2()) +} + +func checkExistsGov(resourceName string) resource.TestCheckFunc { + return checkExistsWithConn(resourceName, acc.ConnV2UsingGov()) +} + +func checkExistsWithConn(resourceName string, conn *admin.APIClient) resource.TestCheckFunc { return func(s *terraform.State) error { rs, ok := s.RootModule().Resources[resourceName] if !ok { @@ -1084,7 +1092,7 @@ func checkExists(resourceName string) resource.TestCheckFunc { if rs.Primary.ID == "" { return fmt.Errorf("no ID is set") } - if _, _, err := acc.ConnV2().ProjectsApi.GetProjectByName(context.Background(), rs.Primary.Attributes["name"]).Execute(); err == nil { + if _, _, err := conn.ProjectsApi.GetProjectByName(context.Background(), rs.Primary.Attributes["name"]).Execute(); err == nil { return nil } return fmt.Errorf("project (%s) does not exist", rs.Primary.ID) @@ -1135,7 +1143,7 @@ func configBasic(orgID, projectName, projectOwnerID string, includeDataSource bo } func configGovWithOwner(orgID, projectName, projectOwnerID string) string { - return fmt.Sprintf(` + return acc.ConfigGovProvider() + fmt.Sprintf(` resource "mongodbatlas_project" "test" { org_id = %[1]q name = %[2]q diff --git a/internal/testutil/acc/project.go b/internal/testutil/acc/project.go index bb18de3fe8..1d1250f7e6 100644 --- a/internal/testutil/acc/project.go +++ b/internal/testutil/acc/project.go @@ -6,20 +6,27 @@ import ( "github.com/hashicorp/terraform-plugin-testing/helper/resource" "github.com/hashicorp/terraform-plugin-testing/terraform" + "go.mongodb.org/atlas-sdk/v20231115013/admin" ) func CheckDestroyProject(s *terraform.State) error { + return checkDestroyProject(ConnV2(), s) +} + +func CheckDestroyProjectGov(s *terraform.State) error { + return checkDestroyProject(ConnV2UsingGov(), s) +} + +func checkDestroyProject(conn *admin.APIClient, s *terraform.State) error { for _, rs := range s.RootModule().Resources { if rs.Type != "mongodbatlas_project" { continue } - - projectRes, _, _ := Conn().Projects.GetOneProjectByName(context.Background(), rs.Primary.ID) + projectRes, _, _ := conn.ProjectsApi.GetProjectByName(context.Background(), rs.Primary.ID).Execute() if projectRes != nil { return fmt.Errorf("project (%s) still exists", rs.Primary.ID) } } - return nil }