Add --insecure flag to podman manifest inspect for Docker compatibility #15359

rhatdan · 2022-08-17T14:28:19Z

Helps fix: #14917

Signed-off-by: Daniel J Walsh dwalsh@redhat.com

Does this PR introduce a user-facing change?

podman manifest inspect now supports --insecure flag for Docker compatibility

edsantiago · 2022-08-17T14:37:09Z

test/system/012-manifest.bats

+
+    run_podman manifest create --insecure test:1.0
+    run_podman images --format '{{.ID}}' --no-trunc
+    [[ "$output" == *"sha256:$iid"* ]]


Aaaaaaaahhhhhh! Evil! I must never have reviewed the initial commit. Please fix this to

assert "$output" =~ "sha256:$iid" "Original image ID still shown in podman-images output"

edsantiago · 2022-08-17T15:59:55Z

test/system/012-manifest.bats

+
+    run_podman manifest create test:1.0
+    run_podman manifest inspect --insecure $output
+    is "$output" ".*\"mediaType\": \"application/vnd.docker.distribution.manifest.list.v2+json\"" "Original image ID still shown in podman-images output"


Nice addition, but comment ("Original image ID...") is misleading. Probably not worth the CI cost to re-push, but if you do, could you also add a comment something like "# --insecure is a NOP, we use it here just to make sure podman accepts it"

cmd/podman/manifest/inspect.go

nalind · 2022-08-25T16:04:24Z

pkg/domain/infra/tunnel/manifest.go

@@ -33,8 +33,17 @@ func (ir *ImageEngine) ManifestExists(ctx context.Context, name string) (*entiti
 }

 // ManifestInspect returns contents of manifest list with given name
-func (ir *ImageEngine) ManifestInspect(_ context.Context, name string) ([]byte, error) {
-	list, err := manifests.Inspect(ir.ClientCtx, name, nil)
+func (ir *ImageEngine) ManifestInspect(ctx context.Context, name string, opts entities.ImageSearchOptions) ([]byte, error) {


Why is this reusing a different method's options structure instead of rolling its own, as most of the others in this file that take options structures do?

nalind · 2022-09-07T21:06:27Z

test/system/012-manifest.bats

+    iid=$output
+
+    run_podman manifest create test:1.0
+    run_podman manifest inspect --insecure $output


$output looks like it'll be the local manifest's ID, which I wouldn't expect us to ask a registry about. Is this testing the right thing?

I was just checking that --insecure option would work, not testing against an insecure registry. Do we have an insecure registry to test against?

The podman login tests fire up a registry container with TLS configured. If we did that here, so long as the client isn't configured to trust the CA certificate (HTTPS, but not verifiable), or there is no certificate (plain old HTTP), a client will have to have a working --insecure flag in order to be able to talk to it at all. But we'd have to be looking for an image using a name rather than an ID.

github-actions · 2022-10-09T00:10:36Z

A friendly reminder that this PR had no activity for 30 days.

rhatdan · 2022-11-01T19:33:35Z

@containers/podman-maintainers PTAL

rhatdan · 2022-11-01T19:34:02Z

@vrothberg @mheon @edsantiago PTAL

edsantiago

Ugh, looks like I need to write a new CI check. For now, can you please:

git rm docs/source/markdown/podman-manifest-instpect.1.md
manually add podman-manifest-inspect.1.md to the file docs/source/markdown/.gitignore (preferably in sorted order)
re-push

Otherwise LGTM, although all I reviewed was docs & tests.

--insecure and --verbose flags for docker compatibility --tls-verify for syntax compatibility and allow users to inspect manifests at remote Container Registiries without requiring tls. Helps fix: containers#14917 Signed-off-by: Daniel J Walsh <dwalsh@redhat.com>

edsantiago · 2022-11-03T21:46:05Z

CI is finally green (lots o' flakes). LGTM but this needs more approval than just mine.

vrothberg

/lgtm

openshift-ci · 2022-11-04T08:47:27Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: edsantiago, rhatdan, vrothberg

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~~ [edsantiago,rhatdan,vrothberg]

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

edsantiago reviewed Aug 17, 2022

View reviewed changes

rhatdan force-pushed the manifest branch from 3e2fb17 to 3cf22fe Compare August 17, 2022 15:02

edsantiago reviewed Aug 17, 2022

View reviewed changes

mheon reviewed Aug 17, 2022

View reviewed changes

cmd/podman/manifest/inspect.go Show resolved Hide resolved

rhatdan force-pushed the manifest branch from 3cf22fe to 92a9341 Compare August 24, 2022 21:05

nalind reviewed Aug 25, 2022

View reviewed changes

rhatdan force-pushed the manifest branch from 92a9341 to 06aea95 Compare September 7, 2022 20:58

nalind reviewed Sep 7, 2022

View reviewed changes

github-actions bot added the stale-pr label Oct 9, 2022

rhatdan removed the stale-pr label Oct 10, 2022

rhatdan force-pushed the manifest branch 2 times, most recently from 51a77d5 to 0bf64b1 Compare October 31, 2022 14:15

github-actions bot added the kind/api-change Change to remote API; merits scrutiny label Oct 31, 2022

rhatdan force-pushed the manifest branch 5 times, most recently from 798a420 to adc69a8 Compare November 1, 2022 17:48

edsantiago requested changes Nov 1, 2022

View reviewed changes

rhatdan force-pushed the manifest branch from adc69a8 to 1a05b9d Compare November 1, 2022 23:20

rhatdan force-pushed the manifest branch from 1a05b9d to 274d0f4 Compare November 3, 2022 12:00

edsantiago approved these changes Nov 3, 2022

View reviewed changes

vrothberg approved these changes Nov 4, 2022

View reviewed changes

openshift-ci bot assigned vrothberg Nov 4, 2022

openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Nov 4, 2022

openshift-merge-robot merged commit 0702b4c into containers:main Nov 4, 2022

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 20, 2023

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

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add --insecure flag to podman manifest inspect for Docker compatibility #15359

Add --insecure flag to podman manifest inspect for Docker compatibility #15359

rhatdan commented Aug 17, 2022 •

edited

edsantiago Aug 17, 2022

edsantiago Aug 17, 2022

nalind Aug 25, 2022

rhatdan Aug 26, 2022 •

edited

nalind Sep 7, 2022

rhatdan Sep 8, 2022

nalind Sep 8, 2022

github-actions bot commented Oct 9, 2022

rhatdan commented Nov 1, 2022

rhatdan commented Nov 1, 2022

edsantiago left a comment

edsantiago commented Nov 3, 2022

vrothberg left a comment

openshift-ci bot commented Nov 4, 2022

Add --insecure flag to podman manifest inspect for Docker compatibility #15359

Add --insecure flag to podman manifest inspect for Docker compatibility #15359

Conversation

rhatdan commented Aug 17, 2022 • edited

Does this PR introduce a user-facing change?

edsantiago Aug 17, 2022

Choose a reason for hiding this comment

edsantiago Aug 17, 2022

Choose a reason for hiding this comment

nalind Aug 25, 2022

Choose a reason for hiding this comment

rhatdan Aug 26, 2022 • edited

Choose a reason for hiding this comment

nalind Sep 7, 2022

Choose a reason for hiding this comment

rhatdan Sep 8, 2022

Choose a reason for hiding this comment

nalind Sep 8, 2022

Choose a reason for hiding this comment

github-actions bot commented Oct 9, 2022

rhatdan commented Nov 1, 2022

rhatdan commented Nov 1, 2022

edsantiago left a comment

Choose a reason for hiding this comment

edsantiago commented Nov 3, 2022

vrothberg left a comment

Choose a reason for hiding this comment

openshift-ci bot commented Nov 4, 2022

rhatdan commented Aug 17, 2022 •

edited

rhatdan Aug 26, 2022 •

edited