-
Notifications
You must be signed in to change notification settings - Fork 761
Add ATEX testing to the upstream CI workflows #14203
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
base: master
Are you sure you want to change the base?
Conversation
f1dae23 to
301dab3
Compare
|
From: https://github.com/ComplianceAsCode/content/actions/runs/19858710726/job/56915038821?pr=14203 I'm afraid the token will only be allowed to be used when we merge the pull request The same code with the same token is working fine on my fork: ggbecker#41 |
|
|
||
| # do faster queries than the default 30 secs, because we don't track | ||
| # many dozens of requests, just one | ||
| class FastRequest(api.Request): |
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.
Consider adding a docstring for why this class is needed.
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.
added, based on the existing test, coming from our productization ATEX scripts
|
|
||
| - name: Run tests on Testing Farm | ||
| env: | ||
| TESTING_FARM_API_TOKEN: ${{ secrets.TESTING_FARM_API_TOKEN }} |
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.
IIRC, this will never be available since we are using pull_request.
We might need break this out into two jobs. One that builds using pull_request, then a second one that uses workflow_run to trigger the tests.
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 have done this separation but gemini tells me this would only work when the file is already merged in the repo for example, that's why we don't see a check running for this workflow.
|
I've addressed all the feedback provided and split the jobs into two workflows, one with the |
|
How would I make a change in the workflow (later down the road) while being able to test the change in a PR, and not hit any of the secrets limitations? For example, if we switch to Will it Just Work? Also nitpicks:
|
ccbbfcb to
8d50245
Compare
I believe it will not. Maybe if you push to a branch belonging to ComplianceAsCode the situation changes, at least that's what worked here #14209. But then I don't know about the |
|
/packit retest-failed |
Also added restrictions to token permissions as suggested by github.
Use env in the shebang for python scripts. Use fixed hash for github actions to prevent supply chain attacks. Remove dead code. Add pcre2 to the requirements.txt file.
There's no need to manage issues from the ATEX PR point of view.
0c53819 to
8b45e57
Compare
Sadly, this is a feature not bug. So that we don't expose our secrets. |
|
@ggbecker: The following test failed, say
Full PR test history. Your PR dashboard. 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-sigs/prow repository. I understand the commands that are listed here. |
Description:
Rationale: