Skip to content
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

feat(vertexai): Make it possible to use Private Service Connect in Vertex AI Index Endpoint #8851

Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
50 changes: 49 additions & 1 deletion mmv1/products/vertexai/IndexEndpoint.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -53,14 +53,18 @@ examples:
network_name: "network-name"
test_vars_overrides:
network_name: 'acctest.BootstrapSharedTestNetwork(t, "vertex-ai-index-endpoint")'
- !ruby/object:Provider::Terraform::Examples
name: "vertex_ai_index_endpoint_with_psc"
primary_resource_id: "index_endpoint"
test_vars_overrides:
network_name: 'acctest.BootstrapSharedTestNetwork(t, "vertex-ai-index-endpoint")'
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It doesn't look like that network_name is being used anywhere in the test. Feel free to remove it if it's not needed

Copy link
Contributor Author

@shotarok shotarok Sep 6, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I confirmed an acceptance test succeeded after removing it. Therefore, I've deleted it in 1a0f1ea.

parameters:
- !ruby/object:Api::Type::String
name: region
description: The region of the index endpoint. eg us-central1
url_param_only: true
immutable: true
properties:
# Intentionally deployedIndexes[] is not included because it's an output-only field and another terraform resource will manage a deployed index.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Any reason we're removing this comment?

Copy link
Contributor Author

@shotarok shotarok Sep 6, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, I initially defined deployedIndexes in this PR, but I reverted it because of #8851 (comment). At that time, I forgot to revert the comment. I'll restore it. Thank you for the catch!

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@shuyama1 I fixed it in 40903e2 👍 When you have time, could you approve CI again? Thanks

- !ruby/object:Api::Type::String
name: 'name'
description: The resource name of the Index.
Expand Down Expand Up @@ -97,3 +101,47 @@ properties:
[Format](https://cloud.google.com/compute/docs/reference/rest/v1/networks/insert): `projects/{project}/global/networks/{network}`.
Where `{project}` is a project number, as in `12345`, and `{network}` is network name.
immutable: true
- !ruby/object:Api::Type::NestedObject
name: privateServiceConnectConfig
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We'll likely need to have default_from_api here to prevent permadiff since we always write this block back

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I added it in f3aecd4.

immutable: true
description: |-
Optional. Configuration for private service connect. `network` and `privateServiceConnectConfig` are mutually exclusive.
shotarok marked this conversation as resolved.
Show resolved Hide resolved
properties:
- !ruby/object:Api::Type::Boolean
name: enablePrivateServiceConnect
description: If set to true, the IndexEndpoint is created without private service access.
- !ruby/object:Api::Type::Array
name: projectAllowlist
description: A list of Projects from which the forwarding rule will target the service attachment.
item_type: Api::Type::String
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are these two subfields also immutable? If so, I think we need to add immutable as well. If not, it's be nice that we add an update test on it.

Plus, can we make at least one of the subfields required. We 'd try to avoid users from sending empty object.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The subfields were immutable although I tested a PATCH request. Based on your advice, I added immutable: and required: in debd2d1.

- !ruby/object:Api::Type::Array
name: deployedIndexes
description: Output only. The indexes deployed in this endpoint.
output: true
item_type: !ruby/object:Api::Type::NestedObject
properties:
- !ruby/object:Api::Type::String
name: id
output: true
description: The user specified ID of the DeployedIndex. The ID can be up to 128 characters long and must start with a letter and only contain letters, numbers, and underscores.
- !ruby/object:Api::Type::String
name: index
output: true
description: The name of the Index this is the deployment of. We may refer to this Index as the DeployedIndex's "original" Index.
- !ruby/object:Api::Type::String
name: displayName
output: true
description: The display name of the DeployedIndex. If not provided upon creation, the Index's displayName is used.
- !ruby/object:Api::Type::NestedObject
name: privateEndpoints
output: true
description: Provides paths for users to send requests directly to the deployed index services running on Cloud via private services access.
properties:
- !ruby/object:Api::Type::String
name: matchGrpcAddress
output: true
description: The ip address used to send match gRPC requests.
- !ruby/object:Api::Type::String
name: serviceAttachment
output: true
description: The name of the service attachment resource. Populated if private service connect is enabled.
shotarok marked this conversation as resolved.
Show resolved Hide resolved
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
resource "google_vertex_ai_index_endpoint" "<%= ctx[:primary_resource_id] %>" {
display_name = "sample-endpoint"
description = "A sample vertex endpoint"
region = "us-central1"
labels = {
label-one = "value-one"
}

private_service_connect_config {
enable_private_service_connect = true
project_allowlist = [
data.google_project.project.number,
]
}
}

data "google_project" "project" {}
Loading