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

Run Mintmaker DependencyUpdateCheck cronjobs as non-root (attempt #2) #5894

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

querti
Copy link
Contributor

@querti querti commented Mar 20, 2025

Mintmaker SA has been granted the anyuid SCC becuase it needs to run its subscription-manager pod as root[1]. A side effect is that now it tries to run all of its pods as root by default. This is not secure and also conflicts with the setting runAsNonRoot: true, resulting in failing jobs. Explicitly run these cronjobs as a non-root user.

This commit should be merged at the same time as [1], otherwise the created jobs will fail with an error. Without the anyuid SCC, pods cannot have arbitrary UIDs - they must be selected from the allowed range. Each namespace has its own range and it is chosen arbitrarily. This means that mintmaker namespace in each cluster has a different allowed UID range. Explicitly setting the UID will cause an error - at least in some clusters. Having anyuid SCC solves this as any UID that doesn't have to adhere to the predefined UID range can be chosen.

Merging this commit after [1] will cause these cronjob pods to be created as a root user, which is not allowed as per the defined security context.

[1] konflux-ci/mintmaker#222

Refers to CLOUDDST-26090

Copy link

openshift-ci bot commented Mar 20, 2025

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

Copy link

openshift-ci bot commented Mar 20, 2025

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: querti

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

The pull request process is described here

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

@querti
Copy link
Contributor Author

querti commented Mar 20, 2025

!!!!!!!! DO NOT MERGE !!!!!!!!

I believe the reason #5868 failed is because it was prematurely merged before new SCC introduced by konflux-ci/mintmaker#222 could be applied.

@qixiang
Copy link
Contributor

qixiang commented Mar 21, 2025

!!!!!!!! DO NOT MERGE !!!!!!!!

I believe the reason #5868 failed is because it was prematurely merged before new SCC introduced by konflux-ci/mintmaker#222 could be applied.

Could you change it in staging first? It should be possible to add a patch in mintmaker/staging/base to modify the cronjobs.

…hat-appstudio#2)

Mintmaker SA has been granted the anyuid SCC becuase it needs to run its
subscription-manager pod as root[1]. A side effect is that now it tries
to run all of its pods as root by default. This is not secure and also
conflicts with the setting runAsNonRoot: true, resulting in failing
jobs. Explicitly run these cronjobs as a non-root user.

This commit should be merged at the same time as [1], otherwise the
created jobs will fail with an error. Without the anyuid SCC, pods
cannot have arbitrary UIDs - they must be selected from the allowed
range. Each namespace has its own range and it is chosen arbitrarily.
This means that mintmaker namespace in each cluster has a different
allowed UID range. Explicitly setting the UID will cause an error - at
least in some clusters. Having anyuid SCC solves this as any UID that
doesn't have to adhere to the predefined UID range can be chosen.

Merging this commit after [1] will cause these cronjob pods to be
created as a root user, which is not allowed as per the defined security
context.

[1] konflux-ci/mintmaker#222
@querti querti force-pushed the mintmaker-run-cronjob-as-nonroot-#2 branch from 4dc60ee to c6cface Compare March 21, 2025 08:37
@querti
Copy link
Contributor Author

querti commented Mar 21, 2025

!!!!!!!! DO NOT MERGE !!!!!!!!
I believe the reason #5868 failed is because it was prematurely merged before new SCC introduced by konflux-ci/mintmaker#222 could be applied.

Could you change it in staging first? It should be possible to add a patch in mintmaker/staging/base to modify the cronjobs.

That's a good idea, I changed it to only modify the cronjobs in stage. Though I think for maximum safety, this patch should be a part of the "promote to stage" PR.

@qixiang
Copy link
Contributor

qixiang commented Mar 21, 2025

That's a good idea, I changed it to only modify the cronjobs in stage. Though I think for maximum safety, this patch should be a part of the "promote to stage" PR.

Nice, you can amend the promote PR to include this commit before we're ready to deploy to stage, (then close this one).

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

Successfully merging this pull request may close these issues.

2 participants