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: additional checks before using S2A #2103

Merged
merged 1 commit into from Aug 8, 2023

Conversation

xmenxk
Copy link
Contributor

@xmenxk xmenxk commented Aug 7, 2023

add a couple more checks before using S2A for mtls

  1. make sure DefaultMTLSEndpoint is not empty
  2. skip s2a if custom http client is provided, because there's no guarantee the client passed in is mtls-capable

go.work.sum Outdated
@@ -1,13 +1,286 @@
cloud.google.com/go v0.110.0 h1:Zc8gqp3+a9/Eyph2KDmcGaPtbKRIoqq4YTlL4NMD0Ys=
cloud.google.com/go v0.110.2 h1:sdFPBr6xG9/wkBbfhmUz/JmZC7X6LavQgcrVINrKiVA=
cloud.google.com/go v0.110.4 h1:1JYyxKMN9hd5dR2MYTPWkGUgcoxVVhg0LKNKEo0qvmk=
cloud.google.com/go v0.110.4/go.mod h1:+EYjdK8e5RME/VY/qLCAtuyALQ9q67dvuum8i+H5xsI=

Choose a reason for hiding this comment

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

I am not familiar with this code base, but should this file be updated separately?

Are all of these dependencies being updated?

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'm asking someone familiar with the repo to take a look

Copy link
Contributor

Choose a reason for hiding this comment

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

Can you just revert this, update your branch and double check that everything is in order? I might need to just manually update this on main so that you don't need to change this.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks @noahdietz I reverted changes to this file. PTAL

internal/cba.go Outdated Show resolved Hide resolved
internal/cba.go Outdated Show resolved Hide resolved
@clundin25
Copy link

@xmenxk can you fix the conventional commit error?

@noahdietz noahdietz added the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Aug 8, 2023
@kokoro-team kokoro-team removed the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Aug 8, 2023
@noahdietz noahdietz requested a review from codyoss August 8, 2023 19:58
@codyoss codyoss changed the title additional checks before using S2A feat: additional checks before using S2A Aug 8, 2023
@codyoss codyoss merged commit c62e5c6 into googleapis:main Aug 8, 2023
5 checks passed
@googleapis googleapis deleted a comment from rwuorinen81 Aug 8, 2023
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.

None yet

5 participants