From ede88cd634cee3b1e87899574c945167b84de53f Mon Sep 17 00:00:00 2001 From: EspenAlbert Date: Tue, 21 May 2024 09:29:34 +0100 Subject: [PATCH 01/19] refactor: use noOpCreate to avoid inconsistent result error --- .../resource_federated_settings_identity_provider.go | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/internal/service/federatedsettingsidentityprovider/resource_federated_settings_identity_provider.go b/internal/service/federatedsettingsidentityprovider/resource_federated_settings_identity_provider.go index 1c60c478d3..c03538ef36 100644 --- a/internal/service/federatedsettingsidentityprovider/resource_federated_settings_identity_provider.go +++ b/internal/service/federatedsettingsidentityprovider/resource_federated_settings_identity_provider.go @@ -22,7 +22,7 @@ const OIDC = "OIDC" func Resource() *schema.Resource { return &schema.Resource{ - CreateContext: resourceMongoDBAtlasFederatedSettingsIdentityProviderRead, + CreateContext: noOpCreate, ReadContext: resourceMongoDBAtlasFederatedSettingsIdentityProviderRead, UpdateContext: resourceMongoDBAtlasFederatedSettingsIdentityProviderUpdate, DeleteContext: resourceMongoDBAtlasFederatedSettingsIdentityProviderDelete, @@ -112,15 +112,14 @@ func Resource() *schema.Resource { } } +func noOpCreate(_ context.Context, _ *schema.ResourceData, _ any) diag.Diagnostics { + return diag.FromErr(errors.New("this resource must be imported")) +} + func resourceMongoDBAtlasFederatedSettingsIdentityProviderRead(ctx context.Context, d *schema.ResourceData, meta any) diag.Diagnostics { // Get client connection. connV2 := meta.(*config.MongoDBClient).Atlas20231115008 - if d.Id() == "" { - d.SetId("") - return nil - } - ids := conversion.DecodeStateID(d.Id()) federationSettingsID := ids["federation_settings_id"] From d9287f093100ec589e61750503d42d45dae9bf50 Mon Sep 17 00:00:00 2001 From: EspenAlbert Date: Tue, 21 May 2024 09:44:13 +0100 Subject: [PATCH 02/19] refactor: use noOpCreate to avoid inconsistent result error in federated_settings_connected_org --- .../resource_federated_settings_connected_org.go | 14 +++++--------- 1 file changed, 5 insertions(+), 9 deletions(-) diff --git a/internal/service/federatedsettingsorgconfig/resource_federated_settings_connected_org.go b/internal/service/federatedsettingsorgconfig/resource_federated_settings_connected_org.go index 44f00cf3c2..5f0d6f5525 100644 --- a/internal/service/federatedsettingsorgconfig/resource_federated_settings_connected_org.go +++ b/internal/service/federatedsettingsorgconfig/resource_federated_settings_connected_org.go @@ -16,7 +16,7 @@ import ( func Resource() *schema.Resource { return &schema.Resource{ - CreateContext: resourceMongoDBAtlasFederatedSettingsOrganizationConfigRead, + CreateContext: noOpCreate, ReadContext: resourceMongoDBAtlasFederatedSettingsOrganizationConfigRead, UpdateContext: resourceMongoDBAtlasFederatedSettingsOrganizationConfigUpdate, DeleteContext: resourceMongoDBAtlasFederatedSettingsOrganizationConfigDelete, @@ -58,22 +58,18 @@ func Resource() *schema.Resource { } } +func noOpCreate(_ context.Context, _ *schema.ResourceData, _ any) diag.Diagnostics { + return diag.FromErr(errors.New("this resource must be imported")) +} + func resourceMongoDBAtlasFederatedSettingsOrganizationConfigRead(ctx context.Context, d *schema.ResourceData, meta any) diag.Diagnostics { - // Get client connection. conn := meta.(*config.MongoDBClient).AtlasV2 - - if d.Id() == "" { - d.SetId("") - return nil - } ids := conversion.DecodeStateID(d.Id()) federationSettingsID := ids["federation_settings_id"] orgID := ids["org_id"] federatedSettingsConnectedOrganization, resp, err := conn.FederatedAuthenticationApi.GetConnectedOrgConfig(context.Background(), federationSettingsID, orgID).Execute() if err != nil { - // case 404 - // deleted in the backend case if resp != nil && resp.StatusCode == http.StatusNotFound { d.SetId("") return nil From 48cc59db77215af7d29fd9295f961656ac38ba28 Mon Sep 17 00:00:00 2001 From: EspenAlbert Date: Tue, 21 May 2024 15:40:46 +0100 Subject: [PATCH 03/19] test: refactor test for mongodbatlas_federated_settings_org_config --- ...e_federated_settings_connected_org_test.go | 39 ++++++++++++++----- 1 file changed, 30 insertions(+), 9 deletions(-) diff --git a/internal/service/federatedsettingsorgconfig/resource_federated_settings_connected_org_test.go b/internal/service/federatedsettingsorgconfig/resource_federated_settings_connected_org_test.go index f6fae92a71..28b4a0c4d9 100644 --- a/internal/service/federatedsettingsorgconfig/resource_federated_settings_connected_org_test.go +++ b/internal/service/federatedsettingsorgconfig/resource_federated_settings_connected_org_test.go @@ -18,7 +18,6 @@ func TestAccFederatedSettingsOrg_basic(t *testing.T) { func basicTestCase(tb testing.TB) *resource.TestCase { tb.Helper() - acc.SkipTestForCI(tb) // affects the org var ( resourceName = "mongodbatlas_federated_settings_org_config.test" @@ -38,16 +37,37 @@ func basicTestCase(tb testing.TB) *resource.TestCase { ImportState: true, ImportStateVerify: false, }, + }, + } +} + +func TestAccFederatedSettingsOrg_update(t *testing.T) { + acc.SkipTestForCI(t) // will delete the MONGODB_ATLAS_FEDERATED_ORG_ID on finish, no workaround: https://github.com/hashicorp/terraform-plugin-testing/issues/85 + var ( + resourceName = "mongodbatlas_federated_settings_org_config.test" + federationSettingsID = os.Getenv("MONGODB_ATLAS_FEDERATION_SETTINGS_ID") + orgID = os.Getenv("MONGODB_ATLAS_FEDERATED_ORG_ID") + idpID = os.Getenv("MONGODB_ATLAS_FEDERATED_IDP_ID") + ) + + resource.Test(t, resource.TestCase{ + PreCheck: func() { acc.PreCheckFederatedSettings(t) }, + ProtoV6ProviderFactories: acc.TestAccProviderV6Factories, + Steps: []resource.TestStep{ { - Config: configBasic(federationSettingsID, orgID, idpID), - ResourceName: resourceName, - ImportStateIdFunc: importStateIDFunc(federationSettingsID, orgID), - ImportState: true, + Config: configBasic(federationSettingsID, orgID, idpID), + ResourceName: resourceName, + ImportStateIdFunc: importStateIDFunc(federationSettingsID, orgID), + ImportState: true, + ImportStateVerify: false, + ImportStatePersist: true, // ensure update will be tested in the next step + }, + { + Config: configBasic(federationSettingsID, orgID, idpID), Check: resource.ComposeTestCheckFunc( checkExists(resourceName), resource.TestCheckResourceAttr(resourceName, "federation_settings_id", federationSettingsID), resource.TestCheckResourceAttr(resourceName, "org_id", orgID), - resource.TestCheckResourceAttr(resourceName, "name", "mongodb_federation_test"), resource.TestCheckResourceAttr(resourceName, "domain_restriction_enabled", "false"), resource.TestCheckResourceAttr(resourceName, "domain_allow_list.0", "reorganizeyourworld.com"), ), @@ -57,10 +77,11 @@ func basicTestCase(tb testing.TB) *resource.TestCase { ResourceName: resourceName, ImportStateIdFunc: importStateIDFunc(federationSettingsID, orgID), ImportState: true, - ImportStateVerify: false, + ImportStateVerify: true, }, }, - } + }, + ) } func checkExists(resourceName string) resource.TestCheckFunc { @@ -100,7 +121,7 @@ func configBasic(federationSettingsID, orgID, identityProviderID string) string federation_settings_id = "%[1]s" org_id = "%[2]s" domain_restriction_enabled = false - domain_allow_list = ["reorganizeyourworld.com"] + domain_allow_list = ["reorganizeyourworld.com", "cfn-test-domain.com"] identity_provider_id = "%[3]s" }`, federationSettingsID, orgID, identityProviderID) } From 6845bc8de72eeb5a2ac91a057a7e483b0c79c2c8 Mon Sep 17 00:00:00 2001 From: EspenAlbert Date: Tue, 21 May 2024 17:00:57 +0100 Subject: [PATCH 04/19] test: cannot do a migration test when there is only an import step --- ...federated_settings_connected_org_migration_test.go | 11 ----------- 1 file changed, 11 deletions(-) delete mode 100644 internal/service/federatedsettingsorgconfig/resource_federated_settings_connected_org_migration_test.go diff --git a/internal/service/federatedsettingsorgconfig/resource_federated_settings_connected_org_migration_test.go b/internal/service/federatedsettingsorgconfig/resource_federated_settings_connected_org_migration_test.go deleted file mode 100644 index 722fab8cb4..0000000000 --- a/internal/service/federatedsettingsorgconfig/resource_federated_settings_connected_org_migration_test.go +++ /dev/null @@ -1,11 +0,0 @@ -package federatedsettingsorgconfig_test - -import ( - "testing" - - "github.com/mongodb/terraform-provider-mongodbatlas/internal/testutil/mig" -) - -func TestMigFederatedSettingsOrg_basic(t *testing.T) { - mig.CreateAndRunTest(t, basicTestCase(t)) -} From 8682b551b8a7cd9abc0135fb7d6eea09bcf9d952 Mon Sep 17 00:00:00 2001 From: EspenAlbert Date: Wed, 22 May 2024 10:39:06 +0100 Subject: [PATCH 05/19] test: skip test in CI and add test for noOpCreate --- ...e_federated_settings_connected_org_test.go | 41 +++++++------------ 1 file changed, 15 insertions(+), 26 deletions(-) diff --git a/internal/service/federatedsettingsorgconfig/resource_federated_settings_connected_org_test.go b/internal/service/federatedsettingsorgconfig/resource_federated_settings_connected_org_test.go index 28b4a0c4d9..7e11609ec8 100644 --- a/internal/service/federatedsettingsorgconfig/resource_federated_settings_connected_org_test.go +++ b/internal/service/federatedsettingsorgconfig/resource_federated_settings_connected_org_test.go @@ -4,6 +4,7 @@ import ( "context" "fmt" "os" + "regexp" "testing" "github.com/hashicorp/terraform-plugin-testing/helper/resource" @@ -12,12 +13,25 @@ import ( "github.com/mongodb/terraform-provider-mongodbatlas/internal/testutil/acc" ) +func TestAccFederatedSettingsOrgCreate_mustImport(t *testing.T) { + resource.ParallelTest(t, resource.TestCase{ + ProtoV6ProviderFactories: acc.TestAccProviderV6Factories, + Steps: []resource.TestStep{ + { + Config: configBasic("not-used", "not-used", "not-used"), + ExpectError: regexp.MustCompile("this resource must be imported"), + }, + }, + }) +} + func TestAccFederatedSettingsOrg_basic(t *testing.T) { resource.ParallelTest(t, *basicTestCase(t)) } func basicTestCase(tb testing.TB) *resource.TestCase { tb.Helper() + acc.SkipTestForCI(tb) // will delete the MONGODB_ATLAS_FEDERATED_ORG_ID on finish, no workaround: https://github.com/hashicorp/terraform-plugin-testing/issues/85 var ( resourceName = "mongodbatlas_federated_settings_org_config.test" @@ -29,30 +43,6 @@ func basicTestCase(tb testing.TB) *resource.TestCase { return &resource.TestCase{ PreCheck: func() { acc.PreCheckFederatedSettings(tb) }, ProtoV6ProviderFactories: acc.TestAccProviderV6Factories, - Steps: []resource.TestStep{ - { - Config: configBasic(federationSettingsID, orgID, idpID), - ResourceName: resourceName, - ImportStateIdFunc: importStateIDFunc(federationSettingsID, orgID), - ImportState: true, - ImportStateVerify: false, - }, - }, - } -} - -func TestAccFederatedSettingsOrg_update(t *testing.T) { - acc.SkipTestForCI(t) // will delete the MONGODB_ATLAS_FEDERATED_ORG_ID on finish, no workaround: https://github.com/hashicorp/terraform-plugin-testing/issues/85 - var ( - resourceName = "mongodbatlas_federated_settings_org_config.test" - federationSettingsID = os.Getenv("MONGODB_ATLAS_FEDERATION_SETTINGS_ID") - orgID = os.Getenv("MONGODB_ATLAS_FEDERATED_ORG_ID") - idpID = os.Getenv("MONGODB_ATLAS_FEDERATED_IDP_ID") - ) - - resource.Test(t, resource.TestCase{ - PreCheck: func() { acc.PreCheckFederatedSettings(t) }, - ProtoV6ProviderFactories: acc.TestAccProviderV6Factories, Steps: []resource.TestStep{ { Config: configBasic(federationSettingsID, orgID, idpID), @@ -80,8 +70,7 @@ func TestAccFederatedSettingsOrg_update(t *testing.T) { ImportStateVerify: true, }, }, - }, - ) + } } func checkExists(resourceName string) resource.TestCheckFunc { From b8ffbc9ae44d987f5e0199e2075f541023a21a27 Mon Sep 17 00:00:00 2001 From: EspenAlbert Date: Wed, 22 May 2024 10:41:12 +0100 Subject: [PATCH 06/19] test: no reason to keep a test that will only be skipped --- ...ted_settings_identity_provider_migration_test.go | 13 ------------- 1 file changed, 13 deletions(-) delete mode 100644 internal/service/federatedsettingsidentityprovider/resource_federated_settings_identity_provider_migration_test.go diff --git a/internal/service/federatedsettingsidentityprovider/resource_federated_settings_identity_provider_migration_test.go b/internal/service/federatedsettingsidentityprovider/resource_federated_settings_identity_provider_migration_test.go deleted file mode 100644 index 8abedc3c44..0000000000 --- a/internal/service/federatedsettingsidentityprovider/resource_federated_settings_identity_provider_migration_test.go +++ /dev/null @@ -1,13 +0,0 @@ -package federatedsettingsidentityprovider_test - -import ( - "testing" - - "github.com/mongodb/terraform-provider-mongodbatlas/internal/testutil/acc" - "github.com/mongodb/terraform-provider-mongodbatlas/internal/testutil/mig" -) - -func TestMigFederatedSettingsIdentityProviderRS_basic(t *testing.T) { - acc.SkipTestForCI(t) // this resource can only be imported - mig.CreateAndRunTest(t, basicTestCase(t)) -} From 5dc00fa3678a1078aedd201c682ec9c5a9afcd79 Mon Sep 17 00:00:00 2001 From: EspenAlbert Date: Wed, 22 May 2024 12:23:44 +0100 Subject: [PATCH 07/19] test: add test for createError --- ...rce_federated_settings_identity_provider_test.go | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/internal/service/federatedsettingsidentityprovider/resource_federated_settings_identity_provider_test.go b/internal/service/federatedsettingsidentityprovider/resource_federated_settings_identity_provider_test.go index fb85e424bd..fb62afbf5b 100644 --- a/internal/service/federatedsettingsidentityprovider/resource_federated_settings_identity_provider_test.go +++ b/internal/service/federatedsettingsidentityprovider/resource_federated_settings_identity_provider_test.go @@ -4,6 +4,7 @@ import ( "context" "fmt" "os" + "regexp" "testing" "github.com/hashicorp/terraform-plugin-testing/helper/resource" @@ -12,6 +13,18 @@ import ( "github.com/mongodb/terraform-provider-mongodbatlas/internal/testutil/acc" ) +func TestAccFederatedSettingsIdentityProvider_createError(t *testing.T) { + resource.ParallelTest(t, resource.TestCase{ + ProtoV6ProviderFactories: acc.TestAccProviderV6Factories, + Steps: []resource.TestStep{ + { + Config: configBasic("not-used", "not-used", "not-used"), + ExpectError: regexp.MustCompile("this resource must be imported"), + }, + }, + }) +} + func TestAccFederatedSettingsIdentityProviderRS_basic(t *testing.T) { resource.ParallelTest(t, *basicTestCase(t)) } From 7c39d8334f676e9fb457099986b3bcbffeec0101 Mon Sep 17 00:00:00 2001 From: EspenAlbert Date: Wed, 22 May 2024 12:23:56 +0100 Subject: [PATCH 08/19] test: renameTest --- .../resource_federated_settings_connected_org_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/service/federatedsettingsorgconfig/resource_federated_settings_connected_org_test.go b/internal/service/federatedsettingsorgconfig/resource_federated_settings_connected_org_test.go index 7e11609ec8..d7b57fcd84 100644 --- a/internal/service/federatedsettingsorgconfig/resource_federated_settings_connected_org_test.go +++ b/internal/service/federatedsettingsorgconfig/resource_federated_settings_connected_org_test.go @@ -13,7 +13,7 @@ import ( "github.com/mongodb/terraform-provider-mongodbatlas/internal/testutil/acc" ) -func TestAccFederatedSettingsOrgCreate_mustImport(t *testing.T) { +func TestAccFederatedSettingsOrgCreate_createError(t *testing.T) { resource.ParallelTest(t, resource.TestCase{ ProtoV6ProviderFactories: acc.TestAccProviderV6Factories, Steps: []resource.TestStep{ From 0c48f0c05a0d9fc2af2074a0f1d8f5f0de97c642 Mon Sep 17 00:00:00 2001 From: EspenAlbert Date: Wed, 22 May 2024 12:26:05 +0100 Subject: [PATCH 09/19] test: remove missleading steps --- ...derated_settings_identity_provider_test.go | 20 +++++++++---------- 1 file changed, 9 insertions(+), 11 deletions(-) diff --git a/internal/service/federatedsettingsidentityprovider/resource_federated_settings_identity_provider_test.go b/internal/service/federatedsettingsidentityprovider/resource_federated_settings_identity_provider_test.go index fb62afbf5b..c78415760e 100644 --- a/internal/service/federatedsettingsidentityprovider/resource_federated_settings_identity_provider_test.go +++ b/internal/service/federatedsettingsidentityprovider/resource_federated_settings_identity_provider_test.go @@ -31,6 +31,7 @@ func TestAccFederatedSettingsIdentityProviderRS_basic(t *testing.T) { func basicTestCase(tb testing.TB) *resource.TestCase { tb.Helper() + acc.SkipTestForCI(tb) // affects the identity provider (resource managed outside of ci) var ( resourceName = "mongodbatlas_federated_settings_identity_provider.test" @@ -45,18 +46,15 @@ func basicTestCase(tb testing.TB) *resource.TestCase { ProtoV6ProviderFactories: acc.TestAccProviderV6Factories, Steps: []resource.TestStep{ { - Config: configBasic(federationSettingsID, ssoURL, issuerURI), - ResourceName: resourceName, - ImportStateIdFunc: importStateIDFunc(federationSettingsID, idpID), - ImportState: true, - ImportStateVerify: false, + Config: configBasic(federationSettingsID, ssoURL, issuerURI), + ResourceName: resourceName, + ImportStateIdFunc: importStateIDFunc(federationSettingsID, idpID), + ImportState: true, + ImportStateVerify: false, }, - { - Config: configBasic(federationSettingsID, ssoURL, issuerURI), - ResourceName: resourceName, - ImportStateIdFunc: importStateIDFunc(federationSettingsID, idpID), - - ImportState: true, + }, + } +} Check: resource.ComposeTestCheckFunc( checkExists(resourceName, idpID), resource.TestCheckResourceAttr(resourceName, "federation_settings_id", federationSettingsID), From d244116e1bfec4efd0e69064a5af70ab44092c04 Mon Sep 17 00:00:00 2001 From: EspenAlbert Date: Wed, 22 May 2024 12:31:26 +0100 Subject: [PATCH 10/19] test: remove missleading test steps --- ...rce_federated_settings_identity_provider_test.go | 13 +++---------- 1 file changed, 3 insertions(+), 10 deletions(-) diff --git a/internal/service/federatedsettingsidentityprovider/resource_federated_settings_identity_provider_test.go b/internal/service/federatedsettingsidentityprovider/resource_federated_settings_identity_provider_test.go index c78415760e..2fc83b2d4f 100644 --- a/internal/service/federatedsettingsidentityprovider/resource_federated_settings_identity_provider_test.go +++ b/internal/service/federatedsettingsidentityprovider/resource_federated_settings_identity_provider_test.go @@ -51,23 +51,16 @@ func basicTestCase(tb testing.TB) *resource.TestCase { ImportStateIdFunc: importStateIDFunc(federationSettingsID, idpID), ImportState: true, ImportStateVerify: false, + ImportStatePersist: true, }, - }, - } -} + { + Config: configBasic(federationSettingsID, ssoURL, issuerURI), Check: resource.ComposeTestCheckFunc( checkExists(resourceName, idpID), resource.TestCheckResourceAttr(resourceName, "federation_settings_id", federationSettingsID), resource.TestCheckResourceAttr(resourceName, "name", "mongodb_federation_test"), ), }, - { - Config: configBasic(federationSettingsID, ssoURL, issuerURI), - ResourceName: resourceName, - ImportStateIdFunc: importStateIDFunc(federationSettingsID, idpID), - ImportState: true, - ImportStateVerify: false, - }, }, } } From 795fa741dea6f58aa217a6e8dd29420113d9f439 Mon Sep 17 00:00:00 2001 From: EspenAlbert Date: Wed, 22 May 2024 12:54:18 +0100 Subject: [PATCH 11/19] test: add import verify step after to ensure test works --- ...resource_federated_settings_identity_provider_test.go | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/internal/service/federatedsettingsidentityprovider/resource_federated_settings_identity_provider_test.go b/internal/service/federatedsettingsidentityprovider/resource_federated_settings_identity_provider_test.go index 2fc83b2d4f..244ba3420a 100644 --- a/internal/service/federatedsettingsidentityprovider/resource_federated_settings_identity_provider_test.go +++ b/internal/service/federatedsettingsidentityprovider/resource_federated_settings_identity_provider_test.go @@ -61,6 +61,13 @@ func basicTestCase(tb testing.TB) *resource.TestCase { resource.TestCheckResourceAttr(resourceName, "name", "mongodb_federation_test"), ), }, + { + Config: configBasic(federationSettingsID, ssoURL, issuerURI), + ResourceName: resourceName, + ImportStateIdFunc: importStateIDFunc(federationSettingsID, idpID), + ImportState: true, + ImportStateVerify: true, + }, }, } } @@ -101,7 +108,7 @@ func configBasic(federationSettingsID, ssoURL, issuerURI string) string { resource "mongodbatlas_federated_settings_identity_provider" "test" { federation_settings_id = "%[1]s" name = "mongodb_federation_test" - associated_domains = ["reorganizeyourworld.com"] + associated_domains = ["cfn-test-domain.com"] sso_debug_enabled = true status = "ACTIVE" sso_url = "%[2]s" From 2c6022418afbcfed20a58fec9516a76f5679b5dc Mon Sep 17 00:00:00 2001 From: EspenAlbert Date: Wed, 22 May 2024 13:14:48 +0100 Subject: [PATCH 12/19] ci: re-enable test in CI and use static resource names --- .../resource_federated_settings_identity_provider_test.go | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/internal/service/federatedsettingsidentityprovider/resource_federated_settings_identity_provider_test.go b/internal/service/federatedsettingsidentityprovider/resource_federated_settings_identity_provider_test.go index 244ba3420a..33c5863479 100644 --- a/internal/service/federatedsettingsidentityprovider/resource_federated_settings_identity_provider_test.go +++ b/internal/service/federatedsettingsidentityprovider/resource_federated_settings_identity_provider_test.go @@ -31,7 +31,6 @@ func TestAccFederatedSettingsIdentityProviderRS_basic(t *testing.T) { func basicTestCase(tb testing.TB) *resource.TestCase { tb.Helper() - acc.SkipTestForCI(tb) // affects the identity provider (resource managed outside of ci) var ( resourceName = "mongodbatlas_federated_settings_identity_provider.test" @@ -58,7 +57,7 @@ func basicTestCase(tb testing.TB) *resource.TestCase { Check: resource.ComposeTestCheckFunc( checkExists(resourceName, idpID), resource.TestCheckResourceAttr(resourceName, "federation_settings_id", federationSettingsID), - resource.TestCheckResourceAttr(resourceName, "name", "mongodb_federation_test"), + resource.TestCheckResourceAttr(resourceName, "name", "SAML-test"), ), }, { @@ -107,8 +106,8 @@ func configBasic(federationSettingsID, ssoURL, issuerURI string) string { return fmt.Sprintf(` resource "mongodbatlas_federated_settings_identity_provider" "test" { federation_settings_id = "%[1]s" - name = "mongodb_federation_test" - associated_domains = ["cfn-test-domain.com"] + name = "SAML-test" + associated_domains = ["reorganizeyourworld.com"] sso_debug_enabled = true status = "ACTIVE" sso_url = "%[2]s" From 3748ec2ef7c62b54a88db98ec5f41c6dfa533d4d Mon Sep 17 00:00:00 2001 From: EspenAlbert Date: Wed, 22 May 2024 16:58:34 +0100 Subject: [PATCH 13/19] test: fix DOMAIN_NOT_IN_FEDERATION_SETTINGS bug --- ..._federated_settings_identity_provider_test.go | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/internal/service/federatedsettingsidentityprovider/resource_federated_settings_identity_provider_test.go b/internal/service/federatedsettingsidentityprovider/resource_federated_settings_identity_provider_test.go index 33c5863479..de1820d018 100644 --- a/internal/service/federatedsettingsidentityprovider/resource_federated_settings_identity_provider_test.go +++ b/internal/service/federatedsettingsidentityprovider/resource_federated_settings_identity_provider_test.go @@ -105,14 +105,14 @@ func importStateIDFunc(federationSettingsID, idpID string) resource.ImportStateI func configBasic(federationSettingsID, ssoURL, issuerURI string) string { return fmt.Sprintf(` resource "mongodbatlas_federated_settings_identity_provider" "test" { - federation_settings_id = "%[1]s" - name = "SAML-test" - associated_domains = ["reorganizeyourworld.com"] - sso_debug_enabled = true - status = "ACTIVE" - sso_url = "%[2]s" - issuer_uri = "%[3]s" - request_binding = "HTTP-POST" + federation_settings_id = %[1]q + name = "SAML-test" + associated_domains = [] + sso_debug_enabled = true + status = "ACTIVE" + sso_url = %[2]q + issuer_uri = %[3]q + request_binding = "HTTP-POST" response_signature_algorithm = "SHA-256" }`, federationSettingsID, ssoURL, issuerURI) } From 7ebaa8aeb66da43fdb2f1e651983fcda6f9e6890 Mon Sep 17 00:00:00 2001 From: EspenAlbert Date: Thu, 23 May 2024 09:28:33 +0100 Subject: [PATCH 14/19] test: try adding domain since at least one domain must be active --- .../resource_federated_settings_identity_provider_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/service/federatedsettingsidentityprovider/resource_federated_settings_identity_provider_test.go b/internal/service/federatedsettingsidentityprovider/resource_federated_settings_identity_provider_test.go index de1820d018..45ee3cce58 100644 --- a/internal/service/federatedsettingsidentityprovider/resource_federated_settings_identity_provider_test.go +++ b/internal/service/federatedsettingsidentityprovider/resource_federated_settings_identity_provider_test.go @@ -107,7 +107,7 @@ func configBasic(federationSettingsID, ssoURL, issuerURI string) string { resource "mongodbatlas_federated_settings_identity_provider" "test" { federation_settings_id = %[1]q name = "SAML-test" - associated_domains = [] + associated_domains = ["cfn-test-domain.com"] sso_debug_enabled = true status = "ACTIVE" sso_url = %[2]q From c28767451d971ccd32390f91fc88d47704db1a46 Mon Sep 17 00:00:00 2001 From: EspenAlbert Date: Thu, 23 May 2024 10:08:39 +0100 Subject: [PATCH 15/19] test: change to a validated domain --- .../resource_federated_settings_identity_provider_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/service/federatedsettingsidentityprovider/resource_federated_settings_identity_provider_test.go b/internal/service/federatedsettingsidentityprovider/resource_federated_settings_identity_provider_test.go index 45ee3cce58..d01426e9c3 100644 --- a/internal/service/federatedsettingsidentityprovider/resource_federated_settings_identity_provider_test.go +++ b/internal/service/federatedsettingsidentityprovider/resource_federated_settings_identity_provider_test.go @@ -107,7 +107,7 @@ func configBasic(federationSettingsID, ssoURL, issuerURI string) string { resource "mongodbatlas_federated_settings_identity_provider" "test" { federation_settings_id = %[1]q name = "SAML-test" - associated_domains = ["cfn-test-domain.com"] + associated_domains = ["mdb-aws-cfn-publishing.com"] sso_debug_enabled = true status = "ACTIVE" sso_url = %[2]q From b7a6a651f715b794dc9d3b8452acc74ea308d810 Mon Sep 17 00:00:00 2001 From: EspenAlbert Date: Thu, 23 May 2024 16:30:28 +0100 Subject: [PATCH 16/19] test: support reading MONGODB_ATLAS_FEDERATED_SETTINGS_ASSOCIATED_DOMAIN --- .github/workflows/acceptance-tests-runner.yml | 4 ++++ .github/workflows/acceptance-tests.yml | 1 + ...ederated_settings_identity_provider_test.go | 18 ++++++++++-------- internal/testutil/acc/pre_check.go | 9 +++++++++ 4 files changed, 24 insertions(+), 8 deletions(-) diff --git a/.github/workflows/acceptance-tests-runner.yml b/.github/workflows/acceptance-tests-runner.yml index 2674fa8265..c1f4d8ec0f 100644 --- a/.github/workflows/acceptance-tests-runner.yml +++ b/.github/workflows/acceptance-tests-runner.yml @@ -73,6 +73,9 @@ on: mongodb_atlas_federated_org_id: type: string required: true + mongodb_atlas_federated_settings_associated_domain: + type: string + required: true secrets: # all secrets are passed explicitly in this workflow mongodb_atlas_public_key: required: true @@ -519,6 +522,7 @@ jobs: MONGODB_ATLAS_FEDERATED_SSO_URL: ${{ inputs.mongodb_atlas_federated_sso_url }} MONGODB_ATLAS_FEDERATED_ISSUER_URI: ${{ inputs.mongodb_atlas_federated_issuer_uri }} MONGODB_ATLAS_FEDERATED_ORG_ID: ${{ inputs.mongodb_atlas_federated_org_id }} + MONGODB_ATLAS_FEDERATED_SETTINGS_ASSOCIATED_DOMAIN: ${{ inputs.mongodb_atlas_federated_settings_associated_domain }} AWS_S3_BUCKET: ${{ secrets.aws_s3_bucket_federation }} AWS_REGION: ${{ vars.aws_region_federation }} AWS_ACCESS_KEY_ID: ${{ secrets.aws_access_key_id }} diff --git a/.github/workflows/acceptance-tests.yml b/.github/workflows/acceptance-tests.yml index 28272bedf4..aa6a6b616f 100644 --- a/.github/workflows/acceptance-tests.yml +++ b/.github/workflows/acceptance-tests.yml @@ -87,3 +87,4 @@ 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_federated_settings_associated_domain: ${{ vars.MONGODB_ATLAS_FEDERATED_SETTINGS_ASSOCIATED_DOMAIN }} diff --git a/internal/service/federatedsettingsidentityprovider/resource_federated_settings_identity_provider_test.go b/internal/service/federatedsettingsidentityprovider/resource_federated_settings_identity_provider_test.go index d01426e9c3..a99bff354f 100644 --- a/internal/service/federatedsettingsidentityprovider/resource_federated_settings_identity_provider_test.go +++ b/internal/service/federatedsettingsidentityprovider/resource_federated_settings_identity_provider_test.go @@ -18,7 +18,7 @@ func TestAccFederatedSettingsIdentityProvider_createError(t *testing.T) { ProtoV6ProviderFactories: acc.TestAccProviderV6Factories, Steps: []resource.TestStep{ { - Config: configBasic("not-used", "not-used", "not-used"), + Config: configBasic("not-used", "not-used", "not-used", "not-used"), ExpectError: regexp.MustCompile("this resource must be imported"), }, }, @@ -38,14 +38,16 @@ func basicTestCase(tb testing.TB) *resource.TestCase { idpID = os.Getenv("MONGODB_ATLAS_FEDERATED_IDP_ID") ssoURL = os.Getenv("MONGODB_ATLAS_FEDERATED_SSO_URL") issuerURI = os.Getenv("MONGODB_ATLAS_FEDERATED_ISSUER_URI") + associatedDomain = os.Getenv("MONGODB_ATLAS_FEDERATED_SETTINGS_ASSOCIATED_DOMAIN") + config = configBasic(federationSettingsID, ssoURL, issuerURI, associatedDomain) ) return &resource.TestCase{ - PreCheck: func() { acc.PreCheckFederatedSettings(tb) }, + PreCheck: func() { acc.PreCheckFederatedSettingsIdentityProvider(tb) }, ProtoV6ProviderFactories: acc.TestAccProviderV6Factories, Steps: []resource.TestStep{ { - Config: configBasic(federationSettingsID, ssoURL, issuerURI), + Config: config, ResourceName: resourceName, ImportStateIdFunc: importStateIDFunc(federationSettingsID, idpID), ImportState: true, @@ -53,7 +55,7 @@ func basicTestCase(tb testing.TB) *resource.TestCase { ImportStatePersist: true, }, { - Config: configBasic(federationSettingsID, ssoURL, issuerURI), + Config: config, Check: resource.ComposeTestCheckFunc( checkExists(resourceName, idpID), resource.TestCheckResourceAttr(resourceName, "federation_settings_id", federationSettingsID), @@ -61,7 +63,7 @@ func basicTestCase(tb testing.TB) *resource.TestCase { ), }, { - Config: configBasic(federationSettingsID, ssoURL, issuerURI), + Config: config, ResourceName: resourceName, ImportStateIdFunc: importStateIDFunc(federationSettingsID, idpID), ImportState: true, @@ -102,17 +104,17 @@ func importStateIDFunc(federationSettingsID, idpID string) resource.ImportStateI } } -func configBasic(federationSettingsID, ssoURL, issuerURI string) string { +func configBasic(federationSettingsID, ssoURL, issuerURI, associatedDomain string) string { return fmt.Sprintf(` resource "mongodbatlas_federated_settings_identity_provider" "test" { federation_settings_id = %[1]q name = "SAML-test" - associated_domains = ["mdb-aws-cfn-publishing.com"] + associated_domains = [%[4]q] sso_debug_enabled = true status = "ACTIVE" sso_url = %[2]q issuer_uri = %[3]q request_binding = "HTTP-POST" response_signature_algorithm = "SHA-256" - }`, federationSettingsID, ssoURL, issuerURI) + }`, federationSettingsID, ssoURL, issuerURI, associatedDomain) } diff --git a/internal/testutil/acc/pre_check.go b/internal/testutil/acc/pre_check.go index 23e52636ce..481eff67b9 100644 --- a/internal/testutil/acc/pre_check.go +++ b/internal/testutil/acc/pre_check.go @@ -271,6 +271,15 @@ func PreCheckFederatedSettings(tb testing.TB) { } } +func PreCheckFederatedSettingsIdentityProvider(tb testing.TB) { + tb.Helper() + if os.Getenv("MONGODB_ATLAS_FEDERATED_ORG_ID") == "" || + os.Getenv("MONGODB_ATLAS_FEDERATED_SETTINGS_ASSOCIATED_DOMAIN") == "" || + os.Getenv("MONGODB_ATLAS_FEDERATION_SETTINGS_ID") == "" { + tb.Fatal("`MONGODB_ATLAS_FEDERATED_ORG_ID` and `MONGODB_ATLAS_FEDERATION_SETTINGS_ID` must be set for federated settings/verify acceptance testing") + } +} + func PreCheckPrivateEndpoint(tb testing.TB) { tb.Helper() if os.Getenv("MONGODB_ATLAS_PRIVATE_ENDPOINT_ID") == "" || From 2e00d90a6496a1969b7348618a2fea52352948d9 Mon Sep 17 00:00:00 2001 From: EspenAlbert Date: Thu, 23 May 2024 17:06:09 +0100 Subject: [PATCH 17/19] test: enable data tests for connected_org --- .../data_source_federated_settings_connected_org_test.go | 5 +---- .../data_source_federated_settings_connected_orgs_test.go | 2 -- 2 files changed, 1 insertion(+), 6 deletions(-) diff --git a/internal/service/federatedsettingsorgconfig/data_source_federated_settings_connected_org_test.go b/internal/service/federatedsettingsorgconfig/data_source_federated_settings_connected_org_test.go index d53d2f11b6..2e8d0ce75e 100644 --- a/internal/service/federatedsettingsorgconfig/data_source_federated_settings_connected_org_test.go +++ b/internal/service/federatedsettingsorgconfig/data_source_federated_settings_connected_org_test.go @@ -10,8 +10,6 @@ import ( ) func TestAccFederatedSettingsOrgDS_basic(t *testing.T) { - acc.SkipTestForCI(t) // affects the org - var ( resourceName = "data.mongodbatlas_federated_settings_org_config.test" federatedSettingsID = os.Getenv("MONGODB_ATLAS_FEDERATION_SETTINGS_ID") @@ -28,8 +26,7 @@ func TestAccFederatedSettingsOrgDS_basic(t *testing.T) { resource.TestCheckResourceAttrSet(resourceName, "federation_settings_id"), resource.TestCheckResourceAttrSet(resourceName, "role_mappings.#"), resource.TestCheckResourceAttrSet(resourceName, "identity_provider_id"), - resource.TestCheckResourceAttrSet(resourceName, "org_id"), - resource.TestCheckResourceAttr(resourceName, "identity_provider_id", "0oad4fas87jL5Xnk1297"), + resource.TestCheckResourceAttr(resourceName, "org_id", orgID), ), }, }, diff --git a/internal/service/federatedsettingsorgconfig/data_source_federated_settings_connected_orgs_test.go b/internal/service/federatedsettingsorgconfig/data_source_federated_settings_connected_orgs_test.go index 41140b6a8f..f581cbe2f0 100644 --- a/internal/service/federatedsettingsorgconfig/data_source_federated_settings_connected_orgs_test.go +++ b/internal/service/federatedsettingsorgconfig/data_source_federated_settings_connected_orgs_test.go @@ -10,8 +10,6 @@ import ( ) func TestAccFederatedSettingsOrgDSPlural_basic(t *testing.T) { - acc.SkipTestForCI(t) // affects the org - var ( resourceName = "data.mongodbatlas_federated_settings_org_configs.test" federatedSettingsID = os.Getenv("MONGODB_ATLAS_FEDERATION_SETTINGS_ID") From 0bdf6d5fda97b26440bcc9fb8c6d58cdf0d15bd0 Mon Sep 17 00:00:00 2001 From: EspenAlbert Date: Fri, 24 May 2024 15:37:20 +0100 Subject: [PATCH 18/19] use spaces instead of tabs --- .../resource_federated_settings_identity_provider_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/internal/service/federatedsettingsidentityprovider/resource_federated_settings_identity_provider_test.go b/internal/service/federatedsettingsidentityprovider/resource_federated_settings_identity_provider_test.go index a99bff354f..fe6b3641be 100644 --- a/internal/service/federatedsettingsidentityprovider/resource_federated_settings_identity_provider_test.go +++ b/internal/service/federatedsettingsidentityprovider/resource_federated_settings_identity_provider_test.go @@ -107,8 +107,8 @@ func importStateIDFunc(federationSettingsID, idpID string) resource.ImportStateI func configBasic(federationSettingsID, ssoURL, issuerURI, associatedDomain string) string { return fmt.Sprintf(` resource "mongodbatlas_federated_settings_identity_provider" "test" { - federation_settings_id = %[1]q - name = "SAML-test" + federation_settings_id = %[1]q + name = "SAML-test" associated_domains = [%[4]q] sso_debug_enabled = true status = "ACTIVE" From b4964e64fcf3b7e5694bb425719387c29179e7b4 Mon Sep 17 00:00:00 2001 From: EspenAlbert Date: Fri, 24 May 2024 15:43:09 +0100 Subject: [PATCH 19/19] add PR comments/suggestions --- ...ce_federated_settings_identity_provider.go | 4 ++-- ...source_federated_settings_connected_org.go | 4 ++-- ...e_federated_settings_connected_org_test.go | 19 ++++++++++--------- 3 files changed, 14 insertions(+), 13 deletions(-) diff --git a/internal/service/federatedsettingsidentityprovider/resource_federated_settings_identity_provider.go b/internal/service/federatedsettingsidentityprovider/resource_federated_settings_identity_provider.go index c03538ef36..6eebc7130f 100644 --- a/internal/service/federatedsettingsidentityprovider/resource_federated_settings_identity_provider.go +++ b/internal/service/federatedsettingsidentityprovider/resource_federated_settings_identity_provider.go @@ -22,7 +22,7 @@ const OIDC = "OIDC" func Resource() *schema.Resource { return &schema.Resource{ - CreateContext: noOpCreate, + CreateContext: resourceCreateNotAllowed, ReadContext: resourceMongoDBAtlasFederatedSettingsIdentityProviderRead, UpdateContext: resourceMongoDBAtlasFederatedSettingsIdentityProviderUpdate, DeleteContext: resourceMongoDBAtlasFederatedSettingsIdentityProviderDelete, @@ -112,7 +112,7 @@ func Resource() *schema.Resource { } } -func noOpCreate(_ context.Context, _ *schema.ResourceData, _ any) diag.Diagnostics { +func resourceCreateNotAllowed(_ context.Context, _ *schema.ResourceData, _ any) diag.Diagnostics { return diag.FromErr(errors.New("this resource must be imported")) } diff --git a/internal/service/federatedsettingsorgconfig/resource_federated_settings_connected_org.go b/internal/service/federatedsettingsorgconfig/resource_federated_settings_connected_org.go index 5f0d6f5525..1a73aa1a5e 100644 --- a/internal/service/federatedsettingsorgconfig/resource_federated_settings_connected_org.go +++ b/internal/service/federatedsettingsorgconfig/resource_federated_settings_connected_org.go @@ -16,7 +16,7 @@ import ( func Resource() *schema.Resource { return &schema.Resource{ - CreateContext: noOpCreate, + CreateContext: resourceCreateNotAllowed, ReadContext: resourceMongoDBAtlasFederatedSettingsOrganizationConfigRead, UpdateContext: resourceMongoDBAtlasFederatedSettingsOrganizationConfigUpdate, DeleteContext: resourceMongoDBAtlasFederatedSettingsOrganizationConfigDelete, @@ -58,7 +58,7 @@ func Resource() *schema.Resource { } } -func noOpCreate(_ context.Context, _ *schema.ResourceData, _ any) diag.Diagnostics { +func resourceCreateNotAllowed(_ context.Context, _ *schema.ResourceData, _ any) diag.Diagnostics { return diag.FromErr(errors.New("this resource must be imported")) } diff --git a/internal/service/federatedsettingsorgconfig/resource_federated_settings_connected_org_test.go b/internal/service/federatedsettingsorgconfig/resource_federated_settings_connected_org_test.go index d7b57fcd84..c55f38c5a9 100644 --- a/internal/service/federatedsettingsorgconfig/resource_federated_settings_connected_org_test.go +++ b/internal/service/federatedsettingsorgconfig/resource_federated_settings_connected_org_test.go @@ -13,12 +13,12 @@ import ( "github.com/mongodb/terraform-provider-mongodbatlas/internal/testutil/acc" ) -func TestAccFederatedSettingsOrgCreate_createError(t *testing.T) { +func TestAccFederatedSettingsOrg_createError(t *testing.T) { resource.ParallelTest(t, resource.TestCase{ ProtoV6ProviderFactories: acc.TestAccProviderV6Factories, Steps: []resource.TestStep{ { - Config: configBasic("not-used", "not-used", "not-used"), + Config: configBasic("not-used", "not-used", "not-used", "not-used"), ExpectError: regexp.MustCompile("this resource must be imported"), }, }, @@ -38,14 +38,15 @@ func basicTestCase(tb testing.TB) *resource.TestCase { federationSettingsID = os.Getenv("MONGODB_ATLAS_FEDERATION_SETTINGS_ID") orgID = os.Getenv("MONGODB_ATLAS_FEDERATED_ORG_ID") idpID = os.Getenv("MONGODB_ATLAS_FEDERATED_IDP_ID") + associatedDomain = os.Getenv("MONGODB_ATLAS_FEDERATED_SETTINGS_ASSOCIATED_DOMAIN") ) return &resource.TestCase{ - PreCheck: func() { acc.PreCheckFederatedSettings(tb) }, + PreCheck: func() { acc.PreCheckFederatedSettingsIdentityProvider(tb) }, ProtoV6ProviderFactories: acc.TestAccProviderV6Factories, Steps: []resource.TestStep{ { - Config: configBasic(federationSettingsID, orgID, idpID), + Config: configBasic(federationSettingsID, orgID, idpID, associatedDomain), ResourceName: resourceName, ImportStateIdFunc: importStateIDFunc(federationSettingsID, orgID), ImportState: true, @@ -53,7 +54,7 @@ func basicTestCase(tb testing.TB) *resource.TestCase { ImportStatePersist: true, // ensure update will be tested in the next step }, { - Config: configBasic(federationSettingsID, orgID, idpID), + Config: configBasic(federationSettingsID, orgID, idpID, associatedDomain), Check: resource.ComposeTestCheckFunc( checkExists(resourceName), resource.TestCheckResourceAttr(resourceName, "federation_settings_id", federationSettingsID), @@ -63,7 +64,7 @@ func basicTestCase(tb testing.TB) *resource.TestCase { ), }, { - Config: configBasic(federationSettingsID, orgID, idpID), + Config: configBasic(federationSettingsID, orgID, idpID, associatedDomain), ResourceName: resourceName, ImportStateIdFunc: importStateIDFunc(federationSettingsID, orgID), ImportState: true, @@ -104,13 +105,13 @@ func importStateIDFunc(federationSettingsID, orgID string) resource.ImportStateI } } -func configBasic(federationSettingsID, orgID, identityProviderID string) string { +func configBasic(federationSettingsID, orgID, identityProviderID, associatedDomain string) string { return fmt.Sprintf(` resource "mongodbatlas_federated_settings_org_config" "test" { federation_settings_id = "%[1]s" org_id = "%[2]s" domain_restriction_enabled = false - domain_allow_list = ["reorganizeyourworld.com", "cfn-test-domain.com"] + domain_allow_list = [%[4]q] identity_provider_id = "%[3]s" - }`, federationSettingsID, orgID, identityProviderID) + }`, federationSettingsID, orgID, identityProviderID, associatedDomain) }