Skip to content

Commit

Permalink
chore: Uses noOpCreate to avoid inconsistent result error and fix/ena…
Browse files Browse the repository at this point in the history
…ble tests in Ci (#2290)

* refactor: use noOpCreate to avoid inconsistent result error

* refactor: use noOpCreate to avoid inconsistent result error in federated_settings_connected_org

* test: refactor test for mongodbatlas_federated_settings_org_config

* test: cannot do a migration test when there is only an import step

* test: skip test in CI and add test for noOpCreate

* test: no reason to keep a test that will only be skipped

* test: add test for createError

* test: renameTest

* test: remove missleading steps

* test: remove missleading test steps

* test: add import verify step after to ensure test works

* ci: re-enable test in CI and use static resource names

* test: fix DOMAIN_NOT_IN_FEDERATION_SETTINGS bug

* test: try adding domain since at least one domain must be active

* test: change to a validated domain

* test: support reading MONGODB_ATLAS_FEDERATED_SETTINGS_ASSOCIATED_DOMAIN

* test: enable data tests for connected_org

* use spaces instead of tabs

* add PR comments/suggestions
  • Loading branch information
EspenAlbert authored May 24, 2024
1 parent 2d19861 commit 2b2e276
Show file tree
Hide file tree
Showing 11 changed files with 89 additions and 86 deletions.
4 changes: 4 additions & 0 deletions .github/workflows/acceptance-tests-runner.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -529,6 +532,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 }}
Expand Down
1 change: 1 addition & 0 deletions .github/workflows/acceptance-tests.yml
Original file line number Diff line number Diff line change
Expand Up @@ -92,3 +92,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 }}
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ const OIDC = "OIDC"

func Resource() *schema.Resource {
return &schema.Resource{
CreateContext: resourceMongoDBAtlasFederatedSettingsIdentityProviderRead,
CreateContext: resourceCreateNotAllowed,
ReadContext: resourceMongoDBAtlasFederatedSettingsIdentityProviderRead,
UpdateContext: resourceMongoDBAtlasFederatedSettingsIdentityProviderUpdate,
DeleteContext: resourceMongoDBAtlasFederatedSettingsIdentityProviderDelete,
Expand Down Expand Up @@ -112,15 +112,14 @@ func Resource() *schema.Resource {
}
}

func resourceCreateNotAllowed(_ 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"]

Expand Down

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import (
"context"
"fmt"
"os"
"regexp"
"testing"

"github.com/hashicorp/terraform-plugin-testing/helper/resource"
Expand All @@ -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", "not-used"),
ExpectError: regexp.MustCompile("this resource must be imported"),
},
},
})
}

func TestAccFederatedSettingsIdentityProviderRS_basic(t *testing.T) {
resource.ParallelTest(t, *basicTestCase(t))
}
Expand All @@ -25,37 +38,36 @@ 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),
ResourceName: resourceName,
ImportStateIdFunc: importStateIDFunc(federationSettingsID, idpID),
ImportState: true,
ImportStateVerify: false,
Config: config,
ResourceName: resourceName,
ImportStateIdFunc: importStateIDFunc(federationSettingsID, idpID),
ImportState: true,
ImportStateVerify: false,
ImportStatePersist: true,
},
{
Config: configBasic(federationSettingsID, ssoURL, issuerURI),
ResourceName: resourceName,
ImportStateIdFunc: importStateIDFunc(federationSettingsID, idpID),

ImportState: true,
Config: config,
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"),
),
},
{
Config: configBasic(federationSettingsID, ssoURL, issuerURI),
Config: config,
ResourceName: resourceName,
ImportStateIdFunc: importStateIDFunc(federationSettingsID, idpID),
ImportState: true,
ImportStateVerify: false,
ImportStateVerify: true,
},
},
}
Expand Down Expand Up @@ -92,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]s"
name = "mongodb_federation_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 = [%[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)
}
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Expand All @@ -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),
),
},
},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ import (

func Resource() *schema.Resource {
return &schema.Resource{
CreateContext: resourceMongoDBAtlasFederatedSettingsOrganizationConfigRead,
CreateContext: resourceCreateNotAllowed,
ReadContext: resourceMongoDBAtlasFederatedSettingsOrganizationConfigRead,
UpdateContext: resourceMongoDBAtlasFederatedSettingsOrganizationConfigUpdate,
DeleteContext: resourceMongoDBAtlasFederatedSettingsOrganizationConfigDelete,
Expand Down Expand Up @@ -58,22 +58,18 @@ func Resource() *schema.Resource {
}
}

func resourceCreateNotAllowed(_ 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
Expand Down

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import (
"context"
"fmt"
"os"
"regexp"
"testing"

"github.com/hashicorp/terraform-plugin-testing/helper/resource"
Expand All @@ -12,52 +13,62 @@ import (
"github.com/mongodb/terraform-provider-mongodbatlas/internal/testutil/acc"
)

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", "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) // affects the org
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"
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),
ResourceName: resourceName,
ImportStateIdFunc: importStateIDFunc(federationSettingsID, orgID),
ImportState: true,
ImportStateVerify: false,
Config: configBasic(federationSettingsID, orgID, idpID, associatedDomain),
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),
ResourceName: resourceName,
ImportStateIdFunc: importStateIDFunc(federationSettingsID, orgID),
ImportState: true,
Config: configBasic(federationSettingsID, orgID, idpID, associatedDomain),
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"),
),
},
{
Config: configBasic(federationSettingsID, orgID, idpID),
Config: configBasic(federationSettingsID, orgID, idpID, associatedDomain),
ResourceName: resourceName,
ImportStateIdFunc: importStateIDFunc(federationSettingsID, orgID),
ImportState: true,
ImportStateVerify: false,
ImportStateVerify: true,
},
},
}
Expand Down Expand Up @@ -94,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"]
domain_allow_list = [%[4]q]
identity_provider_id = "%[3]s"
}`, federationSettingsID, orgID, identityProviderID)
}`, federationSettingsID, orgID, identityProviderID, associatedDomain)
}
9 changes: 9 additions & 0 deletions internal/testutil/acc/pre_check.go
Original file line number Diff line number Diff line change
Expand Up @@ -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") == "" ||
Expand Down

0 comments on commit 2b2e276

Please sign in to comment.