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

chore: Uses noOpCreate to avoid inconsistent result error and fix/enable tests in Ci #2290

Merged
Show file tree
Hide file tree
Changes from 17 commits
Commits
Show all changes
19 commits
Select commit Hold shift + click to select a range
ede88cd
refactor: use noOpCreate to avoid inconsistent result error
EspenAlbert May 21, 2024
d9287f0
refactor: use noOpCreate to avoid inconsistent result error in federa…
EspenAlbert May 21, 2024
48cc59d
test: refactor test for mongodbatlas_federated_settings_org_config
EspenAlbert May 21, 2024
6845bc8
test: cannot do a migration test when there is only an import step
EspenAlbert May 21, 2024
8682b55
test: skip test in CI and add test for noOpCreate
EspenAlbert May 22, 2024
b8ffbc9
test: no reason to keep a test that will only be skipped
EspenAlbert May 22, 2024
5dc00fa
test: add test for createError
EspenAlbert May 22, 2024
7c39d83
test: renameTest
EspenAlbert May 22, 2024
0c48f0c
test: remove missleading steps
EspenAlbert May 22, 2024
d244116
test: remove missleading test steps
EspenAlbert May 22, 2024
795fa74
test: add import verify step after to ensure test works
EspenAlbert May 22, 2024
2c60224
ci: re-enable test in CI and use static resource names
EspenAlbert May 22, 2024
3748ec2
test: fix DOMAIN_NOT_IN_FEDERATION_SETTINGS bug
EspenAlbert May 22, 2024
7ebaa8a
test: try adding domain since at least one domain must be active
EspenAlbert May 23, 2024
c287674
test: change to a validated domain
EspenAlbert May 23, 2024
b7a6a65
test: support reading MONGODB_ATLAS_FEDERATED_SETTINGS_ASSOCIATED_DOMAIN
EspenAlbert May 23, 2024
2e00d90
test: enable data tests for connected_org
EspenAlbert May 23, 2024
0bdf6d5
use spaces instead of tabs
EspenAlbert May 24, 2024
b4964e6
add PR comments/suggestions
EspenAlbert May 24, 2024
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
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 @@ -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 }}
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 @@ -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 }}
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: noOpCreate,
ReadContext: resourceMongoDBAtlasFederatedSettingsIdentityProviderRead,
UpdateContext: resourceMongoDBAtlasFederatedSettingsIdentityProviderUpdate,
DeleteContext: resourceMongoDBAtlasFederatedSettingsIdentityProviderDelete,
Expand Down Expand Up @@ -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"))
EspenAlbert marked this conversation as resolved.
Show resolved Hide resolved
}

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
EspenAlbert marked this conversation as resolved.
Show resolved Hide resolved
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"),
Copy link
Collaborator

Choose a reason for hiding this comment

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

can't be known?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Replaced org_id with a more specific check (value of org_id ).
Removed identity_provider_id as it depends on the environment. Could add an env-var for the specific value but seems fragile and overkill 😅

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: noOpCreate,
ReadContext: resourceMongoDBAtlasFederatedSettingsOrganizationConfigRead,
UpdateContext: resourceMongoDBAtlasFederatedSettingsOrganizationConfigUpdate,
DeleteContext: resourceMongoDBAtlasFederatedSettingsOrganizationConfigDelete,
Expand Down Expand Up @@ -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
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,13 +13,25 @@ import (
"github.com/mongodb/terraform-provider-mongodbatlas/internal/testutil/acc"
)

func TestAccFederatedSettingsOrgCreate_createError(t *testing.T) {
EspenAlbert marked this conversation as resolved.
Show resolved Hide resolved
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) // 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"
Expand All @@ -32,22 +45,19 @@ func basicTestCase(tb testing.TB) *resource.TestCase {
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),
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),
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"),
),
Expand All @@ -57,7 +67,7 @@ func basicTestCase(tb testing.TB) *resource.TestCase {
ResourceName: resourceName,
ImportStateIdFunc: importStateIDFunc(federationSettingsID, orgID),
ImportState: true,
ImportStateVerify: false,
ImportStateVerify: true,
},
},
}
Expand Down Expand Up @@ -100,7 +110,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"]
EspenAlbert marked this conversation as resolved.
Show resolved Hide resolved
identity_provider_id = "%[3]s"
}`, federationSettingsID, orgID, identityProviderID)
}
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