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.

Sign up for GitHub

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

Jump to bottom

feat: allow DirectPath by default + update integration tests #3031

Merged
merged 10 commits into from Jun 30, 2021
Merged

feat: allow DirectPath by default + update integration tests #3031

merged 10 commits into from Jun 30, 2021

Conversation

igorbernstein2
Copy link
Collaborator

  • DirectPath is now opt-out by setting google.bigtable.direct.path.allowed to false
  • Integration tests can now verify that they are running over the desired mode by specifying "bigtable.connection-mode" system property
  • Integration tests update their user agent to signal membership in allow/deny lists
  • DirectPathFallbackIT now supports IPv4

@igorbernstein2 igorbernstein2 requested a review from a team as a code owner June 23, 2021 20:38
@product-auto-label product-auto-label bot added the api: bigtable Issues related to the googleapis/java-bigtable-hbase API. label Jun 23, 2021
@google-cla google-cla bot added the cla: yes This human has signed the Contributor License Agreement. label Jun 23, 2021
@igorbernstein2
feat: allow DirectPath by default + update integration tests
414e5f7 
* DirectPath is now opt-out by setting google.bigtable.direct.path.allowed to false
* Integration tests can now verify that they are running over the desired mode by specifying "bigtable.connection-mode" system property
* Integration tests update their user agent to signal membership in allow/deny lists
* DirectPathFallbackIT now supports IPv4
coryan
coryan reviewed Jun 28, 2021
View reviewed changes
...parent/bigtable-client-core/src/main/java/com/google/cloud/bigtable/util/DirectPathUtil.java Outdated
public Boolean get() {
String osName = System.getProperty("os.name");
if ("Linux".equals(osName)) {
String cmd = "cat /sys/class/dmi/id/product_name";
Copy link
Member

Choose a reason for hiding this comment

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

drive-by:

why cat and then read the output? That file (if it exists) /sys/class/dmi/id/produc_name can be opened like any other file.

Also: I think this won't work on GKE, the containers in that case run without the necessary permissions to read that file.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

These are excellent points. I was trying to keep consistent with all of the other java clients:
googleapis/gax-java#1250

It seems like that file is accessible in gke:
https://github.com/googleapis/gax-java/pull/1250/files/cf494cef60f4d9ef265e5b4690ebe3cb49537893#r589861385

I'll update both libraries to simply read the file w/o exec

Copy link
Member

Choose a reason for hiding this comment

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

Ooops, my recollection was wrong regarding GKE, it was Cloud Run and gVisor, but that may have changed again:

googleapis/google-cloud-cpp#3959

For these purposes it might not matter because using Direct Path with Cloud Run might not be that useful or important. If you are ever interested in supporting Windows, I wrote the code a long time ago for that:

https://github.com/grpc/grpc/blob/master/src/core/lib/security/credentials/alts/check_gcp_environment_windows.cc

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Thanks for the pointer. Though registry access from java is a tough pill to swallow.

@igorbernstein2 igorbernstein2 merged commit 7c33b14 into googleapis:bigtable-1.x Jun 30, 2021
@igorbernstein2 igorbernstein2 deleted the directpath2-1x branch June 30, 2021 14:06
@mutianf mutianf added the release-please:force-run To run release-please label Jun 30, 2021
@release-please release-please bot removed the release-please:force-run To run release-please label Jun 30, 2021
gcf-merge-on-green bot pushed a commit that referenced this pull request Jun 30, 2021
@release-please
chore(bigtable-1.x): release 1.22.0 (#3048)
1515605 
🤖 I have created a release \*beep\* \*boop\*
---
## [1.22.0](https://www.github.com/googleapis/java-bigtable-hbase/compare/v1.21.1...v1.22.0) (2021-06-30)


### Features

* allow DirectPath by default + update integration tests ([#3031](https://www.github.com/googleapis/java-bigtable-hbase/issues/3031)) ([7c33b14](https://www.github.com/googleapis/java-bigtable-hbase/commit/7c33b14362614be0f2d3ad0b5cc95fa70f6a9add))
---


This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please).
mutianf pushed a commit to mutianf/java-bigtable-hbase that referenced this pull request Sep 20, 2022
@igorbernstein2 @mutianf
feat: allow DirectPath by default + update integration tests (googlea…
4e71696 
…pis#3031)

* chore: delete deprecate connection methods

* feat: allow DirectPath by default + update integration tests

* DirectPath is now opt-out by setting google.bigtable.direct.path.allowed to false
* Integration tests can now verify that they are running over the desired mode by specifying "bigtable.connection-mode" system property
* Integration tests update their user agent to signal membership in allow/deny lists
* DirectPathFallbackIT now supports IPv4

* disable dns service config lookup

* format

* format

* copyright

* fix directpath regex

* skip shell exec for reading files

* fix direct path credential probing
mutianf pushed a commit to mutianf/java-bigtable-hbase that referenced this pull request Sep 20, 2022
@release-please @mutianf
chore(bigtable-1.x): release 1.22.0 (googleapis#3048)
06e53fd 
🤖 I have created a release \*beep\* \*boop\*
---
## [1.22.0](https://www.github.com/googleapis/java-bigtable-hbase/compare/v1.21.1...v1.22.0) (2021-06-30)


### Features

* allow DirectPath by default + update integration tests ([googleapis#3031](https://www.github.com/googleapis/java-bigtable-hbase/issues/3031)) ([7c33b14](https://www.github.com/googleapis/java-bigtable-hbase/commit/7c33b14362614be0f2d3ad0b5cc95fa70f6a9add))
---


This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@coryan coryan coryan left review comments

@kolea2 kolea2 kolea2 left review comments

@mutianf mutianf mutianf approved these changes

Assignees
No one assigned
Labels
api: bigtable Issues related to the googleapis/java-bigtable-hbase API. cla: yes This human has signed the Contributor License Agreement.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@igorbernstein2 @coryan @mutianf @kolea2