-
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?
Conversation
…p-port-based-routing-implementation
|
APIx bot: a message has been sent to Docs Slack channel |
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.
Pull request overview
This PR adds support for GCP Private Service Connect (PSC) port-mapping architecture by introducing a port_mapping_enabled attribute to privatelink endpoint resources and data sources. The attribute allows users to explicitly control whether PSC port-mapping is enabled when creating GCP private endpoints.
Key Changes:
- Added
port_mapping_enabledboolean attribute to privatelink_endpoint (optional, ForceNew) and privatelink_endpoint_service (computed) - Implemented test coverage for both explicitly enabled and disabled port mapping scenarios
- Updated documentation for all affected resources and data sources
Reviewed changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| internal/service/privatelinkendpoint/resource.go | Added optional port_mapping_enabled field to resource schema and create/read logic |
| internal/service/privatelinkendpoint/data_source.go | Added computed port_mapping_enabled field to data source schema and read logic |
| internal/service/privatelinkendpointservice/resource.go | Added computed port_mapping_enabled field to resource schema and read logic for GCP |
| internal/service/privatelinkendpointservice/data_source.go | Added computed port_mapping_enabled field to data source schema and read logic for GCP |
| internal/service/privatelinkendpoint/resource_test.go | Added test coverage for port mapping enabled/disabled scenarios and updated existing test assertions |
| docs/resources/privatelink_endpoint.md | Documented the new port_mapping_enabled attribute |
| docs/resources/privatelink_endpoint_service.md | Documented the new port_mapping_enabled attribute |
| docs/data-sources/privatelink_endpoint.md | Documented the new port_mapping_enabled attribute |
| docs/data-sources/privatelink_endpoint_service.md | Documented the new port_mapping_enabled attribute |
| .changelog/4017.txt | Added changelog entries for the enhancement |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| 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" |
Copilot
AI
Dec 22, 2025
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.
The project name references 'feature-flag-enabled' but this test is for explicitly enabled port mapping, not feature flag testing. Consider renaming to something like 'test-acc-tf-p-gcp-port-mapping-enabled' for clarity.
| 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)
lizo-mdb
left a comment
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.
LGTM
| Accepted values are: [AWS regions](https://docs.atlas.mongodb.com/reference/amazon-aws/#amazon-aws), [AZURE regions](https://docs.atlas.mongodb.com/reference/microsoft-azure/#microsoft-azure) and [GCP regions](https://docs.atlas.mongodb.com/reference/google-gcp/#std-label-google-gcp) | ||
| * `timeouts`- (Optional) The duration of time to wait for Private Endpoint to be created or deleted. The timeout value is defined by a signed sequence of decimal numbers with a time unit suffix such as: `1h45m`, `300s`, `10m`, etc. The valid time units are: `ns`, `us` (or `µs`), `ms`, `s`, `m`, `h`. The default timeout for Private Endpoint create & delete is `1h`. Learn more about timeouts [here](https://www.terraform.io/plugin/sdkv2/resources/retries-and-customizable-timeouts). | ||
| * `delete_on_create_timeout`- (Optional) Indicates whether to delete the resource being created if a timeout is reached when waiting for completion. When set to `true` and timeout occurs, it triggers the deletion and returns immediately without waiting for deletion to complete. When set to `false`, the timeout will not trigger resource deletion. If you suspect a transient error when the value is `true`, wait before retrying to allow resource deletion to finish. Default is `true`. | ||
| * `port_mapping_enabled` - (Optional) Flag that indicates whether this endpoint service uses PSC port-mapping. |
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)
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.
| ), | ||
| }, | ||
| { | ||
| ResourceName: resourceName, |
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.
nice to add the import step 👍
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.
it's just git displaying the diff wrong as that was already there but it just got pushed back to the end :)
| resource.TestCheckResourceAttrSet(resourceName, "region"), | ||
| resource.TestCheckResourceAttr(resourceName, "provider_name", providerName), | ||
| resource.TestCheckResourceAttr(resourceName, "region", region), | ||
| resource.TestCheckResourceAttr(resourceName, "port_mapping_enabled", "true"), |
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).
oarbusi
left a comment
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.
LGTM
|
This PR has gone 7 days without any activity and meets the project’s definition of "stale". This will be auto-closed if there is no new activity over the next 7 days. If the issue is still relevant and active, you can simply comment with a "bump" to keep it open, or add the label "not_stale". Thanks for keeping our repository healthy! |
Description
This PR implements support for the new architecture of GCP Private Service Connect (PSC). For this, we add a new attribute named
port_mapping_enabledto bothmongodbatlas_privatelink_endpointandmongodbatlas_privatelink_endpoint_serviceresources and data sources.Changes
Added
port_mapping_enabledfield to:mongodbatlas_privatelink_endpointresource (optional, ForceNew)mongodbatlas_privatelink_endpointdata source (computed)mongodbatlas_privatelink_endpoint_serviceresource (computed)mongodbatlas_privatelink_endpoint_servicedata source (computed)Updated documentation for all affected resources and data sources
Added test coverage:
Link to any related issue(s): CLOUDP-363082
Type of change:
Required Checklist:
Further comments