Skip to content

feat: check actions security action#725

Merged
jorgepiloto merged 86 commits intomainfrom
feat/check-actions-security-action
Apr 29, 2025
Merged

feat: check actions security action#725
jorgepiloto merged 86 commits intomainfrom
feat/check-actions-security-action

Conversation

@moe-ad
Copy link
Copy Markdown
Contributor

@moe-ad moe-ad commented Mar 6, 2025 •
edited
Loading

Closes #692. Closes #680.

@moe-ad moe-ad added this to the ansys/actions@v9 milestone Mar 6, 2025
@moe-ad moe-ad self-assigned this Mar 6, 2025
@ansys-reviewer-bot
Copy link
Copy Markdown
Contributor

ansys-reviewer-bot Bot commented Mar 6, 2025

Thanks for opening a Pull Request. If you want to perform a review write a comment saying:

@ansys-reviewer-bot review

@moe-ad moe-ad marked this pull request as ready for review March 6, 2025 17:23
@moe-ad moe-ad requested a review from a team as a code owner March 6, 2025 17:23
@moe-ad moe-ad marked this pull request as draft March 6, 2025 17:24
@moe-ad moe-ad marked this pull request as ready for review March 7, 2025 13:35
@moe-ad moe-ad marked this pull request as draft March 7, 2025 13:36
@moe-ad moe-ad marked this pull request as ready for review March 7, 2025 13:37
@github-actions github-actions Bot added enhancement General improvements to existing features ci Pipelines maintenance related labels Mar 7, 2025
@moe-ad moe-ad marked this pull request as draft March 7, 2025 14:00
@moe-ad moe-ad force-pushed the feat/check-actions-security-action branch from 9e8f1ed to 18da686 Compare March 7, 2025 15:23
@github-actions github-actions Bot added the docs Issues related to documentation label Apr 22, 2025
@moe-ad moe-ad marked this pull request as ready for review April 23, 2025 08:46
RobPasMue
RobPasMue reviewed Apr 23, 2025
View reviewed changes
Copy link
Copy Markdown
Member

@RobPasMue RobPasMue left a comment

Choose a reason for hiding this comment

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

We need to revert all the refs to the branch by "main". I left some of them only... Apart from that the action looks good

Comment thread .github/workflows/ci_cd_main.yml Outdated
Comment thread .github/workflows/ci_cd_main.yml Outdated
Comment thread .github/workflows/ci_cd_night.yml Outdated
Comment thread .github/workflows/ci_cd_night.yml
Comment thread .github/workflows/ci_cd_night.yml Outdated
Comment thread .github/workflows/ci_cd_release.yml Outdated
Comment thread .github/workflows/ci_cd_release.yml Outdated
Comment thread .github/workflows/ci_cd_release.yml Outdated
Comment thread .github/workflows/ci_cd_release.yml Outdated
Comment thread .github/workflows/ci_cd_release.yml Outdated
@moe-ad
Copy link
Copy Markdown
Contributor Author

moe-ad commented Apr 23, 2025

Thanks for the review @RobPasMue. I am pointing to the PR branch because of testing. I will revert before merging.

SMoraisAnsys
SMoraisAnsys requested changes Apr 24, 2025
View reviewed changes
Copy link
Copy Markdown
Contributor

@SMoraisAnsys SMoraisAnsys left a comment

Choose a reason for hiding this comment

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

Very great job, thanks for your hard work !
I left minor comments, let me know if some of them do not make sense :D

Have you had the time to test your branch using all of the modified actions ? They imply a lot of changes and we might end up breaking multiple projects workflows since we have to cover a large pannel of cases (runners / os / package manager / ...)
If not, could you test all of them in a dummy repo with Github runners + linux and windows + pip and poetry ? This should cover most of the cases and only leave edge cases to handle (if any bug was introduced)

Comment thread build-wheelhouse/action.yml
Comment thread check-actions-security/action.yml Outdated
Comment thread check-vulnerabilities/action.yml
Comment thread check-vulnerabilities/action.yml Outdated
@SMoraisAnsys
Copy link
Copy Markdown
Contributor

SMoraisAnsys commented Apr 24, 2025

@RobPasMue @jorgepiloto @germa89 What do you guys think about testing this PR in our repos to see how it behaves ? There are a lot of changes and if nothing out of the ordinary happens, we'll be able to merge this PR with greater peace of mind :)
I'll try it in pyaedt to see what's happening there. Waiting for @moe-ad 's feedback on my review though

@jorgepiloto
Copy link
Copy Markdown
Member

jorgepiloto commented Apr 25, 2025

Sounds good to me, @SMoraisAnsys.

@moe-ad
Copy link
Copy Markdown
Contributor Author

moe-ad commented Apr 25, 2025

@SMoraisAnsys Thanks for the review. I have addressed your suggestions.

jorgepiloto
jorgepiloto requested changes Apr 25, 2025
View reviewed changes
Copy link
Copy Markdown
Member

@jorgepiloto jorgepiloto left a comment

Choose a reason for hiding this comment

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

Found thanks to https://github.com/ansys/pyaedt/actions/runs/14661989340/job/41148728765?pr=6078

Comment thread tests-pytest/action.yml Outdated
moe-ad and others added 2 commits April 25, 2025 15:12
SMoraisAnsys
SMoraisAnsys reviewed Apr 27, 2025
View reviewed changes
Comment thread _doc-build-linux/action.yml Outdated
@jorgepiloto @SMoraisAnsys
fix: expand variable
9804801 
Co-authored-by: Sébastien Morais <[email protected]>
@RobPasMue
Copy link
Copy Markdown
Member

RobPasMue commented Apr 28, 2025

@RobPasMue @jorgepiloto @germa89 What do you guys think about testing this PR in our repos to see how it behaves ? There are a lot of changes and if nothing out of the ordinary happens, we'll be able to merge this PR with greater peace of mind :) I'll try it in pyaedt to see what's happening there. Waiting for @moe-ad 's feedback on my review though

I can give it a try on my side too, np! But it might take me some time, probably not until Tuesday.

@moe-ad moe-ad mentioned this pull request Apr 28, 2025
Merged
7 tasks
@moe-ad
Copy link
Copy Markdown
Contributor Author

moe-ad commented Apr 29, 2025

@jorgepiloto, @SMoraisAnsys, @RobPasMue, @germa89.
Validated against:

I guess we can merge?

@RobPasMue
Copy link
Copy Markdown
Member

RobPasMue commented Apr 29, 2025

Works for me

@jorgepiloto jorgepiloto merged commit a5e728b into main Apr 29, 2025
12 of 13 checks passed
@jorgepiloto jorgepiloto deleted the feat/check-actions-security-action branch April 29, 2025 09:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@RobPasMue RobPasMue RobPasMue left review comments

@jorgepiloto jorgepiloto jorgepiloto requested changes

@SMoraisAnsys SMoraisAnsys SMoraisAnsys approved these changes

Assignees

@moe-ad moe-ad

Labels

ci Pipelines maintenance related docs Issues related to documentation enhancement General improvements to existing features

Projects

None yet

Milestone

ansys/actions@v10

Development

Successfully merging this pull request may close these issues.

 Implement zizmor to secure our actions Add analysis tool for Github Actions.

5 participants

@moe-ad @SMoraisAnsys @jorgepiloto @RobPasMue @pyansys-ci-bot