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.

Sign up for GitHub

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

Jump to bottom

sast: Add sast-unicode-check task for testing #1401

Merged
merged 2 commits into from
Oct 11, 2024
Merged

sast: Add sast-unicode-check task for testing #1401

merged 2 commits into from
Oct 11, 2024

Conversation

hanchuntao
Copy link
Contributor

Before you complete this pull request ...

Look for any open pull requests in the repository with the title "e2e-tests update" and
see if there are recent e2e-tests updates that will be applicable to your change.

tkdchen
tkdchen reviewed Sep 12, 2024
View reviewed changes
Copy link
Contributor

@tkdchen tkdchen left a comment

Choose a reason for hiding this comment

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

Missing task/task-name/OWNERS as well.

.tekton/pull-request.yaml Outdated Show resolved Hide resolved
@hanchuntao
Copy link
Contributor Author

@tkdchen I want to test this task, do you know how to run the CI? thanks!

@hanchuntao
Copy link
Contributor Author

Missing task/task-name/OWNERS as well.

updated, added OWNERS, thanks

@hanchuntao hanchuntao force-pushed the sast-find-unicode-control branch 2 times, most recently from aac253f to 859526e Compare September 12, 2024 06:19
@kdudka
Copy link

kdudka commented Sep 12, 2024

@hanchuntao Please also include a reference to https://issues.redhat.com/browse/OSH-739 in the commit message.

@hanchuntao hanchuntao force-pushed the sast-find-unicode-control branch 7 times, most recently from 082c7d0 to 8e0d599 Compare September 13, 2024 02:05
kdudka
kdudka suggested changes Sep 13, 2024
View reviewed changes
Copy link

@kdudka kdudka left a comment

Choose a reason for hiding this comment

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

@hanchuntao Please rename the task to sast-unicode-check as stated in the summary of OSH-739.

@hanchuntao hanchuntao force-pushed the sast-find-unicode-control branch 2 times, most recently from ec339ad to c00c8c7 Compare September 19, 2024 03:34
@hanchuntao
Copy link
Contributor Author

@hanchuntao Please rename the task to sast-unicode-check as stated in the summary of OSH-739.

updated

@hanchuntao
Copy link
Contributor Author

@hanchuntao Please also include a reference to https://issues.redhat.com/browse/OSH-739 in the commit message.

updated

@hanchuntao hanchuntao changed the title Add sast-find-unicode-control task for testing Add sast-unicode-check task for testing Sep 19, 2024
@hanchuntao hanchuntao force-pushed the sast-find-unicode-control branch 2 times, most recently from db3e59e to aa7eecb Compare September 19, 2024 06:57
@kdudka kdudka mentioned this pull request Sep 19, 2024
Open
5 tasks
kdudka
kdudka suggested changes Sep 19, 2024
View reviewed changes
Copy link

@kdudka kdudka left a comment

Choose a reason for hiding this comment

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

@hanchuntao Please close the review threads that have been resolved. I do not have permissions to do so.

.tekton/pull-request.yaml Outdated Show resolved Hide resolved
task/sast-unicode-check/0.1/README.md Outdated Show resolved Hide resolved
task/sast-unicode-check/0.1/README.md Outdated Show resolved Hide resolved
task/sast-unicode-check/0.1/sast-unicode-check.yaml Outdated Show resolved Hide resolved
task/sast-unicode-check/0.1/sast-unicode-check.yaml Outdated Show resolved Hide resolved
@hanchuntao hanchuntao force-pushed the sast-find-unicode-control branch 2 times, most recently from 2ec25aa to dd83120 Compare September 20, 2024 03:31
@hanchuntao
Copy link
Contributor Author

/ok-to-test

@hanchuntao
Copy link
Contributor Author 

Failure snippet:
task check-task-pipeline-repo-existence has the status "Failed":
Please contact Build team - #forum-stonesoup-build that the missing repos should be created in:
- https://quay.io/organization/redhat-appstudio-tekton-catalog
- https://quay.io/organization/konflux-ci

@dheerajodha, do you know how to fix this issue in the CI pipeline?

@tkdchen
Copy link
Contributor

tkdchen commented Oct 10, 2024

/retest

@dheerajodha
Copy link
Member

dheerajodha commented Oct 10, 2024 •
edited
Loading

@dheerajodha, do you know how to fix this issue in the CI pipeline?

@hanchuntao The Konflux build PLR has passed now, @tkdchen has opened a PR that fixes the 1 final failing PR check. It shouldn't take long for that PR to get merged, and then I'll merge this PR after that if you don't mind.

@hanchuntao
Copy link
Contributor Author

@dheerajodha Because all of the checks passed, what should we do next? Can somebody with write access merge the code?

@tkdchen tkdchen added this pull request to the merge queue Oct 11, 2024
Merged via the queue into konflux-ci:main with commit e10b7f2 Oct 11, 2024
14 checks passed
@rh-tap-build-team rh-tap-build-team bot mentioned this pull request Oct 11, 2024
Closed
kdudka added a commit to kdudka/build-definitions that referenced this pull request Oct 21, 2024
@kdudka
sast-unicode-check: add support for Trusted Artifacts
9e60dab 
Create the `-oci-ta` variant of the `sast-unicode-check` task.

Related: konflux-ci#1401
Resolves: https://issues.redhat.com/browse/OSH-764
@kdudka kdudka mentioned this pull request Oct 21, 2024
Open
kdudka added a commit to kdudka/build-definitions that referenced this pull request Oct 23, 2024
@kdudka
sast-unicode-check: add support for Trusted Artifacts
838778b 
Create the `-oci-ta` variant of the `sast-unicode-check` task.

Related: konflux-ci#1401
Resolves: https://issues.redhat.com/browse/OSH-764
kdudka added a commit to kdudka/build-definitions that referenced this pull request Oct 24, 2024
@kdudka
sast-unicode-check: make the task work with default params
82046fa 
Before this change, the `sast-unicode-check` was failing with the
default parameters because the script was passing the whole list
of arguments as a single argument:
```
+ mapfile -t fuc_args
+ LANG=en_US.utf8
+ ./find-unicode-control/find_unicode_control.py '-p bidi -v -d -t' /var/workdir/source
+ FUC_EXIT_CODE=2
+ [[ 2 -ne 0 ]]
+ [[ 2 -ne 1 ]]
+ echo 'Failed to run find-unicode-control command'
Failed to run find-unicode-control command
+ cat raw_sast_unicode_check_out.log
usage: find_unicode_control.py [-h] [-p {all,bidi}] [-v] [-d] [-t] [-c CONFIG]
                               path [path ...]
find_unicode_control.py: error: argument -p/--nonprint: invalid choice: ' bidi -v -d -t' (choose from 'all', 'bidi')
+ note='Task sast-unicode-check-oci-ta failed: For details, check Tekton task log.'
++ make_result_json -r ERROR -t 'Task sast-unicode-check-oci-ta failed: For details, check Tekton task log.'
```

Fixes: konflux-ci#1401
Related: konflux-ci#1524
kdudka added a commit to kdudka/build-definitions that referenced this pull request Oct 24, 2024
@kdudka
sast-unicode-check: add support for Trusted Artifacts
31de817 
Create the `-oci-ta` variant of the `sast-unicode-check` task.

Related: konflux-ci#1401
Resolves: https://issues.redhat.com/browse/OSH-764
Closes: konflux-ci#1524
@kdudka
Copy link

kdudka commented Oct 24, 2024

@hanchuntao Are you sure you tested the changes before they were merged?

I do not think this could work. See 82046fa for details.

kdudka added a commit to kdudka/build-definitions that referenced this pull request Oct 24, 2024
@kdudka
sast-unicode-check: make the task work with default params
092a620 
Before this change, the `sast-unicode-check` was failing with the
default parameters because the script was passing the whole list
of arguments as a single argument:
```
+ mapfile -t fuc_args
+ LANG=en_US.utf8
+ ./find-unicode-control/find_unicode_control.py '-p bidi -v -d -t' /var/workdir/source
+ FUC_EXIT_CODE=2
+ [[ 2 -ne 0 ]]
+ [[ 2 -ne 1 ]]
+ echo 'Failed to run find-unicode-control command'
Failed to run find-unicode-control command
+ cat raw_sast_unicode_check_out.log
usage: find_unicode_control.py [-h] [-p {all,bidi}] [-v] [-d] [-t] [-c CONFIG]
                               path [path ...]
find_unicode_control.py: error: argument -p/--nonprint: invalid choice: ' bidi -v -d -t' (choose from 'all', 'bidi')
+ note='Task sast-unicode-check-oci-ta failed: For details, check Tekton task log.'
++ make_result_json -r ERROR -t 'Task sast-unicode-check-oci-ta failed: For details, check Tekton task log.'
```

Fixes: konflux-ci#1401
Related: konflux-ci#1524
kdudka added a commit to kdudka/build-definitions that referenced this pull request Oct 24, 2024
@kdudka
sast-unicode-check: add support for Trusted Artifacts
00a6289 
Create the `-oci-ta` variant of the `sast-unicode-check` task.

Related: konflux-ci#1401
Resolves: https://issues.redhat.com/browse/OSH-764
Closes: konflux-ci#1524
@hanchuntao
Copy link
Contributor Author

hanchuntao commented Oct 25, 2024 •
edited
Loading

@hanchuntao Are you sure you tested the changes before they were merged?

I do not think this could work. See 82046fa for details.

I tested all of the changes. I don't know what happened, let me check it first, thanks

After rerunning this task in my local environment, yes the issue exists.
I ignored this issue, and it also exists in my previous tests: https://konflux.apps.stone-prod-p02.hjvn.p1.openshiftapps.com/application-pipeline/workspaces/chhan/applications/unicode-control-test/pipelineruns/find-unicode-control-on-pull-request-4dfvj
./find-unicode-control/find_unicode_control.py '-p bidi -v -d -t' /workspace/workspace/source

I have filed a new PR to fix this issue: #1535
The latest test result: https://konflux.apps.stone-prod-p02.hjvn.p1.openshiftapps.com/application-pipeline/workspaces/chhan/applications/unicode-control-test/pipelineruns/find-unicode-control-on-pull-request-f82tj

kdudka added a commit to kdudka/build-definitions that referenced this pull request Nov 6, 2024
@kdudka
sast-unicode-check: make the task work with default params
581e219 
Before this change, the `sast-unicode-check` was failing with the
default parameters because the script was passing the whole list
of arguments as a single argument:
```
+ mapfile -t fuc_args
+ LANG=en_US.utf8
+ ./find-unicode-control/find_unicode_control.py '-p bidi -v -d -t' /var/workdir/source
+ FUC_EXIT_CODE=2
+ [[ 2 -ne 0 ]]
+ [[ 2 -ne 1 ]]
+ echo 'Failed to run find-unicode-control command'
Failed to run find-unicode-control command
+ cat raw_sast_unicode_check_out.log
usage: find_unicode_control.py [-h] [-p {all,bidi}] [-v] [-d] [-t] [-c CONFIG]
                               path [path ...]
find_unicode_control.py: error: argument -p/--nonprint: invalid choice: ' bidi -v -d -t' (choose from 'all', 'bidi')
+ note='Task sast-unicode-check-oci-ta failed: For details, check Tekton task log.'
++ make_result_json -r ERROR -t 'Task sast-unicode-check-oci-ta failed: For details, check Tekton task log.'
```

Fixes: konflux-ci#1401
Related: konflux-ci#1524
kdudka added a commit to kdudka/build-definitions that referenced this pull request Nov 6, 2024
@kdudka
sast-unicode-check: add support for Trusted Artifacts
4a91952 
Create the `-oci-ta` variant of the `sast-unicode-check` task.

Related: konflux-ci#1401
Resolves: https://issues.redhat.com/browse/OSH-764
Closes: konflux-ci#1524
kdudka added a commit to kdudka/build-definitions that referenced this pull request Nov 6, 2024
@kdudka
sast-unicode-check: add support for Trusted Artifacts
d2836e2 
Create the `-oci-ta` variant of the `sast-unicode-check` task.

Related: konflux-ci#1401
Resolves: https://issues.redhat.com/browse/OSH-764
Closes: konflux-ci#1524
kdudka added a commit to kdudka/build-definitions that referenced this pull request Nov 11, 2024
@kdudka
sast-unicode-check: make the task work with default params
1cf432b 
Before this change, the `sast-unicode-check` was failing with the
default parameters because the script was passing the whole list
of arguments as a single argument:
```
+ mapfile -t fuc_args
+ LANG=en_US.utf8
+ ./find-unicode-control/find_unicode_control.py '-p bidi -v -d -t' /var/workdir/source
+ FUC_EXIT_CODE=2
+ [[ 2 -ne 0 ]]
+ [[ 2 -ne 1 ]]
+ echo 'Failed to run find-unicode-control command'
Failed to run find-unicode-control command
+ cat raw_sast_unicode_check_out.log
usage: find_unicode_control.py [-h] [-p {all,bidi}] [-v] [-d] [-t] [-c CONFIG]
                               path [path ...]
find_unicode_control.py: error: argument -p/--nonprint: invalid choice: ' bidi -v -d -t' (choose from 'all', 'bidi')
+ note='Task sast-unicode-check-oci-ta failed: For details, check Tekton task log.'
++ make_result_json -r ERROR -t 'Task sast-unicode-check-oci-ta failed: For details, check Tekton task log.'
```

Fixes: konflux-ci#1401
Related: konflux-ci#1524
kdudka added a commit to kdudka/build-definitions that referenced this pull request Nov 11, 2024
@kdudka
sast-unicode-check: add support for Trusted Artifacts
7b71a19 
Create the `-oci-ta` variant of the `sast-unicode-check` task.

Related: konflux-ci#1401
Resolves: https://issues.redhat.com/browse/OSH-764
Closes: konflux-ci#1524
kdudka added a commit to kdudka/build-definitions that referenced this pull request Nov 13, 2024
@kdudka
sast-unicode-check: make the task work with default params
c75ac32 
Before this change, the `sast-unicode-check` was failing with the
default parameters because the script was passing the whole list
of arguments as a single argument:
```
+ mapfile -t fuc_args
+ LANG=en_US.utf8
+ ./find-unicode-control/find_unicode_control.py '-p bidi -v -d -t' /var/workdir/source
+ FUC_EXIT_CODE=2
+ [[ 2 -ne 0 ]]
+ [[ 2 -ne 1 ]]
+ echo 'Failed to run find-unicode-control command'
Failed to run find-unicode-control command
+ cat raw_sast_unicode_check_out.log
usage: find_unicode_control.py [-h] [-p {all,bidi}] [-v] [-d] [-t] [-c CONFIG]
                               path [path ...]
find_unicode_control.py: error: argument -p/--nonprint: invalid choice: ' bidi -v -d -t' (choose from 'all', 'bidi')
+ note='Task sast-unicode-check-oci-ta failed: For details, check Tekton task log.'
++ make_result_json -r ERROR -t 'Task sast-unicode-check-oci-ta failed: For details, check Tekton task log.'
```

Fixes: konflux-ci#1401
Related: konflux-ci#1524
kdudka added a commit to kdudka/build-definitions that referenced this pull request Nov 13, 2024
@kdudka
sast-unicode-check: add support for Trusted Artifacts
424fdf9 
Create the `-oci-ta` variant of the `sast-unicode-check` task.

Related: konflux-ci#1401
Resolves: https://issues.redhat.com/browse/OSH-764
Closes: konflux-ci#1524
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jperezdealgaba jperezdealgaba jperezdealgaba left review comments

@dheerajodha dheerajodha dheerajodha left review comments

@Josh-Everett Josh-Everett Josh-Everett approved these changes

@kdudka kdudka kdudka approved these changes

@mmorhun mmorhun Awaiting requested review from mmorhun

@tnevrlka tnevrlka Awaiting requested review from tnevrlka

@chmeliik chmeliik Awaiting requested review from chmeliik

@tkdchen tkdchen Awaiting requested review from tkdchen

@14rcole 14rcole Awaiting requested review from 14rcole

@jsztuka jsztuka Awaiting requested review from jsztuka

@hongweiliu17 hongweiliu17 Awaiting requested review from hongweiliu17

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
9 participants
@hanchuntao @kdudka @tnevrlka @dheerajodha @tkdchen @Josh-Everett @chmeliik @jperezdealgaba @openshift-merge-robot