-
Notifications
You must be signed in to change notification settings - Fork 117
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
Enhancement proposal: process controlled HTTP response code and headers #3
Comments
@alexellis Could you please take a look at this? |
So in the streaming mode your HTTP response header is returned immediately as soon as the first byte is transferred. You cannot send a HTTP response header a second time. |
Correct. All the headers and status code must be sent in a single bundle in this design. |
Unless I'm not following along then what I just mentioned would mean your change couldn't work in the streaming mode. Perhaps if you need an error after the fact link headers might be better suited or checking on the client-end to create a checksum from the result. What is your concrete use-case here? |
My concrete use case is using an openfaas function to provide authentication responses for nginx subrequest authentication. The nginx server needs a 200 code for access granted and a 400 code for access denied. This proposal can be accomplished by putting a shim struct between the http.ResponseWriter and the stdout of the called process which will detect the first byte of the output and at that point, if there was a http response code or headers provided by in the pipe, using those for the response, defaulting to a 200 code if nothing was provided. This shim would only need to do this check on the first byte and would otherwise skip that test, instead passing the data along to the http.ResponseWriter transparently. |
I'd like to see this, too. Most usecases I can think of for openfaas or similar things require the ability to return non-200 status codes. |
@alexellis I'm thinking perhaps I didn't do the best job explaining what my idea is in full. In my proposal, a side channel will be opened and, when the first byte of output would be sent back to the client, that side pipe is checked first. If that pipe contains any data, it dumps that before the first byte of stdout. After the first byte is sent to the client, that check no longer occurs. So in this design the program would still be required to set status code and headers before sending the body of the response, but it would allow something other than a 200 to be returned. |
Hi guys - we are working on something that will allow a http status to be returned. Using a pipe as well as stdio seems orthogonal and I'm not keen on opening additional files or pipes per request - as these have to be managed and garbage collected. Under high load these could result in undesired side-effects. If you're on the OpenFaaS slack already you can read more about the plans in #contributors with some related notes at openfaas/faas#494 To join the Slack community just email [email protected] |
The issue you've linked to doesn't cover the streaming mode, only the forking and afterburn methods. My proposed solution works for streaming, and could easily be used for both forking and afterburn as well. But lets discuss it over slack rather than continuing to go back and forth in this thread. |
Going forward the new of-watchdog will most likely retain the forking mode and the HTTP mode only. The HTTP mode will allow a user to return any response status code / header. The forking mode I would not see changing much beyond its current interface. It is possible to set an alternative HTTP code with a non-zero exit code. This currently maps like this:
What programming language would you like to use to return the 200/400 status code? |
Based on your wording, and looking at the README to confirm the mode types, I must assume that by 'forking mode' you mean 'Serializing Fork' and not 'Streaming Fork' because my proposed solution is primarily aimed at the Streaming Fork mode, which is still a forking mode. It now sounds like you are saying you are planning to remove the Streaming Forking mode all together, which would break backwards compatibility which you expressly state that the of-watchdog process will maintain. Serializing Fork mode, as stated in the readme, reads everything into a buffer before passing it to the end user. This means you are limited to the memory your computer has and prevents doing large files which is the primary use case of the Streaming Fork mode. As for the current interface of Serializing Fork, which again I must assume you mean as your post is ambiguous, returning either 200 or 500 is not sufficient for some use cases, including the use case I stated above of using Openfaas for subrequest authentication of an nginx server. To more directly address the HTTP mode you refer to, I am uncertain why someone would bother using the of-watchdog process if their code is already providing an HTTP interface, instead of just having their process listen on the port directly. Adding of-watchdog in front of it seems to add unneeded overhead. The language used to return the status codes should be irrelevant per your post as the Serializing Fork mode cannot return the 400 status codes, and the HTTP interface does not need to know anything about the language in process function's language. Again, I would be happy to discuss this with you in slack, and have already sent a request to your email address to join the slack server, as you suggested. I await your invite. |
Thank you for sharing your thoughts and opinions. For those who do not want to to consume HTTP within their process the forking mode will still be available. The OpenFaaS-incubator repository contains experimental projects which do not provide a 1.0 API - that means that they are subject to change. Future versions of the watchdog will of course maintain compatibility with the existing watchdog. To summarise before locking the thread - there will be two options for returning a HTTP status code:
|
Derek lock |
Brief summary including motivation/context
This change is to the streaming mode of of-watchdog to add a named pipe with which the running subprograms can send control messages back to of-watchdog in specify the HTTP response code and headers. This design will be done in such a way so as to not impact the behavior for any called functions which do not make use of the pipe provided. The motivation behind this change is that I need functions to be able to return specific response codes other than 200 for functions which I call.
Any design changes
The of-watchdog process will create a new named pipe when a function is called, then set the environment variable
CONTROL_PIPE
before calling the sub process. The of-watchdog will then listen on the control pipe file, if it recieves a JSON blob of the following format, it will use it to set the response code and headers before writing the output of the sub command. If no JSON blob is received before the subprocess begins to output on it's standard out, an HTTP response code of 200 will be sent with the first message.Pros + Cons
Pros: Functions will now be able to return custom response codes and headers per request.
Cons: None.
Effort required up front
A few days of coding time.
Effort required for CI/CD, release, ongoing maintenance
Minimal, if any.
Migration strategy / backwards-compatibility
This change will be 100% backwards compatible with the existing program.
Mock-up screenshots or examples of how the CLI would work
N/A
The text was updated successfully, but these errors were encountered: