-
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! |
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
Copilot reviewed 18 out of 18 changed files in this pull request and generated 3 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| private_link_id = mongodbatlas_privatelink_endpoint.test.private_link_id | ||
| provider_name = "GCP" | ||
| endpoint_service_id = google_compute_network.default.name | ||
| endpoint_service_id = "the-endpoint-group-name" |
Copilot
AI
Jan 22, 2026
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 placeholder value 'the-endpoint-group-name' is not descriptive enough for example code. Consider using a more meaningful placeholder like 'my-endpoint-group' or adding a comment explaining what value should be used here.
|
|
||
| locals { | ||
| endpoint_service_id = google_compute_network.default.name | ||
| endpoint_service_id = "the-endpoint-group-name" |
Copilot
AI
Jan 22, 2026
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.
Same placeholder value as line 59. These should match the actual resource reference (like it was before: google_compute_network.default.name) or be clearly documented as placeholders.
| endpoint_service_id = "the-endpoint-group-name" | |
| endpoint_service_id = mongodbatlas_privatelink_endpoint_service.test.endpoint_service_id |
| * `private_link_id` - (Required) Unique identifier of the private endpoint service for which you want to retrieve a private endpoint. | ||
| * `endpoint_service_id` - (Required) Unique identifier of the `AWS` or `AZURE` or `GCP` resource. | ||
| * `provider_name` - (Required) Cloud provider for which you want to create a private endpoint. Atlas accepts `AWS` or `AZURE` or `GCP`. | ||
| * `endpoint_service_id` - (Required) Unique identifier of the interface endpoint you created in your VPC with the `AWS`, `AZURE`, or `GCP` resource. For GCP legacy architecture, this can be any identifier string. For GCP port-based architecture (when `port_mapping_enabled = true` on the endpoint resource), this should be the forwarding rule name. |
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.
why should? do we need any confirmation to change the wording?
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.
thank you! no, this must be the forwarding rule name, I will change the wording
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.
updated. thank you!
| * `private_endpoint_ip_address` - (Optional) Private IP address of the private endpoint network interface you created in your Azure VNet. Only for `AZURE`. | ||
| * `gcp_project_id` - (Optional) Unique identifier of the GCP project in which you created your endpoints. Only for `GCP`. | ||
| * `endpoints` - (Optional) Collection of individual private endpoints that comprise your endpoint group. Only for `GCP`. See below. | ||
| * `private_endpoint_ip_address` - (Optional) Private IP address of the private endpoint network interface. **Required for `AZURE`.** For GCP port-based architecture (when `port_mapping_enabled = true` on the endpoint resource), this is required and should be the IP address of the forwarding rule. For GCP legacy architecture, this is not used. |
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.
remove should if possible
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.
updated. thank you!
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
Copilot reviewed 18 out of 18 changed files in this pull request and generated 3 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| resource/mongodbatlas_privatelink_endpoint_service: Adds `port_mapping_enabled` attribute | ||
| ``` | ||
|
|
||
| ```release-note:enhancement |
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 only other resource and ds in Private Endpoint Services group is https://registry.terraform.io/providers/mongodb/mongodbatlas/latest/docs/resources/private_endpoint_regional_mode, can you confirm nothing changes there?
|
|
||
| ## Example with GCP (Port-Based Architecture) | ||
|
|
||
| The new port-based architecture uses port mapping to reduce resource provisioning. Unlike the legacy architecture that requires dedicated resources for each Atlas node, the new design uses a single set of resources to support up to 1000 nodes through a port mapping network endpoint group (NEG), enabling direct targeting of specific nodes using only one customer IP address. Enable it by setting `port_mapping_enabled = true` on the endpoint resource. |
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.
similarly to calling port-based architecture to the new architecture, don't know if we can use a different name for old one instead of legacy
| "port_mapping_enabled": { | ||
| Type: schema.TypeBool, | ||
| Optional: true, | ||
| ForceNew: 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.
discussed offline, to remove ForceNew and throw an error in Update func
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.
updated. thank you!
| Config: configWithPortMapping(orgID, projectName, providerName, region, true), | ||
| Check: resource.ComposeAggregateTestCheckFunc( | ||
| checkExists(resourceName), | ||
| resource.TestCheckResourceAttrSet(resourceName, "project_id"), |
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: consider using helper functions for checks
| }, | ||
| "port_mapping_enabled": { | ||
| Type: schema.TypeBool, | ||
| Computed: 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.
add description
| return diag.FromErr(fmt.Errorf(errorEndpointSetting, "aws_connection_status", endpointServiceID, err)) | ||
| } | ||
|
|
||
| if err := d.Set("interface_endpoint_id", serviceEndpoint.GetInterfaceEndpointId()); err != nil { |
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.
remove these sets, as they're provided by customer in the TF config
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.
updated. thank you!
| Type: schema.TypeString, | ||
| Computed: true, | ||
| }, | ||
| "endpoint_group_name": { |
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.
don't remove as it'll be a breaking change if some customers has it in the config, that will fail. I recommend to keep it, remove it from the documentation, and add a code comment saying that's it's not being used, maybe create a ticket to remove if TF 3.0.0
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.
updated. TY!
| } | ||
|
|
||
| func isGCPPortBasedArchitectureInput(providerName string, hasGCPProjectID, hasPrivateEndpointIP, hasEndpoints bool) bool { | ||
| return providerName == "GCP" && hasGCPProjectID && hasPrivateEndpointIP && !hasEndpoints |
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: being "GCP" we already know hasGCPProjectID is true so no needed
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.
similarly we've already checked that hasPrivateEndpointIP and hasEndpoints are the opposite in GCP so only need to check one
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.
updated. thank you!
| if !hasGCPProjectID { | ||
| return diag.FromErr(errors.New("`gcp_project_id` must be set for GCP")) | ||
| } | ||
| if (hasPrivateEndpointIP && hasEndpoints) || (!hasPrivateEndpointIP && !hasEndpoints) { |
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.
| if (hasPrivateEndpointIP && hasEndpoints) || (!hasPrivateEndpointIP && !hasEndpoints) { | |
| if (hasPrivateEndpointIP == hasEndpoints) { |
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.
updated. thank you!
| return providerName == "GCP" && hasGCPProjectID && hasPrivateEndpointIP && !hasEndpoints | ||
| } | ||
|
|
||
| func isGCPLegacyArchitectureInput(providerName string, hasGCPProjectID, hasPrivateEndpointIP, hasEndpoints bool) bool { |
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.
recommend to remove isGCPLegacyArchitectureInput and use !isGCPPortBasedArchitectureInput instead
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.
updated. thank you!
| } | ||
| if isGCPLegacyArchitectureInput(providerName, hasGCPProjectID, hasPrivateEndpointIP, hasEndpoints) { |
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.
| } | |
| if isGCPLegacyArchitectureInput(providerName, hasGCPProjectID, hasPrivateEndpointIP, hasEndpoints) { | |
| } else { |
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.
updated. thank you!
| return diag.FromErr(fmt.Errorf(errorEndpointSetting, "private_endpoint_resource_id", endpointServiceID, err)) | ||
| } | ||
|
|
||
| if strings.EqualFold(providerName, "azure") { |
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.
| if strings.EqualFold(providerName, "azure") { | |
| if providerName == constant.AZURE |
or even better with switch(provideName) {
case constant.AZURE:
...
case constant.GCP:
.....
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.
updated. thank you!
| return diag.FromErr(fmt.Errorf(errorEndpointSetting, "gcp_status", endpointServiceID, err)) | ||
| } | ||
|
|
||
| if privateEndpoint.GetPortMappingEnabled() && privateEndpoint.Endpoints != nil && len(*privateEndpoint.Endpoints) == 1 { |
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.
| if privateEndpoint.GetPortMappingEnabled() && privateEndpoint.Endpoints != nil && len(*privateEndpoint.Endpoints) == 1 { | |
| if privateEndpoint.GetPortMappingEnabled() && len(privateEndpoint.GetEndpoints()) == 1 { |
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.
updated. thank you!
…p-port-based-routing-implementation
…p-port-based-routing-implementation
Description
This PR implements support for the new GCP port-based architecture, which uses port mapping to reduce resource provisioning. Unlike the legacy architecture that requires dedicated resources for each Atlas node, the new design uses a single set of resources to support up to 1000 nodes through port mapping, enabling direct targeting of specific nodes using only one customer IP address.
Changes Overview
New Attributes
port_mapping_enabled(Optional, Computed)mongodbatlas_privatelink_endpointresource (Optional, settable)mongodbatlas_privatelink_endpointdata source (Computed, read-only)mongodbatlas_privatelink_endpoint_serviceresource (Computed, read-only)mongodbatlas_privatelink_endpoint_servicedata source (Computed, read-only)trueon the endpoint resource, enables the new port-based architecture for GCPfalse(legacy architecture)gcp_endpoint_status(Computed)mongodbatlas_privatelink_endpoint_serviceresource and data sourceport_mapping_enabled = true)INITIATING,AVAILABLE,FAILED,DELETINGReused Attributes
private_endpoint_ip_address(Optional)endpointsattributeendpoint_service_id(Required)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