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

Add hidden trust command to alias existing trust commands #16463

Closed
wants to merge 1 commit into from

Conversation

rhatdan
Copy link
Member

@rhatdan rhatdan commented Nov 9, 2022

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

Does this PR introduce a user-facing change?

podman trust hidden command added for docker compatibility.

@openshift-ci openshift-ci bot added the do-not-merge/release-note-label-needed Enforce release-note requirement, even if just None label Nov 9, 2022
@openshift-ci
Copy link
Contributor

openshift-ci bot commented Nov 9, 2022

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: rhatdan

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:

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

@openshift-ci openshift-ci bot added approved Indicates a PR has been approved by an approver from all required OWNERS files. release-note and removed do-not-merge/release-note-label-needed Enforce release-note requirement, even if just None labels Nov 9, 2022
Copy link
Member

@Luap99 Luap99 left a comment

Choose a reason for hiding this comment

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

I don't see how this helps.
If you actually care for compatibility you need to match the actual commands with their input and output.

Per https://docs.docker.com/engine/reference/commandline/trust/
there is the inspect, key, revoke, sign, signer command, but no set or show which you add here.

@rhatdan
Copy link
Member Author

rhatdan commented Nov 9, 2022

It makes it more discoverable. We can look at fixing up the others. But for now users who do podman trust see no failure.

@Luap99
Copy link
Member

Luap99 commented Nov 10, 2022

It makes it more discoverable. We can look at fixing up the others. But for now users who do podman trust see no failure.

I disagree, nobody uses podman trust. This is just a stub command to attach subcommands, if you want to be docker compatible you actually need to implement the correct subcommands. As of now you add podman trust set/show which do not exists on docker, so what is the point?

@rhatdan
Copy link
Member Author

rhatdan commented Nov 11, 2022

Let's discuss at the next standup, and get consensus. My goal was to allow users to discover how Podman defines trust.

podman trust
Error: unrecognized command `podman trust`
Try 'podman --help' for more information

Versus

 $ ./bin/podman trust
Manage container image trust policy

Description:
  Manages which registries you trust as a source of container images based on their location.
  The location is determined by the transport and the registry host of the image.  Using this container image docker://quay.io/podman/stable as an example, docker is the transport and quay.io is the registry host.

Usage:
  podman trust [command]

Available Commands:
  set         Set default trust policy or a new trust policy for a registry
  show        Display trust policy for the system

Error: missing command 'podman trust COMMAND'

Signed-off-by: Daniel J Walsh <dwalsh@redhat.com>
@vrothberg
Copy link
Member

For the record: consensus was to not create an alias but return a meaningful error to users that would direct them to image trust.

@vrothberg
Copy link
Member

Friendly ping. @rhatdan do you want to kick it over the finish line?

@github-actions
Copy link

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

@github-actions
Copy link

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

@vrothberg
Copy link
Member

Friendly ping, @rhatdan :^)

@rhatdan
Copy link
Member Author

rhatdan commented Apr 4, 2023

Closing for now, since I have no time to work on it, and it would be complicated to implement.

@rhatdan rhatdan closed this Apr 4, 2023
@github-actions 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 3, 2023
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Sep 3, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. locked - please file new issue/PR Assist humans wanting to comment on an old issue or PR with locked comments. release-note
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants