Skip to content
This repository has been archived by the owner on Jul 30, 2021. It is now read-only.

apiserver: run as non-root user. #789

Merged
merged 2 commits into from
Jan 8, 2018
Merged

apiserver: run as non-root user. #789

merged 2 commits into from
Jan 8, 2018

Conversation

diegs
Copy link
Contributor

@diegs diegs commented Dec 5, 2017

Changes the apiserver defaults to listen on port 6443 and use a non-root
user. The port matches the upstream default and reflects best practices.

Production users can put the apiserver behind a load-balancer to forward
443 to 6443.

@diegs diegs self-assigned this Dec 5, 2017
@diegs diegs requested a review from dghubble December 5, 2017 05:39
@k8s-ci-robot k8s-ci-robot added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. labels Dec 5, 2017
@diegs diegs mentioned this pull request Dec 5, 2017
Closed
@rphillips
Copy link
Contributor

though the build is failing

@dghubble
Copy link
Contributor

dghubble commented Dec 5, 2017

You might have a read through poseidon/terraform-render-bootstrap#17 where there was some discussion about the repercussions of this change. I'm not opposed to moving toward it, but it necessitates a v0.10.0 imo.

@diegs
Copy link
Contributor Author

diegs commented Dec 5, 2017

@dghubble yes, v0.10.0 would be prudent, though this is basically just changing the default of an option that has always existed (and updating all the examples to use the new default).

PR tests are very flaky. I'm close to making them better in #784

@redbaron
Copy link
Contributor

redbaron commented Dec 6, 2017

Do we need to change port at all? Running as non root and adding CAP_NET_BIND_SERVICE capability in security context should be enough

@diegs
Copy link
Contributor Author

diegs commented Dec 6, 2017

I did some research on using CAP_NET_BIND_SERVICE in the past. Due to Docker limitations granting it to the user does not work. It must be added to the binary, which would require a modification to the hyperkube image, which upstream will not accept. They recommend using port 6443 which is actually a reasonable stance in my opinion.

@redbaron
Copy link
Contributor

redbaron commented Dec 6, 2017

Googled, found your comments :) Can't initcontainer help here? Set capability on hyperkube binary at start time. Hmm, probably just binding to a non privileged port is easier.

@diegs diegs force-pushed the 6443 branch 2 times, most recently from 825d02d to 6b0c30f Compare December 14, 2017 01:18
@k8s-ci-robot k8s-ci-robot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Jan 2, 2018
@diegs
Copy link
Contributor Author

diegs commented Jan 2, 2018

coreosbot run e2e checkpointer

1 similar comment
@diegs
Copy link
Contributor Author

diegs commented Jan 3, 2018

coreosbot run e2e checkpointer

@xiang90
Copy link
Contributor

xiang90 commented Jan 3, 2018

@diegs

are all the tests failure tracked? it is useful to link the failure issues in the rerun comment so we know all flakes are tracked. thanks!

@diegs
Copy link
Contributor Author

diegs commented Jan 3, 2018

Flake is #816. Was just doing some testing to see if #814 solves this use case (it does!)

diegs added 2 commits January 8, 2018 11:09
@diegs
checkpointer: bump default checkpointer version.
5414b8b 
Also use new flag (in this checkpointer version) to vastly shorten the
grace period in the checkpoint tests.
@diegs
apiserver: run as non-root user.
7370202 
Changes the apiserver defaults to listen on port 6443 and use a non-root
user. The port matches the upstream default and reflects best practices.

Production users can put the apiserver behind a load-balancer to forward
443 to 6443.
@k8s-ci-robot k8s-ci-robot added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. and removed size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Jan 8, 2018
@diegs
Copy link
Contributor Author

diegs commented Jan 8, 2018

coreosbot run e2e

@diegs diegs merged commit fa0a51f into kubernetes-retired:master Jan 8, 2018
@diegs diegs deleted the 6443 branch January 8, 2018 21:53
wking added a commit to wking/bootkube that referenced this pull request Mar 28, 2018
@wking
Documentation/root-requirements: API server no longer needs root
b392b3b 
Catch up with 7370202 apiserver: (run as non-root user, 2017-12-04,
kubernetes-retired#789).
@wking wking mentioned this pull request Mar 28, 2018
Merged
rphillips pushed a commit that referenced this pull request Mar 28, 2018
@wking @rphillips
Documentation/root-requirements: API server no longer needs root (#943)
fbcb680 
Catch up with 7370202 apiserver: (run as non-root user, 2017-12-04,
#789).
@wking wking mentioned this pull request Mar 30, 2018
Closed
dghubble added a commit to poseidon/terraform-render-bootstrap that referenced this pull request Jun 19, 2018
@dghubble
Change apiserver port from 443 to 6443
b455fb2 
* Consumers MUST update load balanacers, firewall rules,
security groups, and utilities to correspond
* Drop root privileges in apiserver pods
* kubernetes-retired/bootkube#789
@dghubble dghubble mentioned this pull request Jun 19, 2018
Merged
dghubble added a commit to poseidon/terraform-render-bootstrap that referenced this pull request Jun 20, 2018
@dghubble
Change apiserver port from 443 to 6443
dcd8ae0 
* Requires updating load balancers, firewall rules,
security groups, and potentially routers/balancers
* Temporarily allow apiserver_port override to accommodate
edge cases or migration
* kubernetes-retired/bootkube#789
dghubble added a commit to poseidon/terraform-render-bootstrap that referenced this pull request Jun 20, 2018
@dghubble
Change apiserver port from 443 to 6443
2bcf61b 
* Requires updating load balancers, firewall rules,
security groups, and potentially routers/balancers
* Temporarily allow apiserver_port override to accommodate
edge cases or migration
* kubernetes-retired/bootkube#789
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@rphillips rphillips rphillips approved these changes

@dghubble dghubble Awaiting requested review from dghubble

Assignees

@diegs diegs

Labels
cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. size/M Denotes a PR that changes 30-99 lines, ignoring generated files.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@diegs @rphillips @dghubble @redbaron @xiang90 @k8s-ci-robot