Only open save output file with WRONLY

[rhatdan]

The previous code fails on a MAC when opening /dev/stdout

Fixes: #12402

[NO NEW TESTS NEEDED] No easy way to test this.

Signed-off-by: Daniel J Walsh [email protected]

What this PR does / why we need it:

How to verify it

Which issue(s) this PR fixes:

Special notes for your reviewer:

[openshift-ci]

[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:

• OWNERS [rhatdan]

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

[rhatdan]

@vrothberg @mtrmac @xandris PTAL

[mtrmac]

No, it’s the Create above that needs to be changed AFAICS.

(changing that Open(, which is only used to read data for, O_WRONLY can’t work anyway, AFAICS.)

---

Please have someone with a working Podman on Mac to test this, as well…

[rhatdan]

@mtrmac Better.

[mtrmac]

Yes, this is what I think is going wrong and how to fix it, but please wait for a confirmation from someone who has Podman remote actually set up.

[rhatdan]

I think this is only podman on a MAC too, correct. I could not get this to fail on Linux.

[mtrmac]

It might be possible to reproduce on Linux using something like

touch output;
chmod 0200 output # u=w,u-r
podman save $other-args -o output # Must be podman-remote!

where the -o output is new, to prevent triggering the (Linux-only)

podman/cmd/podman/images/save.go

Line 115 in 7324d94

pipePath, cleanup, err := setupPipe()

code. But that’s just reading the code, I could easily be missing something.

[xandris]

Works for me. I checked out this branch, built it, and ran it on my Mac, comparing the behavior to the Homebrew-installed version:

❯ ./bin/darwin/podman save centos:7 | sha256sum
08129aca6f66e3aa07368ffb8576e6612b67bd1ca624a7fd155d03aa9608ee8a  -
❯ podman save centos:7 | sha256sum
Error: open /dev/stdout: permission denied

[Luap99]

Can we add a comment here that this mode is important.

[rhatdan]

Sure, could you explain to me why? Or what should be in the comment?

   default:
	// This code was added to allow for opening stdout use on MAC. The mode 0644 is important.
	f, err = os.OpenFile(opts.Output, os.O_WRONLY|os.O_CREATE|os.O_TRUNC, 0644)
}

[Luap99]

I mean the flags os.O_WRONLY. If someone has to rework this they might not think about it and change it back to os.Create() for example.
Maybe adding a link to the issue would help

[rhatdan]

Ok, I will write that.

[vrothberg]

LGTM, nice work!

[Luap99]

/lgtm