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

Authentication Support for Java & Go SDKs #971

Merged
merged 14 commits into from
Sep 4, 2020
Merged

Authentication Support for Java & Go SDKs #971

merged 14 commits into from
Sep 4, 2020

Conversation

mrzzy
Copy link
Collaborator

@mrzzy mrzzy commented Sep 1, 2020

What this PR does / why we need it:

Implements support for authentication to Go and Java SDKs, which is required for interacting with a authentication enabled Feast Serving:

  • Update Feast clients to attach OIDC token used for authentication into metadata when enabled.
  • Allows users to provide credentials via Google, OAuth Client Credentials request, or a Static token.
    • Google: Uses user provided Service Account Credentials (ie JSON) to obtain token.
    • OAuth Client Credentials: Makes a client credentials request to obtain token.
    • Static: Uses a user provided static token.

Continuation of #504

Which issue(s) this PR fixes:

Fixes #950

Does this PR introduce a user-facing change?:

Adds authentication support to Go and Java SDKs via Google, OAuth Client Credentials request, or a Static token.

@mrzzy
Copy link
Collaborator Author

mrzzy commented Sep 2, 2020

/retest

@mrzzy mrzzy changed the title WIP: Authentication Support for Java & Go SDKs. Authentication Support for Java & Go SDKs. Sep 3, 2020
@mrzzy mrzzy added area/sdks kind/feature New feature or request labels Sep 3, 2020
@mrzzy mrzzy changed the title Authentication Support for Java & Go SDKs. Authentication Support for Java & Go SDKs Sep 3, 2020
.prow/config.yaml Outdated Show resolved Hide resolved
*/
public class GoogleAuthCredentials extends CallCredentials {
private final IdTokenCredentials credentials;
private static final String BEARER_TYPE = "Bearer";
private static final Metadata.Key<String> AUTHORIZATION_METADATA_KEY =
Metadata.Key.of("Authorization", ASCII_STRING_MARSHALLER);

/**
* Constructa new GoogleAuthCredentials with given options.
Copy link
Member

Choose a reason for hiding this comment

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

Typo

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

fixed.

Copy link
Collaborator Author

@mrzzy mrzzy Sep 4, 2020

Choose a reason for hiding this comment

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

Updated.

@@ -5,11 +5,13 @@ set -o pipefail
[[ $1 == "True" ]] && ENABLE_AUTH="true" || ENABLE_AUTH="false"
echo "Authenication enabled : ${ENABLE_AUTH}"

test -z ${GOOGLE_APPLICATION_CREDENTIALS} && GOOGLE_APPLICATION_CREDENTIALS="/etc/gcloud/service-account.json"
test -z ${GOOGLE_APPLICATION_CREDENTIALS} && GOOGLE_APPLICATION_CREDENTIALS=""
Copy link
Member

Choose a reason for hiding this comment

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

Why are we clearing that configuration? Isn't that a default?

Copy link
Collaborator Author

@mrzzy mrzzy Sep 3, 2020

Choose a reason for hiding this comment

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

install_gcloud_sdk() tries to activate service account credential if this variable is set. Under the current .prow/config.yaml test-end-to-end does not expose the service account key.json and the end to end test fails because it cant find key.json. I added a if conditional to install_gcloud_sdk() to check this variable is set before trying to activate service account credentials.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Reverted.

tests/e2e/java/pom.xml Outdated Show resolved Hide resolved
tests/e2e/go/e2e_test.go Outdated Show resolved Hide resolved
@mrzzy
Copy link
Collaborator Author

mrzzy commented Sep 4, 2020

Reverted the E2E test changes for another PR.

@feast-ci-bot
Copy link
Collaborator

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: mrzzy, pyalex

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

The pull request process is described here

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

@pyalex
Copy link
Collaborator

pyalex commented Sep 4, 2020

/lgtm

@feast-ci-bot feast-ci-bot merged commit 653645d into feast-dev:master Sep 4, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Authentication & Authorization Support for Java and Go SDKs
4 participants