-
Notifications
You must be signed in to change notification settings - Fork 16
Add egress_private_endpoint_domain_names resource #309
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
Add egress_private_endpoint_domain_names resource #309
Conversation
702f294 to
2aaa768
Compare
2aaa768 to
25e65c3
Compare
7658e8d to
fc98588
Compare
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.
Minor docs suggestions, otherwise LGTM
| ### Required | ||
|
|
||
| - `cluster_id` (String) cluster_id identifies the cluster to which this egress private endpoint applies | ||
| - `domain_names` (List of String) domain_names are the domain names to associate with the egress private endpoint. |
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.
| - `domain_names` (List of String) domain_names are the domain names to associate with the egress private endpoint. | |
| - `domain_names` (List of Strings) domain_names contains a list of domain names to associate with the egress private endpoint. |
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.
List of String is auto generated, so I can't change it. I've made the suggested wording change though!
bec53cf to
77eb382
Compare
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 code is all looking good to me here. I was wondering why we need a separate resource for this? It seems like we could just add the domain_names list to the cockroach_egress_private_endpoint resource. If there is a reason we should probably document it somewhere. Maybe just the commit message.
|
Good question! The expected use case for this feature includes a bit of sequencing:
If domain names were configured in |
|
The example in the docs describes how this works with Confluent Cloud: resource "confluent_private_link_attachment" "main" {
cloud = "AWS"
region = "us-east-1"
display_name = "main-private-link-attachment"
environment {
id = confluent_environment.main.id
}
}
resource "cockroach_egress_private_endpoint" "confluent_cloud" {
cluster_id = cockroach_cluster.my_cluster.id
region = "us-east-1"
target_service_type = "PRIVATE_SERVICE"
target_service_identifier = confluent_private_link_attachment.main.aws.vpc_endpoint_service_name
}
resource "confluent_private_link_attachment_connection" "cockroach_cloud" {
display_name = "cockroach-cloud-access-point"
environment {
id = confluent_environment.main.id
}
aws {
vpc_endpoint_id = cockroach_egress_private_endpoint.confluent_cloud.endpoint_connection_id
}
private_link_attachment {
id = confluent_private_link_attachment.main.id
}
}
resource "cockroach_egress_private_endpoint_domain_names" "confluent_cloud" {
cluster_id = cockroach_cluster.my_cluster.id
endpoint_id = cockroach_egress_private_endpoint.confluent_cloud.id
domain_names = [
"*.us-east-1.aws.private.confluent.cloud"
]
depends_on = [
confluent_private_link_attachment_connection.cockroach_cloud
]
} |
This commit implements custom domain names for an egress private endpoint as a Terraform resource. See below for official Cockroach Cloud documentation for the feature. This feature is currently in limited access and is not yet available to all customers. This resource is separate from the existing egress private endpoint resource for sequencing reasons: in order for domain names to be successfully added to an egress private endpoint, the endpoint must be in the AVAILABLE state. However, it is possible that other resources might be needed to make the endpoint AVAILABLE. Therefore, we have two resources, to allow Terraform to create one separately from the other. https://www.cockroachlabs.com/docs/cockroachcloud/egress-private-endpoints.html
77eb382 to
7906828
Compare
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.
Perf, thanks for the explanation.
This commit implements custom domain names for an egress private endpoint as a Terraform resource. See below for official Cockroach Cloud documentation for the feature. This feature is currently in limited access and is not yet available to all customers.
This resource is separate from the existing egress private endpoint resource for sequencing reasons: in order for domain names to be successfully added to an egress private endpoint, the endpoint must be in the AVAILABLE state. However, it is possible that other resources might be needed to make the endpoint AVAILABLE. Therefore, we have two resources, to allow Terraform to create one separately from the other.
https://www.cockroachlabs.com/docs/cockroachcloud/egress-private-endpoints.html
This PR is waiting for two things:
Commit checklist
make generate)