-
Notifications
You must be signed in to change notification settings - Fork 119
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
add sast-shell-check task #1437
base: main
Are you sure you want to change the base?
Conversation
Marking as draft for now since we'll need to comply with some sast convention first. |
0e75cb1
to
489ea23
Compare
016a826
to
3819a60
Compare
3819a60
to
b6bdb3e
Compare
beabb08
to
cfaec5b
Compare
@kdudka I added 2 commits for review. I'll squash them into one single commit before it's getting merged. |
cfaec5b
to
319a9aa
Compare
1c1285c
to
b7aa95f
Compare
ac33aad
to
9431a2f
Compare
Commits squashed for brevity. |
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.
The code changes look good. Two remarks though:
- The CI is red because of two ShellCheck issues: https://github.com/konflux-ci/build-definitions/actions/runs/11229456532/job/31227287880?pr=1437 - I think we could use a ShellCheck directive and a small code change to make the CI green again.
- Out of OSH-210, this seems to be the only pull request that does not handle trust to the internal Red Hat CA, which is needed to access the internal KFP git repo. I think we should add the optional
trusted-ca
volume based on config map and optionally install the CA bundle from there, as done in sast-snyk-check: increased version to 0.3 #1359.
752fadd
to
a95fdc8
Compare
It's weird the pipeline got stuck, I'm getting: Can anybody tell how can I re-trigger the CI? |
you can ask the reviewers to approve your request to trigger the CI |
/ok-to-test |
9d8b4e9
to
9bfa582
Compare
Hey can anyone from the konflux team please give a review on this? Thanks! |
Added the |
@brunoapimentel @tkdchen @owtaylor @14rcole @Josh-Everett @jsztuka would you mind give a reivew when got time please? This has been pending for some time. |
Rebased / added missing OWNERS file for the oci-ta task. |
No description provided.