plugin/pkg/auth: add webhook implementation of authorizer.Authorizer plugin #20347
Conversation
|
We found a Contributor License Agreement for you (the sender of this pull request) and all commit authors, but as best as we can tell these commits were authored by someone else. If that's the case, please add them to this pull request and have them confirm that they're okay with these commits being contributed to Google. If we're mistaken and you did author these commits, just reply here to confirm. |
|
Can one of the admins verify that this patch is reasonable to test? (reply "ok to test", or if you trust the user, reply "add to whitelist") If this message is too spammy, please complain to ixdy. |
1 similar comment
|
Can one of the admins verify that this patch is reasonable to test? (reply "ok to test", or if you trust the user, reply "add to whitelist") If this message is too spammy, please complain to ixdy. |
|
Labelling this PR as size/XS |
k8s-github-robot
added
the
size/XS
ericchiang
force-pushed
the
authz_grpc
branch
2 times, most recently
from
d0d5862 to
ef512d2
Compare
ericchiang
changed the title
|
PR needs rebase |
k8s-github-robot
added
the
needs-rebase
|
Reassigning to @erictune |
|
This is definitely one of those things that start to make it desirable to have out of tree plugins compiled easily into our build process. If every authorizer in the world is compiled in, we're going to have massive binaries (the cloud providers today are already kind of large) |
|
@smarterclayton True. But it would be nice if (most? all?) plugins shipped with a GRPC (or whatever RPC) implementation; then it wouldn't matter where they lived and people could easily develop plugins without getting bogged down learning k8s internals. |
|
@smarterclayton I am confused by your feedback. You are opposed to having stuff compiled into k8s but you want out of tree plugins compiled into the build system? Is your feedback that you would like to see a way for someone to take the FooAuthz external plugin and compile it into the k8s API process instead of having to launch a FooAuthz service separately on cluster? |
|
I think we're nowhere close to that point, and that still requires
compilation. My point here is not that grpc or this plugin are bad,
but that we're pushing limits of how many external packages can be
imported. We have no clean decoupling between all of the valid
extension points of Kube and all of the myriad of Go ecosystem tools
that we can use to talk to the external world.
Cloud provider and contrib/mesos are other good examples of sprawl, as
are the proposed Kerberos auth extensions, direct LDAP backends to
auth (which is far more common than grpc), and various flavored of
not-quite standard oauth/openid connect clients.
Is the goal here to ensure the exact Kube binaries released have built
in plugins that talk to specific CoreOS auth tools? Or that you can
maintain and ship a distribution of Kube that compiles in these
features easily? The latter has clear value to everyone, the former is
less clear.
I'm also concerned about the rapid pace of grpc and likelihood two
different consumers (etcd + ???) depend on it in different ways.
|
|
I'm probably OK with extensions that clearly mirror Kube interfaces, but I am concerned by grpc versions across this plus etcd plus others. I also don't know that authorizer is ready to be totally stable forever (i.e. what backcompat guarantees do we provide). What guarantees do you guys think you need to make this useful? Minor version compat? Additive only changes? |
|
We've talked about doing this for admission control as well, and an http implementation would mirror this with the exception that we would need to use external versioned structs and declare a formal support policy for how we change those. |
I totally get your point here; we were hoping that by introducing a network plugin, we can others could implement plugins as services, and thus reduce the need for external packages.
Neither, I think. Definitely not the former - the auth tool that this GRPC plugin would talk to is yet to be born. And neither do we really want to distribute our own Kubernetes - we want to contribute to upstream, and ensure that our tools work with upstream. In this case it's not clear what a well-loved standard for authz is, so instead of baking our own protocol and trying to get that mainlined into Kubernetes, it seems more generally useful to have a network based plugin that mirrors the kubernetes Go interface. |
@ericchiang - you probably should make this go-generate-able, yes? |
|
The bar is typically a bit higher - we can make an exception for some
types of stuff, but I don't know that we've clearly spec'd the
authorizer interface stability guarantees. We should definitely do
that for 1.3 so this can make progress - it just is a discussion that
hasn't happened yet.
|
Sorry, lost context there - which bar? |
|
My first thought is that we don't use gRPC anywhere else, and it seems arbitrary to start using it here. |
|
Baking in an RPC is a big support/compatibility commitment. We don't currently have any outgoing RPCs or webhooks, although we've definitely talked about webhooks, and specifically about auth webhooks. So thanks in advance for bearing with us if we spend a lot of time here |
I've used gRPC because we prefer it to REST, but the overall goal is to provide a webhook like mechanism for authz. If it would fit into the existing project better, REST would be fine.
Since the existing API already has a lot of good language around API stability / compatibility would it be more reasonable to rethink this as an API addition of a REST webhook? That would allow us to add swagger definitions for documentation (rather than pointing others to a .proto file) and piggyback off of established rules for API groups and deprecation. |
|
@cjcullen you have been looking at adding a generic REST authorizer. You might be interested in this PR. If this PR pivots from gRPC to REST, as has been suggested, you might want to collaborate to see if you can use the same REST impl. Also wonder if the remote service you are targeting would prefer to be called via gRPC over REST? If so, that might sway the whole "lets not link in gRPC" conversation. |
gRPC is not necessary for the remote service. Since the gRPC part of this PR seems to be the main issue, and because there's already been discussion of the REST implementation, I'll go ahead and rework this to use REST. |
|
GCE e2e test build/test passed for commit 7a436c554370470649f5a67953eb06b03e267c3e. |
|
PR needs rebase |
k8s-github-robot
added
the
needs-rebase
ericchiang
force-pushed
the
authz_grpc
branch
from
7a436c5 to
b50ede6
Compare
k8s-github-robot
removed
the
needs-rebase
|
GCE e2e test build/test passed for commit b50ede6. |
|
@erictune would love to see this in 1.2 (noticed it's a v1.2-candidate). Please let me know if you have any further changes you'd like to see. |
|
lgtm |
erictune
added
the
lgtm
|
@k8s-bot test this Tests are more than 48 hours old. Re-running tests. |
|
GCE e2e test build/test passed for commit b50ede6. |
|
@k8s-bot test this [submit-queue is verifying that this PR is safe to merge] |
|
GCE e2e test build/test passed for commit b50ede6. |
|
@k8s-bot test this issue #IGNORE |
|
GCE e2e test build/test passed for commit b50ede6. |
|
@k8s-bot test this [submit-queue is verifying that this PR is safe to merge] |
|
GCE e2e build/test passed for commit b50ede6. |
|
Automatic merge from submit-queue |
Auto commit by PR queue bot
UPSTREAM: 62943: set updated replicas in statefulsets Origin-commit: caded921683fb178f30744cd1d19501d74ad1ef8
Following up on the proposal in #19828 this is an implementation of the authorizer.Authorizer plugin which calls out to a remote gRPC service to drive authorization. The details are in that issues, but in summary we (CoreOS) would like a way to build services that drive authentication separate from k8s.
Due to the current vendored state of the protobuf and grpc packages, I've had to cherry pick updates from #19931 to generate code that would compile. That PR is not passing tests, but I'd still like feedback on this PR and will update the commits when #19931 is resolved.
cc @philips @sym3tri @bobbyrullo