-
Notifications
You must be signed in to change notification settings - Fork 43
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
WIP: Openshift 4.11 Pod security standards #260
WIP: Openshift 4.11 Pod security standards #260
Conversation
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: CathalOConnorRH The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
8e50c32
to
3881e4e
Compare
@CathalOConnorRH @Mo3m3n has already worked on implementing this in his PR #241 and there is a discussion going on there. Could you please check that and see if this PR is bringing other changes than what he submitted there? |
I believe these are different, @Mo3m3n is implementing pod security on ckcp deployment, this PR is implementing pod security on the pods created by the pipeline runs which will be on the workload clusters |
3881e4e
to
cd44021
Compare
@CathalOConnorRH Shouldn't this rather be handled by the Openshift pipelines operator ? |
@Mo3m3n There is an upstream story opened for this for v1.9 but no release date set yet. This will secure pipelines sooner and allow us to resolve any issues we may encounter. |
Setting as draft, as my understanding is that this PR might not be merged. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@CathalOConnorRH the item that's missing here is the role binding.
Looking at what the Tekton operator does, it creates a RoleBinding
in every namespace to go along with the pipelines
service account. It seems that we would need to replicate this logic in the workspace controller.
include.release.openshift.io/ibm-cloud-managed: 'true' | ||
include.release.openshift.io/self-managed-high-availability: 'true' | ||
include.release.openshift.io/single-node-developer: 'true' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We don't need these - these are special annotations used by OLM for OpenShift (I believe)
include.release.openshift.io/single-node-developer: 'true' | ||
kubernetes.io/description: >- | ||
pipelines-scc-v2 is a close replica of pipelines-scc scc. pipelines-scc-v2 has allowPrivilegeEscalation=false. | ||
release.openshift.io/create-only: 'true' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Likewise - I don' think we need this, since ArgoCD is going to be doing the syncing.
@@ -0,0 +1,14 @@ | |||
--- | |||
kind: ClusterRole |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I guess the key question is if this ClusterRole needs to be created on the workload cluster, or if it needs to be created in KCP.
@CathalOConnorRH has this PR gone stale? If it is not needed anymore please close it. If it is still relevant please respond to the feedback and rebase the changes. |
@CathalOConnorRH: PR needs rebase. 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. |
@CathalOConnorRH Could you check if this PR is relevant anymore with the changes to the repo? If not, please close it. |
Update PipelinesAsCode to 0.6.0
Created new SCC to match baseline pod security standards by setting allowPrivilegeEscalation: false and updating clusterrole to use new SCC. This is required from Openshift 4.11 onwards.