carvel-dev / kapp

kapp is a simple deployment tool focused on the concept of "Kubernetes application" — a set of resources with the same label
https://carvel.dev/kapp
Apache License 2.0
931 stars 110 forks source link

kapp reads confirmation prompt from stdin not tty #1003

Open ringerc opened 3 months ago

ringerc commented 3 months ago

What steps did you take:

simplified example (real case uses a json input not jq --null-input):

#!/bin/bash -e -u -o pipefail
jq -r --null-input '["app1","app2"]|.[]' | while read -r app; do
  kapp deploy -a "$app" -f "manifests/$app.yaml"
done

What happened:

Op:      ...
Wait to: ...

Continue? [yN]: app1
invalid input (not y, n, yes, or no)
Continue? [yN]: app2
invalid input (not y, n, yes, or no)

What did you expect:

kapp to prompt for confirmation and read input from the tty since --yes was not passed.

Anything else you would like to add:

The bug here is that kapp tries to read interactive confirmation from stdin. That's rarely correct; the user may

kustomize build foo | kapp deploy -f -`

for example. But within the while loop, stdin points to the output of the jq command. kapp wasn't given -f - or anything so it isn't expected to read from stdin.

It should do what sudo and other tools do - detect an interactive tty and read directly from the tty, rather than stdin.

Workaround

To work around the bug, dup the stdin fd before you enter the loop or pass the pipe, e.g.

#!/bin/bash -e -u -o pipefail
# save stdin fd, so we can tell kapp to use it for prompting
exec 3<&0
jq -r --null-input '["app1","app2"]|.[]' | while read -r app; do
  # pass kapp the original stdin
  kapp deploy -a "$app" -f "manifests/$app.yaml" <&3
done

or

#!/bin/bash -e -u -o pipefail
exec 3<&0
kustomize build foo | kapp deploy -a myapp -f -

Environment:


Vote on this request

This is an invitation to the community to vote on issues, to help us prioritize our backlog. Use the "smiley face" up to the right of this comment to vote.

👍 "I would like to see this addressed as soon as possible" 👎 "There are other more important things to focus on right now"

We are also happy to receive and review Pull Requests if you want to help working on this issue.

praveenrewar commented 2 months ago

@ringerc Thanks for creating the issue. In general, folks just pass -y when they want to pipe the output from some other tool to kapp or when they want to deploy multiple apps. I agree that kapp should be reading the confirmation from tty, so I am marking it as accepted. I am not sure when the maintainers will be able to pick this up, however we would be happy to review if you would like to contribute a PR.

praveenrewar commented 2 months ago

Also, you might find kapp app-group deploy interesting if you are trying to deploy multiple apps based on folder structure.

ringerc commented 2 months ago

Thanks for the tip