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

fix: Marks project.region_usage_restrictions as OptionalComputed to avoid provider upgrade errors #2291

Merged
Merged
Show file tree
Hide file tree
Changes from 12 commits
Commits
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
3 changes: 3 additions & 0 deletions .changelog/2291.txt
Original file line number Diff line number Diff line change
@@ -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
```
18 changes: 18 additions & 0 deletions .github/workflows/acceptance-tests-runner.yml
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,15 @@ on:
mongodb_atlas_federated_org_id:
type: string
required: true
mongodb_atlas_gov_org_id:
type: string
required: true
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
Expand Down Expand Up @@ -107,6 +116,10 @@ on:
required: true
mongodb_atlas_private_endpoint_dns_name:
required: true
mongodb_atlas_gov_private_key:
required: true
mongodb_atlas_gov_public_key:
required: true
azure_directory_id:
required: true
azure_resource_group_name:
Expand All @@ -128,6 +141,11 @@ env:
MONGODB_ATLAS_ORG_ID: ${{ inputs.mongodb_atlas_org_id }}
MONGODB_ATLAS_PUBLIC_KEY: ${{ secrets.mongodb_atlas_public_key }}
MONGODB_ATLAS_PRIVATE_KEY: ${{ secrets.mongodb_atlas_private_key }}
MONGODB_ATLAS_GOV_PUBLIC_KEY: ${{ secrets.mongodb_atlas_gov_public_key }}
MONGODB_ATLAS_GOV_PRIVATE_KEY: ${{ secrets.mongodb_atlas_gov_private_key }}
MONGODB_ATLAS_GOV_BASE_URL: ${{ inputs.mongodb_atlas_gov_base_url }}
MONGODB_ATLAS_GOV_ORG_ID: ${{ inputs.mongodb_atlas_gov_org_id }}


jobs:

Expand Down
5 changes: 5 additions & 0 deletions .github/workflows/acceptance-tests.yml
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,8 @@ jobs:
secrets:
mongodb_atlas_public_key: ${{ inputs.atlas_cloud_env == 'qa' && secrets.MONGODB_ATLAS_PUBLIC_KEY_CLOUD_QA || secrets.MONGODB_ATLAS_PUBLIC_KEY_CLOUD_DEV }}
mongodb_atlas_private_key: ${{ inputs.atlas_cloud_env == 'qa' && secrets.MONGODB_ATLAS_PRIVATE_KEY_CLOUD_QA || secrets.MONGODB_ATLAS_PRIVATE_KEY_CLOUD_DEV }}
mongodb_atlas_gov_public_key: ${{ inputs.atlas_cloud_env == 'qa' && secrets.MONGODB_ATLAS_GOV_PUBLIC_KEY_QA || secrets.MONGODB_ATLAS_GOV_PUBLIC_KEY_DEV }}
mongodb_atlas_gov_private_key: ${{ inputs.atlas_cloud_env == 'qa' && secrets.MONGODB_ATLAS_GOV_PRIVATE_KEY_QA || secrets.MONGODB_ATLAS_GOV_PRIVATE_KEY_DEV }}
ca_cert: ${{ secrets.CA_CERT }}
aws_account_id: ${{ secrets.AWS_ACCOUNT_ID }}
aws_access_key_id: ${{ secrets.AWS_ACCESS_KEY_ID }}
Expand Down Expand Up @@ -92,4 +94,7 @@ jobs:
mongodb_atlas_federated_sso_url: ${{ vars.MONGODB_ATLAS_FEDERATED_SSO_URL }}
mongodb_atlas_federated_issuer_uri: ${{ vars.MONGODB_ATLAS_FEDERATED_ISSUER_URI }}
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 }}
9 changes: 9 additions & 0 deletions contributing/testing-best-practices.md
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,15 @@
- Data sources are tested in the same tests as the resources, e.g. [commonChecks](https://github.com/mongodb/terraform-provider-mongodbatlas/blob/66c44e62c9afe04ffe8be0dbccaec682bab830e6/internal/service/searchindex/resource_search_index_test.go#L262-L263).
- Helper functions such as `resource.TestCheckTypeSetElemNestedAttrs` or `resource.TestCheckTypeSetElemAttr` can be used to check resource and data source attributes more easily, e.g. [resource_serverless_instance_test.go](https://github.com/mongodb/terraform-provider-mongodbatlas/blob/66c44e62c9afe04ffe8be0dbccaec682bab830e6/internal/service/serverlessinstance/resource_serverless_instance_test.go#L61).

### Cloud Gov tests

1. Use [`PreCheck: PreCheckGovBasic`](https://github.com/mongodb/terraform-provider-mongodbatlas/blob/CLOUDP-250271_cloud_gov/internal/testutil/acc/pre_check.go#L98)
2. Use the [`acc.ConfigGovProvider`](https://github.com/mongodb/terraform-provider-mongodbatlas/blob/CLOUDP-250271_cloud_gov/internal/testutil/acc/provider.go#L61) together with your normal terraform config
3. Modify the `checkExist` and `CheckDestroy` to use `acc.ConnV2UsingGov`
4. Follow naming convention:
1. `TestAccGovProject_withProjectOwner`, note prefix: `TestAccGov`
2. `TestMigGovProject_regionUsageRestrictionsDefault`, note prefix: `TestMigGov`

## Migration tests

- There must be at least one `basic migration test` for each resource that leverages on the `basic acceptance tests` using helper test functions such as `CreateAndRunTest`, e.g. [TestMigServerlessInstance_basic](https://github.com/mongodb/terraform-provider-mongodbatlas/blob/66c44e62c9afe04ffe8be0dbccaec682bab830e6/internal/service/serverlessinstance/resource_serverless_instance_migration_test.go#L10).
Expand Down
3 changes: 2 additions & 1 deletion internal/service/project/resource_project.go
Original file line number Diff line number Diff line change
Expand Up @@ -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{
Expand Down Expand Up @@ -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,
}

Expand Down
42 changes: 42 additions & 0 deletions internal/service/project/resource_project_migration_test.go
Original file line number Diff line number Diff line change
@@ -1,7 +1,9 @@
package project_test

import (
"fmt"
"os"
"regexp"
"strings"
"testing"

Expand Down Expand Up @@ -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(
Copy link
Collaborator

Choose a reason for hiding this comment

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

nit: based on what we were discussing last Thurs, have you done a quick manual check to verify the test is actually failing if you write a wrong check assertion? It would help us make sure the test is actually set-up correctly.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yes, I did it :D Next time, I'll try to check in the test before the fix so it can be seen in the CI too (test added push and then fix push) 🚀

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)
}
26 changes: 17 additions & 9 deletions internal/service/project/resource_project_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Expand Down Expand Up @@ -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 {
Expand All @@ -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)
Expand Down Expand Up @@ -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
Expand Down
11 changes: 11 additions & 0 deletions internal/testutil/acc/factory.go
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,17 @@ func ConnV2UsingProxy(proxyPort *int) *admin.APIClient {
return client.(*config.MongoDBClient).AtlasV2
}

func ConnV2UsingGov() *admin.APIClient {
cfg := config.Config{
PublicKey: os.Getenv("MONGODB_ATLAS_GOV_PUBLIC_KEY"),
PrivateKey: os.Getenv("MONGODB_ATLAS_GOV_PRIVATE_KEY"),
BaseURL: os.Getenv("MONGODB_ATLAS_GOV_BASE_URL"),
RealmBaseURL: os.Getenv("MONGODB_ATLAS_GOV_BASE_URL"),
}
client, _ := cfg.NewClient(context.Background())
return client.(*config.MongoDBClient).AtlasV2
}

func init() {
TestAccProviderV6Factories = map[string]func() (tfprotov6.ProviderServer, error){
ProviderNameMongoDBAtlas: func() (tfprotov6.ProviderServer, error) {
Expand Down
17 changes: 5 additions & 12 deletions internal/testutil/acc/pre_check.go
Original file line number Diff line number Diff line change
Expand Up @@ -97,19 +97,12 @@ func GetProjectTeamsIDsWithPos(pos int) string {

func PreCheckGovBasic(tb testing.TB) {
tb.Helper()
if os.Getenv("MONGODB_ATLAS_PUBLIC_KEY") == "" ||
os.Getenv("MONGODB_ATLAS_PRIVATE_KEY") == "" ||
os.Getenv("MONGODB_ATLAS_ORG_ID_GOV") == "" {
tb.Fatal("`MONGODB_ATLAS_PUBLIC_KEY`, `MONGODB_ATLAS_PRIVATE_KEY`and `MONGODB_ATLAS_ORG_ID_GOV` must be set for acceptance testing")
}
}

func PreCheckGov(tb testing.TB) {
tb.Helper()
if os.Getenv("MONGODB_ATLAS_PROJECT_ID_GOV") == "" {
tb.Fatal("`MONGODB_ATLAS_PROJECT_ID_GOV` must be set for acceptance testing")
if os.Getenv("MONGODB_ATLAS_GOV_PUBLIC_KEY") == "" ||
os.Getenv("MONGODB_ATLAS_GOV_PRIVATE_KEY") == "" ||
os.Getenv("MONGODB_ATLAS_GOV_BASE_URL") == "" ||
os.Getenv("MONGODB_ATLAS_GOV_ORG_ID") == "" {
tb.Fatal("`MONGODB_ATLAS_GOV_BASE_URL`, `MONGODB_ATLAS_GOV_PUBLIC_KEY`, `MONGODB_ATLAS_GOV_PRIVATE_KEY`and `MONGODB_ATLAS_GOV_ORG_ID` must be set for acceptance testing")
}
PreCheckGovBasic(tb)
}

func PreCheckGPCEnv(tb testing.TB) {
Expand Down
13 changes: 10 additions & 3 deletions internal/testutil/acc/project.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}

Expand Down
23 changes: 23 additions & 0 deletions internal/testutil/acc/provider.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,9 @@
package acc

import (
"fmt"
"os"

"github.com/hashicorp/terraform-plugin-testing/helper/resource"
)

Expand Down Expand Up @@ -38,3 +41,23 @@ func providerAWS() *resource.ExternalProvider {
Source: "hashicorp/aws",
}
}

// ConfigProvider creates a new provider with credentials explicit in config.
//
// This can be used when you want credentials different from the default env-vars.
func ConfigProvider(publicKey, privateKey, baseURL string) string {
return fmt.Sprintf(`
provider %[1]q {
public_key = %[2]q
private_key = %[3]q
base_url = %[4]q
}
`, ProviderNameMongoDBAtlas, publicKey, privateKey, baseURL)
}

// ConfigGovProvider creates provider using MONGODB_ATLAS_GOV_* env vars.
//
// Remember to use PreCheckGovBasic when using this.
func ConfigGovProvider() string {
return ConfigProvider(os.Getenv("MONGODB_ATLAS_GOV_PUBLIC_KEY"), os.Getenv("MONGODB_ATLAS_GOV_PRIVATE_KEY"), os.Getenv("MONGODB_ATLAS_GOV_BASE_URL"))
}