Skip to content

Authentication Support for Java & Go SDKs#971

Merged
feast-ci-bot merged 14 commits intofeast-dev:masterfrom
mrzzy:auth-go-java-sdk
Sep 4, 2020
Merged

Authentication Support for Java & Go SDKs#971
feast-ci-bot merged 14 commits intofeast-dev:masterfrom
mrzzy:auth-go-java-sdk

Conversation

@mrzzy
Copy link
Copy Markdown
Collaborator

@mrzzy mrzzy commented Sep 1, 2020 •
edited
Loading

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
Copy Markdown
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
woop
woop reviewed Sep 3, 2020
View reviewed changes
woop
woop reviewed Sep 3, 2020
View reviewed changes
common/src/main/java/feast/common/auth/credentials/GoogleAuthCredentials.java Outdated
Copy link
Copy Markdown
Member

@woop woop Sep 3, 2020

Choose a reason for hiding this comment

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

Typo

Copy link
Copy Markdown
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.

fixed.

Copy link
Copy Markdown
Collaborator Author

@mrzzy mrzzy Sep 4, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

Updated.

woop
woop reviewed Sep 3, 2020
View reviewed changes
woop
woop reviewed Sep 3, 2020
View reviewed changes
infra/scripts/test-end-to-end.sh Outdated
Copy link
Copy Markdown
Member

@woop woop Sep 3, 2020

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
Copy Markdown
Collaborator Author

@mrzzy mrzzy Sep 3, 2020 •
edited
Loading

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
Copy Markdown
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.

Reverted.

woop
woop reviewed Sep 3, 2020
View reviewed changes
woop
woop reviewed Sep 3, 2020
View reviewed changes
woop
woop reviewed Sep 3, 2020
View reviewed changes
@mrzzy
Copy link
Copy Markdown
Collaborator Author

mrzzy commented Sep 4, 2020 •
edited
Loading

Reverted the E2E test changes for another PR.

mrzzy added 11 commits September 4, 2020 14:52
@mrzzy
Fix typo in authorization grpc metadata should not caps first letter
ea40082
@mrzzy
Infer token audience from serving service address in Go SDK.
4daf188
@mrzzy
Add javadocs to OAuthCredentials, GoogleAuthCredentials & JwtCallCred…
253b320 
…entials as they are to become user facing apis.
@mrzzy
Move JwtCallCredentials to common module as static JWT CallCredential…
23bd5ad 
…s implementation.
@mrzzy
Go SDK: Replace AuthProvider with Credential implementation.
f1e28d9
@mrzzy
Update Java SDK to support authentication with CallCredentials implem…
c2c87f4 
…entations.
@mrzzy
Fix issue where GrpcMessageInterceptor obtains subject claim from aut…
3348f44 
…horization instead of authentication options.
@mrzzy
Fix unhandled IllegalStateException throw by getSubjectFromAuth in Gr…
1e6d930 
…pcMessageInterceptor
@mrzzy
Fix lint
8c92c5b
@mrzzy
Fix typo in GoogleAuthCredentials docs
9571064
@mrzzy
Rename 'oAuth' to 'oauth' to make naming less weird.
918543e
@feast-ci-bot
Copy link
Copy Markdown
Collaborator

feast-ci-bot commented Sep 4, 2020

[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

Details 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
Copy Markdown
Collaborator

pyalex commented Sep 4, 2020

/lgtm

@feast-ci-bot feast-ci-bot merged commit 653645d into feast-dev:master Sep 4, 2020
@mrzzy mrzzy mentioned this pull request Sep 7, 2020
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@woop woop woop left review comments

@pyalex pyalex pyalex approved these changes

@davidheryanto davidheryanto Awaiting requested review from davidheryanto

@khorshuheng khorshuheng Awaiting requested review from khorshuheng

@zhilingc zhilingc Awaiting requested review from zhilingc

Assignees

@pyalex pyalex

Labels

approved area/sdks kind/feature New feature or request lgtm size/XL

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Authentication & Authorization Support for Java and Go SDKs

4 participants

@mrzzy @feast-ci-bot @pyalex @woop