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

fix: Fixes v1.0.0 versions and below to correctly get entries by UUID #132

Closed
wants to merge 1 commit into from

Conversation

asraa
Copy link
Contributor

@asraa asraa commented Jul 8, 2022

Signed-off-by: Asra Ali <[email protected]>
@asraa
Copy link
Contributor Author

asraa commented Jul 8, 2022

@laurentsimon I had to make these fixes from v1.0.0: I needed to update rekor (and then also cosign) to pull in new sharding libraries, and then update logic in the entry we needed to verify.

Because v1.0.0 is a tag, I can't easily merge in.

Because v1.0.0 and below verifiers only use search index, they are effectively totally broken now.

@laurentsimon laurentsimon self-requested a review July 8, 2022 17:05
@laurentsimon
Copy link
Contributor

laurentsimon commented Jul 8, 2022

@laurentsimon I had to make these fixes from v1.0.0: I needed to update rekor (and then also cosign) to pull in new sharding libraries, and then update logic in the entry we needed to verify.

Because v1.0.0 is a tag, I can't easily merge in.

How do we plan to merge this PR?

Is it do-able to backport the change by re-writing the commit history and re-running the release? Just curious how much work this would be.

Or we could create a new branch off v1.0.0 to backport the changes then re-run the release. Not ideal because verification would need to pass a branch... I'm not sure if we can have a branch v1.0.1 and a tag v1.0.0 :) Probably not?

I'm wondering how we can best respond to a similar problem in the future.

Because v1.0.0 and below verifiers only use search index, they are effectively totally broken now.

But this PR is fixing it.. correct?

@asraa
Copy link
Contributor Author

asraa commented Jul 11, 2022

Or we could create a new branch off v1.0.0 to backport the changes then re-run the release. Not ideal because verification would need to pass a branch... I'm not sure if we can have a branch v1.0.1 and a tag v1.0.0 :) Probably not?

I think yes, we should create a new branch. If we call this branch v1.0.0, and then delete the tag and create a new release for v1.0.0, would that work?

In the future, we should create a branch for each release I think. If a release has a major issue like this, then we should always be using verifier at the highest patch version anyway, right?

@laurentsimon
Copy link
Contributor

Or we could create a new branch off v1.0.0 to backport the changes then re-run the release. Not ideal because verification would need to pass a branch... I'm not sure if we can have a branch v1.0.1 and a tag v1.0.0 :) Probably not?

I think yes, we should create a new branch. If we call this branch v1.0.0, and then delete the tag and create a new release for v1.0.0, would that work?

I think it would. We'll need to update the SHA256SUM.md too. Let's keep all the assets around in case :)

In the future, we should create a branch for each release I think. If a release has a major issue like this, then we should always be using verifier at the highest patch version anyway, right?

I think so. So basically we'd create a release at HEAD, and then create the release-v1.0.0 branch? I think there's one caveat I've come across:

  1. If 2 branches (main and release-v1.0.0) are at the same commit, creating a release from release-v1.0.0 will trigger the workflow on the main branch. During verification, no need for --branch release-v1.0.0 and verification will pass
  2. If we edit an old branch and re-release, the branch will be branch1, so verification will need --branch release-branch1.

To be consistent, maybe we could push a commit to main before we release for a release branch (what should that commit be..), this way it will always need --branch release-branch1. Note that this second branch could also help when doing pre-release tests (instead of doing them at main)

@laurentsimon
Copy link
Contributor

laurentsimon commented Jul 11, 2022

Or we could create a new branch off v1.0.0 to backport the changes then re-run the release. Not ideal because verification would need to pass a branch... I'm not sure if we can have a branch v1.0.1 and a tag v1.0.0 :) Probably not?

I think yes, we should create a new branch. If we call this branch v1.0.0, and then delete the tag and create a new release for v1.0.0, would that work?

In the future, we should create a branch for each release I think. If a release has a major issue like this, then we should always be using verifier at the highest patch version anyway, right?

Is there a way to keep track of previous release somewhere? That can be useful when someone asks us why their verifier is different fr the one in our release, which will cause confusion. I think keeping the old assets somewhere should be enough for audit and traceability. Wdut?

Let's start a PR/issue to discuss this

@asraa
Copy link
Contributor Author

asraa commented Jul 11, 2022

Closing in favor of #140

@asraa asraa closed this Jul 11, 2022
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.

[e2e]: go schedule main config-ldflags-main slsa3
2 participants