Skip to content

Conversation

tonistiigi
Copy link
Member

This fallback case was accidentally left out of https://github.com/moby/buildkit/pull/4563/files

It can appear for example if new dockerfile frontend resolves SBOM generator image.

@@ -484,7 +484,31 @@ func (c *grpcClient) Solve(ctx context.Context, creq client.SolveRequest) (res *

func (c *grpcClient) ResolveSourceMetadata(ctx context.Context, op *opspb.SourceOp, opt sourceresolver.Opt) (*sourceresolver.MetaResponse, error) {
if c.caps.Supports(pb.CapSourceMetaResolver) != nil {
return nil, errors.Errorf("fallback not implemented")
var ref string
if strings.HasPrefix(op.Identifier, "docker-image://") {
Copy link
Member

Choose a reason for hiding this comment

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

strings.CutPrefix can replace HasPrefix and TrimPrefix.

if strings.HasPrefix(retRef, "docker-image://") {
op.Identifier = "docker-image://" + retRef
} else if strings.HasPrefix(retRef, "oci-layout://") {
op.Identifier = "oci-layout://" + retRef
Copy link
Member

@jedevc jedevc Feb 14, 2024

Choose a reason for hiding this comment

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

If HasPrefix, prepend it again? I assume the line above should be op.Identifier.

Copy link
Member

@jedevc jedevc Feb 14, 2024

Choose a reason for hiding this comment

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

I think this is still wrong. ref has had the prefix stripped already, so the condition never triggers.

This should be:

if strings.HasPrefix(op.Identifier, "docker-image://") {
	op.Identifier = "docker-image://" + retRef
}

That was we actually are checking the prefix, and making sure the output has the same prefix.

@tonistiigi tonistiigi force-pushed the sourceresolver-fallback branch 2 times, most recently from f18756f to 4fe9630 Compare February 14, 2024 16:27
Signed-off-by: Tonis Tiigi <tonistiigi@gmail.com>
Signed-off-by: Tonis Tiigi <tonistiigi@gmail.com>
@tonistiigi tonistiigi force-pushed the sourceresolver-fallback branch from 4fe9630 to 0184fde Compare February 14, 2024 17:18
@tonistiigi tonistiigi requested a review from jedevc February 14, 2024 19:24
@tonistiigi
Copy link
Member Author

Self-merge to unblock CI

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.

2 participants