fix: Removes escape logic on IP address in mongodbatlas_access_list_api_key #1998
Conversation
| CheckDestroy: checkDestroy, | ||
| Steps: []resource.TestStep{ | ||
| { | ||
| ExternalProviders: acc.ExternalProviders("1.14.0"), // testing provider version before this resource was migrated to Atlas SDK |
There was a problem hiding this comment.
consider using SkipIfVersionBelow and/or IsProviderVersionAtLeast instead.
the issue with acc.ExternalProviders is that it will always check against that version ignoring MONGODB_ATLAS_LAST_VERSION, that can be confusing.
There was a problem hiding this comment.
While this is true, I think we have to revise the provider versions we are passing for running migrations tests in the CI (https://github.com/mongodb/terraform-provider-mongodbatlas/blob/master/.github/workflows/code-health.yml#L91) and test suite. As of the moment we are simply running against latest so any test using SkipIfVersionBelow is likely being skipped if it is not the latest release.
There was a problem hiding this comment.
using SkipIfVersionBelow(1.14.0), updated
| }, | ||
| }, | ||
| { | ||
| ProtoV6ProviderFactories: acc.TestAccProviderV6Factories, |
There was a problem hiding this comment.
you can use mig.TestStepCheckEmptyPlan
| @@ -57,9 +57,54 @@ func TestAccMigrationProjectAccesslistAPIKey_SettingCIDRBlock(t *testing.T) { | |||
| PreCheck: func() { mig.PreCheckBasic(t) }, | |||
| CheckDestroy: checkDestroy, | |||
| Steps: []resource.TestStep{ | |||
| { | |||
There was a problem hiding this comment.
[q] since we already had a migration test defined for setting CIDR block, this would have been captured if we ran the migration test with a provider version of 1.14.0 or below right?
There was a problem hiding this comment.
by "this" are you referring to this bug? If so, then no migration tests would not have captured this bug as we didn't have one for this specific use-case
There was a problem hiding this comment.
Okay, was struggling to find the difference between the existing test and the new one. It has to do with cidrBlock value right?
There was a problem hiding this comment.
right, the existing test uses subnet mask "/32", the new one uses "/16" so I named the test to have "WideCIDR"
| }, | ||
| }) | ||
| } | ||
|
|
||
| func TestAccMigrationProjectAccesslistAPIKey_SettingCIDRBlock(t *testing.T) { | ||
| mig.SkipIfVersionBelow(t, "1.14.0") |
There was a problem hiding this comment.
Could you document this line? Might be beneficial to quickly know the why in the future
There was a problem hiding this comment.
+1, as an example we're also documenting SkipTestForCI so we know why they're being skipped
Description
Removes escape logic on IP address in
mongodbatlas_access_list_api_keysince the new Atlas SDK escapes characters by default.Link to any related issue(s): https://jira.mongodb.org/browse/CLOUDP-235189
Resolves #1984
Type of change:
Required Checklist:
Further comments