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

deprecate insecure http flags and remove already deprecated flags #59018

Merged
merged 1 commit into from Jan 30, 2018

Conversation

@hzxuzhonghu
Member

hzxuzhonghu commented Jan 30, 2018

What this PR does / why we need it:

  1. deprecate insecure-bind-address insecure-port flags
  2. remove flags public-address-override address port They are mark deprecated in #36604, which is more than a year ago.

Which issue(s) this PR fixes (optional, in fixes #<issue number>(, fixes #<issue_number>, ...) format, will close the issue(s) when PR gets merged):
Fixes #58951

Special notes for your reviewer:

Release note:

Deprecate insecure flags `--insecure-bind-address`, `--insecure-port` and remove  `--public-address-override`.
@hzxuzhonghu

This comment has been minimized.

Member

hzxuzhonghu commented Jan 30, 2018

/assign @deads2k @sttts

@hzxuzhonghu

This comment has been minimized.

Member

hzxuzhonghu commented Jan 30, 2018

/assign @gmarek

@@ -1518,7 +1518,6 @@ function start-kube-apiserver {
# Calculate variables and assemble the command line.
local params="${API_SERVER_TEST_LOG_LEVEL:-"--v=2"} ${APISERVER_TEST_ARGS:-} ${CLOUD_CONFIG_OPT}"
params+=" --address=127.0.0.1"

This comment has been minimized.

@hzxuzhonghu

hzxuzhonghu Jan 30, 2018

Member

--address is removed, insecure-bind-address is deprecated.

@hzxuzhonghu

This comment has been minimized.

Member

hzxuzhonghu commented Jan 30, 2018

/assign @bowei

@k8s-ci-robot

This comment has been minimized.

Contributor

k8s-ci-robot commented Jan 30, 2018

@hzxuzhonghu: The following test failed, say /retest to rerun them all:

Test name Commit Details Rerun command
pull-kubernetes-e2e-gke 9152862 link /test pull-kubernetes-e2e-gke

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.

@hzxuzhonghu

This comment has been minimized.

Member

hzxuzhonghu commented Jan 30, 2018

kops is using --address, should not execute until kops stop using --address.

  1. remove flags public-address-override address port They are mark deprecated in #36604, which is more than a year ago.

@hzxuzhonghu hzxuzhonghu force-pushed the hzxuzhonghu:deprecate-http branch from 9152862 to 24c687f Jan 30, 2018

@k8s-ci-robot k8s-ci-robot added size/S and removed size/M labels Jan 30, 2018

@hzxuzhonghu

This comment has been minimized.

Member

hzxuzhonghu commented Jan 30, 2018

/unassign @bowei @gmarek

@sttts

This comment has been minimized.

Contributor

sttts commented Jan 30, 2018

kops is using --address, should not execute until kops stop using --address.

Are there issues in those projects to remove the deprecated use?

@sttts

This comment has been minimized.

Contributor

sttts commented Jan 30, 2018

/lgtm

@k8s-ci-robot k8s-ci-robot added the lgtm label Jan 30, 2018

@k8s-ci-robot

This comment has been minimized.

Contributor

k8s-ci-robot commented Jan 30, 2018

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: hzxuzhonghu, sttts

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 /approve in a comment
You can cancel your approval by writing /approve cancel in a comment

@hzxuzhonghu

This comment has been minimized.

Member

hzxuzhonghu commented Jan 30, 2018

Are there issues in those projects to remove the deprecated use?

Yes, I create one kubernetes/kops#4355

@k8s-merge-robot

This comment has been minimized.

Contributor

k8s-merge-robot commented Jan 30, 2018

/test all [submit-queue is verifying that this PR is safe to merge]

@k8s-merge-robot

This comment has been minimized.

Contributor

k8s-merge-robot commented Jan 30, 2018

Automatic merge from submit-queue. If you want to cherry-pick this change to another branch, please follow the instructions here.

@k8s-merge-robot k8s-merge-robot merged commit 2f175bc into kubernetes:master Jan 30, 2018

12 of 13 checks passed

Submit Queue Required Github CI test is not green: pull-kubernetes-verify
Details
cla/linuxfoundation hzxuzhonghu authorized
Details
pull-kubernetes-bazel-build Job succeeded.
Details
pull-kubernetes-bazel-test Job succeeded.
Details
pull-kubernetes-cross Skipped
pull-kubernetes-e2e-gce Job succeeded.
Details
pull-kubernetes-e2e-gce-device-plugin-gpu Job succeeded.
Details
pull-kubernetes-e2e-gke Skipped
pull-kubernetes-e2e-kops-aws Job succeeded.
Details
pull-kubernetes-kubemark-e2e-gce Job succeeded.
Details
pull-kubernetes-node-e2e Job succeeded.
Details
pull-kubernetes-unit Job succeeded.
Details
pull-kubernetes-verify Job succeeded.
Details

@hzxuzhonghu hzxuzhonghu deleted the hzxuzhonghu:deprecate-http branch Jan 30, 2018

@justinsb

This comment has been minimized.

Member

justinsb commented Feb 2, 2018

Do we have a solution for #43784 ?

@deads2k

This comment has been minimized.

Contributor

deads2k commented Feb 5, 2018

Do we have a solution for #43784 ?

Health checks? /healthz isn't confidential and default policy allows anonymous access.

rfranzke added a commit to gardener/gardener that referenced this pull request Mar 2, 2018

Do not longer expose insecure kube-apiserver HTTP server
As of Kubernetes 1.10, the insecure flags will be deprecated: kubernetes/kubernetes#59018
Currently, there is no other way to allow unauthenticated health checks (requests on kube-apiserver's /healthz endpoint) other than allowing anonymous requests (which we do not want). Related issue: kubernetes/kubernetes#43784
We are now going to use the basic authentication credentials which the kubelet will use to reach the /healthz endpoint.
fs.IntVar(&s.BindPort, "insecure-port", s.BindPort, ""+
"The port on which to serve unsecured, unauthenticated access. It is assumed "+
"that firewall rules are set up such that this port is not reachable from outside of "+
"the cluster and that port 443 on the cluster's public address is proxied to this "+
"port. This is performed by nginx in the default setup. Set to zero to disable")
"port. This is performed by nginx in the default setup. Set to zero to disable.")
fs.MarkDeprecated("insecure-port", "This flag will be removed in a future version.")

This comment has been minimized.

@liggitt

liggitt Mar 28, 2018

Member

marking this deprecated actually omits it from the help, which means the fact that it is enabled by default and you need to "Set to zero to disable." is invisible to the end user. That is not good.

This comment has been minimized.

@sttts

sttts Mar 29, 2018

Contributor

Never understood this behaviour.

@mwthink

This comment has been minimized.

mwthink commented Apr 13, 2018

The API server still serves on localhost:8080 by default. Is this expected to be the case going forward, or is insecure access being removed entirely?

If the latter, it should be disabled by default, but if that's the case, what about health endpoints? Am I going to need a sidecar container to validate the TLS and proxy /healthz over HTTP?

Edit - For anyone coming along later, check out #43784

@deads2k

This comment has been minimized.

Contributor

deads2k commented Apr 16, 2018

The API server still serves on localhost:8080 by default. Is this expected to be the case going forward, or is insecure access being removed entirely?

Nothing should be relying on the insecure port. It is a target for future removal.

@javefang

This comment has been minimized.

javefang commented Apr 16, 2018

@deads2k we are running the apiserver as a static pod on master nodes, which currently requires the apiserver to listen on an insecure port 8080 on localhost for kubelet to be able to talk to it. All other worker nodes' kubelet uses the "--bootstrap-kubeconfig" to perform TLS bootstrapping once the apiserver is up.

I tried to use the same "--bootstrap-kubeconfig" for kubelet running on master, but it complains that it can't reach the apiserver (for obvious reason) and crash, without starting the apiserver/controller-manager/scheduler pods defined in the manifests.

Is there a proper way for kubelet on master to talk to the apiserver without utilising the --insecure-port and --insecure-bind-address?

Environment:
OS: Centos 7.3
Docker: 1.12.6
Kubernetes 1.10.1

@deads2k

This comment has been minimized.

Contributor

deads2k commented Apr 16, 2018

Short lived client certificates are a good option since the kubelet running the kube-apiserver is effectively root on the cluster already.

dghubble added a commit to poseidon/terraform-render-bootkube that referenced this pull request Jun 28, 2018

Explicitly disable apiserver 127.0.0.1 insecure port
* Although the --insecure-port flag is deprecated, apiserver
continues to default to listening on 127.0.0.1:8080
* Explicitly disable insecure local listener since its unused
* kubernetes/kubernetes#59018 (comment)
* 5f3546b
@mrmm

This comment has been minimized.

mrmm commented Oct 2, 2018

Update, thanks @justinsb implemented this for Kops (PR kubernetes/kops#5234)
Cheers 🍺

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment