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

Change TreeID to be of type string instead of int64 #712

Merged
merged 1 commit into from
Mar 9, 2022

Conversation

priyawadhwa
Copy link
Contributor

@priyawadhwa priyawadhwa commented Mar 7, 2022

When printing the TreeID with rekor-cli loginfo, if the output is parsed through jq
then the TreeID gets rounded down as an int because it is bigger than JSON allows Numbers to be.

This is how jq works and is mentioned in the FAQ: https://github.com/stedolan/jq/wiki/FAQ#numbers

Switching this to a string will preserve the actual Tree ID.

How this used to work:

% go run ./cmd/rekor-cli loginfo --rekor_server http://localhost:3000 --format json
Persisted log state matches the current state of the log
{"TreeSize":0,"RootHash":"e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855","TimestampNanos":1646669010959148926,"TreeID":1593213237895900003}

# With jq, note TreeID is rounded down
% go run ./cmd/rekor-cli loginfo --rekor_server http://localhost:3000 --format json | jq
Persisted log state matches the current state of the log
{
  "TreeSize": 0,
  "RootHash": "e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855",
  "TimestampNanos": 1646669014825668000,
  "TreeID": 1593213237895900000
}

Signed-off-by: Priya Wadhwa priya@chainguard.dev

cc @lkatalin

Summary

Ticket Link

Fixes

Release Note


@priyawadhwa priyawadhwa requested a review from a team as a code owner March 7, 2022 16:04
Copy link
Contributor

@lkatalin lkatalin left a comment

Choose a reason for hiding this comment

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

Great catch, @priyawadhwa , thank you!!

Copy link
Member

@bobcallaway bobcallaway left a comment

Choose a reason for hiding this comment

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

one minor comment but LGTM

openapi.yaml Show resolved Hide resolved
When printing the TreeID with rekor-cli loginfo, if the output is parsed through jq
then the TreeID gets rounded down as an int because it is bigger than JSON allows Numbers to be.

This is how jq works and is mentioned in the FAQ: https://github.com/stedolan/jq/wiki/FAQ#numbers

Switching this to a string will preserve the actual Tree ID.

Signed-off-by: Priya Wadhwa <priya@chainguard.dev>
@@ -43,7 +43,7 @@ type logInfoCmdOutput struct {
TreeSize int64
RootHash string
TimestampNanos uint64
TreeID int64
TreeID string
Copy link
Member

Choose a reason for hiding this comment

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Weirdly I'm still seeing the same issue with this even though it should be a string...

@dlorenc dlorenc merged commit b719032 into sigstore:main Mar 9, 2022
@github-actions github-actions bot added this to the v1.0.0 milestone Mar 9, 2022
@priyawadhwa priyawadhwa deleted the tree-id-string branch March 9, 2022 11:56
@dlorenc dlorenc mentioned this pull request Mar 9, 2022
This pull request was closed.
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.

5 participants