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

Migrate to a CI build with SLSA provenance #896

Merged
merged 8 commits into from
Feb 14, 2023

Conversation

pnacht
Copy link
Contributor

@pnacht pnacht commented Jan 23, 2023

Closes #844.

CI build with SLSA provenance

As discussed in #847 (comment), this PR will allow jackson's maintainer to migrate from releasing on their local machine to a CI release, while keeping the mvn release:prepare workflow they're used to.

This is done via the new release.sh script, which:

  • runs mvn release:clean release:prepare (without pushing to GitHub)
  • makes a few changes to the output (described below)
  • pushes the new commits and tag to GitHub

When the new tag is pushed to GitHub, the new release.yml workflow kicks in and performs the final mvn release:perform step, as well as generating SLSA provenance.

Work done by release.sh

mvn release:prepare effectively does three things:

  • it creates a new commit where all SNAPSHOT versions are replaced by non-SNAPSHOT versions
  • it creates a tag pointing to this commit
  • it creates a new commit back to SNAPSHOT versions (if the next version)

release.sh has to rewrite the commits created by mvn release:prepare.

This is because mvn release:perform requires a release.properties file that is created by release:prepare,* but that isn't actually committed. Since perform will be run in the CI, we need release.properties to be committed as well.

We must therefore rewrite the commits so that release.properties is added to the first commit and then deleted in the second commit (so the final result is unchanged). In changing the commits, we also need to recreate the tag.

Work done by release.yml

The workflow itself is straightforward: it checks out the code (on the commit tied to the pushed tag) and then runs mvn release:perform.

Once the release is complete, it calculates the jar's hash and uses it to create a SLSA provenance attestation file, which is added as a workflow artifact.

The workflow only runs on "pure" tags without metadata; tags ending with -rc, .pr or b are ignored (those were the variants I found in the project's list of tags, let me know if there are others you'd like to omit).

Likely incorrect value

The workflow copied the actions/setup-java arguments from main.yml, which likely isn't correct (especially the server-id: sonatype-nexus-snapshots. Unfortunately, after checking out the pom.xml, I couldn't figure out what the correct ID would be.


* Technically, release:perform accepts command line arguments, but it doesn't work with GitHub URLs. And regardless, making the GitHub workflow rely on release.properties reduces the potential damage from accidentally pushing a random tag. In such a case, the workflow will simply fail.

@cowtowncoder
Copy link
Member

@pnacht Thank you for the PR! I'll need to have another and hope to do that soon.

One quick question: have I asked for CLA yet? If not, I'd need this:

https://github.com/FasterXML/jackson/blob/master/contributor-agreement.pdf

before first merge (but only once, good for all future contributions, PRs etc).
The usual way is to print it, fill & sign, scan/photo, email to info at fasterxml dot com.

Looking forward to merging this!

@cowtowncoder
Copy link
Member

Ok, thank you again @pnacht. Aside from CLA (which I'll need) things make sense.

There are 2 things, one smaller, one bigger that I am not sure about.

Both are related to publishing of Release Candidates (current version numbering scheme being "2.15.0-rc1" etc -- apparently many Maven tools do not recognize prX which I'd personally prefer -- and OSGi mandated "2.15.0.rc1" is apparently also problematic for other tools... so this is by process of elimination).

So: I do need to publish these too, but would the idea be that I would just use the old method of using Maven release plugin outside of SLSA workflow? That'd be fine for me but not sure if that makes sense or not: maybe provenance aspects are important for (pre-) Release Candidates too, if we are to get users to test them?

Aside from that I was wondering if tags to consider should be opt-in rather than opt-out?
Format for valid version numbers is [\d\.\d\d?\.\d\d?] but I guess there's the "jackson-core-" prefix that release plugin adds. Or maybe GH Action that does tagging uses just version number?
But basically it might be easier and more reliable to indicate what is a valid version number, instead of trying to rule out known "not a version to release" versions?

on:
push:
tags:
- "*"
Copy link
Member

Choose a reason for hiding this comment

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

As per my note, should perhaps verify that the tag ends in something like "2.15.3"; although if Release Candidates were to use this workflow too (should they?) would need to accommodate those too.

@pnacht
Copy link
Contributor Author

pnacht commented Jan 27, 2023

Ah, I hadn't noticed that the RCs are actually published to Maven! I thought they were only used to test deployment to the Sonatype Nexus Staging area.

As is, the workflow rejects RCs (tags: - "!*-rc*"), but that can be removed easily enough.

Provenance is as relevant for RCs as for regular releases, and as far as I'm aware, there's no reason not to add it to RCs once the infrastructure has been set up.

As for the tag names, I can certainly make the filter more specific and validate it (^jackson-core-\d+\.\d+\.\d+(-rc\d*)?$).

What do you think?

Also, what should I do about the server-id parameter? It's currently set to sonatype-nexus-snapshots, which I assume isn't the right place for actual releases.

@cowtowncoder
Copy link
Member

@pnacht Right, release candidates are indeed pushed to MC as that makes it easier for developers to test them. Having to use, say, SNAPSHOT builds means very few developers help with pre-release candidates (based on my experience).
Improvement to tag name validation sounds good.

On server-id: this must be due to default main.yml only publishing -SNAPSHOT builds so sonatype-nexus-snapshot would indeed be the right target. For releases I think I have locally sonatype-nexus-staging which is where actual release go I think. Settings come via parent poms, ultimately from fom.fasterxml:oss-parent.

@pnacht
Copy link
Contributor Author

pnacht commented Jan 30, 2023

Gotcha.

One clarification for someone not fluent in Maven: will the parent pom (up to oss-parent) be fetched automatically? That is, can I simply set server-id: sonatype-nexus-staging and it'll figure it out? Or do I need to make some other changes to mimic your local setup in the CI?

@cowtowncoder
Copy link
Member

Gotcha.

One clarification for someone not fluent in Maven: will the parent pom (up to oss-parent) be fetched automatically? That is, can I simply set server-id: sonatype-nexus-staging and it'll figure it out? Or do I need to make some other changes to mimic your local setup in the CI?

Sorry, missed this question. Yes, parent pom settings are inherited so you can just refer to that value.

@pnacht
Copy link
Contributor Author

pnacht commented Feb 8, 2023

Sorry for the delay, but I've just made the requested changes. The workflow now uses server-id: sonatype-nexus-staging and validates that the version tag follows the Release Plugin's "jackson-core-1.2.3(-rc1)?" format.

@cowtowncoder cowtowncoder added the cla-needed PR looks good (although may also require code review), but CLA needed from submitter label Feb 13, 2023
@cowtowncoder
Copy link
Member

Ok great @pnacht !

Just one question: looks like this runs for all tags, but validation step will fail for non-compliant tags: is this correct?
If so that's perfect: we only want to stage releases with compliant version tags.

Aside from that all I need is the CLA as per my earlier note, and then I can merge this PR.

@pnacht
Copy link
Contributor Author

pnacht commented Feb 13, 2023

Just one question: looks like this runs for all tags, but validation step will fail for non-compliant tags: is this correct? If so that's perfect: we only want to stage releases with compliant version tags.

That's correct, the workflow only progresses past the first step if the tag name is compliant with the standard Release Plugin format.

And if a compliant tag is pushed but lacks the release.properties file (created by release:prepare and momentarily committed via the changes done in release.sh), then the release will fail at the release step, since release:perform requires it.

For examples, see these workflow runs on my fork:

Aside from that all I need is the CLA as per my earlier note, and then I can merge this PR.

Slipped my mind! Just sent it over.

@cowtowncoder cowtowncoder merged commit b3c6899 into FasterXML:2.15 Feb 14, 2023
@cowtowncoder cowtowncoder added this to the 2.15.0 milestone Feb 14, 2023
@cowtowncoder cowtowncoder removed the cla-needed PR looks good (although may also require code review), but CLA needed from submitter label Feb 14, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add SLSA provenance via build script
2 participants