Skip to content

Support custom-endpoint in sidecar mounter#1316

Draft
uriel-guzman wants to merge 1 commit intoGoogleCloudPlatform:mainfrom
uriel-guzman:tpc-support
Draft

Support custom-endpoint in sidecar mounter#1316
uriel-guzman wants to merge 1 commit intoGoogleCloudPlatform:mainfrom
uriel-guzman:tpc-support

Conversation

@uriel-guzman
Copy link
Copy Markdown
Collaborator

What type of PR is this?

Uncomment only one /kind <> line, hit enter to put that in a new line, and remove leading whitespaces from that line:

/kind api-change
/kind bug
/kind cleanup
/kind design
/kind documentation
/kind failing-test
/kind feature
/kind flake

What this PR does / why we need it:

Which issue(s) this PR fixes:

Fixes #

Special notes for your reviewer:

Does this PR introduce a user-facing change?:


@google-oss-prow
Copy link
Copy Markdown

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

@google-oss-prow
Copy link
Copy Markdown

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: uriel-guzman

The full list of commands accepted by this bot can be found here.

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

Copy link
Copy Markdown

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request introduces support for a custom endpoint in the GCS Fuse CSI driver sidecar mounter. Changes include updating the ServiceManager interface and its implementations to accept a customEndpoint parameter, which is then used when initializing the storage client. A review comment highlights that the custom endpoint should also be applied to the control client to ensure consistency during the pre-mount phase.

Comment on lines +467 to +469
if customEndpoint != "" {
storageOpts = append(storageOpts, option.WithEndpoint(customEndpoint))
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

high

The customEndpoint is currently only applied to the storageClient. However, the controlClient (initialized at line 476) is used for the CheckBucketExists call during the sidecar's pre-mount phase. If a custom endpoint is required for storage access, it is highly likely that the Storage Control API also needs to be directed to a corresponding custom endpoint. Failing to set this will cause the CheckBucketExists call to attempt to reach the default Google Cloud endpoints, which may fail or be blocked in restricted environments.

Suggested change
if customEndpoint != "" {
storageOpts = append(storageOpts, option.WithEndpoint(customEndpoint))
}
if customEndpoint != "" {
storageOpts = append(storageOpts, option.WithEndpoint(customEndpoint))
controlOpts = append(controlOpts, option.WithEndpoint(customEndpoint))
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant