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

Support for Google Application Default Credentials #8394

Merged

Conversation

fahadshamiinsta
Copy link
Contributor

@fahadshamiinsta fahadshamiinsta commented Jul 3, 2023

Description

This change provides the support for Application Default Credentials for the repository-gcs plugin to support the snapshot api requests.
The solution uses the existent google auth library. The google client is instantiated being wrapped in a doPrivileged block i.e using SocketAccess class to avoid Access Denied errors and to connect to google cloud services.

Related Issues

Resolves #8375

Check List

  • New functionality includes testing.
    • All tests pass
  • New functionality has been documented.
    • New functionality has javadoc added
  • Commits are signed per the DCO using --signoff
  • Commit changes are listed out in CHANGELOG.md file (See: Changelog)

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.

@github-actions
Copy link
Contributor

github-actions bot commented Jul 3, 2023

Gradle Check (Jenkins) Run Completed with:

@github-actions
Copy link
Contributor

github-actions bot commented Jul 3, 2023

Gradle Check (Jenkins) Run Completed with:

Copy link
Member

@dblock dblock left a comment

Choose a reason for hiding this comment

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

Assuming it works (see comment below), this needs at the very least tests, please.

CHANGELOG.md Outdated Show resolved Hide resolved
@peternied
Copy link
Member

Thanks for creating this pull request - I'll take another look after tests have been added.

@opensearch-trigger-bot
Copy link
Contributor

This PR is stalled because it has been open for 30 days with no activity. Remove stalled label or comment or this will be closed in 7 days.

@opensearch-trigger-bot opensearch-trigger-bot bot added the stalled Issues that have stalled label Aug 14, 2023
Copy link
Contributor

❌ Gradle check result for a10ffa5: FAILURE

Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change?

Signed-off-by: fahadshamiinsta <fshami@netapp.com>
Copy link
Contributor

❌ Gradle check result for 736b2f2: FAILURE

Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change?

@opensearch-trigger-bot opensearch-trigger-bot bot removed the stalled Issues that have stalled label Jan 14, 2024
@fahadshamiinsta
Copy link
Contributor Author

FAILURE

#10006

Signed-off-by: fahadshamiinsta <fshami@netapp.com>
Copy link
Contributor

❌ Gradle check result for ae2f832: FAILURE

Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change?

Copy link
Contributor

❕ Gradle check result for 21a2ffb: UNSTABLE

  • TEST FAILURES:
      1 org.opensearch.search.SearchWeightedRoutingIT.testMultiGetWithNetworkDisruption_FailOpenEnabled

Please review all flaky tests that succeeded after retry and create an issue if one does not already exist to track the flaky failure.

@fahadshamiinsta
Copy link
Contributor Author

fahadshamiinsta commented Jan 15, 2024

Close! I have some naming asks, I think the way the classes and methods are named is incorrect.

Iterate to green please!

Is there a doc on how to use this? Do we need to write one?

@dblock please have another look:
I have referenced the flaky tests to the PR,
added another test to increase the coverage for GoogleApplicationDefaultCredentials ,
provided a link to the google doc in the class comment for default credentials,
I had to do extra commits to trigger the ci checks (not sure if there is another way)

@dblock dblock merged commit 4c283a7 into opensearch-project:main Jan 16, 2024
32 of 33 checks passed
@dblock dblock added the backport 2.x Backport to 2.x branch label Jan 16, 2024
@opensearch-trigger-bot
Copy link
Contributor

The backport to 2.x failed:

The process '/usr/bin/git' failed with exit code 128

To backport manually, run these commands in your terminal:

# Navigate to the root of your repository
cd $(git rev-parse --show-toplevel)
# Fetch latest updates from GitHub
git fetch
# Create a new working tree
git worktree add ../.worktrees/OpenSearch/backport-2.x 2.x
# Navigate to the new working tree
pushd ../.worktrees/OpenSearch/backport-2.x
# Create a new branch
git switch --create backport/backport-8394-to-2.x
# Cherry-pick the merged commit of this pull request and resolve the conflicts
git cherry-pick -x --mainline 1 4c283a7c79b20e73ccf3378124893451197b85a3
# Push it to GitHub
git push --set-upstream origin backport/backport-8394-to-2.x
# Go back to the original working tree
popd
# Delete the working tree
git worktree remove ../.worktrees/OpenSearch/backport-2.x

Then, create a pull request where the base branch is 2.x and the compare/head branch is backport/backport-8394-to-2.x.

@dblock
Copy link
Member

dblock commented Jan 17, 2024

@fahadshamiinsta you will need to manually backport this if you want it in the next 2.x release (2.12)

yallen-ic pushed a commit to yallen-ic/OpenSearch2110Fork that referenced this pull request Jan 30, 2024
…t#8394)

* fixed conflicts

Signed-off-by: fahadshamiinsta <fshami@netapp.com>

* applying spotless Java check

Signed-off-by: fahadshamiinsta <fshami@netapp.com>

* added a comment to test helper  method

Signed-off-by: fahadshamiinsta <fshami@netapp.com>

* added a comment to GoogleApplicationDefaultCredentials class with document reference

Signed-off-by: fahadshamiinsta <fshami@netapp.com>

* to rerun gradle checks

Signed-off-by: fahadshamiinsta <fshami@netapp.com>

* increasing coverage by adding another test

Signed-off-by: fahadshamiinsta <fshami@netapp.com>

* test name change

Signed-off-by: fahadshamiinsta <fshami@netapp.com>

* rerun ci

Signed-off-by: fahadshamiinsta <fshami@netapp.com>

* rerun ci

Signed-off-by: fahadshamiinsta <fshami@netapp.com>

* force push to rerun ci

Signed-off-by: fahadshamiinsta <fshami@netapp.com>

* pushing to trigger ci checks

Signed-off-by: fahadshamiinsta <fshami@netapp.com>

---------

Signed-off-by: fahadshamiinsta <fshami@netapp.com>
(cherry picked from commit 4c283a7)
yallen-ic pushed a commit to yallen-ic/OpenSearch2110Fork that referenced this pull request Jan 30, 2024
…t#8394)

* fixed conflicts

Signed-off-by: fahadshamiinsta <fshami@netapp.com>

* applying spotless Java check

Signed-off-by: fahadshamiinsta <fshami@netapp.com>

* added a comment to test helper  method

Signed-off-by: fahadshamiinsta <fshami@netapp.com>

* added a comment to GoogleApplicationDefaultCredentials class with document reference

Signed-off-by: fahadshamiinsta <fshami@netapp.com>

* to rerun gradle checks

Signed-off-by: fahadshamiinsta <fshami@netapp.com>

* increasing coverage by adding another test

Signed-off-by: fahadshamiinsta <fshami@netapp.com>

* test name change

Signed-off-by: fahadshamiinsta <fshami@netapp.com>

* rerun ci

Signed-off-by: fahadshamiinsta <fshami@netapp.com>

* rerun ci

Signed-off-by: fahadshamiinsta <fshami@netapp.com>

* force push to rerun ci

Signed-off-by: fahadshamiinsta <fshami@netapp.com>

* pushing to trigger ci checks

Signed-off-by: fahadshamiinsta <fshami@netapp.com>

---------

Signed-off-by: fahadshamiinsta <fshami@netapp.com>
(cherry picked from commit 4c283a7)
andrross pushed a commit to yallen-ic/OpenSearch2110Fork that referenced this pull request Jan 30, 2024
…t#8394)

* fixed conflicts

Signed-off-by: fahadshamiinsta <fshami@netapp.com>

* applying spotless Java check

Signed-off-by: fahadshamiinsta <fshami@netapp.com>

* added a comment to test helper  method

Signed-off-by: fahadshamiinsta <fshami@netapp.com>

* added a comment to GoogleApplicationDefaultCredentials class with document reference

Signed-off-by: fahadshamiinsta <fshami@netapp.com>

* to rerun gradle checks

Signed-off-by: fahadshamiinsta <fshami@netapp.com>

* increasing coverage by adding another test

Signed-off-by: fahadshamiinsta <fshami@netapp.com>

* test name change

Signed-off-by: fahadshamiinsta <fshami@netapp.com>

* rerun ci

Signed-off-by: fahadshamiinsta <fshami@netapp.com>

* rerun ci

Signed-off-by: fahadshamiinsta <fshami@netapp.com>

* force push to rerun ci

Signed-off-by: fahadshamiinsta <fshami@netapp.com>

* pushing to trigger ci checks

Signed-off-by: fahadshamiinsta <fshami@netapp.com>

---------

Signed-off-by: fahadshamiinsta <fshami@netapp.com>
(cherry picked from commit 4c283a7)
andrross pushed a commit to yallen-ic/OpenSearch2110Fork that referenced this pull request Jan 30, 2024
…t#8394)

* fixed conflicts

Signed-off-by: fahadshamiinsta <fshami@netapp.com>

* applying spotless Java check

Signed-off-by: fahadshamiinsta <fshami@netapp.com>

* added a comment to test helper  method

Signed-off-by: fahadshamiinsta <fshami@netapp.com>

* added a comment to GoogleApplicationDefaultCredentials class with document reference

Signed-off-by: fahadshamiinsta <fshami@netapp.com>

* to rerun gradle checks

Signed-off-by: fahadshamiinsta <fshami@netapp.com>

* increasing coverage by adding another test

Signed-off-by: fahadshamiinsta <fshami@netapp.com>

* test name change

Signed-off-by: fahadshamiinsta <fshami@netapp.com>

* rerun ci

Signed-off-by: fahadshamiinsta <fshami@netapp.com>

* rerun ci

Signed-off-by: fahadshamiinsta <fshami@netapp.com>

* force push to rerun ci

Signed-off-by: fahadshamiinsta <fshami@netapp.com>

* pushing to trigger ci checks

Signed-off-by: fahadshamiinsta <fshami@netapp.com>

---------

Signed-off-by: fahadshamiinsta <fshami@netapp.com>
(cherry picked from commit 4c283a7)
@andrross andrross added the v2.12.0 Issues and PRs related to version 2.12.0 label Jan 30, 2024
kotwanikunal pushed a commit that referenced this pull request Jan 31, 2024
* fixed conflicts

Signed-off-by: fahadshamiinsta <fshami@netapp.com>

* applying spotless Java check

Signed-off-by: fahadshamiinsta <fshami@netapp.com>

* added a comment to test helper  method

Signed-off-by: fahadshamiinsta <fshami@netapp.com>

* added a comment to GoogleApplicationDefaultCredentials class with document reference

Signed-off-by: fahadshamiinsta <fshami@netapp.com>

* to rerun gradle checks

Signed-off-by: fahadshamiinsta <fshami@netapp.com>

* increasing coverage by adding another test

Signed-off-by: fahadshamiinsta <fshami@netapp.com>

* test name change

Signed-off-by: fahadshamiinsta <fshami@netapp.com>

* rerun ci

Signed-off-by: fahadshamiinsta <fshami@netapp.com>

* rerun ci

Signed-off-by: fahadshamiinsta <fshami@netapp.com>

* force push to rerun ci

Signed-off-by: fahadshamiinsta <fshami@netapp.com>

* pushing to trigger ci checks

Signed-off-by: fahadshamiinsta <fshami@netapp.com>

---------

Signed-off-by: fahadshamiinsta <fshami@netapp.com>
(cherry picked from commit 4c283a7)

Co-authored-by: Fahad Shami <48306098+fahadshamiinsta@users.noreply.github.com>
yallen-ic pushed a commit to yallen-ic/OpenSearch2110Fork that referenced this pull request Jan 31, 2024
…t#8394)

* fixed conflicts

Signed-off-by: fahadshamiinsta <fshami@netapp.com>

* applying spotless Java check

Signed-off-by: fahadshamiinsta <fshami@netapp.com>

* added a comment to test helper  method

Signed-off-by: fahadshamiinsta <fshami@netapp.com>

* added a comment to GoogleApplicationDefaultCredentials class with document reference

Signed-off-by: fahadshamiinsta <fshami@netapp.com>

* to rerun gradle checks

Signed-off-by: fahadshamiinsta <fshami@netapp.com>

* increasing coverage by adding another test

Signed-off-by: fahadshamiinsta <fshami@netapp.com>

* test name change

Signed-off-by: fahadshamiinsta <fshami@netapp.com>

* rerun ci

Signed-off-by: fahadshamiinsta <fshami@netapp.com>

* rerun ci

Signed-off-by: fahadshamiinsta <fshami@netapp.com>

* force push to rerun ci

Signed-off-by: fahadshamiinsta <fshami@netapp.com>

* pushing to trigger ci checks

Signed-off-by: fahadshamiinsta <fshami@netapp.com>

---------

Signed-off-by: fahadshamiinsta <fshami@netapp.com>
(cherry picked from commit 4c283a7)
yallen-ic pushed a commit to yallen-ic/OpenSearch2110Fork that referenced this pull request Feb 5, 2024
…t#8394)

* fixed conflicts

Signed-off-by: fahadshamiinsta <fshami@netapp.com>

* applying spotless Java check

Signed-off-by: fahadshamiinsta <fshami@netapp.com>

* added a comment to test helper  method

Signed-off-by: fahadshamiinsta <fshami@netapp.com>

* added a comment to GoogleApplicationDefaultCredentials class with document reference

Signed-off-by: fahadshamiinsta <fshami@netapp.com>

* to rerun gradle checks

Signed-off-by: fahadshamiinsta <fshami@netapp.com>

* increasing coverage by adding another test

Signed-off-by: fahadshamiinsta <fshami@netapp.com>

* test name change

Signed-off-by: fahadshamiinsta <fshami@netapp.com>

* rerun ci

Signed-off-by: fahadshamiinsta <fshami@netapp.com>

* rerun ci

Signed-off-by: fahadshamiinsta <fshami@netapp.com>

* force push to rerun ci

Signed-off-by: fahadshamiinsta <fshami@netapp.com>

* pushing to trigger ci checks

Signed-off-by: fahadshamiinsta <fshami@netapp.com>

---------

Signed-off-by: fahadshamiinsta <fshami@netapp.com>
(cherry picked from commit 4c283a7)
peteralfonsi pushed a commit to peteralfonsi/OpenSearch that referenced this pull request Mar 1, 2024
…t#8394)

* fixed conflicts

Signed-off-by: fahadshamiinsta <fshami@netapp.com>

* applying spotless Java check

Signed-off-by: fahadshamiinsta <fshami@netapp.com>

* added a comment to test helper  method

Signed-off-by: fahadshamiinsta <fshami@netapp.com>

* added a comment to GoogleApplicationDefaultCredentials class with document reference

Signed-off-by: fahadshamiinsta <fshami@netapp.com>

* to rerun gradle checks

Signed-off-by: fahadshamiinsta <fshami@netapp.com>

* increasing coverage by adding another test

Signed-off-by: fahadshamiinsta <fshami@netapp.com>

* test name change

Signed-off-by: fahadshamiinsta <fshami@netapp.com>

* rerun ci

Signed-off-by: fahadshamiinsta <fshami@netapp.com>

* rerun ci

Signed-off-by: fahadshamiinsta <fshami@netapp.com>

* force push to rerun ci

Signed-off-by: fahadshamiinsta <fshami@netapp.com>

* pushing to trigger ci checks

Signed-off-by: fahadshamiinsta <fshami@netapp.com>

---------

Signed-off-by: fahadshamiinsta <fshami@netapp.com>
rayshrey pushed a commit to rayshrey/OpenSearch that referenced this pull request Mar 18, 2024
…t#8394)

* fixed conflicts

Signed-off-by: fahadshamiinsta <fshami@netapp.com>

* applying spotless Java check

Signed-off-by: fahadshamiinsta <fshami@netapp.com>

* added a comment to test helper  method

Signed-off-by: fahadshamiinsta <fshami@netapp.com>

* added a comment to GoogleApplicationDefaultCredentials class with document reference

Signed-off-by: fahadshamiinsta <fshami@netapp.com>

* to rerun gradle checks

Signed-off-by: fahadshamiinsta <fshami@netapp.com>

* increasing coverage by adding another test

Signed-off-by: fahadshamiinsta <fshami@netapp.com>

* test name change

Signed-off-by: fahadshamiinsta <fshami@netapp.com>

* rerun ci

Signed-off-by: fahadshamiinsta <fshami@netapp.com>

* rerun ci

Signed-off-by: fahadshamiinsta <fshami@netapp.com>

* force push to rerun ci

Signed-off-by: fahadshamiinsta <fshami@netapp.com>

* pushing to trigger ci checks

Signed-off-by: fahadshamiinsta <fshami@netapp.com>

---------

Signed-off-by: fahadshamiinsta <fshami@netapp.com>
shiv0408 pushed a commit to Gaurav614/OpenSearch that referenced this pull request Apr 25, 2024
…t#8394)

* fixed conflicts

Signed-off-by: fahadshamiinsta <fshami@netapp.com>

* applying spotless Java check

Signed-off-by: fahadshamiinsta <fshami@netapp.com>

* added a comment to test helper  method

Signed-off-by: fahadshamiinsta <fshami@netapp.com>

* added a comment to GoogleApplicationDefaultCredentials class with document reference

Signed-off-by: fahadshamiinsta <fshami@netapp.com>

* to rerun gradle checks

Signed-off-by: fahadshamiinsta <fshami@netapp.com>

* increasing coverage by adding another test

Signed-off-by: fahadshamiinsta <fshami@netapp.com>

* test name change

Signed-off-by: fahadshamiinsta <fshami@netapp.com>

* rerun ci

Signed-off-by: fahadshamiinsta <fshami@netapp.com>

* rerun ci

Signed-off-by: fahadshamiinsta <fshami@netapp.com>

* force push to rerun ci

Signed-off-by: fahadshamiinsta <fshami@netapp.com>

* pushing to trigger ci checks

Signed-off-by: fahadshamiinsta <fshami@netapp.com>

---------

Signed-off-by: fahadshamiinsta <fshami@netapp.com>
Signed-off-by: Shivansh Arora <hishiv@amazon.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport 2.x Backport to 2.x branch backport-failed enhancement Enhancement or improvement to existing feature or request security Anything security related v2.12.0 Issues and PRs related to version 2.12.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Support for GCP Application Default Credentials strategy
6 participants