Default ABAC to off in GCE (for new clusters). #51367
Conversation
k8s-ci-robot
added
size/XS
k8s-github-robot
added
the
release-note
|
@kubernetes/sig-auth-pr-reviews |
k8s-ci-robot
added
the
sig/auth
| @@ -1270,6 +1270,7 @@ function parse-master-env() { | |||
| REQUESTHEADER_CA_CERT_BASE64=$(get-env-val "${master_env}" "REQUESTHEADER_CA_CERT") | |||
| PROXY_CLIENT_CERT_BASE64=$(get-env-val "${master_env}" "PROXY_CLIENT_CERT") | |||
| PROXY_CLIENT_KEY_BASE64=$(get-env-val "${master_env}" "PROXY_CLIENT_KEY") | |||
| ENABLE_LEGACY_ABAC=$(get-env-val "${master_env}" "ENABLE_LEGACY_ABAC") | |||
There was a problem hiding this comment.
Does this set it to "" and prevent the defaulting in config-default?
There was a problem hiding this comment.
Yes, which means the only way to change this on an existing cluster is to modify the existing kube-env in metadata.
I'm not sure of a better way to handle updates. Either, we do this, and changing the existing value is kinda gross, or we go off of the config-default value, and we (possibly surprisingly) update the value on existing clusters. This felt like the less intrusive approach.
There was a problem hiding this comment.
Is there a well defined procedure for changing existing cluster parameters generally?
There was a problem hiding this comment.
we definitely should not update the value on existing clusters during upgrade. existing workloads should continue to run as-is on an upgraded cluster.
updating the default for new clusters is what I'd like to see, though the interaction between metadata env and the config-default.sh scripts is not clear to me (I can't tell which runs first, which informs the other, etc).
|
@bgrant0607 since you had concerns about ABAC defaulting when we initially went for it :). |
bgrant0607
added
the
release-note-action-required
|
The deprecation policy doesn't really cover this: Technically, kube-up is deprecated and not part of Kubernetes proper, but is part of the release bundle. I'm ok with changing default behavior for new clusters at this point. RBAC has been well publicized and has been enabled by default for one minor release. |
|
I agree with the intent of the PR, I'll defer to @roberthbailey on whether this change effectively accomplishes the following:
|
|
I'm confused by the title of this issue -- we recently removed gke support from kube-up and this PR only touches config files used by the gce implementation. |
cjcullen
changed the title
|
/retest |
|
/test pull-kubernetes-e2e-gce-etcd3 |
|
Are we still hoping to get this in for 1.8? |
|
comment at #51367 (comment) stands... it LGTM assuming the three scenarios described have been tested |
|
Just verified: Creating a new cluster w/ Running Modifying the metadata to set Creating a new cluster w/ Running |
|
And yes, I think we still want this in for 1.8 (Our GCE tests already run w/ ABAC off). |
|
Agree. CI has run with ABAC off since 1.6. /lgtm |
k8s-ci-robot
added
the
lgtm
|
One last question, does creating a 1.7 cluster populate metadata with ENABLE_LEGACY_ABAC=true? |
|
A 1.7 cluster with this script will set |
|
/test pull-kubernetes-e2e-gce-bazel |
I'm asking about a 1.7 cluster created with 1.7 scripts. Does metadata contain the environment variable set to true in that case? |
Yes |
|
sounds good to me. @roberthbailey has approval |
|
/approve |
|
/approve no-issue |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: cjcullen, liggitt, mikedanese, roberthbailey Associated issue requirement bypassed by: mikedanese The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these OWNERS Files:
You can indicate your approval by writing |
k8s-github-robot
added
the
approved
|
/test all Tests are more than 96 hours old. Re-running tests. |
|
Automatic merge from submit-queue (batch tested with PRs 48970, 52497, 51367, 52549, 52541). If you want to cherry-pick this change to another branch, please follow the instructions here.. |
|
@cjcullen: The following tests failed, say
Full PR test history. Your PR dashboard. Please help us cut down on flakes by linking to an open issue when you hit one in your PR. 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. |
What this PR does / why we need it:
Disables the legacy ABAC authorizer by default on GCE/GKE clusters using kube-up.sh. Existing clusters upgrading to 1.8 will keep their existing configuration.
Release note: