Skip to content

Add validation for project-id returned by metadata server - #2868

Merged
yihanzhen merged 9 commits into
googleapis:masterfrom
yihanzhen:fix-project-id-from-metadata-server
Feb 13, 2018
Merged

Add validation for project-id returned by metadata server#2868
yihanzhen merged 9 commits into
googleapis:masterfrom
yihanzhen:fix-project-id-from-metadata-server

Conversation

@yihanzhen

@yihanzhenyihanzhen commented Feb 6, 2018

Copy link
Copy Markdown
Contributor

Check whether the HttpResponse from metadata server contains a field Metadata-Flavor in its header when requesting project-id.
If yes, the response will be parsed as the project-id;
If not, the response will be considered invalid and ServiceOptions will continue to look for project-id elsewhere.

@yihanzhen
yihanzhen requested a review from pongadFebruary 6, 2018 18:58
@googlebotgooglebot added the cla: yes This human has signed the Contributor License Agreement. label Feb 6, 2018
@yihanzhen

yihanzhen commented Feb 6, 2018

Copy link
Copy Markdown
ContributorAuthor

This fixes #2440

wsh
wsh suggested changes Feb 6, 2018
View reviewed changes

@wshwsh left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Please double check the null safety of the changes to ServiceOptions!

Comment threadgoogle-cloud-core/src/test/java/com/google/cloud/ServiceOptionsTest.java Outdated
}

@Test
public void testResponseHeaderContainsMetaDataFlavor() throws Exception {

This comment was marked as spam.

Sign in to view

Comment threadgoogle-cloud-core/src/test/java/com/google/cloud/ServiceOptionsTest.java
assertNull(ServiceOptions.getServiceAccountProjectId(credentialsFile.getPath()));
}

@Test

This comment was marked as spam.

Sign in to view

Comment threadgoogle-cloud-core/src/main/java/com/google/cloud/ServiceOptions.java Outdated
@InternalApi("Visible for testing")
static boolean headerContainsMetadataFlavor(HttpResponse response) {
return response.getHeaders()
.getFirstHeaderStringValue("Metadata-Flavor").equals("Google");

This comment was marked as spam.

Sign in to view

yihanzhen
yihanzhen commented Feb 6, 2018
View reviewed changes
Comment threadgoogle-cloud-core/src/test/java/com/google/cloud/ServiceOptionsTest.java Outdated
HttpTransport mockHttpTransport = Mockito.mock(MockHttpTransport.class);
MockLowLevelHttpResponse mockResponse = new MockLowLevelHttpResponse();
mockResponse.addHeader("Metadata-Flavor", "Google");
Mockito.when(mockHttpTransport.buildRequest(Matchers.anyString(), Matchers.anyString()).execute()).thenReturn(mockResponse);

This comment was marked as spam.

Sign in to view

This comment was marked as spam.

Sign in to view

wsh
wsh suggested changes Feb 6, 2018
View reviewed changes
Comment threadgoogle-cloud-core/pom.xml Outdated
<dependency>
<groupId>org.mockito</groupId>
<artifactId>mockito-all</artifactId>
<version>1.9.5</version>

This comment was marked as spam.

Sign in to view

This comment was marked as spam.

Sign in to view

Comment threadgoogle-cloud-core/src/main/java/com/google/cloud/ServiceOptions.java Outdated
return response.getHeaders()
.getFirstHeaderStringValue("Metadata-Flavor").equals("Google");
String metadataFlavorValue = response.getHeaders().getFirstHeaderStringValue("Metadata-Flavor");
return metadataFlavorValue != null && metadataFlavorValue.equals("Google");

This comment was marked as spam.

Sign in to view

Comment threadgoogle-cloud-core/src/test/java/com/google/cloud/ServiceOptionsTest.java Outdated
HttpTransport mockHttpTransport = Mockito.mock(MockHttpTransport.class);
MockLowLevelHttpResponse mockResponse = new MockLowLevelHttpResponse();
mockResponse.addHeader("Metadata-Flavor", "Google");
Mockito.when(mockHttpTransport.buildRequest(Matchers.anyString(), Matchers.anyString()).execute()).thenReturn(mockResponse);

This comment was marked as spam.

Sign in to view

pongad
pongad reviewed Feb 7, 2018
View reviewed changes
Comment threadgoogle-cloud-core/pom.xml Outdated
<dependency>
<groupId>org.mockito</groupId>
<artifactId>mockito-all</artifactId>
<version>1.9.5</version>

This comment was marked as spam.

Sign in to view

Comment threadgoogle-cloud-core/src/main/java/com/google/cloud/ServiceOptions.java Outdated
@InternalApi("Visible for testing")
static boolean headerContainsMetadataFlavor(HttpResponse response) {
String metadataFlavorValue = response.getHeaders().getFirstHeaderStringValue("Metadata-Flavor");
return metadataFlavorValue != null && metadataFlavorValue.equals("Google");

This comment was marked as spam.

Sign in to view

@yihanzhen
Add null check in ServiceOptions
c358a00
Add a null check test in ServiceOptionsTest
yihanzhen
yihanzhen commented Feb 7, 2018
View reviewed changes
Comment threadgoogle-cloud-core/src/test/java/com/google/cloud/ServiceOptionsTest.java Outdated
assertThat(ServiceOptions.headerContainsMetadataFlavor(httpResponse)).isTrue();
}

private HttpResponse createHttpResponseWithHeader(final Map<String, String> headers) throws Exception {

This comment was marked as spam.

Sign in to view

This comment was marked as spam.

Sign in to view

@yihanzhen

yihanzhen commented Feb 7, 2018

Copy link
Copy Markdown
ContributorAuthor

Can you take a look? @wsh@pongad Thanks!

wsh
wsh suggested changes Feb 7, 2018
View reviewed changes
Comment threadgoogle-cloud-core/src/main/java/com/google/cloud/ServiceOptions.java Outdated
static boolean headerContainsMetadataFlavor(HttpResponse response) {
String metadataFlavorValue = response.getHeaders().getFirstHeaderStringValue("Metadata-Flavor");
return metadataFlavorValue != null && metadataFlavorValue.equals("Google");
return "Google".equals(response.getHeaders().getFirstHeaderStringValue("Metadata-Flavor"));

This comment was marked as spam.

Sign in to view

Comment threadgoogle-cloud-core/src/test/java/com/google/cloud/ServiceOptionsTest.java

headers.put("Metadata-Flavor", "Google");
httpResponse = createHttpResponseWithHeader(headers);
assertThat(ServiceOptions.headerContainsMetadataFlavor(httpResponse)).isTrue();

This comment was marked as spam.

Sign in to view

Comment threadgoogle-cloud-core/src/test/java/com/google/cloud/ServiceOptionsTest.java Outdated
assertThat(ServiceOptions.headerContainsMetadataFlavor(httpResponse)).isTrue();
}

private HttpResponse createHttpResponseWithHeader(final Map<String, String> headers) throws Exception {

This comment was marked as spam.

Sign in to view

@yihanzhen

yihanzhen commented Feb 7, 2018

Copy link
Copy Markdown
ContributorAuthor

Another look? @wsh

wsh
wsh approved these changes Feb 7, 2018
View reviewed changes
Comment threadgoogle-cloud-core/src/test/java/com/google/cloud/ServiceOptionsTest.java
}

@Test
public void testResponseHeaderDoesNotContainMetaDataFlavor() throws Exception {

This comment was marked as spam.

Sign in to view

Comment threadgoogle-cloud-core/src/test/java/com/google/cloud/ServiceOptionsTest.java
public LowLevelHttpResponse execute() throws IOException {
MockLowLevelHttpResponse response = new MockLowLevelHttpResponse();
for (Map.Entry<String, String> entry : headers.entrySet()) {
for (Map.Entry<String, String> entry : headers.entries()) {

This comment was marked as spam.

Sign in to view

This comment was marked as spam.

Sign in to view

This comment was marked as spam.

Sign in to view

pongad
pongad approved these changes Feb 13, 2018
View reviewed changes

@pongadpongad left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

IT failures OK, they usually don't run on PRs anyway.

@yihanzhen
yihanzhen merged commit f2a2387 into googleapis:masterFeb 13, 2018
chingor13 pushed a commit that referenced this pull request Feb 20, 2026
@cloud-java-bot
chore: Update generation configuration at Sat Feb 15 02:23:07 UTC 2025 (
69833d1
#2868)
suztomo pushed a commit that referenced this pull request Mar 9, 2026
@renovate-bot
deps: update dependency org.graalvm.buildtools:junit-platform-native …
d01031c
…to v0.9.26 (#2868)
chingor13 pushed a commit that referenced this pull request Mar 24, 2026
@renovate-bot
deps: update dependency org.junit.vintage:junit-vintage-engine to v5.…
030d281
…10.2 (#2868)
chingor13 pushed a commit that referenced this pull request Mar 24, 2026
@release-please
chore(main): release 6.60.1 (#2918)
81293aa
🤖 I have created a release *beep* *boop*
---
## [6.60.1](https://togithub.com/googleapis/java-spanner/compare/v6.60.0...v6.60.1) (2024-02-23)
### Dependencies
* Update dependency com.google.cloud:google-cloud-monitoring to v3.37.0 ([#2920](https://togithub.com/googleapis/java-spanner/issues/2920)) ([c2115a6](https://togithub.com/googleapis/java-spanner/commit/c2115a698b00f2dce810b7a48ad7b722d085200c))
* Update dependency org.graalvm.buildtools:native-maven-plugin to v0.10.0 ([#2861](https://togithub.com/googleapis/java-spanner/issues/2861)) ([d32adc3](https://togithub.com/googleapis/java-spanner/commit/d32adc3587f388464f3f5c5e42ecd94b9b49bb2f))
* Update dependency org.graalvm.buildtools:native-maven-plugin to v0.10.1 ([#2919](https://togithub.com/googleapis/java-spanner/issues/2919)) ([cbd6629](https://togithub.com/googleapis/java-spanner/commit/cbd66293b45aac7b6d3c6951af2fd1b3880839e9))
* Update dependency org.json:json to v20240205 ([#2913](https://togithub.com/googleapis/java-spanner/issues/2913)) ([98fa243](https://togithub.com/googleapis/java-spanner/commit/98fa243025b624b397a4589cca6d94934496d011))
* Update dependency org.junit.vintage:junit-vintage-engine to v5.10.2 ([#2868](https://togithub.com/googleapis/java-spanner/issues/2868)) ([030d281](https://togithub.com/googleapis/java-spanner/commit/030d281a120cfc56a628e19dd4cb01f986f283ab))
* Update opentelemetry.version to v1.35.0 ([#2902](https://togithub.com/googleapis/java-spanner/issues/2902)) ([f2cdf12](https://togithub.com/googleapis/java-spanner/commit/f2cdf126be9cd3fc1741a9cf2d29041d34a372a9))
---
This PR was generated with [Release Please](https://togithub.com/googleapis/release-please). See [documentation](https://togithub.com/googleapis/release-please#release-please).
chingor13 pushed a commit that referenced this pull request Mar 30, 2026
@renovate-bot
chore(deps): update storage release dependencies to v2.47.0 (#2868)
2abf71e
rahul2393 pushed a commit to akash329d/google-cloud-java that referenced this pull request May 21, 2026
@igorbernstein2
chore: increase severity of session creation exhaustion debug tag (go…
a9ecf66
…ogleapis#2868)
Change-Id: I1a4f684c2cb939fbf1e1ec4705e360bfaeeb4a46
Thank you for opening a Pull Request! Before submitting your PR, there are a few things you can do to make sure it goes smoothly:
- [ ] Make sure to open an issue as a [bug/issue](https://togithub.com/googleapis/java-bigtable/issues/new/choose) before writing your code! That way we can discuss the change, evaluate designs, and agree on the general idea
- [ ] Ensure the tests and linter pass
- [ ] Code coverage does not decrease (if any source code was changed)
- [ ] Appropriate docs were updated (if necessary)
- [ ] Rollback plan is reviewed and LGTMed
- [ ] All new data plane features have a completed end to end testing plan
Fixes #<issue_number_goes_here> ☕️
If you write sample code, please follow the [samples format](
https://togithub.com/GoogleCloudPlatform/java-docs-samples/blob/main/SAMPLE_FORMAT.md).
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

2 more reviewers
@wshwshwsh approved these changes
@pongadpongadpongad approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

cla: yesThis human has signed the Contributor License Agreement.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@yihanzhen@wsh@pongad@googlebot