Buildah bud should not use stdin by default

[rhatdan]

Fixes: containers/podman#8066
Is reporting that buildah is eating stdin. I don't beleive
we should be using stdin when doing a buildah bud command
unless buildah bud - is specified. After this PR, the
- Dockerfile is still handled.

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

What type of PR is this?

/kind api-change
/kind bug
/kind cleanup
/kind deprecation
/kind design
/kind documentation
/kind failing-test
/kind feature
/kind flake
/kind other

What this PR does / why we need it:

How to verify it

Which issue(s) this PR fixes:

Special notes for your reviewer:

Does this PR introduce a user-facing change?

[openshift-ci-robot]

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

@nalind PTAL

[rhatdan]

Fixes: #2705

[nalind]

LGTM, though this does add a difference between how we handle RUN instructions and buildah run.

[rhatdan]

I think that is ok. The question I have is should we add an --interactive mode to docker build to add stdin back in.
I know where have an issue somewhere talking about how buildah bud or podman build hung waiting for input from apt-update.
If we added -i flag then dnf update and other commands could wait for user input.

[edsantiago]

LGTM. Could you add this test, or something equivalent?

@test "bud does not gobble stdin" {
  _prefetch alpine

  ctxdir=${TESTDIR}/bud
  mkdir -p $ctxdir
  cat >$ctxdir/Dockerfile <<EOF
FROM alpine
RUN true
EOF

  random_msg=$(head -10 /dev/urandom | tr -dc a-zA-Z0-9 | head -c12)

  # Prior to #2708, buildah bud would gobble up its stdin even if it
  # didn't actually use it. This prevented the use of 'cmdlist | bash';
  # if 'buildah bud' was in cmdlist, everything past it would be lost.
  #
  # This is ugly but effective: it checks that buildah passes stdin untouched.
  passthru=$(echo "$random_msg" | (run_buildah bud --quiet --signature-policy ${TESTSDIR}/policy.json -t stdin-test ${ctxdir} >/dev/null; cat))

  expect_output --from="$passthru" "$random_msg" "stdin was passed through"
}

[edsantiago]

P.S. the test (as expected) fails on current master, passes with this PR.

[rhatdan]

@TomSweeneyRedHat @edsantiago @vrothberg @giuseppe @ashley-cui This is ready to go in. Merge me.

[edsantiago]

/lgtm
/hold

[rhatdan]

/hold cancel

[hansborr]

I think that is ok. The question I have is should we add an --interactive mode to docker build to add stdin back in.
I know where have an issue somewhere talking about how buildah bud or podman build hung waiting for input from apt-update.
If we added -i flag then dnf update and other commands could wait for user input.

I would like to see this added.

This change broke something for me -- I used to have a dockerfile which prompted for user input during the RUN stage (running smbpasswd).
This was to avoid having a cleartext password in the file, but to allow the hashed password to be baked into the image.

After updating to the newest buildah, that no longer works.

I was able to work around it, but I expect there are probably other people who will encounter this.

[rhatdan]

Interested in opening a PR, it is actually a fairly easy change to add an option for this.

BTW, Please open an issue for this anyways, rather then in this closed PR.