search

fullstorydev / grpcurl

Like cURL, but for gRPC: Command-line tool for interacting with gRPC servers
MIT License
10.67k stars 502 forks source link

Headers can be read from stdin? #116

Open jammerful opened 4 years ago

jammerful commented 4 years ago

I have a use case where I'd the headers to be able to be read from stdin, in particular I'm passing credentials through the headers and I don't want the credentials to show up as command line parameters which exposes the credentials.

Would it be possible to allow the headers to be read from stdin like the request data? I could put in a PR if this is simple, would you accept it?

I was thinking of putting a switch like this for reading from stdin just like the request data: https://github.com/fullstorydev/grpcurl/blob/master/cmd/grpcurl/grpcurl.go#L489

jhump commented 4 years ago

I think that's reasonable. But it could conflict with reading the request body from stdin. I think maybe the logic should look like so:

  1. If only headers are read from stdin, then read stdin, line-by-line. Each line must be a header in name: value form. Blank lines are ignored as are lines that start with a # (I'm a sucker for allowing comments if/when it's easy to support!). It probably needs to trim, too -- though maybe only trim from the left (to avoid leading whitespace in the header name, but allow trailing whitespace in the value?).
  2. If only the request is read from stdin, then the current behavior should be preserved.
  3. If both are to be read from stdin, it should have the headers first, one per line, and then a blank line, and then the request body (much like an HTTP 1.1 request might look).

What do you think? I doubt I'll have time to implement this soon, but I'll accept a patch if you're up for this.

jhump commented 4 years ago

Actually, I just had a thought: what if instead we added a flag that allowed header values to be re-written via environment variable interpolation?

So instead of having to feed the headers in via stdin, you could do something like so:

grpcurl -expand-headers -H 'name: ${value}'

Note use of single quotes, so the shell won't expand ${value}. Instead, grpcurl could do that, by search for ${...} sequences and replacing them with the corresponding environment variable.

What do you think about that? I can actually see cases where both would be useful -- ability to interpolate env vars into the values and ability to pass via stdin... Anyhow, I think I'd take a patch that had either (or both!) of these approaches implemented.

jammerful commented 4 years ago

Thanks for the second solution, I think that is much simpler and I'll put a PR in for that.

jammerful commented 4 years ago

I'll have a PR opened shortly, already have the patch ready.

After putting in the PR, I would need a new release cut and build artifacts published. Is this possible?

jammerful commented 4 years ago

@jhump Thanks for merging my PR, is there anyway you can cut a new release with accompanying binaries? Also should we close this issue, or should we leave it open for also accepting headers from stdin?

jhump commented 4 years ago

@jammerful, we can leave this issue open as an enhancement request to also accept via stdin.

I should be able to do a release soon, early next week at the latest. I'm looking for other issues that I might be able to address, too, just so it's not such a small release.

jammerful commented 4 years ago

@jhump Thanks, let me know if I can help at all.

jhump commented 4 years ago

@jammerful, FYI, a new v1.4.0 has been released that includes the change you landed.