Add sysctls to the ouput of `describe` on PSPs #65218

stlaz · 2018-06-19T13:02:56Z

What this PR does / why we need it:
When promoting the sysctls feature for PSPs, the output of the
kubectl describe command was forgotten about. This commit
adds the AllowedUnsafeSysctls and ForbiddenSysctls fields
to the output of that command.

Which issue(s) this PR fixes :
Fixes #65181

Release notes:

NONE

php-coder · 2018-06-19T13:09:34Z

/ok-to-test

php-coder

A handful comments from me. Looks good overall!

php-coder · 2018-06-19T13:12:45Z

pkg/printers/internalversion/describe_test.go

@@ -2248,6 +2249,7 @@ func TestDescribePodSecurityPolicy(t *testing.T) {
 			Name: "mypsp",
 		},
 		Spec: policy.PodSecurityPolicySpec{
+			AllowedUnsafeSysctls: []string{"kernel.*", ",net.ipv4.ip_local_port_range"},


What's about forbidden sysctls?

",net.ipv4.ip_local_port_range"

Why it's started from a comma?

I thought I'd test only one of those two since many of the other attributes seem to be left out as well (e.g. FlexVolume) and forbidden sysctls have the same display method as allowed unsafe ones. I can add them if you want, though.

php-coder · 2018-06-19T13:15:01Z

pkg/printers/internalversion/describe_test.go

@@ -2228,6 +2228,7 @@ func TestDescribePodSecurityPolicy(t *testing.T) {
 		"Required Drop Capabilities:\\s*<none>",
 		"Allowed Capabilities:\\s*<none>",
 		"Allowed Volume Types:\\s*<none>",
+		"Allowed Unsafe Sysctls:\\skernel.*,net.ipv4.ip_local_port_range",


Because these are not just a plain string but regexps, I'd suggest to:

replace \\s by \\s*

quote . and * that are part of the expected output rather than a regexp itself

Ah, you're right, I completely missed the dot and the asterisk somehow got left out as well. Thanks!

php-coder · 2018-06-19T13:17:41Z

/sig auth
/sig cli

ingvagabund · 2018-06-19T15:34:39Z

pkg/printers/internalversion/describe_test.go

@@ -2248,6 +2249,7 @@ func TestDescribePodSecurityPolicy(t *testing.T) {
 			Name: "mypsp",
 		},
 		Spec: policy.PodSecurityPolicySpec{
+			AllowedUnsafeSysctls: []string{"kernel.*", "net.ipv4.ip_local_port_range"},


Can you add test for ForbiddenSysctls field as well? Just in case it gets removed by accident?

When promoting the sysctls feature for PSPs, the output of the `kubectl describe` command was forgotten about. This commit adds the `AllowedUnsafeSysctls` and `ForbiddenSysctls` fields to the output of that command.

stlaz · 2018-06-20T06:11:15Z

The latest patch now also includes the test for "Forbidden Sysctls"

k8s-ci-robot · 2018-06-20T06:37:29Z

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

Test name	Commit	Details	Rerun command
pull-kubernetes-kubemark-e2e-gce	2d6c640892e8c4ac83eab6cd97f44c3667b6fdf4	link	`/test pull-kubernetes-kubemark-e2e-gce`

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.

php-coder · 2018-06-20T10:12:32Z

PTAL @kubernetes/sig-cli-maintainers

/retest
/lgtm

soltysh

/lgtm
/approve

k8s-ci-robot · 2018-06-21T10:57:55Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: php-coder, soltysh, stlaz

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:

~~pkg/printers/OWNERS~~ [soltysh]

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

k8s-github-robot · 2018-06-22T04:12:11Z

Automatic merge from submit-queue (batch tested with PRs 65064, 65218, 65260, 65241, 64372). If you want to cherry-pick this change to another branch, please follow the instructions here.

k8s-ci-robot requested review from rootfs and soltysh June 19, 2018 13:03

k8s-ci-robot added release-note-none Denotes a PR that doesn't merit a release note. and removed do-not-merge/release-note-label-needed Indicates that a PR should not merge because it's missing one of the release note labels. labels Jun 19, 2018

k8s-ci-robot removed the needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. label Jun 19, 2018

php-coder suggested changes Jun 19, 2018

View reviewed changes

k8s-ci-robot added sig/auth Categorizes an issue or PR as relevant to SIG Auth. sig/cli Categorizes an issue or PR as relevant to SIG CLI. labels Jun 19, 2018

stlaz force-pushed the sysctls_describe branch 2 times, most recently from cc5573d to 2d6c640 Compare June 19, 2018 13:41

ingvagabund reviewed Jun 19, 2018

View reviewed changes

Add sysctls to the ouput of describe on PSPs

dd667ec

When promoting the sysctls feature for PSPs, the output of the `kubectl describe` command was forgotten about. This commit adds the `AllowedUnsafeSysctls` and `ForbiddenSysctls` fields to the output of that command.

stlaz force-pushed the sysctls_describe branch from 2d6c640 to dd667ec Compare June 20, 2018 06:09

k8s-ci-robot assigned php-coder Jun 20, 2018

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Jun 20, 2018

k8s-ci-robot assigned soltysh Jun 21, 2018

soltysh approved these changes Jun 21, 2018

View reviewed changes

k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jun 21, 2018

k8s-github-robot merged commit b5ea862 into kubernetes:master Jun 22, 2018

stlaz mentioned this pull request Apr 1, 2022

REQUEST: New membership for stlaz kubernetes/org#3352

Closed

9 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add sysctls to the ouput of `describe` on PSPs #65218

Add sysctls to the ouput of `describe` on PSPs #65218

stlaz commented Jun 19, 2018 •

edited

Loading

php-coder commented Jun 19, 2018

php-coder left a comment

php-coder Jun 19, 2018

php-coder Jun 19, 2018 •

edited

Loading

stlaz Jun 19, 2018

php-coder Jun 19, 2018

stlaz Jun 19, 2018

php-coder commented Jun 19, 2018

ingvagabund Jun 19, 2018

stlaz Jun 19, 2018

stlaz commented Jun 20, 2018

k8s-ci-robot commented Jun 20, 2018 •

edited

Loading

php-coder commented Jun 20, 2018

soltysh left a comment

k8s-ci-robot commented Jun 21, 2018

k8s-github-robot commented Jun 22, 2018

Add sysctls to the ouput of describe on PSPs #65218

Add sysctls to the ouput of describe on PSPs #65218

Conversation

stlaz commented Jun 19, 2018 • edited Loading

php-coder commented Jun 19, 2018

php-coder left a comment

Choose a reason for hiding this comment

php-coder Jun 19, 2018

Choose a reason for hiding this comment

php-coder Jun 19, 2018 • edited Loading

Choose a reason for hiding this comment

stlaz Jun 19, 2018

Choose a reason for hiding this comment

php-coder Jun 19, 2018

Choose a reason for hiding this comment

stlaz Jun 19, 2018

Choose a reason for hiding this comment

php-coder commented Jun 19, 2018

ingvagabund Jun 19, 2018

Choose a reason for hiding this comment

stlaz Jun 19, 2018

Choose a reason for hiding this comment

stlaz commented Jun 20, 2018

k8s-ci-robot commented Jun 20, 2018 • edited Loading

php-coder commented Jun 20, 2018

soltysh left a comment

Choose a reason for hiding this comment

k8s-ci-robot commented Jun 21, 2018

k8s-github-robot commented Jun 22, 2018

Add sysctls to the ouput of `describe` on PSPs #65218

Add sysctls to the ouput of `describe` on PSPs #65218

stlaz commented Jun 19, 2018 •

edited

Loading

php-coder Jun 19, 2018 •

edited

Loading

k8s-ci-robot commented Jun 20, 2018 •

edited

Loading