-
Notifications
You must be signed in to change notification settings - Fork 38.7k
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
Enable basic auth username rotation for GCI #44590
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -182,11 +182,11 @@ function mount-master-pd { | |
chgrp -R etcd "${mount_point}/var/etcd" | ||
} | ||
|
||
# replace_prefixed_line ensures: | ||
# append_or_replace_prefixed_line ensures: | ||
# 1. the specified file exists | ||
# 2. existing lines with the specified ${prefix} are removed | ||
# 3. a new line with the specified ${prefix}${suffix} is appended | ||
function replace_prefixed_line { | ||
function append_or_replace_prefixed_line { | ||
local -r file="${1:-}" | ||
local -r prefix="${2:-}" | ||
local -r suffix="${3:-}" | ||
|
@@ -287,30 +287,32 @@ function create-master-auth { | |
local -r basic_auth_csv="${auth_dir}/basic_auth.csv" | ||
if [[ -n "${KUBE_PASSWORD:-}" && -n "${KUBE_USER:-}" ]]; then | ||
if [[ -e "${basic_auth_csv}" && "${METADATA_CLOBBERS_CONFIG:-false}" == "true" ]]; then | ||
sed -i "/,${KUBE_USER},admin,system:masters$/d" "${basic_auth_csv}" | ||
# The following is for the legacy form of the password line. | ||
sed -i "/,${KUBE_USER},admin$/d" "${basic_auth_csv}" | ||
# If METADATA_CLOBBERS_CONFIG is true, we want to rewrite the file | ||
# completely, because if we're changing KUBE_USER and KUBE_PASSWORD, we | ||
# have nothing to match on. The file is replaced just below with | ||
# append_or_replace_prefixed_line. | ||
rm "${basic_auth_csv}" | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The original sed nastiness was chosen rather than a file remove because there was a belief that in GCE a customer might have manually added their own users. This fix is clearly fine in GKE, but I'm not sure if our original concerns about additional GCE user accounts no longer applies. If it does still apply then this does not seem safe. |
||
fi | ||
replace_prefixed_line "${basic_auth_csv}" "${KUBE_PASSWORD},${KUBE_USER}," "admin,system:masters" | ||
append_or_replace_prefixed_line "${basic_auth_csv}" "${KUBE_PASSWORD},${KUBE_USER}," "admin,system:masters" | ||
fi | ||
local -r known_tokens_csv="${auth_dir}/known_tokens.csv" | ||
if [[ -n "${KUBE_BEARER_TOKEN:-}" ]]; then | ||
replace_prefixed_line "${known_tokens_csv}" "${KUBE_BEARER_TOKEN}," "admin,admin,system:masters" | ||
append_or_replace_prefixed_line "${known_tokens_csv}" "${KUBE_BEARER_TOKEN}," "admin,admin,system:masters" | ||
fi | ||
if [[ -n "${KUBE_CONTROLLER_MANAGER_TOKEN:-}" ]]; then | ||
replace_prefixed_line "${known_tokens_csv}" "${KUBE_CONTROLLER_MANAGER_TOKEN}," "system:kube-controller-manager,uid:system:kube-controller-manager" | ||
append_or_replace_prefixed_line "${known_tokens_csv}" "${KUBE_CONTROLLER_MANAGER_TOKEN}," "system:kube-controller-manager,uid:system:kube-controller-manager" | ||
fi | ||
if [[ -n "${KUBE_SCHEDULER_TOKEN:-}" ]]; then | ||
replace_prefixed_line "${known_tokens_csv}" "${KUBE_SCHEDULER_TOKEN}," "system:kube-scheduler,uid:system:kube-scheduler" | ||
append_or_replace_prefixed_line "${known_tokens_csv}" "${KUBE_SCHEDULER_TOKEN}," "system:kube-scheduler,uid:system:kube-scheduler" | ||
fi | ||
if [[ -n "${KUBELET_TOKEN:-}" ]]; then | ||
replace_prefixed_line "${known_tokens_csv}" "${KUBELET_TOKEN}," "kubelet,uid:kubelet,system:nodes" | ||
append_or_replace_prefixed_line "${known_tokens_csv}" "${KUBELET_TOKEN}," "kubelet,uid:kubelet,system:nodes" | ||
fi | ||
if [[ -n "${KUBE_PROXY_TOKEN:-}" ]]; then | ||
replace_prefixed_line "${known_tokens_csv}" "${KUBE_PROXY_TOKEN}," "system:kube-proxy,uid:kube_proxy" | ||
append_or_replace_prefixed_line "${known_tokens_csv}" "${KUBE_PROXY_TOKEN}," "system:kube-proxy,uid:kube_proxy" | ||
fi | ||
if [[ -n "${NODE_PROBLEM_DETECTOR_TOKEN:-}" ]]; then | ||
replace_prefixed_line "${known_tokens_csv}" "${NODE_PROBLEM_DETECTOR_TOKEN}," "system:node-problem-detector,uid:node-problem-detector" | ||
append_or_replace_prefixed_line "${known_tokens_csv}" "${NODE_PROBLEM_DETECTOR_TOKEN}," "system:node-problem-detector,uid:node-problem-detector" | ||
fi | ||
local use_cloud_config="false" | ||
cat <<EOF >/etc/gce.conf | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: e.g.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This was an intentional change; KUBE_USER and KUBE_PASSWORD aren't examples, they are actually the things that are overwritten. https://english.stackexchange.com/questions/1629/e-g-versus-i-e/1631
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ahh gotcha.
I'd say "Indicates if the KUBE_USER and KUBE_PASSWORD values..." because I won't be the only one who misunderstands this, and assumes this env var has wider scope than it does.