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

bash subshell file descriptors produce nothing #894

Closed
richard-fairthorne opened this issue Feb 18, 2024 · 11 comments
Closed

bash subshell file descriptors produce nothing #894

richard-fairthorne opened this issue Feb 18, 2024 · 11 comments
Assignees
Labels
discussion This issue is not a bug or feature and a conversation is needed to find an appropriate resolution

Comments

@richard-fairthorne
Copy link

What steps did you take:
[A clear and concise description steps and yaml files that can be used to reproduce the problem.]

ytt -f <(echo hello: world)

What happened:
[A small description of the result or the yaml that was generated]

Nothing was produced.

What did you expect:
[A description of what was expected and the yaml that should have been generated]

I expected:

hello: world

Anything else you would like to add:
[Additional information that will assist in solving the issue.]

cat <(echo hello:world) results in hello: world as expected.

Environment:

  • ytt version (use ytt --version): ytt version 0.48.0
  • OS (e.g. from /etc/os-release): Darwin RichBook.local 23.0.0 Darwin Kernel Version 23.0.0: Fri Sep 15 14:41:43 PDT 2023; root:xnu-10002.1.13~1/RELEASE_ARM64_T6000 arm64

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.

@richard-fairthorne richard-fairthorne added bug This issue describes a defect or unexpected behavior carvel triage This issue has not yet been triaged for relevance labels Feb 18, 2024
@prembhaskal
Copy link
Contributor

i checked and confirm ytt does not work that way in bash(GNU bash, version 5.2.15(1)-release (aarch64-apple-darwin22.1.0)) and zsh(zsh 5.9 (x86_64-apple-darwin23.0)).
We have documented that kapp works this way, I am not sure ytt should work this way, I will check and confirm.

@richard-fairthorne
Copy link
Author

We have documented that kapp works this way, I am not sure ytt should work this way, I will check and confirm.

Thanks. Bash turns that expression into a valid file name. In other words, it is a valid file name. Every program which accepts a file name should work with the <( cmd ) syntax in bash or compatible shells.

@prembhaskal
Copy link
Contributor

here https://github.com/carvel-dev/ytt/blob/develop/pkg/files/file.go#L273 I see an option to read from named pipe, so i think this is a bug.

@prembhaskal prembhaskal added carvel accepted This issue should be considered for future work and that the triage process has been completed can be replicated A bug that has been reproduced and removed carvel triage This issue has not yet been triaged for relevance labels Feb 22, 2024
@prembhaskal prembhaskal self-assigned this Feb 22, 2024
@richard-fairthorne
Copy link
Author

richard-fairthorne commented Feb 22, 2024

Thank you. I'm not 100% sure, but I think that changing != to == will fix it. Why negate the result of the comparison?

The same should apply to the isSymLink line.

@prembhaskal
Copy link
Contributor

The condition is correct. if namedpipe bit is set in fileinfo, its bitwise AND with namedpipemode should be non zero.

@prembhaskal
Copy link
Contributor

also feel free to submit a PR if you figure out a fix.

@richard-fairthorne
Copy link
Author

The condition is correct. if namedpipe bit is set in fileinfo, its bitwise AND with namedpipemode should be non zero.

Of course you're correct. I had to do a double take on that one. I'm on another project now, but when I get back to the one which is relying on YTT, if it's not fixed yet, i'll submit a PR.

@hoegaarden
Copy link

hoegaarden commented Feb 24, 2024

TL,DR: ytt -f test.yaml=<(echo 'foo: bar') seems to work as expected.

The thing is, that the process substitution gives you a file name like /dev/fd/63 (at least on linux). ytt treats this as "data" (see file marks), as this file does not have a filename extension which would mark it differently.

But with the above shown approach you can give the thing a name, and thus a filename extension, which let's ytt know you want this thing to be interpreted as "yaml-template".

BTW: there's some sort of a special behavior when you have ytt consume StdIn. When you tell ytt to read from -, then ytt creates a file called stdin.yaml (see here), however if you tell ytt to read from e.g. /dev/stdin that does not happen, the file does not have an extension which would mark it as "yaml-template" and thus treats it as "data".
Example:

: echo 'foo: bar' | ytt -f /dev/stdin
: 
: echo 'foo: bar' | ytt -f -
foo: bar
:

Some other maybe interesting bits:

Same thing also happens with named pipes; e.g. when you do a mkfifo /tmp/somePipe vs. mkfifo /tmp/somePipe.yaml and consume those two with ytt, the former would be interpreted as "data", the later as "yaml-template".

The code comment suggests, that process substitution results in a pipe, however over here1 it actually results in a symlink to a pipe. I have to assume that e.g. on Darwin and others, process substitution might end up in being represented as a pipe (without symlink).

"external symlinks" are not allowed by default, however symlinks to pipes in /dev/$PID/fd are allowed -- but only on Linux (I don't think Darwin et al have a /proc filesystem and thus would not produce this "special error"). Therefore it might also be the case, that on other systems than linux, if they do use symlinks for process substitution, one might need to allow "external symlinks" with --allow-symlink-destination .... I currently only have Linux boxen available, so can't really test that.

If that is true, that's a bit odd, because this would mean that ytt behaves slightly different on Linux than on other systems.

You could also explicitly tell ytt which file mark you want to use for your process substitutions, e.g. ytt -f <(echo 'foo: bar') --file-mark '63:type=yaml-plain', but I don't think you have any guarantees on the names of the pipes/links.

And lastly, I found --debug to be very helpful in understanding which files ytt considers and how it interprets those.

So all in all, I think:

  • ytt works correctly and does read process substitutions (again, only able to verify that on Linux)
  • there might be slightly different behavior for unnamed pipes in Linux and others
  • there could maybe be better docs on how to use process substitutions/anonymous pipes, named pipes, even StdIn (- vs. /dev/stdin) and other "special" things

Footnotes

  1. Linux & bash 5.2.15 & ytt 0.48.0

@richard-fairthorne
Copy link
Author

TL,DR: ytt -f test.yaml=<(echo 'foo: bar') seems to work as expected.

@hoegaarden Thank you for that thorough, and very useful response. Better documentation on pipes would help big time.

For me, the problem is solved. I am convinced that I was wrong to view this as a bug. If everyone is in agreement, please feel free to close the ticket.

@prembhaskal
Copy link
Contributor

@hoegaarden thanks for the detail explanation. But I am still bit unsure why the pipes are treated as plain files (without extension) whereas the stdin is treated as .yaml

@prembhaskal prembhaskal added discussion This issue is not a bug or feature and a conversation is needed to find an appropriate resolution and removed carvel accepted This issue should be considered for future work and that the triage process has been completed labels Feb 26, 2024
@prembhaskal
Copy link
Contributor

ok so I saw this in test now

# test pipe redirect (on Linux, pipe is symlinked)
diff <(../../ytt -f pipe.yml=<(cat ../../examples/k8s-relative-rolling-update/config.yml)) ../../examples/k8s-relative-rolling-update/expected.txt
so whatever you (@hoegaarden ) explained makes sense now.

@prembhaskal prembhaskal removed bug This issue describes a defect or unexpected behavior can be replicated A bug that has been reproduced labels Feb 26, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
discussion This issue is not a bug or feature and a conversation is needed to find an appropriate resolution
Projects
Archived in project
Development

No branches or pull requests

3 participants