pypdf icon indicating copy to clipboard operation
pypdf copied to clipboard

git tag was not created for 4.3.0

Open MartinThoma opened this issue 1 year ago • 32 comments

This is mostly a reminder for me to check why the git tag was not created for the commit REL: 4.3.0. Something does not work as expected with https://github.com/py-pdf/pypdf/blob/main/.github/workflows/release.yaml

image

MartinThoma avatar Jul 14 '24 20:07 MartinThoma

I wonder if it's a limitation of using secrets.GITHUB_TOKEN for trying to create a protected tag, where the user associated doesn't have admin/maintainer rights on the repo? Might need to use a PAT, though that feels like it would bypass the entire reason to have protected tags.

MasterOdin avatar Jul 15 '24 16:07 MasterOdin

As far as I remember, we already had discussions about PATs, but tried to avoid it as they have a limited lifetime as well. According to the docs, our approach should work, although it seems like push protections prevent this from actually running correctly.

stefan6419846 avatar Jul 15 '24 18:07 stefan6419846

Yes, my guess was as well that this is a security mechanism. I'm uncertain if there is a reasonable way around it.

In the end, I also would trust you @stefan6419846 / @pubpub-zz enough to handle releases for pypdf. So I'm also thinking about simply giving you maintainer permissions.

MartinThoma avatar Jul 15 '24 20:07 MartinThoma

It looks to me like there's a tag there (at least now), but it's not signed, which is a departure from previous releases.

kitterma avatar Jul 18 '24 20:07 kitterma

Yes, the tag has been created in the meantime, but it should have been created automatically - this is what this issue seeks to further analyze. In this case, it seems like there is some setting which show an issue on the GitHub UI:

ksnip_20240719-090637

Do you rely on all tags being signed?

stefan6419846 avatar Jul 19 '24 07:07 stefan6419846

We do.

Scott K

kitterma avatar Jul 19 '24 09:07 kitterma

I could force-push a signed tag. Could that cause issues?

The workflow will attempt to create a new release on pypi, but that will fail (which is ok)

MartinThoma avatar Jul 19 '24 09:07 MartinThoma

In Debian our tooling is set up to pull from a signed tag, so for us, that's what's most important.

kitterma avatar Jul 19 '24 23:07 kitterma

@pubpub-zz @stefan6419846 You have done a great job as core contributors to pypdf for an extended period of time. I trust your intentions and abilities, hence I gave both of you the "Maintainer" permissions on the pypdf repository. That should mean you can trigger the release by merging a "REL: XYZ" commit in future.

MartinThoma avatar Jul 21 '24 09:07 MartinThoma

@kitterma pypdf releases relatively often. Is it fine when we just release the next release with a signed git tag? We could release 4.3.1 today :-)

edit: PR is prepared https://github.com/py-pdf/pypdf/pull/2764

MartinThoma avatar Jul 21 '24 09:07 MartinThoma

Yes. I can wait for the next release.

Scott K

kitterma avatar Jul 21 '24 11:07 kitterma

Release 4.3.1 failed in CI again.

As far as I understand, the executing user (probably GitHub Actions in this case) needs push permissions for protected branches - even if just creating a tag: https://github.blog/changelog/2021-11-19-allow-bypassing-required-pull-requests/

stefan6419846 avatar Jul 21 '24 19:07 stefan6419846

I've created the tag manually via git tag -s 4.3.1, pasting the part of the changelog, git push --tags.

I'll look more into that next week :eyes: :smiling_face_with_tear:

MartinThoma avatar Jul 21 '24 19:07 MartinThoma

That worked. Thanks.

kitterma avatar Jul 22 '24 03:07 kitterma

@MartinThoma Have you been able to progress in your analysis? Just for information we have some PRs in the pipe / already merged that should be released in 5.0.0 although we are not ready yet to tag it

pubpub-zz avatar Aug 28 '24 07:08 pubpub-zz

@MartinThoma +1 ?

pubpub-zz avatar Sep 03 '24 18:09 pubpub-zz

@kitterma Can you tell me if release 5.0.0 is good for you ?

pubpub-zz avatar Sep 17 '24 19:09 pubpub-zz

@MartinThoma Will you have time to fix this issue or should we consider that we have to use the workaround using git tag if so can you update the documentation ?

pubpub-zz avatar Sep 17 '24 21:09 pubpub-zz

@kitterma Can you tell me if release 5.0.0 is good for you ?

No. I can see the tag, but it's not signed. The commit is signed, but the tag is not. If you look in the GitHub U/I both 4.2.0 and 5.0.0 both show as verified, but if you click on the little verified tag for the tag it shows a signed tag in 4.2.0 (and 4.3.1) and a signed commit in 5.0.0. I guess this is progress, but it's the tag we need to verify.

kitterma avatar Sep 18 '24 01:09 kitterma

Could you please elaborate why each tag needs to be signed and verifiable in this case? While I appreciate a good relationship and factual discussion between downstream and upstream, I do not really understand why you as the downstream packager for a distro have to strictly rely on each of our upstream tags to actually be signed? (The goal is to further automate creating tags once we manage to fix the remaining CI issues and AFAIK these tags will not be signed as well unless we explicitly implement it.)

stefan6419846 avatar Sep 18 '24 05:09 stefan6419846

at next release (most probably by end of the month) I will test if the tag can be signed.

pubpub-zz avatar Sep 18 '24 21:09 pubpub-zz

Could you please elaborate why each tag needs to be signed and verifiable in this case? While I appreciate a good relationship and factual discussion between downstream and upstream, I do not really understand why you as the downstream packager for a distro have to strictly rely on each of our upstream tags to actually be signed? (The goal is to further automate creating tags once we manage to fix the remaining CI issues and AFAIK these tags will not be signed as well unless we explicitly implement it.)

We try to have a mechanism in place to be able to verify that the code we import into the distribution matches what upstream released. Since Pypi no longer supports signed tarballs, for Python projects a signed Git tag is often the best way to do that. The signed commit only validates that the commit hasn't been modified. It doesn't tell you anything about the tag. Thanks for working on this.

kitterma avatar Sep 21 '24 18:09 kitterma

@MartinThoma From my latest attempt, I don't have permissions to push a commit onto the server. Can you check this

I've also detected a few hickups in the make_release.py (use of UTF-8 and issue while parsing authors -PR in progress)

I've tried to produce a release where a tag is created, however, the web interface does not seem to use my PGP signature 😫 @kitterma sorry for the moment I can not produce a signed tag.

pubpub-zz avatar Sep 29 '24 10:09 pubpub-zz

I've also detected a few hickups in the make_release.py (use of UTF-8 and issue while parsing authors -PR in progress)

UTF-8 is the default Python encoding for quite some time now. This sounds like a Windows-only issue.

Edit: I just did a quick test for the authors handling and everything worked as expected on Ubuntu 22.04.

stefan6419846 avatar Sep 29 '24 10:09 stefan6419846

@MartinThoma From my latest attempt, I don't have permissions to push a commit onto the server. Can you check this

@MartinThoma +1?

pubpub-zz avatar Oct 06 '24 08:10 pubpub-zz

From my latest attempt, I don't have permissions to push a commit onto the server. Can you check this

What do you mean by this? Which commands did you try? AFAIK you are not able to push to main directly, but to any other branch on this repository.

stefan6419846 avatar Oct 06 '24 09:10 stefan6419846

Mistake in my words: I mean push a tag

pubpub-zz avatar Oct 06 '24 10:10 pubpub-zz

Tag was once again not signed. I know it's possible because the ones before 5.0.0 were signed.

kitterma avatar Oct 06 '24 18:10 kitterma

Tag was once again not signed. I know it's possible because the ones before 5.0.0 were signed.

We are aware of this, but not every pypdf maintainer has a corresponding setup and/or signs all commits/tags. Releases before 5.0.0 were created manually by Martin, whose required maintenance work we are trying to reduce and thus the last tags were not pushed by him. While this issue is open, there probably will be no general change. Our goal is to further automate creating tags, although it is not clear whether we will have a clean way to sign these tags through GitHub CI without too much overhead.

Apart from this, I am not aware of any pydf policy which would document such signing as a hard requirement. I personally do not consider signing essential. IMHO it is the downstream maintainers choice and while I appreciate good interactions between upstream and downstream maintainers, I currently see no value in pinging upstream about this after every release.

stefan6419846 avatar Oct 06 '24 18:10 stefan6419846

On October 6, 2024 6:54:51 PM UTC, Stefan @.***> wrote:

Tag was once again not signed. I know it's possible because the ones before 5.0.0 were signed.

We are aware of this, but not every pypdf maintainer has a corresponding setup and/or signs all commits/tags. Releases before 5.0.0 were created manually by Martin, whose required maintenance work we are trying to reduce and thus the last tags were not pushed by him. While this issue is open, there probably will be no general change. Our goal is to further automate creating tags, although it is not clear whether we will have a clean way to sign these tags through GitHub CI without too much overhead.

Apart from this, I am not aware of any pydf policy which would document such signing as a hard requirement. I personally do not consider signing essential. IMHO it is the downstream maintainers choice and while I appreciate good interactions between upstream and downstream maintainers, I currently see no value in pinging upstream about this after every release.

Sure. After 5.0.0 I was told that the next release would have a signed tag. It didn't. I think it's fair to mention that.

I don't think moving away from having a cryptographically verifiable release artifact is a good idea. Obviously it's your call, so do what you will.

It used to be one could upload signed tarballs to Pypi, but they did away with that.

In cases where I'm upstream, I have manually uploaded signed tarballs to GitHub as well as signed the tags. I'm open to alternatives, but would (as I'm sure is clear) prefer that there's something.

Please just let me know what the plan is. We'll adapt.

Scott K

kitterma avatar Oct 06 '24 19:10 kitterma