SCC: Promote sysctl annotations to fields by stlaz · Pull Request #20151 · openshift/origin

stlaz · 2018-06-29T11:54:29Z

This PR promotes the experimental SCCs' sysctl-related annotations to fields.

promote sysctls from annotations to SCC fields
fix the sysctl-related tests disabled in the 1.11 rebase
extend the defaul privileged SCC to allow all sysctls

Trello: https://trello.com/c/WP50iqpB/222-add-forbiddensysctls-and-allowedunsafesysctls-options-to-sccs

php-coder · 2018-06-29T12:41:14Z

pkg/oc/cli/describe/describer.go

+		}
+		if len(scc.ForbiddenSysctls) > 0 {
+			fmt.Fprintf(out, "  Forbidden Sysctls:\t%s\n", sysctlsToString(scc.ForbiddenSysctls))
+		}


Looks like we always show the fields. If their content is empty, we just represent it as <all> or <none>. So, could you remove these if-s then?

php-coder · 2018-06-29T13:32:55Z

pkg/security/apis/security/validation/validation.go

+}
+
+const sysctlPatternSegmentFmt string = "([a-z0-9][-_a-z0-9]*)?[a-z0-9*]"
+const SysctlPatternFmt string = "(" + kapivalidation.SysctlSegmentFmt + "\\.)*" + sysctlPatternSegmentFmt


Could we make it non-pubic or something else depends on it?

I believe it should be alright to make it private, I'll change that.

openshift-bot · 2018-07-09T18:07:32Z

/test cross

stlaz · 2018-07-11T06:48:48Z

Apparently one of the dependencies is breaking the build:

# github.com/openshift/origin/vendor/github.com/spf13/viper
vendor/github.com/spf13/viper/viper.go:688:9: undefined: cast.ToInt32

liggitt · 2018-07-20T15:17:30Z

/hold cancel
/approve
adding lgtm from #20151 (comment)

openshift-bot · 2018-07-20T16:05:45Z

/retest

Please review the full test history for this PR and help us cut down flakes.

openshift-bot · 2018-07-20T18:07:00Z

/retest

Please review the full test history for this PR and help us cut down flakes.

liggitt · 2018-07-20T19:02:53Z

travis failure is due to old commit checking, fixed in #20381

CI is green otherwise

This reverts commit b5133dc.

liggitt · 2018-07-20T19:09:18Z

fixed bump commit title, re-tagging

liggitt · 2018-07-20T19:09:21Z

/lgtm

openshift-ci-robot · 2018-07-20T19:09:26Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: liggitt, simo5, stlaz

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

The pull request process is described here

Details

Needs approval from an approver in each of these files:

~~OWNERS~~ [liggitt]

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

openshift-bot · 2018-07-21T04:09:38Z

/retest

Please review the full test history for this PR and help us cut down flakes.

openshift-ci-robot · 2018-07-21T09:26:12Z

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

Test name	Commit	Details	Rerun command
ci/openshift-jenkins/extended_image_registry	`eff8d35`	link	`/test extended_image_registry`
ci/openshift-jenkins/extended_builds	`8b46d16`	link	`/test extended_builds`
ci/openshift-jenkins/extended_image_ecosystem	`8b46d16`	link	`/test extended_image_ecosystem`
ci/openshift-jenkins/cross	`8b46d16`	link	`/test cross`
ci/prow/cross	`898677a`	link	`/test cross`

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.

Details

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.

openshift-bot · 2018-07-21T10:12:40Z

/retest

Please review the full test history for this PR and help us cut down flakes.

openshift-ci-robot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Jun 29, 2018

openshift-ci-robot requested review from enj and liggitt June 29, 2018 11:54

openshift-ci-robot added the size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. label Jun 29, 2018

stlaz mentioned this pull request Jun 29, 2018

SCC: add {AllowedUnsafe,Forbidden}Sysctls openshift/api#60

Merged

php-coder reviewed Jun 29, 2018

View reviewed changes

stlaz force-pushed the sysctl_promotion branch 2 times, most recently from 17a47f7 to eff8d35 Compare June 29, 2018 13:24

php-coder reviewed Jun 29, 2018

View reviewed changes

stlaz force-pushed the sysctl_promotion branch from eff8d35 to b3b1c9e Compare June 29, 2018 19:49

openshift-bot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jul 4, 2018

stlaz force-pushed the sysctl_promotion branch from b3b1c9e to 8b46d16 Compare July 10, 2018 16:34

openshift-bot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jul 10, 2018

openshift-bot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jul 16, 2018

stlaz force-pushed the sysctl_promotion branch from 8b46d16 to 1018439 Compare July 16, 2018 11:36

openshift-bot added needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. and removed needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. labels Jul 16, 2018

stlaz force-pushed the sysctl_promotion branch from 1018439 to d1b8219 Compare July 17, 2018 13:16

openshift-ci-robot added size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. and removed size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. labels Jul 17, 2018

stlaz force-pushed the sysctl_promotion branch from d1b8219 to 6c9d002 Compare July 18, 2018 07:38

openshift-bot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jul 18, 2018

openshift-ci-robot added size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. and removed size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. labels Jul 18, 2018

stlaz force-pushed the sysctl_promotion branch from 6c9d002 to 8432567 Compare July 19, 2018 08:08

openshift-bot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jul 20, 2018

stlaz force-pushed the sysctl_promotion branch from fb346d1 to fe9fc21 Compare July 20, 2018 06:06

openshift-ci-robot removed the size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. label Jul 20, 2018

openshift-ci-robot added the size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. label Jul 20, 2018

Promote {allowedUnsafe,forbidden}Sysctls from annotations to fields

6cc02df

liggitt force-pushed the sysctl_promotion branch from 898677a to 1a60d00 Compare July 20, 2018 15:11

openshift-ci-robot added size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. and removed size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. labels Jul 20, 2018

openshift-ci-robot added approved Indicates a PR has been approved by an approver from all required OWNERS files. and removed do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. labels Jul 20, 2018

liggitt added lgtm Indicates that a PR is ready to be merged. api-approved approved Indicates a PR has been approved by an approver from all required OWNERS files. and removed approved Indicates a PR has been approved by an approver from all required OWNERS files. labels Jul 20, 2018

stlaz added 4 commits July 20, 2018 15:07

bump(*)

180410e

ran make update && ./hack/update-generated-swagger-spec.sh

3f797af

Revert "disable sysctl tests that don't support SCC."

2a7e444

This reverts commit b5133dc.

Bootstrap the "privileged" SCC with all sysctls allowed

d4a61f6

liggitt force-pushed the sysctl_promotion branch from 1a60d00 to d4a61f6 Compare July 20, 2018 19:08

openshift-ci-robot removed the lgtm Indicates that a PR is ready to be merged. label Jul 20, 2018

openshift-ci-robot assigned liggitt Jul 20, 2018

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

openshift-merge-robot merged commit 70e941d into openshift:master Jul 21, 2018

Conversation

stlaz commented Jun 29, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

php-coder Jun 29, 2018

Choose a reason for hiding this comment

Uh oh!

stlaz Jun 29, 2018

Choose a reason for hiding this comment

Uh oh!

php-coder Jun 29, 2018

Choose a reason for hiding this comment

Uh oh!

stlaz Jun 29, 2018

Choose a reason for hiding this comment

Uh oh!

openshift-bot commented Jul 9, 2018

Uh oh!

stlaz commented Jul 11, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

liggitt commented Jul 20, 2018

Uh oh!

openshift-bot commented Jul 20, 2018

Uh oh!

openshift-bot commented Jul 20, 2018

Uh oh!

liggitt commented Jul 20, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

liggitt commented Jul 20, 2018

Uh oh!

liggitt commented Jul 20, 2018

Uh oh!

openshift-ci-robot commented Jul 20, 2018

Uh oh!

openshift-bot commented Jul 21, 2018

Uh oh!

openshift-ci-robot commented Jul 21, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

openshift-bot commented Jul 21, 2018

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

8 participants

stlaz commented Jun 29, 2018 •

edited

Loading

stlaz commented Jul 11, 2018 •

edited

Loading

liggitt commented Jul 20, 2018 •

edited

Loading

openshift-ci-robot commented Jul 21, 2018 •

edited

Loading