fix podman load oci-archive's tar with name #7386

zhangguanzhang · 2020-08-20T06:26:00Z

Fixes: #7337
fixes this problem:

$ podman pull alpine
$ podman save --format oci-archive alpine:latest > /tmp/FOO.tar
$ podman load -i /tmp/FOO.tar foo
Error: error pulling "foo": unable to pull dir:/tmp/FOO.tar: error determining pull goal for image "dir:/tmp/FOO.tar": error parsing dest reference name "localhost/tmp/FOO.tar": error parsing named reference "localhost/tmp/FOO.tar": invalid reference format: repository name must be lowercase

Signed-off-by: zhangguanzhang [email protected]

rhatdan · 2020-08-20T10:35:14Z

@mtrmac @vrothberg PTAL

edsantiago

Thanks for looking into this. Code is still broken (very unhappy tests), and I can't offer suggestions there, but here's one suggestion re: tests.

edsantiago · 2020-08-20T12:06:35Z

test/system/120-load.bats

+
+    run_podman load -i $archive test.com/foo:test
+    verify_name test.com/foo:test
+    run_podman rmi test.com/foo:test


This is a nice set of tests, but they're actually unrelated to the purpose of this particular test block ("save to pipe"). They would belong better in a separate @test. I would also like to see an explicit test for CAPS in the archive name. So maybe something like:

@test "podman save/load: oci-archive" { # FIXME: remove this if #7371 gets fixed safe_archive=$PODMAN_TMPDIR/restoreme.tar run_podman save -o $safe_archive $IMAGE archive=$PODMAN_TMPDIR/MYIMAGE-$(random_string 8).tar run_podman save --format oci-archive -o $archive $IMAGE run_podman rmi $IMAGE function _load_and_verify() { run_podman load -i $archive "$1" run_podman images -a --format '{{.Repository}}:{{.Tag}}' is "$output" "$2" "podman load, with tag $1" run_podman rmi "$2" } _load_and_verify "foo" "localhost/foo:latest" _load_and_verify "foo:test" "localhost/foo:test" _load_and_verify "test.com/foo" "test.com/foo:latest" _load_and_verify "test.com/foo:test" "test.com/foo:test" # Restore original test image with real IID (FIXME: #7371) run_podman load -i $safe_archive }

zhangguanzhang · 2020-08-20T12:16:52Z

ci has some errors, I will try to fix it

mtrmac

I really don’t understand what this is fixing and how. This is the third PR with, what seems to me, wildly different approach to supposedly the same problem.

You probably understand the situation, after spending days on this, but without writing it down we don’t know whether we are seeing the same thing or considering the same goal (and at least I can’t spend the same days to retrace all your steps now.)

Please make a clear record for the future:

As a part of the code base:
- End-user documentation: what is the goal / intended behavior of the feature
- Programmer documentation: What is the semantics of the involved functions / data structures.
- (If it is not obvious, which should not be the case after the above:) How the implementation works and matches the function contracts.
As part of this PR / process:
- What is failing. Was the previous goal of the code incorrect? Or just incorrectly developed?
  - (podman load -i Path-With-Caps label : invalid reference format: repository name must be lowercase #7337 only lists an error reported by the dir: transport. If this is focusing on oci-archive:, please write down what is the problem encountered when using oci-archive: that this is supposed to be fixing. So far I’m just guessing based on the the very different PRs and I can’t see any connection to upper-case file names.)
- How is this PR fixing this

(Others: See #6811 (comment) for some context around this, it’s not at all clear what we are doing here.)

mtrmac · 2020-08-20T12:19:36Z

libpod/image/pull.go

@@ -176,6 +176,24 @@ func (ir *Runtime) pullGoalFromImageReference(ctx context.Context, srcRef types.
 		}, nil

 	case OCIArchive:
+		// imgName such as "oci-archive:/tmp/FOO.tar:localhost/foo:latest"
+		// or "oci-archive:/tmp/FOO.tar:domain.com/foo:tag1", "oci-archive:/tmp/FOO.tar"
+		imgNameSli := strings.SplitN(imgName, ":", 3)


Don’t use imgName, except maybe for the destination; start with the well-typed srcRef.

oci-archive: paths can include : on Windows.

(Eventually this should get a docker/archive.Reader-like interface similar to the concept of #6811 . But that’s much more work.)

mtrmac · 2020-08-20T12:19:39Z

libpod/image/pull.go

+		// or "oci-archive:/tmp/FOO.tar:domain.com/foo:tag1", "oci-archive:/tmp/FOO.tar"
+		imgNameSli := strings.SplitN(imgName, ":", 3)
+		if len(imgNameSli) == 3 {
+			// Fixes: https://github.com/containers/podman/issues/7337


How does this relate to podman load -i Path-With-Caps label : invalid reference format: repository name must be lowercase #7337 at all, when the previous version did not use the path component of srcRef for dest either AFAICS?

Why is fixing the single-image case enough? Can’t multi-image archives use upper-case names as well?

Still no answer to this.

mtrmac · 2020-08-20T12:22:21Z

libpod/runtime_img.go

@@ -283,14 +283,12 @@ func (r *Runtime) LoadImage(ctx context.Context, name, inputFile string, writer
 			return dockerarchive.ParseReference(inputFile) // FIXME? We should add dockerarchive.NewReference()
 		},
 		func() (types.ImageReference, error) {
-			return ociarchive.NewReference(inputFile, name) // name may be ""


This is incorrect. OCI reference annotations have no semantics other than a set of allowed characters, and it’s perfectly possible fo the annotation in the image to have no slash. It must be possible to refer to such an image.

oci: and oci-archive: should behave consistently; this changes the two in different ways.

I can’t see any relationship to podman load -i Path-With-Caps label : invalid reference format: repository name must be lowercase #7337 either way. One goal per PR, please; or at least one commit per concept; or at the very least write down in the PR description what else the PR does!

mtrmac · 2020-08-20T12:24:48Z

libpod/runtime_img.go

 			}
-			return nil, nil
+			return layout.NewReference(inputFile, iName)


(In both the ociarchive and layout code above:)

AFAICS this is unrelated to podman load -i Path-With-Caps label : invalid reference format: repository name must be lowercase #7337 , and probably belongs to a separate PR.

If the separate NewReference(inputFile, name) attempt remains (as it should), this case should only try prepending DefaultLocalRegistry. It’s a good point that in the "" case that shouldn’t happen, but in that case these functions should return nil, nil, not try to load the archive the second time using the same reference that has already failed.

mtrmac · 2020-08-20T12:29:49Z

pkg/domain/infra/abi/images.go

@@ -452,7 +452,8 @@ func (ir *ImageEngine) Load(ctx context.Context, opts entities.ImageLoadOptions)
 	if !opts.Quiet {
 		writer = os.Stderr
 	}
-	name, err := ir.Libpod.LoadImage(ctx, opts.Name, opts.Input, writer, opts.SignaturePolicy)
+	imgName := opts.Name + ":" + opts.Tag
+	name, err := ir.Libpod.LoadImage(ctx, imgName, opts.Input, writer, opts.SignaturePolicy)


Hum. Paging @mheon @jwhonce

⚠️ What’s the semantics of ImageLoadOptions? We must establish that and document that. (See also podman load/save: support multi-image docker archive #6811 (comment) )

Why is it safe to change this behavior now?

pkg/domain/infra/tunnel.ImageEngine.Load uses Name+Tag in yet another different way. Which one is correct?

edsantiago · 2020-08-20T15:41:12Z

I really don’t understand what this is fixing and how. This is the third PR with, what seems to me, wildly different approach to supposedly the same problem

@mtrmac there is strong precedent for the following to just work:

# podman load -i FOO.tar myname
# podman load -i FOO.tar myname:mytag

ref: existing tests.

This is currently broken when FOO.tar is an oci-archive. As a podman user, I do not wish to care what format a tarball is in, I just want podman load to work as it has always worked. And I don't want to have to worry about upper-case vs lower-case tar file names. Does that address your concern?

mtrmac · 2020-08-20T15:57:55Z

No-one has actually written down how that fails internally. The quoted error code mentioning dir: is of course unrelated, it’s just the one LoadImage happens to report. How does it actually fail, what are we fixing?
If this is related to use of capital letters in file names, and that failing substantially similarly as the dir: case on creating a storage reference with capital letters (is that the case??), AFAICS that just doesn’t really match what the PRs are doing. Nothing in the existing code is using the file name in a Docker-reference context (in particular as the second parameter to getSinglePullRefPairGoal) already.
The PRs somehow hint at ignoring the user-specified name (“myname” above), and using data from the archive instead, but “myname” does not use capital letters.

I’m not disputing that things are broken, and it’s entirely fair to say that I should do my own research; I’m afraid I can’t spend that time this week; and looking at how all over the place the previous PRs have been, I’d like the PR author to show the analysis that has already been done.

(It’s also quite possible I’ve just been an idiot and I’ve been overlooking an entirely obvious problem, and rejecting the clearly correct way to solve it. In that case, showing the existing analysis should hopefully also help.)

zhangguanzhang · 2020-08-21T01:23:51Z

No-one has actually written down how that fails internally. The quoted error code mentioning dir: is of course unrelated, it’s just the one LoadImage happens to report. How does it actually fail, what are we fixing?

If this is related to use of capital letters in file names, and that failing substantially similarly as the dir: case on creating a storage reference with capital letters (is that the case??), AFAICS that just doesn’t really match what the PRs are doing. Nothing in the existing code is using the file name in a Docker-reference context (in particular as the second parameter to getSinglePullRefPairGoal) already.

The PRs somehow hint at ignoring the user-specified name (“myname” above), and using data from the archive instead, but “myname” does not use capital letters.

I’m not disputing that things are broken, and it’s entirely fair to say that I should do my own research; I’m afraid I can’t spend that time this week; and looking at how all over the place the previous PRs have been, I’d like the PR author to show the analysis that has already been done.

(It’s also quite possible I’ve just been an idiot and I’ve been overlooking an entirely obvious problem, and rejecting the clearly correct way to solve it. In that case, showing the existing analysis should hopefully also help.)

podman/libpod/runtime_img.go

Lines 285 to 318 in 516196f

    
           for _, referenceFn := range []func() (types.ImageReference, error){ 
        
           	func() (types.ImageReference, error) { 
        
           		return dockerarchive.ParseReference(inputFile) // FIXME? We should add dockerarchive.NewReference() 
        
           	}, 
        
           	func() (types.ImageReference, error) { 
        
           		return ociarchive.NewReference(inputFile, name) // name may be "" 
        
           	}, 
        
           	func() (types.ImageReference, error) { 
        
           		// prepend "localhost/" to support local image saved with this semantics 
        
           		if !strings.Contains(name, "/") { 
        
           			return ociarchive.NewReference(inputFile, fmt.Sprintf("%s/%s", image.DefaultLocalRegistry, name)) 
        
           		} 
        
           		return nil, nil 
        
           	}, 
        
           	func() (types.ImageReference, error) { 
        
           		return directory.NewReference(inputFile) 
        
           	}, 
        
           	func() (types.ImageReference, error) { 
        
           		return layout.NewReference(inputFile, name) 
        
           	}, 
        
           	func() (types.ImageReference, error) { 
        
           		// prepend "localhost/" to support local image saved with this semantics 
        
           		if !strings.Contains(name, "/") { 
        
           			return layout.NewReference(inputFile, fmt.Sprintf("%s/%s", image.DefaultLocalRegistry, name)) 
        
           		} 
        
           		return nil, nil 
        
           	}, 
        
           } { 
        
           	src, err = referenceFn() 
        
           	if err == nil && src != nil { 
        
           		if newImages, err = r.ImageRuntime().LoadFromArchiveReference(ctx, src, signaturePolicy, writer); err == nil { 
        
           			return getImageNames(newImages), nil 
        
           		} 
        
           	}

When importing images, these methods will be tried from front to back
In fact, when we import the oci-archive format and use name coverage， neither the second method nor the third method passed normally, so the error message is the e thrown by the last method.

When the passed image name is not empty, the final method returns an error
https://github.com/containers/image/blob/22129f19a77890293540e67843f28860f5a77895/oci/layout/oci_transport.go#L180-L215
so this case will return a error
https://github.com/containers/podman/blob/master/libpod/image/pull.go#L178

zhangguanzhang · 2020-08-21T01:26:57Z

and the title of the #7337 is wrong，Actually fix this problem:

$ podman pull alpine
$ podman save --format oci-archive alpine:latest > /tmp/FOO.tar
$ podman load -i /tmp/FOO.tar foo
Error: error pulling "foo": unable to pull dir:/tmp/FOO.tar: error determining pull goal for image "dir:/tmp/FOO.tar": error parsing dest reference name "localhost/tmp/FOO.tar": error parsing named reference "localhost/tmp/FOO.tar": invalid reference format: repository name must be lowercase

mtrmac · 2020-08-22T16:13:01Z

When the passed image name is not empty, the final method returns an error
https://github.com/containers/image/blob/22129f19a77890293540e67843f28860f5a77895/oci/layout/oci_transport.go#L180-L215

Yes… and what’s wrong about that? The ref.image in ociReference was not found in the manifest, and so the load reports an error. Is the error just “foo not found in FOO.tar”?

Are we just back at #6811 (comment) ?

Either the name parameter indicates what to pull, and in that case it should do so for docker-archive:, or it doesn’t, and in that case it should be dropped from this function, changing behavior of all the other transports.

Even hypothetically granting the status quo of different transports behaving in different ways, and as a more limited case: either the name parameter to Runtime.LoadImage for oci-archive: images specifies what image inside the archive to use (and in that case it should be used for all OCI archives, whether single-image or multi-image), or it doesn’t (and in that case it should be ignored for all OCI archives, whether single-image or multi-image. It makes no sense to me to have a parameter that means different things for single-image and multi-image archives , when most of the call stack has no idea which one it is.

rhatdan · 2020-09-11T10:24:21Z

@zhangguanzhang Are you still working on this one?

zhangguanzhang · 2020-09-11T10:43:14Z

@zhangguanzhang Are you still working on this one?
You can discuss this with @QiWang19 , I'm going on vacation

rhatdan · 2020-09-11T11:35:18Z

@zhangguanzhang Thanks enjoy your vacation, I just returned.

rhatdan · 2020-10-01T20:18:18Z

@zhangguanzhang What is going on with this?

zhangguanzhang · 2020-10-01T23:03:40Z

@zhangguanzhang What is going on with this?

@QiWang19 Is there any progress on your side regarding this matter recently?

Signed-off-by: zhangguanzhang <[email protected]>

rhatdan · 2020-10-04T11:53:23Z

/approve

openshift-ci-robot · 2020-10-04T11:53:26Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: rhatdan, zhangguanzhang

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

~~OWNERS~~ [rhatdan]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

mtrmac · 2020-10-05T08:59:11Z

libpod/image/pull.go

@@ -201,6 +201,24 @@ func (ir *Runtime) pullGoalFromImageReference(ctx context.Context, srcRef types.
 		}, nil

 	case OCIArchive:
+		// imgName such as "/tmp/FOO.tar:" or "/tmp/FOO.tar:domain.com/foo:tag1"
+		imgName := strings.TrimSuffix(srcRef.StringWithinTransport(), ":")


What does the TrimSuffix do?

mtrmac · 2020-10-05T08:59:48Z

libpod/image/pull.go

+		// or "oci-archive:/tmp/FOO.tar:domain.com/foo:tag1", "oci-archive:/tmp/FOO.tar"
+		imgNameSli := strings.SplitN(imgName, ":", 3)
+		if len(imgNameSli) == 3 {
+			// Fixes: https://github.com/containers/podman/issues/7337


Still no answer to this.

mtrmac

Even hypothetically granting the status quo of different transports behaving in different ways, and as a more limited case: either the name parameter to Runtime.LoadImage for oci-archive: images specifies what image inside the archive to use (and in that case it should be used for all OCI archives, whether single-image or multi-image), or it doesn’t (and in that case it should be ignored for all OCI archives, whether single-image or multi-image. It makes no sense to me to have a parameter that means different things for single-image and multi-image archives , when most of the call stack has no idea which one it is.

Please make a clear record for the future:

As a part of the code base:
- End-user documentation: what is the goal / intended behavior of the feature
- Programmer documentation: What is the semantics of the involved functions / data structures.
- (If it is not obvious, which should not be the case after the above:) How the implementation works and matches the function contracts.
As part of this PR / process:
- What is failing. Was the previous goal of the code incorrect? Or just incorrectly developed?
  - (podman load -i Path-With-Caps label : invalid reference format: repository name must be lowercase #7337 only lists an error reported by the dir: transport. If this is focusing on oci-archive:, please write down what is the problem encountered when using oci-archive: that this is supposed to be fixing. So far I’m just guessing based on the the very different PRs and I can’t see any connection to upper-case file names.)
- How is this PR fixing this

zhangguanzhang · 2020-10-05T11:02:37Z

Even hypothetically granting the status quo of different transports behaving in different ways, and as a more limited case: either the name parameter to Runtime.LoadImage for oci-archive: images specifies what image inside the archive to use (and in that case it should be used for all OCI archives, whether single-image or multi-image), or it doesn’t (and in that case it should be ignored for all OCI archives, whether single-image or multi-image. It makes no sense to me to have a parameter that means different things for single-image and multi-image archives , when most of the call stack has no idea which one it is.

Please make a clear record for the future:

As a part of the code base:

End-user documentation: what is the goal / intended behavior of the feature

Programmer documentation: What is the semantics of the involved functions / data structures.

(If it is not obvious, which should not be the case after the above:) How the implementation works and matches the function contracts.

As part of this PR / process:

What is failing. Was the previous goal of the code incorrect? Or just incorrectly developed?

(podman load -i Path-With-Caps label : invalid reference format: repository name must be lowercase #7337 only lists an error reported by the dir: transport. If this is focusing on oci-archive:, please write down what is the problem encountered when using oci-archive: that this is supposed to be fixing. So far I’m just guessing based on the the very different PRs and I can’t see any connection to upper-case file names.)

How is this PR fixing this

Is this feature needed or not planned to support this feature?

mtrmac · 2020-10-05T11:12:09Z

Is this feature needed or not planned to support this feature?

Which “this”?

zhangguanzhang · 2020-10-05T11:34:30Z

Is this feature needed or not planned to support this feature?

Which “this”?

podman load -i /tmp/FOO.tar foo:testTag

QiWang19 · 2020-10-05T13:54:08Z

#7337 seems not caused by Path-With-Caps. The issue can be regenerated even with a path using lower cases.
@zhangguanzhang Are you trying to fix the [name:tag]` inconsistencies problem #7387? From the comments there the optional argument is going to be dropped in Podman v3.0. I have no idea what is a good fix for now.
@mtrmac @vrothberg WDTY?

mtrmac · 2020-10-05T14:31:41Z

@zhangguanzhang Are you trying to fix the [name:tag]` inconsistencies problem #7387? From the comments there the optional argument is going to be dropped in Podman v3.0. I have no idea what is a good fix for now.
@mtrmac @vrothberg WDTY?

For the name inconsistencies, I guess if no-one makes an executive decision one way or the other to use one of the semantics and possibly break users, we can always at least document the current behavior (both for the API and the CLI), and document both the behavior and that it is deprecated. In general I don’t think that changing small aspects of the inconsistent behavior would be materially better, as long as an inconsistency remains.

OTOH… doesn’t some of the API come directly from Docker? At least that one should have clearer semantics, giving us a specific target that must work. That might involve actually implementing that in a reliable way (a nameToSearchForInArchive / nameToTagTheResultWithLocally parameter that does exactly what the API means), and we could preserve the current mess for CLI users (legacyNameParameterWithConfusingSemantics) — or it might be just enough to push us to implement only the Docker API-compatible behavior and break the other one.

openshift-ci-robot requested review from baude and umohnani8 August 20, 2020 06:26

zhangguanzhang force-pushed the podman-load-oci-with-name branch 6 times, most recently from 9d0fb2d to 6013880 Compare August 20, 2020 10:01

edsantiago reviewed Aug 20, 2020

View reviewed changes

mtrmac requested changes Aug 20, 2020

View reviewed changes

zhangguanzhang force-pushed the podman-load-oci-with-name branch 3 times, most recently from f893afc to 483ccbc Compare August 20, 2020 14:27

zhangguanzhang requested a review from edsantiago August 22, 2020 15:56

mtrmac mentioned this pull request Aug 28, 2020

Define an error to allow an external reference containers/image#1029

Closed

QiWang19 self-assigned this Sep 11, 2020

fix podman load oci-archive's tar with name

13b36ee

Signed-off-by: zhangguanzhang <[email protected]>

zhangguanzhang force-pushed the podman-load-oci-with-name branch from 483ccbc to 13b36ee Compare October 4, 2020 06:47

openshift-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Oct 4, 2020

mtrmac reviewed Oct 5, 2020

View reviewed changes

mtrmac requested changes Oct 5, 2020

View reviewed changes

zhangguanzhang closed this Oct 18, 2020

QiWang19 mentioned this pull request Oct 27, 2020

Drop name argument from Load API #8112

Merged

github-actions bot added the locked - please file new issue/PR Assist humans wanting to comment on an old issue or PR with locked comments. label Sep 24, 2023

github-actions bot locked as resolved and limited conversation to collaborators Sep 24, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix podman load oci-archive's tar with name #7386

fix podman load oci-archive's tar with name #7386

zhangguanzhang commented Aug 20, 2020 •

edited

Loading

rhatdan commented Aug 20, 2020

edsantiago left a comment

edsantiago Aug 20, 2020

zhangguanzhang Aug 20, 2020

zhangguanzhang commented Aug 20, 2020

mtrmac left a comment •

edited

Loading

mtrmac Aug 20, 2020

mtrmac Aug 20, 2020

mtrmac Oct 5, 2020

mtrmac Aug 20, 2020

mtrmac Aug 20, 2020

mtrmac Aug 20, 2020

edsantiago commented Aug 20, 2020

mtrmac commented Aug 20, 2020

zhangguanzhang commented Aug 21, 2020

zhangguanzhang commented Aug 21, 2020

mtrmac commented Aug 22, 2020

rhatdan commented Sep 11, 2020

zhangguanzhang commented Sep 11, 2020

rhatdan commented Sep 11, 2020

rhatdan commented Oct 1, 2020

zhangguanzhang commented Oct 1, 2020

rhatdan commented Oct 4, 2020

openshift-ci-robot commented Oct 4, 2020

mtrmac Oct 5, 2020

mtrmac Oct 5, 2020

mtrmac left a comment

zhangguanzhang commented Oct 5, 2020

mtrmac commented Oct 5, 2020

zhangguanzhang commented Oct 5, 2020 •

edited

Loading

QiWang19 commented Oct 5, 2020

mtrmac commented Oct 5, 2020

fix podman load oci-archive's tar with name #7386

fix podman load oci-archive's tar with name #7386

Conversation

zhangguanzhang commented Aug 20, 2020 • edited Loading

rhatdan commented Aug 20, 2020

edsantiago left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

zhangguanzhang commented Aug 20, 2020

mtrmac left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

edsantiago commented Aug 20, 2020

mtrmac commented Aug 20, 2020

zhangguanzhang commented Aug 21, 2020

zhangguanzhang commented Aug 21, 2020

mtrmac commented Aug 22, 2020

rhatdan commented Sep 11, 2020

zhangguanzhang commented Sep 11, 2020

rhatdan commented Sep 11, 2020

rhatdan commented Oct 1, 2020

zhangguanzhang commented Oct 1, 2020

rhatdan commented Oct 4, 2020

openshift-ci-robot commented Oct 4, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mtrmac left a comment

Choose a reason for hiding this comment

zhangguanzhang commented Oct 5, 2020

mtrmac commented Oct 5, 2020

zhangguanzhang commented Oct 5, 2020 • edited Loading

QiWang19 commented Oct 5, 2020

mtrmac commented Oct 5, 2020

zhangguanzhang commented Aug 20, 2020 •

edited

Loading

mtrmac left a comment •

edited

Loading

zhangguanzhang commented Oct 5, 2020 •

edited

Loading