-
Notifications
You must be signed in to change notification settings - Fork 160
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
control: allow to query for latest TRC #4601
control: allow to query for latest TRC #4601
Conversation
Currently querying for the latest TRC was not allowed, because TRC ID validation would fail. However it should be completely fine to query for the latest TRC. This commit fixes it.
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.
Reviewable status: 0 of 1 files reviewed, 1 unresolved discussion (waiting on @jiceatscion and @lukedirtwalker)
control/trust/grpc/proto.go
line 70 at r1 (raw file):
// If the query is for the latest version don't validate the ID fully, only // the ISD ID. if id.Base.IsLatest() && id.Serial.IsLatest() {
IMO it is also legit to ask for latest serial on a fixed base version.
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.
Reviewable status: 0 of 1 files reviewed, 1 unresolved discussion (waiting on @jiceatscion and @oncilla)
control/trust/grpc/proto.go
line 70 at r1 (raw file):
Previously, oncilla (Dominik Roos) wrote…
IMO it is also legit to ask for latest serial on a fixed base version.
The DB does not think so:
scion/private/storage/trust/sqlite/db.go
Lines 84 to 86 in 4c929f6
if id.Base.IsLatest() != id.Serial.IsLatest() { | |
return cppki.SignedTRC{}, serrors.New("unsupported TRC ID for query", "id", id) | |
} |
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.
Reviewed 1 of 1 files at r1, all commit messages.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @jiceatscion)
control/trust/grpc/proto.go
line 70 at r1 (raw file):
Previously, lukedirtwalker (Lukas Vogel) wrote…
The DB does not think so:
scion/private/storage/trust/sqlite/db.go
Lines 84 to 86 in 4c929f6
if id.Base.IsLatest() != id.Serial.IsLatest() { return cppki.SignedTRC{}, serrors.New("unsupported TRC ID for query", "id", id) }
Fine. Overruled by our past selves
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.
Reviewed all commit messages.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @lukedirtwalker)
Currently querying for the latest TRC was not allowed, because TRC ID validation would fail. However it should be completely fine to query for the latest TRC. This commit fixes it.