UPSTREAM: 58720: Ensure that the runtime mounts RO volumes read-only by joelsmith · Pull Request #18255 · openshift/origin

joelsmith · 2018-01-23T23:55:44Z

This is a backport of kubernetes/kubernetes#58720

This change makes it so that containers cannot write to secret, configMap, downwardAPI and projected volumes since the runtime will now mount them read-only. This change makes things less confusing for a user since any attempt to update a secret volume will result in an error rather than a successful change followed by a revert by the kubelet when the volume next syncs.

Which issue(s) this PR fixes
N/A

Release note:

Containers now mount secret, configMap, downwardAPI and projected volumes read-only.

jsafrane · 2018-02-02T12:30:13Z

++ Building go targets for linux/amd64: cmd/openshift cmd/oc cmd/oadm cmd/template-service-broker vendor/k8s.io/kubernetes/cmd/hyperkube pkg/network/sdn-cni-plugin vendor/github.com/containernetworking/plugins/plugins/ipam/host-local vendor/github.com/containernetworking/plugins/plugins/main/loopback
# github.com/openshift/origin/vendor/k8s.io/kubernetes/pkg/features
vendor/k8s.io/kubernetes/pkg/features/kube_features.go:276:54: undefined: feature.Deprecated

joelsmith · 2018-02-04T04:58:54Z

/test unit

sjenning · 2018-02-05T15:34:28Z

vendor/k8s.io/kubernetes/test/e2e/common/downwardapi_volume.go

 				{
 					Name:      "podinfo",
-					MountPath: "/etc",
+					MountPath: "/etc/podinfo",


thanks for fixing this! I noticed this while working on a flake for this test and was surprised when it overwrote /etc :P

sjenning · 2018-02-05T15:36:39Z

vendor/k8s.io/kubernetes/cluster/addons/dashboard/dashboard-controller.yaml

      - name: kubernetes-dashboard-certs
-        secret:
-          secretName: kubernetes-dashboard-certs
+        emptyDir: {}


does this work? does the dashboard create its own certs at start time?

@joelsmith could you respond here?

irrelevant for this pick.

aveshagarwal · 2018-02-06T22:16:57Z

@joelsmith could you check for failures if there is anything relevant?

aveshagarwal · 2018-02-06T22:17:24Z

@smarterclayton could you approve this?

smarterclayton · 2018-02-07T07:16:58Z

/approve

sjenning · 2018-02-07T21:37:27Z

/retest

aveshagarwal · 2018-02-08T16:12:03Z

Since upstream PR is merged, is it ok to give lgtm here? @derekwaynecarr

derekwaynecarr · 2018-02-08T16:17:30Z

/approved
/lgtm

openshift-ci-robot · 2018-02-08T16:17:35Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: derekwaynecarr, joelsmith, smarterclayton

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

Details

Needs approval from an approver in each of these OWNERS Files:

~~vendor/k8s.io/kubernetes/cluster/OWNERS~~ [derekwaynecarr,smarterclayton]
~~vendor/k8s.io/kubernetes/pkg/OWNERS~~ [derekwaynecarr,smarterclayton]
~~vendor/k8s.io/kubernetes/test/OWNERS~~ [derekwaynecarr,smarterclayton]

You can indicate your approval by writing /approve in a comment
You can cancel your approval by writing /approve cancel in a comment

openshift-merge-robot · 2018-02-09T03:57:20Z

Automatic merge from submit-queue (batch tested with PRs 18423, 18255, 18526, 18539, 18509).

openshift-ci-robot requested review from gnufied, jsafrane, rootfs, screeley44 and smarterclayton January 23, 2018 23:55

openshift-merge-robot added the vendor-update Touching vendor dir or related files label Jan 23, 2018

openshift-ci-robot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Jan 23, 2018

joelsmith changed the title **What this PR does / why we need it**: UPSTREAM: 58720: Ensure that the runtime mounts RO volumes read-only Jan 23, 2018

joelsmith force-pushed the master branch from a9bfcbb to 9e27823 Compare January 23, 2018 23:57

openshift-ci-robot added size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. and removed size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Jan 23, 2018

joelsmith force-pushed the master branch from 9e27823 to bbc362f Compare February 1, 2018 20:25

openshift-ci-robot added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. and removed size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. labels Feb 1, 2018

UPSTREAM: 58720: Ensure that the runtime mounts RO volumes read-only

62cd305

joelsmith force-pushed the master branch from bbc362f to 62cd305 Compare February 2, 2018 22:48

sjenning reviewed Feb 5, 2018

View reviewed changes

openshift-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Feb 7, 2018

openshift-ci-robot assigned derekwaynecarr Feb 8, 2018

openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Feb 8, 2018

openshift-merge-robot merged commit 6ce0af0 into openshift:master Feb 9, 2018

liggitt mentioned this pull request Mar 12, 2018

[3.9] UPSTREAM: 61045: subpath fixes #18957

Merged

Conversation

joelsmith commented Jan 23, 2018

Uh oh!

jsafrane commented Feb 2, 2018

Uh oh!

joelsmith commented Feb 4, 2018

Uh oh!

sjenning Feb 5, 2018

Choose a reason for hiding this comment

Uh oh!

sjenning Feb 5, 2018

Choose a reason for hiding this comment

Uh oh!

aveshagarwal Feb 6, 2018

Choose a reason for hiding this comment

Uh oh!

derekwaynecarr Feb 8, 2018

Choose a reason for hiding this comment

Uh oh!

aveshagarwal commented Feb 6, 2018

Uh oh!

aveshagarwal commented Feb 6, 2018

Uh oh!

smarterclayton commented Feb 7, 2018 via email

Uh oh!

sjenning commented Feb 7, 2018

Uh oh!

aveshagarwal commented Feb 8, 2018

Uh oh!

derekwaynecarr commented Feb 8, 2018

Uh oh!

openshift-ci-robot commented Feb 8, 2018

Uh oh!

openshift-merge-robot commented Feb 9, 2018

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

8 participants