-
Notifications
You must be signed in to change notification settings - Fork 41
Introduce v0.2 TrustedRoot, un-deprecate log ID #690
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
Conversation
@@ -67,50 +67,50 @@ class Signature(betterproto.Message): | |||
|
|||
|
|||
@dataclass(eq=False, repr=False) | |||
class DsseRequestV002(betterproto.Message): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@woodruffw any guesses as to why my build output differs from CI? I can just omit this file, I’ve seen this before though.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For now, I've reverted this change. Will dig in more later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, no idea unfortunately. My best guess is that something in your local resolution is diverging from the CI's resolution, producing different codegen, but I have no idea what would cause that 😞
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Changes looks good, thanks.
As we learned when we introduced a new SigningConfig version, adding fields to the proto isn't a breaking change, but setting these fields in JSON and unmarshalling the object is a breaking change. Since we've added operator and checkpoint_key_id to the Instance message, this requires introduced a new TrustedRoot version, which I've done in this PR as v0.2. As with SigningConfig, I've made it optional for clients to indefinitely support old versions, but it should be little work for a client to support both versions concurrently. I've also un-deprecated log ID. This was premature since no clients supported checkpoint key ID yet. With Rekor v2, we'll be setting the same value in both log_id and checkpoint_key_id, so clients can continue to ignore checkpoint_key_id for now. Signed-off-by: Hayden B <[email protected]>
f44a457
to
559aa06
Compare
Do y'all have opinions on if I should cut a new minor release vs a patch release? Given this is a new TrustedRoot release, I'm leaning towards a new minor 0.5 |
A new minor seems like the correct choice 👍 |
As we learned when we introduced a new SigningConfig version, adding fields to the proto isn't a breaking change, but setting these fields in JSON and unmarshalling the object is a breaking change. Since we've added operator and checkpoint_key_id to the Instance message, this requires introduced a new TrustedRoot version, which I've done in this PR as v0.2. As with SigningConfig, I've made it optional for clients to indefinitely support old versions, but it should be little work for a client to support both versions concurrently.
I've also un-deprecated log ID. This was premature since no clients supported checkpoint key ID yet. With Rekor v2, we'll be setting the same value in both log_id and checkpoint_key_id, so clients can continue to ignore checkpoint_key_id for now.
Summary
Release Note
Documentation