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.

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

MAINT - CI improvements (security and maintenance) #2077

Open
wants to merge 27 commits into
base: main
Choose a base branch
from

Conversation

trallard
Copy link
Collaborator

@trallard trallard commented Dec 12, 2024

I thought I would do some winter/summer cleaning (depending on your location) β›„πŸŒž.

This PR adds several improvements/updates to our CI with a focus on improving the contributor experience and security
Details below:

πŸ”’ Security focused

  • Use SHA for third-party actions and our internal action for development setup
  • Replace potentially dangerous trigger workflow_run for workflow_call and use as a reusable workflow
  • Create and use a dedicated environment for releases:
    • Created a pst-release environment in the repo (restricted to main only)
    • Use pst-release for our release-PST step in publish.yml
    • Add pst-release as the default env in PyPI
  • Add a zizmor.yml workflow to run static analysis on our GH workflows
  • Add explicit persist-credentials: false to relevant actions (where we do not need further git operations)

πŸ‘©πŸ½β€πŸŽ€ Contributor experience

  • Prevent the pre-release.yml workflow from running in repos not under the pydata org (forks)
  • Our CI workflow has grown significantly with the various tests and checks. This PR splits it into:
  • CI.yml: pytest, a11y-tests, profiling, coverage
  • docs.yml: docs-related checks like building across OSes and Python and Sphinx versions, check for broken links (new, note that I had to fix some broken links to get this in πŸ™ˆ and there seem to be still some others to fix)
  • Add tox run -e docs-linkcheck to check for broken links in our docs

🧰 Maintenance

  • Adds Python 3.13 to our testing matrices (3.12 is left as the default until we are confident all is ok with 3.13)
  • Add an explicit ubuntu-22.04 target as ubuntu-latest will soon be 24.04 (being rolled out right now) -> I think I might actually have explicit versions on both and only change to latest (or not) when the rollout is completed

Questions / notes

  • @drammock, we have "sphinx-theme-builder @ https://github.com/pradyunsg/sphinx-theme-builder/archive/87214d0671c943992c05e3db01dca997e156e8d6.zip", in our project. tool and tox.ini. I do not believe this pin is needed anymore, so I would like to remove it, too. WDYT?
  • @drammock did you create the token for Anaconda.org? I would like to make this an environment secret (vs a repository secret as it is right now)
  • Also, while adding a new environment, I noticed a github-pages environment that I do not think we are using, so I'd like to delete it.
  • Finally, I deleted a leftover PYPI_TOKEN, which should have been removed when we changed to trusted publishers.

@trallard trallard added kind: maintenance Improving maintainability and reducing technical debt tag: CI Pull requests that update GitHub Actions code labels Dec 12, 2024
@github-advanced-security
Copy link

This pull request sets up GitHub code scanning for this repository. Once the scans have completed and the checks have passed, the analysis results for this pull request branch will appear on this overview. Once you merge this pull request, the 'Security' tab will show more code scanning analysis results (for example, for the default branch). Depending on your configuration and choice of analysis tool, future pull requests will be annotated with code scanning analysis results. For more information about GitHub code scanning, check out the documentation.

Copy link

Coverage report

This PR does not seem to contain any modification to coverable code.

Comment on lines 21 to 22
uses: ./.github/workflows/CI.yml
# needed for the coverage action
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Once this is merged, this will need pinning to a SHA.

tests:
uses: ./.github/workflows/CI.yml
# needed for the coverage action
permissions:
contents: write
pull-requests: write
# calls our docs workflow (build docs, check broken links, lighthouse)
docs:
uses: ./.github/workflows/docs.yml
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Once this is merged, this will need pinning to a SHA.

docs/.jupyterlite.doit.db-journal Outdated Show resolved Hide resolved
runs-on: ubuntu-latest
needs: [build-package]
permissions:
id-token: write # needed for PyPI upload
environment:

Choose a reason for hiding this comment

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

Double-checking that this GitHub Environment is required on the Trusted Publisher in PyPI?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I added it to my checklist so I remember to add it

.github/workflows/publish.yml Outdated Show resolved Hide resolved
.github/workflows/publish.yml Outdated Show resolved Hide resolved
@drammock
Copy link
Collaborator

@drammock, we have "sphinx-theme-builder @ https://github.com/pradyunsg/sphinx-theme-builder/archive/87214d0671c943992c05e3db01dca997e156e8d6.zip", in our project. tool and tox.ini. I do not believe this pin is needed anymore, so I would like to remove it, too. WDYT?

my fixes to STB were done in May and August of 2023, and the most recent release was March 2023. So we do still need the pin (or to install from tip of main would be fine too --- or convince Pradyun to cut a release πŸ˜„).
https://github.com/pradyunsg/sphinx-theme-builder/commits/main/

@drammock did you create the token for Anaconda.org? I would like to make this an environment secret (vs a repository secret as it is right now)

I don't remember creating it. But it might have been me. Are we pushing nightlies to the scientific python channel or something, is that what it's used for?

@trallard
Copy link
Collaborator Author

@drammock yes we are using the Scientific Python channel so I am pretty confident it was you then.

my fixes to STB were done in May and August of 2023, and the most recent release was March 2023. So we do still need the pin (or to install from tip of main would be fine too --- or convince Pradyun to cut a release πŸ˜„).
pradyunsg/sphinx-theme-builder@main (commits)

ha! I can do both, use main for now and bug Pradyun (I am pretty good at the latter now)

@drammock
Copy link
Collaborator

@drammock yes we are using the Scientific Python channel so I am pretty confident it was you then.

ok, feel free to do whatever you need to w/r/t org vs repo vs env secrets, and LMK if/when you want a second opinion.

@trallard
Copy link
Collaborator Author

@drammock if you could create a new token for the Scientific Python nightly and add it to the pst-release environment here in the repo that would be grand so that we can keep all things related to releases in that env.

@drammock
Copy link
Collaborator

@drammock if you could create a new token for the Scientific Python nightly and add it to the pst-release environment here in the repo that would be grand so that we can keep all things related to releases in that env.

OK this is done now, sorry for the delay @trallard. It's added with name ANACONDA_ORG_UPLOAD_TOKEN to mimic the old org-level token; LMK if you want a different name and I can rename it (or maybe you can rename it yourself?). Please ping me when this is merged if you need me to delete the org-level token.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind: maintenance Improving maintainability and reducing technical debt tag: CI Pull requests that update GitHub Actions code
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants