Add PSP support #389
Add PSP support #389
Conversation
andreas-kupries
force-pushed
the
aku-9EmTR4JL-psp-support
branch
from
b388cce
to
1e689bc
Compare
andreas-kupries
force-pushed
the
aku-9EmTR4JL-psp-support
branch
from
1e689bc
to
adb7c67
Compare
Limited set of fixed names. Reference variables in chart values.yml for user to configure. No kube objects for undefined psp's. Service account cloning in case of conflicting psp uses by the igroups using the account PSP information is per-job, merged into a per-role value for use with service account
andreas-kupries
force-pushed
the
aku-9EmTR4JL-psp-support
branch
from
adb7c67
to
94cd7fd
Compare
model/roles.go
Outdated
|
|
||
| // GroupPolicy determines the name of the pod security policy | ||
| // governing the specified instance group. | ||
| func GroupPolicy(instanceGroup *InstanceGroup) string { |
There was a problem hiding this comment.
Why is this an exported function; it only seems to be called from within this file.
Also would prefer groupPodSecurityPolicy for consistency with other method names.
There was a problem hiding this comment.
Sure, will change.
There was a problem hiding this comment.
Changed with commit 30f483d
kube/pod.go
Outdated
| // This role requires a custom service account | ||
| block := helm.Block("") | ||
| if settings.CreateHelmChart { | ||
| block = helm.Block(authModeRBAC) | ||
| } | ||
| spec.Add("serviceAccountName", role.Run.ServiceAccount, block) | ||
| } | ||
|
|
||
| // Note: This group's PSP reference is not used, as it is not |
There was a problem hiding this comment.
I don't understand what "group's PSP" means here. Aren't all PSPs on jobs and not on groups?
So not sure what this comment is supposed to be helping with here; it only leaves me more confused. Also seems to be the only reference to PSPs in this file. Why is it needed?
There was a problem hiding this comment.
While PSP's are on jobs all the PSPs referenced by the jobs of an instance group are merged together into a single policy for the entire group. Because all the jobs currently still run in a single container/pod with a single PSP attached to it.
Will check if I can phrase it better.
There was a problem hiding this comment.
Ok, checked. This comment is out of date. It harkens back to an interim state where the instance group structure actually had a PSP field. Then this moved to Jobs, and the IG's psp is computed (see groupPodSecurityPolicy), and then immediately moved to the associated service account.
Nothing is stored in the IG any longer.
I will remove this comment.
There was a problem hiding this comment.
Changed in commit 273e933
kube/rbac.go
Outdated
| @@ -41,6 +41,31 @@ func NewRBACAccount(name string, account model.AuthAccount, settings ExportSetti | |||
| resources = append(resources, binding) | |||
| } | |||
|
|
|||
| // Integrate a PSP, via ClusterRoleBinding to ClusterRole | |||
| if account.PodSecurityPolicy != "" && account.PodSecurityPolicy != "null" { | |||
There was a problem hiding this comment.
resolveSecurityPolicies always sets the value to either nonprivileged or privileged, so this conditional should always be true. How can the value be "" or "null" ?
There was a problem hiding this comment.
The "null" might be a leftover from some interim state. The ""-case might happen in tests which do not go through rSP. Will check.
There was a problem hiding this comment.
It looks to be wrong. Only one test case affected, which explicitly has no PSP set. I will drop the conditions and remove that test.
There was a problem hiding this comment.
Changed in commit 273e933
model/roles.go
Outdated
| // As service accounts can reference only one PSP the operation makes | ||
| // clones of the base SA as needed. Note that the clones reference the same | ||
| // roles as the base, and that the roles are not cloned. | ||
| func (m *RoleManifest) resolveSecurityPolicies() error { |
There was a problem hiding this comment.
For consistency this should include Pod, as in resolvePodSecurityPolicies.
There was a problem hiding this comment.
Ok. Will change.
There was a problem hiding this comment.
Changed with commit 30f483d
kube/rbac.go
Outdated
| @@ -76,3 +101,40 @@ func NewRBACRole(name string, role model.AuthRole, settings ExportSettings) (hel | |||
|
|
|||
| return container.Sort(), nil | |||
| } | |||
|
|
|||
| // authPSP creates a block condition checking for RBAC and the named PSP | |||
| func authPSP(psp string) helm.NodeModifier { | |||
There was a problem hiding this comment.
Don't like the function name (given that there is also authPSPRoleName).
Maybe authPSPCondition?
There was a problem hiding this comment.
Like it. Will change.
There was a problem hiding this comment.
Changed with commit 30f483d
Ref: https://trello.com/c/9EmTR4JL/775-3-add-psp-support-to-fissile-and-role-manifests
Ref: https://github.com/SUSE/fissile/wiki/Managing-Pod-Security-Policies