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

Generic evergreen CI and maintenance ergonomics #62

Closed
stdedos opened this issue Apr 24, 2024 · 13 comments
Closed

Generic evergreen CI and maintenance ergonomics #62

stdedos opened this issue Apr 24, 2024 · 13 comments
Labels

Comments

@stdedos
Copy link
Collaborator

stdedos commented Apr 24, 2024

P.S. Since you mentioned liking evergreen CI and maintenance ergonomics, here's a few of my GitHub Actions that you may enjoy having here:

Originally posted by @webknjaz in #15 (comment)

@stdedos
Copy link
Collaborator Author

stdedos commented Apr 24, 2024

Unfortunately @webknjaz, all action approvals must go through @Pierre-Sassoulas.
The org has very strict actions - and rightfully so, as pylint is a huge cornerstone piece of software.

I am splitting this here only for posterity.

(@Pierre-Sassoulas just an FYI, if something would happen to interest you. Not a requirement/request of any kind.)

@stdedos stdedos closed this as not planned Won't fix, can't repro, duplicate, stale Apr 24, 2024
@stdedos stdedos added the CI label Apr 24, 2024
@Pierre-Sassoulas
Copy link
Member

Hey :) I'm not the only person that can allow this, any pylint-dev admin can. If there is a good reason to add a github action we can do it, but it increase the attack surface on all pylint-dev repoes (i.e. someone open a merge request using a compromised actions from an authorized namespace, the more namespace we allow the likelier it is that one of them is compromised) so it has to be a big benefit. The pypy's way to publish package seems like a nice one we could add, the other two seems more anecdotal (without looking too much into it).

@stdedos
Copy link
Collaborator Author

stdedos commented Apr 24, 2024

I'm not the only person that can allow this, any pylint-dev admin can.

By "Admin", I assume you mean Owner? (https://github.com/orgs/pylint-dev/people)

I am not one - and even if I was, I'd be very reluctant to do it anyway 🙃


Unfortunately, we (https://github.com/pylint-dev/pylint-pytest) cannot use the https://github.com/pypa/gh-action-pypi-publish action. I need to register a trusted publisher to an existing PyPI project, but I do not have (yet) Owner rights.

I have been thinking to ask ownership once I released v2 (soon ™️), or "when it just comes".

@Pierre-Sassoulas
Copy link
Member

By "Admin", I assume you mean Owner?

Possibly yes. Those that can modify the github action's setting regex filter for pylint-dev.

Unfortunately, we (https://github.com/pylint-dev/pylint-pytest) cannot use the https://github.com/pypa/gh-action-pypi-publish action.

I just authorized the two github actions from the pypa (https://github.com/orgs/pypa/repositories?type=all&q=gh-action) at the pylint-dev level.

I need to register a trusted publisher to an existing PyPI project, but I do not have (yet) Owner rights.

This is going to depend on the original author of pylint-pytest, right ?

I have been thinking to ask ownership once I released v2 (soon ™️), or "when it just comes".

If you're talking about ownership in pylint-dev the path to admin-ship is documented here : https://pylint.readthedocs.io/en/stable/development_guide/contributor_guide/governance.html#how-to-become-an-admin. The context with the xz exploit is not conducive to easing up this process. If you're talking about the pylint-pytest repo you should already have ownership rights if at all possible using github's option.

@stdedos
Copy link
Collaborator Author

stdedos commented Apr 24, 2024

This is going to depend on the original author of pylint-pytest, right ?

Right. Especially because of the xz-issue, I am not interested to push immediately.
I was planning asking sometime at the 6mo/1y mark, and it was "coming up" around v2 (that everything of the original owner, minus git-history stuff, was expunged).

If you're talking about ownership in pylint-dev the path to admin-ship is documented here:

I meant pypi ownership

@webknjaz
Copy link

(i.e. someone open a merge request using a compromised actions from an authorized namespace, the more namespace we allow the likelier it is that one of them is compromised) so it has to be a big benefit.

I know that some orgs (especially corporate/enterprise) solve this by forking said actions. They then have use copies of actions under their control. Updating those forks would involve auditing the diff, of course. This is evidently more maintenance burden but it might make sense for actions that are planned to be used in more than one repo in the org. JFYI.

The pypy's way to publish package seems like a nice one we could add,

**PyPI. FWIW I've been collaborating with the security folks implementing the Warehouse of Trusted Publishing (and other things) and we were able to implement tokenless publishing in my action (under PyPA) at the stage of the early private beta and were able to go GA during the Packaging Summit on PyCon last year. This action is featured on GitHub's own docs site (with a few things that we're trying to correct there, though) and is suggested in the starter workflow (but that one I'm annoyed about since they pin to a several years old hash and many people don't get a version that is capable of Trusted Publishing which is something we're trying to get them to fix lately as well).

As you mentioned the fear of supply chain attacks which I also share, I recommend you follow my guide closely — especially the notes on splitting the jobs for building the dists and publishing them (it's an attack surface for privilege elevation through build dependency poisoning that reaches beyond just PyPI access in my opinion, which is why I insisted on using better examples throughout the documents I could reach..)

Another supply chain-related recommendation is to switch to pinning actions in the workflow definitions with commit SHAs which are immutable (unlike branches or tags). Even in this action's README I mention this recommendation (but don't show in the snippets because it'd be a pain to update the docs all the time). Dependabot understands how to update hash-based versions, by the way. In case it was the reason this wasn't done in this repo.

When I contribute my actions to some repos, I try to follow their conventions when they use hashes for everything like https://github.com/pyca/cryptography/blob/41ca410/.github/workflows/ci.yml#L420. In other cases, I suggest pinning even when those projects don't normally use hashes: https://github.com/python/cpython/blob/4b10e20/.github/workflows/build.yml#L574 — one would thing they'd want to be more careful 🤷‍♂️

But most of the time I don't pin the actions that I own because it makes it easier to ship updates, it's a balance.

the other two seems more anecdotal (without looking too much into it).

alls-green closes the gap for a very real problem on GitHub specifically. Its usefulness is mostly appreciated by projects that have huge matrices of jobs and use branch protection. People can get annoyed by having to remember to update tens of CI checks in branch protection on each matrix update so they like to have a single check to list there. There's a "Why?" section in readme explaining some limitations.

As for checkout-python-sdist, I need to work on documenting it better. It's one of the reasons it's not used widely (it's mostly in my projects and maybe one or two other PyPA folks use it so far). It's distilled from my experiments with releasing exactly the artifacts that got tested and not a separate rebuild. Plus, it allows keeping sdists downstream-friendly over time. Though, it's probably not the time since there's a small incompatibly with the workflow here.

This is going to depend on the original author of pylint-pytest, right ?

@stdedos you can still integrate the action, though, and prepare for future tokenless setup. Meanwhile, using the long-living API token remains working. Technically, it's not much less secure, it's just easier to configure everything w/o tokens (well, the tokens still exist under the hood — short lived / 15 min).

The major difference is that API tokens are bound to user accounts so if you kick a user out of a PyPI project, their tokens will lose access. With trusted publishing, OTOH, the trust is connected to the projects directly and will not get revoked when project's maintainers change (the trust is set up against a specific workflow in a specific repository + a specific GitHub Environment if set).

By the way, using GitHub Environments can let you have workflows that pause before actually doing the release and wait for a human from the allowlist to click a button approving the thing to actually happen. This means that regardless of who is added on PyPI, it is possible to control who releases on the GitHub side more granularly.

Also, on the topic of granularity, my guide shows the use of Sigstore to publish signatures for the artifacts. But William made a PEP regarding providing a better thing right in PyPI so hopefully over time my action will just do that as a part of the process, seamlessly.

I meant pypi ownership

There's been talk over in Warehouse of deprecating the Maintainer role with more OIDC support so that leaves us with Owner and maybe some RBAC in the future (PyPI orgs already have RBAC, though). So perhaps you'll need to solve this sooner.

Anyway, if you get to updating the workflows, feel free to tag me for reviews :)

@stdedos
Copy link
Collaborator Author

stdedos commented Apr 25, 2024

There's been talk over in Warehouse of deprecating the Maintainer role with more OIDC support so that leaves us with Owner and maybe some RBAC in the future (PyPI orgs already have RBAC, though). So perhaps you'll need to solve this sooner.

How much time? Given the speed that I have seen so far (... from 2 features), I think I should have ... 1 year, at least 😅

@stdedos you can still integrate the action, though, and prepare for future tokenless setup.

eeeh ... maybe. Let's not do before it's possible 😅 (it's the third time I've looked into this now, but "given the limitation", it is possible to do something and then forget it)

Another supply chain-related recommendation is to switch to pinning actions in the workflow definitions with commit SHAs which are immutable (unlike branches or tags).

If dependabot can handle updating pins (with commented-out versions), I'd take it as an update. Feel free to start an MR, if interested.

@webknjaz
Copy link

There's been talk over in Warehouse of deprecating the Maintainer role with more OIDC support so that leaves us with Owner and maybe some RBAC in the future (PyPI orgs already have RBAC, though). So perhaps you'll need to solve this sooner.

How much time? Given the speed that I have seen so far (... from 2 features), I think I should have ... 1 year, at least 😅

No idea, I'm only passively involved. But I think that was blocked on more OIDC providers and I recently noticed that you can set it up for platforms other than GitHub now. So that's unblocked.
Anyway, when this happens, there will likely be some official communications. Just thought I'd give some heads up.

@stdedos you can still integrate the action, though, and prepare for future tokenless setup.

eeeh ... maybe. Let's not do before it's possible 😅 (it's the third time I've looked into this now, but "given the limitation", it is possible to do something and then forget it)

Fair!

Another supply chain-related recommendation is to switch to pinning actions in the workflow definitions with commit SHAs which are immutable (unlike branches or tags).

If dependabot can handle updating pins (with commented-out versions), I'd take it as an update. Feel free to start an MR, if interested.

I've seen the versions in comments in some repos and I think Dependabot was updating those too.. Though, it needs checking.
I just pointed this out mostly because I saw a declaration that everyone here cares about the supply chain attacks, which is not compatible with using moving pointers for versions 🤷‍♂️

Hopefully, GitHub will complete the implementation of immutable action artifacts and this will stop being as dangerous..

@webknjaz
Copy link

Double checked the comment version thing. Found this example of dependabot doing the comment bumps properly: pyca/cryptography#10650.

@Pierre-Sassoulas
Copy link
Member

Pierre-Sassoulas commented Apr 25, 2024

We (pylint and pytest) use hashes tagging for external github actions (automatically upgraded by dependabot) but it means we need to review everything for each upgrade so the github actions need to remove at least as much labor to be worth it.

@stdedos
Copy link
Collaborator Author

stdedos commented Apr 25, 2024

We (pylint and pytest) use hashes tagging for external github actions (automatically upgraded by dependabot)

Yeah, it seems that I don't have almost anything else either (except codecov/codecov-action@v3, and some low-prio in-brewing lycheeverse/lychee-action - but that's low prio, and only locally)

So I guess, in reality, no update to do on that front (and the one update that is to do, can be handled as part of #43 - when I get around figuring out how to make their action not break unnecessarily)

@webknjaz
Copy link

@stdedos Codecov's v4 was so broken at the time of release that I ended up recommending not updating for a while, in @aio-libs. I reported a lot of issues there but haven't had time to revise it: https://github.com/orgs/aio-libs/discussions/36.
Although, they did fix a lot. So I'm checking one bug at a time. I think it's mostly only rare cases that may remain broken (I have an old PR in setuptools to fix coverage on Cywgin that crashes, for example).

It's known that the service is unreliable which is why many folks adopt https://hynek.me/articles/ditch-codecov-python/ instead. However personally, I think it's a good idea to combine the approaches.

In the past, Codecov introduced untested changes in their uploader that broke the official action user's for quite a while and I ended up having enough time to send PRs fixing their product and enclose very detailed troubleshooting stories..

Periodically I think I should just make my own wrapper with proper retries but that won't fix their backend 🤷‍♂️
I think we're running own uploader in @ansible since 2020.

@stdedos
Copy link
Collaborator Author

stdedos commented Apr 25, 2024

I'd really want to have something OSS for that. ... but with all of the things that I'm doing, trying to out-do Codecov is no small feat for me.

Regardless of how tr...."unrealiable" their upload process is (let alone pinning etc)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants