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

resource-uri: handle two-part resource paths #917

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

pmores
Copy link

@pmores pmores commented Feb 20, 2025

Two-part resource paths have so far resulted in an error. This however seems incorrect since the repository part of a resource path is specified as optional in the kbs-protocol definition(*).

This change could conceivably break a caller that relies on ResourceUri to bail out if a resource path only has two parts. Arguably such a caller would itself be faulty though, given the kbs-protocol definition.

(*) https://github.com/confidential-containers/trustee/blob/main/kbs/docs/kbs_attestation_protocol.md#secret-resource

Two-part resource paths have so far resulted in an error.  This however
seems incorrect since the repository part of a resource path is specified
as optional in the kbs-protocol definition(*).

This change could conceivably break a caller that relies on ResourceUri
to bail out if a resource path only has two parts.  Arguably such a caller
would itself be faulty though, given the kbs-protocol definition.

(*) https://github.com/confidential-containers/trustee/blob/main/kbs/docs/kbs_attestation_protocol.md#secret-resource

Signed-off-by: Pavel Mores <pmores@redhat.com>
@pmores
Copy link
Author

pmores commented Feb 20, 2025

Please do review carefully since I'm very new here and chances of me overlooking something are definitely solidly non-zero.

Copy link
Member

@fitzthum fitzthum left a comment

Choose a reason for hiding this comment

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

Seems good. One nit.

let (repository, r#type, tag) = if values.len() == 3 {
(values[0].into(), values[1].into(), values[2].into())
} else if values.len() == 2 {
println!("two-part resource path, filling in \"default\" for repository");
Copy link
Member

Choose a reason for hiding this comment

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

probably want to use log::info! here instead

@Xynnn007
Copy link
Member

Please see my comment confidential-containers/trustee#712 (comment)

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.

3 participants