-
Notifications
You must be signed in to change notification settings - Fork 898
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
Added securityContext profile to cronjob in istio-system and remove PSS workflow warnings #2848
Conversation
Why are you suing while instead of for ? https://stackoverflow.com/questions/49110/how-do-i-write-a-for-loop-in-bash |
What range would you like me to put in the for loop's condition? |
The range is up to you. 60 seconds is a good start. |
Perhaps I should change this PR for cronjob to just constraining loop time to the script. |
Maybe you need to check the script and OCI image |
I also thought the curlimages/curl image might have some issue so I added the latest verion 8.9.1 and pushed in one of the above commits. Seems like it's still facing the same. |
Please also check that you use a rootless image. Most bitnami images support runasnonroot. |
I believe the following was the reason for the failing cronjob even in my local system. I added runAsUser:999 to counter it. |
Now it is down to
After fixing this we can create PRs to upstream all patches into the right places/repositories |
Shall I fix these warnings in this PR itself? |
changes to the m2m script should trigger many workflows in the CICD. There seem to be missing trigger paths. |
Yes, that happened due to the typo Hansini made in her previous merged PR replacing /common/oauth2-proxy with common/oidc-client/oauth2-proxy. If she creates the PR, I could rebase and run the tests here again, or I could push a PR to fix the trigger paths myself and then rebase. |
Rebase to master first and see what is needed. Then lets merge and continue in a new PR, because this one here is getting too big. |
Signed-off-by: biswajit-9776 <[email protected]>
Signed-off-by: biswajit-9776 <[email protected]>
Signed-off-by: biswajit-9776 <[email protected]>
Signed-off-by: biswajit-9776 <[email protected]>
Signed-off-by: biswajit-9776 <[email protected]>
Signed-off-by: biswajit-9776 <[email protected]>
Signed-off-by: biswajit-9776 <[email protected]>
Signed-off-by: biswajit-9776 <[email protected]>
Signed-off-by: biswajit-9776 <[email protected]>
Signed-off-by: biswajit-9776 <[email protected]>
Signed-off-by: biswajit-9776 <[email protected]>
Signed-off-by: biswajit-9776 <[email protected]>
Signed-off-by: biswajit-9776 <[email protected]>
Signed-off-by: biswajit-9776 <[email protected]>
Let me change the trigger paths for the m2m tests in this PR itself. |
Signed-off-by: biswajit-9776 <[email protected]>
Signed-off-by: biswajit-9776 <[email protected]>
Signed-off-by: biswajit-9776 <[email protected]>
Signed-off-by: biswajit-9776 <[email protected]>
Signed-off-by: biswajit-9776 <[email protected]>
Hey @juliusvonkohout, the UID 1000 works for the cronjob as of now. It passes the m2m tests in 25s for the latest commits. I have also increased the time of cronjob test from 60s to 100s. In future we may change it as per requirement. |
@@ -11,7 +11,7 @@ on: | |||
- common/cert-manager/** | |||
- common/oauth2-proxy/** | |||
- common/istio*/** | |||
- common/oidc-client/** | |||
- common/** |
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.
This line is probably too much. Let's revisit it in your follow up PR
/lgtm |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: juliusvonkohout 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 |
…SS workflow warnings (kubeflow#2848) * Added securityContext profile to cronjob in istio-system Signed-off-by: biswajit-9776 <[email protected]> * Limited script to run for a minute Signed-off-by: biswajit-9776 <[email protected]> * Undo change to script Signed-off-by: biswajit-9776 <[email protected]> * Added time constraint to waiting for job loop Signed-off-by: biswajit-9776 <[email protected]> * Added if condition to script Signed-off-by: biswajit-9776 <[email protected]> * Added version to curl image Signed-off-by: biswajit-9776 <[email protected]> * Undo change to curl image Signed-off-by: biswajit-9776 <[email protected]> * Fixed failing cronjob Signed-off-by: biswajit-9776 <[email protected]> * Refactored the script Signed-off-by: biswajit-9776 <[email protected]> * Added workflow job to clear PSS warnings Signed-off-by: biswajit-9776 <[email protected]> * Removed cluster-local-gateway PSS patch Signed-off-by: biswajit-9776 <[email protected]> * Fixed typo in patches Signed-off-by: biswajit-9776 <[email protected]> * Debugging failing warnings Signed-off-by: biswajit-9776 <[email protected]> * Empty commit Signed-off-by: biswajit-9776 <[email protected]> * Fixed trigger paths for m2m tests Signed-off-by: biswajit-9776 <[email protected]> * Remove debug commands Signed-off-by: biswajit-9776 <[email protected]> * Fixed typo Signed-off-by: biswajit-9776 <[email protected]> * Increases cronjob time from 60s to 100s Signed-off-by: biswajit-9776 <[email protected]> * Change UID to debug Signed-off-by: biswajit-9776 <[email protected]> --------- Signed-off-by: biswajit-9776 <[email protected]> Signed-off-by: Patrick Schönthaler <[email protected]>
Pull Request Template for Kubeflow manifests Issues
✏️ A brief description of the changes
📦 List any dependencies that are required for this change
🐛 If this PR is related to an issue, please put the link to the issue here.
✅ Contributor checklist
DCO
check)cla/google
check)