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

Semantic versioning for slsa-github-generator releases #1253

Closed
behnazh-w opened this issue Nov 22, 2022 · 6 comments
Closed

Semantic versioning for slsa-github-generator releases #1253

behnazh-w opened this issue Nov 22, 2022 · 6 comments
Labels
type:discussion A point of discussion

Comments

@behnazh-w
Copy link
Contributor

The current versions of slsa-github-generator releases are not accurately following the semantic versioning rules, which causes confusions as the bots, such as Renovate send update PRs.

For instance, v1.2.1 is bumped to v1.2.2, which means the updated version should fix a bug but generally v1.2.1 should still work as there is no breaking change. However, looking at the release notes of v1.2.1, looks like v1.2.1 is not working anymore (which is not the same as deprecated actually). Based on semantic versioning rules for deprecated functionality, I would have expected to see v1.3.0 as a version that will be deprecated but still works, and v2.0.0 to actually remove the functionality with a breaking change.

That being said, I think the current version of slsa-github-generator releases have to consider two aspects while bumping a version:

  1. The API itself, e.g., has the inputs outputs changed that could result in a breaking change?
  2. Availability: unfortunately the current version still relies on Rekor from Sigstore, so if it breaks, the whole generator breaks too. In this case versioning becomes meaningless and semantic versioning rules can't really help.

To avoid all these problems, it seems crucial to remove reliance on Rekor to begin with, and only then the versions can be meaningful.

@behnazh-w behnazh-w added status:triage Issue that has not been triaged type:bug Something isn't working labels Nov 22, 2022
@ianlewis
Copy link
Member

ianlewis commented Nov 23, 2022

I agree in principle that software should continue to work even if you don't upgrade and I think in hindsight it might have been prudent for us to not GA slsa-github-generator workflows or make any assertions about it working until Rekor's public instance was GA and we had better tests in place for Rekor changes. We and the sigstore team didn't foresee the issues with new Rekor versions. I also agree we'd like to give users options and not rely on Rekor as much.

However, transparency logs are an important part of keyless signing and verification so even if we reduce our reliance on the public instance of Rekor, we would still need to rely on some instance of Rekor somewhere. We are considering adding support for private instances of Rekor which would allow users to not rely on the public instance, but requires users to run their own instance of Rekor.

We are also considering adding "offline" verification to the slsa-verifier but this only partly solves the issue as IIUC it's just simply caching TUF roots. We can't really remove reliance on Rekor without requiring users to bring and manage their own keys, which has its own security issues.

Based on semantic versioning rules for deprecated functionality, I would have expected to see v1.3.0 as a version that will be deprecated but still works, and v2.0.0 to actually remove the functionality with a breaking change.

I'm curious about your thoughts but I'm not sure I understand you fully. How your are describing semver is how I understand it as well. I'm not sure semver really makes it clear what to do when a particular version of the software no longer works and an upgrade is required even though the API has not changed.

Some questions in my mind:

  1. What do you think we should have done?
  2. Is there a better way to communicate that a previous version is not working at all and you need to upgrade? Maybe "deprecate" was the wrong word to use to describe v1.2.0 and v1.2.1?
  3. Would it have made more sense to release v1.2.2 as v2.0.0 because v1.2.1 was no longer functional even though the API didn't change?

@behnazh-w
Copy link
Contributor Author

Thanks @ianlewis for the answer.

until Rekor's public instance was GA and we had better tests in place for Rekor changes.

So we should expect v1.2.2 which was released after Rekor's public instance was GA to be available even if Rekor moves on and introduces a breaking change. We really need to make sure this can happen by proper API versioning and support from the Rekor side. Otherwise, provenance reusable workflows should not be used in production IMO.

I'm not sure semver really makes it clear what to do when a particular version of the software no longer works and an upgrade is required even though the API has not changed.

I agree with you that semver doesn't currently specify how to deal with availability of a service. But I would argue it's the responsibility of the maintainers of the Rekor instance to solve this issue by providing a versioned service, which cannot just become unavailable suddenly. That's a standard expectation in industry AFAIK.

What do you think we should have done?

I would have not started with version 1.0.0 because that's a production ready version. Please see the major version zero rule, which would have been more suitable. Going forward, we need meaningful semantic versioning for the workflows, and that can only be achieved if Rekor instance, as a dependency of the generator workflows, accurately follows it too.

Is there a better way to communicate that a previous version is not working at all and you need to upgrade? Maybe "deprecate" was the wrong word to use to describe v1.2.0 and v1.2.1?
Would it have made more sense to release v1.2.2 as v2.0.0 because v1.2.1 was no longer functional even though the API didn't change?

I think "deprecate" was not the right word. I would have said the workflow is not "available" anymore and bump to v2.0.0 to communicate the breaking effect (but ideally this should have been using the major version zero to avoid bumping as the workflows were not stable yet).

@ianlewis
Copy link
Member

ianlewis commented Nov 24, 2022

So we should expect v1.2.2 which was released after Rekor's public instance was GA to be available even if Rekor moves on and introduces a breaking change. We really need to make sure this can happen by proper API versioning and support from the Rekor side. Otherwise, provenance reusable workflows should not be used in production IMO.

This is something I expect from Rekor moving forward now that it is GA. We are also working with them to add tests to ensure that older clients do not break when the server is upgraded.

I'm not sure semver really makes it clear what to do when a particular version of the software no longer works and an upgrade is required even though the API has not changed.

I agree with you that semver doesn't currently specify how to deal with availability of a service. But I would argue it's the responsibility of the maintainers of the Rekor instance to solve this issue by providing a versioned service, which cannot just become unavailable suddenly. That's a standard expectation in industry AFAIK.

Yes. This is standard practice and I expect it from Rekor moving forward. Since it wasn't GA it broke older clients a few times in the past but in the future if that happens I expect it to be caught before release, or for service to be restored to supported clients.

What do you think we should have done?

I would have not started with version 1.0.0 because that's a production ready version. Please see the major version zero rule, which would have been more suitable. Going forward, we need meaningful semantic versioning for the workflows, and that can only be achieved if Rekor instance, as a dependency of the generator workflows, accurately follows it too.

I think that's fair. We should have waited for GA of Rekor to make our workflows GA. Though, the cat was kind of out of the bag once we released v1.0.0. We can't really undo that decision even if it was made in error.

I think "deprecate" was not the right word. I would have said the workflow is not "available" anymore and bump to v2.0.0 to communicate the breaking effect (but ideally this should have been using the major version zero to avoid bumping as the workflows were not stable yet).

I see. I think "available" is the wrong word since it's still available in the strict sense because we aren't going to delete the tags from the git repo. How about "not supported"? Does that work?

@ianlewis ianlewis added type:discussion A point of discussion and removed type:bug Something isn't working status:triage Issue that has not been triaged labels Nov 24, 2022
@behnazh-w
Copy link
Contributor Author

I see. I think "available" is the wrong word since it's still available in the strict sense because we aren't going to delete the tags from the git repo. How about "not supported"? Does that work?

"not supported" might imply it's not maintained anymore. How about "withdrawn"? Or anything that communicates the workflows does not work anymore.

@ianlewis
Copy link
Member

ianlewis commented Nov 25, 2022

Thanks. I updated the warnings on v1.2.0 and v1.2.1 releases to the following. Hopefully this is strong enough language that they should not be used and that they will not work.

v1.2.1
DO NOT USE THIS RELEASE. This version will no longer work and is not supported due to errors described in #1163. Please upgrade to v1.2.2 or later.

v1.2.0
DO NOT USE THIS RELEASE. This version will no longer work and is not supported due to errors described in #942. Please upgrade to v1.2.2 or later.

@ianlewis
Copy link
Member

I'm going to close this since there aren't any actions left to do. We've improved our practices somewhat, like keeping a good change log and requiring conventional commits to avoid semantic versioning issues in the future.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type:discussion A point of discussion
Projects
None yet
Development

No branches or pull requests

2 participants