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

kapp reads confirmation prompt from stdin not tty #1003

Open
ringerc opened this issue Aug 15, 2024 · 3 comments
Open

kapp reads confirmation prompt from stdin not tty #1003

ringerc opened this issue Aug 15, 2024 · 3 comments
Labels
bug This issue describes a defect or unexpected behavior carvel accepted This issue should be considered for future work and that the triage process has been completed

Comments

@ringerc
Copy link

ringerc commented Aug 15, 2024

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:

  • kapp version 0.63.2
  • Ubuntu 24.04 LTS
  • Kubernetes Server Version: v1.29.2

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.

@ringerc ringerc added bug This issue describes a defect or unexpected behavior carvel triage This issue has not yet been reviewed for validity labels Aug 15, 2024
@renuy renuy added the discussion This issue is not a bug or feature and a conversation is needed to find an appropriate resolution label Aug 30, 2024
@renuy renuy moved this to To Triage in Carvel Aug 30, 2024
@renuy renuy removed the carvel triage This issue has not yet been reviewed for validity label Aug 30, 2024
@praveenrewar praveenrewar added carvel accepted This issue should be considered for future work and that the triage process has been completed and removed discussion This issue is not a bug or feature and a conversation is needed to find an appropriate resolution labels Sep 5, 2024
@praveenrewar
Copy link
Member

@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
Copy link
Member

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

@ringerc
Copy link
Author

ringerc commented Sep 12, 2024

Thanks for the tip

@renuy renuy moved this from To Triage to Unprioritized in Carvel Sep 20, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug This issue describes a defect or unexpected behavior carvel accepted This issue should be considered for future work and that the triage process has been completed
Projects
Status: Unprioritized
Development

No branches or pull requests

3 participants