Skip to content

Auth plugin to be used for Galley dial out#11068

Closed
jeffmendoza wants to merge 4 commits intoistio:release-1.1from
jeffmendoza:galley-callout
Closed

Auth plugin to be used for Galley dial out#11068
jeffmendoza wants to merge 4 commits intoistio:release-1.1from
jeffmendoza:galley-callout

Conversation

@jeffmendoza
Copy link
Copy Markdown
Contributor

This interacts with #10834. A section will be added to the config file to allow specification of dial out including which type of auth to use. Auth will reference a plugin as specified in this PR. Config will include a string map to be passed to auth plugin.

Next step is to add an auth plugin that will use in-cluster mtls (if configured).

@jeffmendoza jeffmendoza requested a review from ozevren January 18, 2019 00:04
@istio-testing istio-testing added the do-not-merge/work-in-progress Block merging of a PR because it isn't ready yet. label Jan 18, 2019
@istio-testing
Copy link
Copy Markdown
Collaborator

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: jeffmendoza
To fully approve this pull request, please assign additional approvers.
We suggest the following additional approver: hklai

If they are not already assigned, you can assign the PR to them by writing /assign @hklai in a comment when ready.

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

@istio-testing istio-testing requested a review from hklai January 18, 2019 00:04
}

grpcOpts := []grpc.DialOption{
grpc.WithPerRPCCredentials(oauth.TokenSource{creds.TokenSource}),
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.

oauth.TokenSource composite literal uses unkeyed fields (from govet)

"golang.org/x/oauth2/google"
"google.golang.org/grpc"
"google.golang.org/grpc/credentials"
"google.golang.org/grpc/credentials/oauth"
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.

File is not goimports-ed (from goimports)

package none

import (
"google.golang.org/grpc"
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.

File is not goimports-ed (from goimports)

"istio.io/istio/galley/pkg/authplugin"
)

func returnAuth(config map[string]string) ([]grpc.DialOption, error) {
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.

returnAuth - config is unused (from unparam)

@jeffmendoza
Copy link
Copy Markdown
Contributor Author

/test istio-pilot-e2e-envoyv2-v1alpha3

@jeffmendoza jeffmendoza changed the title [WIP] Auth plugin to be used for Galley dial out Auth plugin to be used for Galley dial out Jan 24, 2019
@istio-testing istio-testing removed the do-not-merge/work-in-progress Block merging of a PR because it isn't ready yet. label Jan 24, 2019
@jeffmendoza
Copy link
Copy Markdown
Contributor Author

Removed [WIP] from title. This can be merged, it wont be called until more code is added.

@istio-testing
Copy link
Copy Markdown
Collaborator

@jeffmendoza: The following tests failed, say /retest to rerun them all:

Test name Commit Details Rerun command
prow/istio-integ-k8s-tests.sh cab2ba1 link /test istio-integ-k8s-tests
prow/istio-pilot-multicluster-e2e.sh cab2ba1 link /test istio-pilot-multicluster-e2e
Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here.

findDC = google.FindDefaultCredentials
}

func returnAuth(config map[string]string) ([]grpc.DialOption, error) {
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 looks like we don't have a need for config for the time being. Should we remove that?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Correct, it is not being used. The main reason we would want it is if we wanted to specify a service account file through config, instead of setting the GOOGLE_APPLICATION_CREDENTIALS env var.
Other auth plugins might want some sort of parameters as well.

@jeffmendoza
Copy link
Copy Markdown
Contributor Author

Superseded by #11291

@jeffmendoza jeffmendoza closed this Feb 1, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants