Modify "sudoer" group and "privileged" SCC to seamlessly work together #16213
Conversation
openshift-ci-robot
added
the
size/XS
There was a problem hiding this comment.
@php-coder you need to regenerate the bootstrap policy YAML files.
UPDATE_BOOTSTRAP_POLICY_FIXTURE_DATA=true hack/test-go.sh pkg/cmd/server/bootstrappolicy
| @@ -136,6 +136,7 @@ func GetOpenshiftBootstrapClusterRoles() []rbac.ClusterRole { | |||
| }, | |||
| Rules: []rbac.PolicyRule{ | |||
| rbac.NewRule("impersonate").Groups(userGroup, legacyUserGroup).Resources(authorizationapi.SystemUserResource, authorizationapi.UserResource).Names(SystemAdminUsername).RuleOrDie(), | |||
| rbac.NewRule("impersonate").Groups(userGroup, legacyUserGroup).Resources(authorizationapi.SystemGroupResource, authorizationapi.GroupResource).Names(MastersGroup).RuleOrDie(), | |||
There was a problem hiding this comment.
@deads2k we seem to have 3 different copies of MastersGroup. I do not know which one we want to use, and if we want to delete some of said copies.
There was a problem hiding this comment.
@deads2k we seem to have 3 different copies of MastersGroup. I do not know which one we want to use, and if we want to delete some of said copies.
We probably don't want the package dependencies for most constants.
| @@ -285,15 +285,15 @@ func GetBootstrapSecurityContextConstraints(sccNameToAdditionalGroups map[string | |||
| // GetBoostrapSCCAccess provides the default set of access that should be passed to GetBootstrapSecurityContextConstraints. | |||
| func GetBoostrapSCCAccess(infraNamespace string) (map[string][]string, map[string][]string) { | |||
| groups := map[string][]string{ | |||
| SecurityContextConstraintPrivileged: {ClusterAdminGroup, NodesGroup}, | |||
| SecurityContextConstraintPrivileged: {ClusterAdminGroup, NodesGroup, MastersGroup}, | |||
There was a problem hiding this comment.
@deads2k do we want to go ahead an remove ClusterAdminGroup?
There was a problem hiding this comment.
@deads2k do we want to go ahead an remove ClusterAdminGroup
Mark the constant deprecated, but I wouldn't remove it. If people used it (and it looks reasonable to use), a tighten shouldn't break them.
test/cmd/policy.sh
Outdated
| @@ -10,6 +10,7 @@ os::cmd::expect_success_and_text 'oc whoami --as deads' "deads" | |||
|
|
|||
| os::cmd::expect_success 'oc adm policy add-cluster-role-to-user sudoer wheel' | |||
| os::cmd::try_until_text 'oc policy who-can impersonate systemusers system:admin' "wheel" | |||
| os::cmd::try_until_text 'oc policy who-can impersonate systemgroups system:masters' "wheel" | |||
There was a problem hiding this comment.
also check:
oc policy who-can impersonate users system:admin
oc policy who-can impersonate groups system:masters
…ystem:masters group (--as-group).
…stem:masters group. This change improves UX and allows to a member of the sudoer group to, for example, create a privileged pod using impersonation: $ oc create -f privileged.pod --as system:admin
php-coder
force-pushed
the
gh16092_scc_and_impersonation
branch
from
9f94f06
to
228bc1c
Compare
openshift-ci-robot
added
size/S
|
/lgtm |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: enj, php-coder The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these OWNERS Files:
You can indicate your approval by writing |
openshift-merge-robot
added
the
approved
|
/retest Please review the full test history for this PR and help us cut down flakes. |
|
Automatic merge from submit-queue |
Prior this change, a member of "sudoer" group couldn't create a privileged pod (or perform
oc execagainst it). Also before, a user couldn't impersonatesystem:mastersgroup. Now all of these are became possible.PTAL @openshift/sig-security
Fixes #16092