Skip to content

Conversation

@anik120
Copy link
Contributor

@anik120 anik120 commented Aug 19, 2022

This PR applies the restricted security level defined by the Pod Security
Admission Controller
. It also modifies
the deployments to add the required securityContext for the pods deployed to be
admitted at the restricted level.

@openshift-ci openshift-ci bot requested review from exdx and tylerslaton August 19, 2022 21:27
@openshift-merge-robot openshift-merge-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Aug 23, 2022
@openshift-merge-robot openshift-merge-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Aug 25, 2022
This PR applies the `restricted` security level [defined by the Pod Security
Admission Controller](https://kubernetes.io/docs/tasks/configure-pod-container/enforce-standards-namespace-labels/). It also modifies
the deployments to add the required securityContext for the pods deployed to be
admitted at the `restricted` level.
Copy link
Contributor

@tylerslaton tylerslaton left a comment

Choose a reason for hiding this comment

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

Overall this looks good to me, just a small comment on it. The structure you've gone with here reflects our currently set precedence. We'll need an approve as well as some overrides from @timflannagan when this is ready.

/lgtm

metadata:
labels:
control-plane: controller-manager
openshift.io/scc: "anyuid"
Copy link
Contributor

@tylerslaton tylerslaton Aug 25, 2022

Choose a reason for hiding this comment

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

I'd like to see a comment describing why we're setting this label to this value with a link to the documentation. Do you think that would be too invasive?

Comment on lines +7 to +8
pod-security.kubernetes.io/enforce: restricted
pod-security.kubernetes.io/enforce-version: v1.24
Copy link
Contributor

Choose a reason for hiding this comment

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

Same from above applied to these lines.

@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Aug 25, 2022
@openshift-ci
Copy link
Contributor

openshift-ci bot commented Aug 25, 2022

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: anik120, tylerslaton
Once this PR has been reviewed and has the lgtm label, please assign timflannagan for approval by writing /assign @timflannagan in a comment. For more information see:The Kubernetes Code Review Process.

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

Details Needs approval from an approver in each of these files:

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

@anik120
Copy link
Contributor Author

anik120 commented Aug 25, 2022

/hold

Till we make necessary changes to rukpak's workloads to make them compatible for the restricted profile.

@openshift-ci openshift-ci bot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Aug 25, 2022
@openshift-ci
Copy link
Contributor

openshift-ci bot commented Aug 25, 2022

@anik120: The following test failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name Commit Details Required Rerun command
ci/prow/e2e a14a4a1 link false /test e2e

Full PR test history. Your PR dashboard.

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-merge-robot openshift-merge-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Aug 27, 2022
@openshift-merge-robot
Copy link
Contributor

@anik120: PR needs rebase.

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.

@timflannagan
Copy link
Contributor

/close

@openshift-ci openshift-ci bot closed this Sep 2, 2022
@openshift-ci
Copy link
Contributor

openshift-ci bot commented Sep 2, 2022

@timflannagan: Closed this PR.

Details

In response to this:

/close

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. lgtm Indicates that a PR is ready to be merged. needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants