Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

(Successor of #21) Add an allow list parameter, allowedSysctls, to the Forbidden sysctl constraint template #253

Merged
merged 14 commits into from Nov 29, 2022

Conversation

ordovicia
Copy link
Contributor

This PR is a successor of #21 and adds description for the new field, as per #21 (comment).

Quoting PR description from #21:

The K8sPSPForbiddenSysctls constraint template allows limiting sysctls available to pods using a deny list. This change adds another option, allowedSysctls, which allows a constraint to specify an allow list in addition to, or instead of, the deny list.

The matching logic is as follows:

  1. When specified, any sysctl not in the allow list is considered to be forbidden.
  2. The allow list can be omitted.
  3. The deny list takes precedence.

shomron and others added 2 commits November 2, 2022 10:08
…constraint template

The `K8sPSPForbiddenSysctls` constraint template allows limiting sysctls
available to pods using a deny list. This change adds another option,
`allowedSysctls`, which allows a constraint to specify an allow list in
addition to, or instead of, the deny list.

The matching logic is as follows:

1. When specified, any sysctl not in the allow list is considered to be forbidden.
2. The allow list can be omitted.
3. The deny list takes precedence.

Signed-off-by: Oren Shomron <shomron@gmail.com>
Signed-off-by: Hidehito Yabuuchi <hdht.ybuc@gmail.com>
Signed-off-by: Hidehito Yabuuchi <hdht.ybuc@gmail.com>
Signed-off-by: Hidehito Yabuuchi <hdht.ybuc@gmail.com>
Signed-off-by: Hidehito Yabuuchi <hdht.ybuc@gmail.com>
Signed-off-by: Hidehito Yabuuchi <hdht.ybuc@gmail.com>
…ctls

Signed-off-by: Hidehito Yabuuchi <hdht.ybuc@gmail.com>
…sctls

Signed-off-by: Hidehito Yabuuchi <hdht.ybuc@gmail.com>
Signed-off-by: Hidehito Yabuuchi <hdht.ybuc@gmail.com>
@ordovicia ordovicia requested review from maxsmythe and nilekhc and removed request for maxsmythe and nilekhc November 7, 2022 03:13
Copy link
Contributor

@maxsmythe maxsmythe left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM after nit!

violation[{"msg": msg, "details": {}}] {
sysctl := input.review.object.spec.securityContext.sysctls[_].name
not allowed_sysctl(sysctl)
msg := sprintf("The sysctl %v is not explictly allowed, pod: %v. Allowed sysctls: %v", [sysctl, input.review.object.metadata.name, input.parameters.allowedSysctls])
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should add a comment that input.parameters.allowedSysctls in sprintf() is load-bearing.

Without that line, this change would be backwards-incompatible and that seems non-obvious. Adding a comment will help prevent accidentally breaking this requirement in the future.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry, what specifically do you mean by "load-bearing"?

…s 1.0.0

Signed-off-by: Hidehito Yabuuchi <hdht.ybuc@gmail.com>
@ordovicia ordovicia requested review from ritazh and removed request for sozercan November 14, 2022 02:51
@ordovicia
Copy link
Contributor Author

Sorry, I accidentally removed a review request from @sozercan

@ordovicia
Copy link
Contributor Author

Hi, @maxsmythe @ritazh @sozercan cloud you please take a look again and merge this?

Signed-off-by: Hidehito Yabuuchi <hdht.ybuc@gmail.com>
Copy link
Member

@ritazh ritazh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@maxsmythe maxsmythe merged commit 377cb91 into open-policy-agent:master Nov 29, 2022
@ordovicia ordovicia deleted the sysctl-allow-list branch November 29, 2022 04:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants