search

carvel-dev / ytt

YAML templating tool that works on YAML structure instead of text
https://carvel.dev/ytt
Apache License 2.0
1.68k stars 137 forks source link

bash subshell file descriptors produce nothing #894

Closed richard-fairthorne closed 8 months ago

richard-fairthorne commented 9 months ago

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:

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.

prembhaskal commented 8 months ago

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 commented 8 months ago

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 commented 8 months ago

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.

richard-fairthorne commented 8 months ago

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 commented 8 months ago

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

prembhaskal commented 8 months ago

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

richard-fairthorne commented 8 months ago

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 commented 8 months ago

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 here[^1] 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:

[^1]: Linux & bash 5.2.15 & ytt 0.48.0

richard-fairthorne commented 8 months ago

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 commented 8 months ago

@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 commented 8 months ago

ok so I saw this in test now https://github.com/carvel-dev/ytt/blob/6d2001d043a758577173db242fc1c6585db44b5b/test/e2e/assets/test_pipe_redirect.sh#L5-L6 so whatever you (@hoegaarden ) explained makes sense now.