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

Create /var/lib/etcd with 0700 #71885

Merged
merged 1 commit into from Dec 10, 2018

Conversation

@dims
Copy link
Member

dims commented Dec 9, 2018

If we let the hostpath with DirectoryOrCreate to create this directory
it defaults to 0755. A default install should use 0700 for better
security especially if the directory is not present.

Change-Id: Idc0266685895767b0d1c5710c8a4fb704805652f

What type of PR is this?
/kind bug

What this PR does / why we need it:

Which issue(s) this PR fixes:

Fixes kubernetes/kubeadm#1308

Special notes for your reviewer:

Does this PR introduce a user-facing change?:

 kubeadm: Create /var/lib/etcd with correct permissions (0700) by default.
@dims

This comment has been minimized.

Copy link
Member

dims commented Dec 9, 2018

/sig cluster-lifecycle

@k8s-ci-robot k8s-ci-robot requested review from chuckha and yagonobre Dec 9, 2018

@yagonobre
Copy link
Member

yagonobre left a comment

Thanks @dims
/lgtm

Create /var/lib/etcd with 0700
If we let the hostpath with DirectoryOrCreate to create this directory
it defaults to 0755. A default install should use 0700 for better
security especially if the directory is not present.

Change-Id: Idc0266685895767b0d1c5710c8a4fb704805652f

@dims dims force-pushed the dims:create-etcd-with-0700-permissions branch from 5fb89b0 to 836f413 Dec 9, 2018

@k8s-ci-robot k8s-ci-robot removed the lgtm label Dec 9, 2018

@rosti

rosti approved these changes Dec 10, 2018

Copy link
Contributor

rosti left a comment

Looks great! Thank you @dims !

/lgtm

@k8s-ci-robot k8s-ci-robot added the lgtm label Dec 10, 2018

@neolit123
Copy link
Member

neolit123 left a comment

thanks @dims
could you please prefix the release note with kubeadm: ...

/hold
/approve
/priority important-longerm

i guess this only fixes the case where the path was not created already by something else as @yagonobre outlined: https://kubernetes.io/docs/concepts/storage/volumes/#hostpath

@k8s-ci-robot

This comment has been minimized.

Copy link
Contributor

k8s-ci-robot commented Dec 10, 2018

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: dims, neolit123

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@dims

This comment has been minimized.

Copy link
Member

dims commented Dec 10, 2018

@neolit123 done!

@neolit123

This comment has been minimized.

Copy link
Member

neolit123 commented Dec 10, 2018

thanks
/priority important-longterm
/hold cancel

@rosti

This comment has been minimized.

Copy link
Contributor

rosti commented Dec 10, 2018

/test pull-kubernetes-e2e-kops-aws

@k8s-ci-robot k8s-ci-robot merged commit 0b13221 into kubernetes:master Dec 10, 2018

17 of 18 checks passed

pull-kubernetes-e2e-kops-aws Job triggered.
Details
cla/linuxfoundation dims 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-100-performance Job succeeded.
Details
pull-kubernetes-e2e-gce-device-plugin-gpu Job succeeded.
Details
pull-kubernetes-e2e-gke Skipped
pull-kubernetes-e2e-kubeadm-gce Skipped
pull-kubernetes-integration Job succeeded.
Details
pull-kubernetes-kubemark-e2e-gce-big Job succeeded.
Details
pull-kubernetes-local-e2e Skipped
pull-kubernetes-local-e2e-containerized Skipped
pull-kubernetes-node-e2e Job succeeded.
Details
pull-kubernetes-typecheck Job succeeded.
Details
pull-kubernetes-verify Job succeeded.
Details
tide In merge pool.
Details
@luxas
Copy link
Member

luxas left a comment

/lgtm
Thanks a lot @dims :)

@luxas

This comment has been minimized.

Copy link
Member

luxas commented Dec 11, 2018

@timothysc shall we cherrypick this?

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