-
Notifications
You must be signed in to change notification settings - Fork 2.4k
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
manifest, push: use source
as destination
if not specified
#18395
manifest, push: use source
as destination
if not specified
#18395
Conversation
`manifest push <source>` must work as-is if source is actually a valid path and no destination is provided, buildah must internally choose source as its destination just like `podman push`. PR is similar to: containers/podman#18395 Signed-off-by: Aditya R <[email protected]>
1558a1d
to
711ca62
Compare
`manifest push <source>` must work as-is if `source` is actually a valid path and no destination is provided, `podman` must internally choose `source` as its `destination` just like `podman push` See: https://github.com/containers/podman/blob/main/cmd/podman/images/push.go#L161 Closes: containers#18360 Signed-off-by: Aditya R <[email protected]>
711ca62
to
bab4217
Compare
@@ -114,7 +114,7 @@ func push(cmd *cobra.Command, args []string) error { | |||
return err | |||
} | |||
listImageSpec := args[0] | |||
destSpec := args[1] | |||
destSpec := args[len(args)-1] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
@containers/podman-maintainers PTAL |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
/lgtm
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: flouthoc, 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:
Approvers can indicate their approval by writing |
Expect(session).To(ExitWithError()) | ||
// Push should actually fail since its not valid registry | ||
Expect(session.ErrorToString()).To(ContainSubstring("requested access to the resource is denied")) | ||
Expect(session.OutputToString()).To(Not(ContainSubstring("accepts 2 arg(s), received 1"))) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this NOT check is pointless, you would need to match ErrorToString
to string in order to actually check what you are trying to do. General speaking given that you already check for the error message above you could just drop this check.
`manifest push <source>` must work as-is if source is actually a valid path and no destination is provided, buildah must internally choose source as its destination just like `podman push`. PR is similar to: containers/podman#18395 Signed-off-by: Aditya R <[email protected]>
`manifest push <source>` must work as-is if source is actually a valid path and no destination is provided, buildah must internally choose source as its destination just like `podman push`. PR is similar to: containers/podman#18395 Signed-off-by: Aditya R <[email protected]>
manifest push <source>
must work as-is ifsource
is actually a valid path and no destination is provided,podman
must internally choosesource
as itsdestination
just likepodman push
See: https://github.com/containers/podman/blob/main/cmd/podman/images/push.go#L161
Closes: #18360
Does this PR introduce a user-facing change?