-
Notifications
You must be signed in to change notification settings - Fork 210
feat: Adds port_mapping_enabled attribute to privatelink_endpoint and privatelink_endpoint_service #4017
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
base: CLOUDP-346617-dev-gcp-port-based
Are you sure you want to change the base?
feat: Adds port_mapping_enabled attribute to privatelink_endpoint and privatelink_endpoint_service #4017
Changes from 11 commits
332be39
9175dce
8a9562f
ca4cacf
ed3e818
d6725b9
7545b99
3752b5a
cfb1e0f
3a32c9f
df750a7
be386c1
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,15 @@ | ||
| ```release-note:enhancement | ||
| resource/mongodbatlas_privatelink_endpoint_service: Adds `port_mapping_enabled` attribute | ||
| ``` | ||
|
|
||
| ```release-note:enhancement | ||
| resource/mongodbatlas_privatelink_endpoint: Adds `port_mapping_enabled` attribute | ||
| ``` | ||
|
|
||
| ```release-note:enhancement | ||
| data-source/mongodbatlas_privatelink_endpoint_service: Adds `port_mapping_enabled` attribute | ||
| ``` | ||
|
|
||
| ```release-note:enhancement | ||
| data-source/mongodbatlas_privatelink_endpoint: Adds `port_mapping_enabled` attribute | ||
| ``` |
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -106,6 +106,81 @@ func TestAccNetworkRSPrivateLinkEndpointGCP_basic(t *testing.T) { | |||||
| resource.TestCheckResourceAttrSet(resourceName, "region"), | ||||||
| resource.TestCheckResourceAttr(resourceName, "provider_name", providerName), | ||||||
| resource.TestCheckResourceAttr(resourceName, "region", region), | ||||||
| resource.TestCheckResourceAttr(resourceName, "port_mapping_enabled", "false"), | ||||||
| ), | ||||||
| }, | ||||||
| { | ||||||
| ResourceName: resourceName, | ||||||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nice to add the import step 👍
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. it's just git displaying the diff wrong as that was already there but it just got pushed back to the end :) |
||||||
| ImportStateIdFunc: importStateIDFunc(resourceName), | ||||||
| ImportState: true, | ||||||
| ImportStateVerify: true, | ||||||
| }, | ||||||
| }, | ||||||
| }) | ||||||
| } | ||||||
|
|
||||||
| func TestAccNetworkRSPrivateLinkEndpointGCP_basic_with_new_architecture_explicitly_enabled(t *testing.T) { | ||||||
| var ( | ||||||
| resourceName = "mongodbatlas_privatelink_endpoint.test" | ||||||
| orgID = os.Getenv("MONGODB_ATLAS_ORG_ID") | ||||||
| projectName = "test-acc-tf-p-gcp-port-based-routing-feature-flag-enabled" | ||||||
|
||||||
| projectName = "test-acc-tf-p-gcp-port-based-routing-feature-flag-enabled" | |
| projectName = "test-acc-tf-p-gcp-port-mapping-enabled" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Note: This will be updated to use the random name once the feature is officially released (GA) and not gated by a feature-flag (before merging to master)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[nit] not sure if it is worth testing the attribute changes:
null -> true
true -> false
Also, does this value have a default? (What is returned by the API if the field is not set?)
What is the difference between false and null? Will null -> false and ForceNew leading to an unnecessary DELETE+CREATE plan?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
-
Attribute changes (null -> true, true -> false):
These transitions were not tested since I thought it is not needed because of ForceNew. I will add those. -
Default value:
Atlas defaults this attribute to false. Should we do the same? -
null vs false:
I will check this as I do not have a response now (I personally expect yes).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[nit] Is there any example we can update also? I struggle a bit to understand this feature seeing only this variable description
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is the description available on Atlas and unfortunately we are not able to update it. (we need DELETE + CREATE)
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I just wanted to know if we have an example in repo_root/examples/* .tf that we could update?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This makes total sense. I will add an example for this.