alihan/fn-serverless
1
0
Fork 0
mirror of https://github.com/fnproject/fn.git synced 2022-10-28 21:29:17 +03:00
Code Issues Packages Projects Releases Wiki Activity

make headers quasi-consistent (#660)

Browse Source 
possible breakages:

* `FN_HEADER` on cold are no longer `s/-/_/` -- this is so that cold functions
can rebuild the headers as they were when they came in on the request (fdks,
specifically), there's no guarantee that a reversal `s/_/-/` is the original
header on the request.
* app and route config no longer `s/-/_/` -- it seemed really weird to rewrite
the users config vars on these. should just pass them exactly as is to env.
* headers no longer contain the environment vars (previously, base config; app
config, route config, `FN_PATH`, etc.), these are still available in the
environment.

this gets rid of a lot of the code around headers, specifically the stuff that
shoved everything into headers when constructing a call to begin with. now we
just store the headers separately and add a few things, like FN_CALL_ID to
them, and build a separate 'config' now to store on the call. I thought
'config' was more aptly named, 'env' was confusing, though now 'config' is
exactly what 'base_vars' was, which is only the things being put into the env.
we weren't storing this field in the db, this doesn't break unless there are
messages in a queue from another version, anyway, don't think we're there and
don't expect any breakage for anybody with field name changes.

this makes the configuration stuff pretty straight forward, there's just two
separate buckets of things, and cold just needs to mash them together into the
env, and otherwise hot containers just need to put 'config' in the env, and then
hot format can shove 'headers' in however they'd like. this seems better than
my last idea about making this easier but worse (RIP).

this means:

* headers no longer contain all vars, the set of base vars can only be found
in the environment.
* headers is only the headers from request + call_id, deadline, method, url
* for cold, we simply add the headers to the environment, prepending
`FN_HEADER_` to them, BUT NOT upper casing or `s/-/_/`
* fixes issue where async hot functions would end up with `Fn_header_`
prefixed headers
* removes idea of 'base' vars and 'env'. this was a strange concept. now we just have
'config' which was base vars, and headers, which was base_env+headers; i.e.
they are disjoint now.
* casing for all headers will lean to be `My-Header` style, which should help
with consistency. notable exceptions for cold only are FN_CALL_ID, FN_METHOD,
and FN_REQUEST_URL -- this is simply to avoid breakage, in either hot format
they appear as `Fn_call_id` still.
* removes FN_PARAM stuff
* updated doc with behavior

weird things left:

`Fn_call_id` e.g. isn't a correctly formatted http header, it should likely be
`Fn-Call-Id` but I wanted to live to fight another day on this one, it would
add some breakage.

examples to be posted of each format below

closes #329
This commit is contained in:
Reed Allman
2018-01-09 10:08:30 -08:00
committed by GitHub
parent 9c2a2a7fe7
commit 20089c4e83
11 changed files with 170 additions and 253 deletions
Download Patch File Download Diff File Expand all files Collapse all files

153
api/agent/call.go
View File

@@ -6,7 +6,6 @@ import (
"fmt"
"io"
"net/http"
"strconv"
"strings"
"time"
@@ -63,7 +62,7 @@ func fixupRequestURL(req *http.Request) string {
return req.URL.String()
}
func FromRequest(appName, path string, req *http.Request, params Params) CallOpt {
func FromRequest(appName, path string, req *http.Request) CallOpt {
return func(a *agent, c *call) error {
app, err := a.da.GetApp(req.Context(), appName)
if err != nil {
@@ -79,62 +78,8 @@ func FromRequest(appName, path string, req *http.Request, params Params) CallOpt
route.Format = models.FormatDefault
}
url := fixupRequestURL(req)
id := id.New().String()
// baseVars are the vars on the route & app, not on this specific request [for hot functions]
baseVars := make(map[string]string, len(app.Config)+len(route.Config)+3)
// add app & route config before our standard additions
for k, v := range app.Config {
k = toEnvName("", k)
baseVars[k] = v
}
for k, v := range route.Config {
k = toEnvName("", k)
baseVars[k] = v
}
baseVars["FN_FORMAT"] = route.Format
baseVars["FN_APP_NAME"] = appName
baseVars["FN_PATH"] = route.Path
// TODO: might be a good idea to pass in: envVars["FN_BASE_PATH"] = fmt.Sprintf("/r/%s", appName) || "/" if using DNS entries per app
baseVars["FN_MEMORY"] = fmt.Sprintf("%d", route.Memory)
baseVars["FN_TYPE"] = route.Type
// envVars contains the full set of env vars, per request + base
envVars := make(map[string]string, len(baseVars)+len(params)+len(req.Header)+3)
for k, v := range baseVars {
envVars[k] = v
}
envVars["FN_CALL_ID"] = id
envVars["FN_METHOD"] = req.Method
envVars["FN_REQUEST_URL"] = url
headerVars := make(map[string]string, len(req.Header))
for k, v := range req.Header {
if !noOverrideVars(k) { // NOTE if we don't do this, they'll leak in (don't want people relying on this behavior)
headerVars[toEnvName("FN_HEADER", k)] = strings.Join(v, ", ")
}
}
// add all the env vars we build to the request headers
for k, v := range envVars {
if noOverrideVars(k) {
// overwrite the passed in request headers explicitly with the generated ones
req.Header.Set(k, v)
} else {
req.Header.Add(k, v)
}
}
for k, v := range headerVars {
envVars[k] = v
}
// TODO this relies on ordering of opts, but tests make sure it works, probably re-plumb/destroy headers
// TODO async should probably supply an http.ResponseWriter that records the logs, to attach response headers to
if rw, ok := c.w.(http.ResponseWriter); ok {
@@ -147,6 +92,11 @@ func FromRequest(appName, path string, req *http.Request, params Params) CallOpt
}
}
// add our per call headers in here
req.Header.Set("FN_METHOD", req.Method)
req.Header.Set("FN_REQUEST_URL", reqURL(req))
req.Header.Set("FN_CALL_ID", id)
// this ensures that there is an image, path, timeouts, memory, etc are valid.
// NOTE: this means assign any changes above into route's fields
err = route.Validate()
@@ -167,10 +117,10 @@ func FromRequest(appName, path string, req *http.Request, params Params) CallOpt
Timeout: route.Timeout,
IdleTimeout: route.IdleTimeout,
Memory: route.Memory,
BaseEnv: baseVars,
EnvVars: envVars,
Config: buildConfig(app, route),
Headers: req.Header,
CreatedAt: strfmt.DateTime(time.Now()),
URL: url,
URL: reqURL(req),
Method: req.Method,
}
@@ -179,22 +129,36 @@ func FromRequest(appName, path string, req *http.Request, params Params) CallOpt
}
}
func noOverrideVars(key string) bool {
// descrepency in casing b/w req headers and env vars, force matches
return overrideVars[strings.ToUpper(key)]
func buildConfig(app *models.App, route *models.Route) models.Config {
conf := make(models.Config, 8+len(app.Config)+len(route.Config))
for k, v := range app.Config {
conf[k] = v
}
for k, v := range route.Config {
conf[k] = v
}
conf["FN_FORMAT"] = route.Format
conf["FN_APP_NAME"] = app.Name
conf["FN_PATH"] = route.Path
// TODO: might be a good idea to pass in: "FN_BASE_PATH" = fmt.Sprintf("/r/%s", appName) || "/" if using DNS entries per app
conf["FN_MEMORY"] = fmt.Sprintf("%d", route.Memory)
conf["FN_TYPE"] = route.Type
return conf
}
// overrideVars means that the app config, route config or header vars
// must not overwrite the generated values in call construction.
var overrideVars = map[string]bool{
"FN_FORMAT": true,
"FN_APP_NAME": true,
"FN_PATH": true,
"FN_MEMORY": true,
"FN_TYPE": true,
"FN_CALL_ID": true,
"FN_METHOD": true,
"FN_REQUEST_URL": true,
func reqURL(req *http.Request) string {
if req.URL.Scheme == "" {
if req.TLS == nil {
req.URL.Scheme = "http"
} else {
req.URL.Scheme = "https"
}
}
if req.URL.Host == "" {
req.URL.Host = req.Host
}
return req.URL.String()
}
// TODO this currently relies on FromRequest having happened before to create the model
@@ -208,31 +172,7 @@ func FromModel(mCall *models.Call) CallOpt {
if err != nil {
return err
}
// HACK: only applies to async below, for async we need to restore
// content-length and content-type of the original request, which are
// derived from Payload and original content-type which now is in
// Fn_header_content_type
if c.Type == models.TypeAsync {
// Hoist original request content type into async request
if req.Header.Get("Content-Type") == "" {
content_type, ok := c.EnvVars["Fn_header_content_type"]
if ok {
req.Header.Set("Content-Type", content_type)
}
}
// Ensure content-length in async requests for protocol/http DumpRequestTo()
if req.ContentLength == -1 || req.Header.Get("Content-Length") == "" {
req.ContentLength = int64(len(c.Payload))
req.Header.Set("Content-Length", strconv.FormatInt(int64(len(c.Payload)), 10))
}
}
for k, v := range c.EnvVars {
// TODO if we don't store env as []string headers are messed up
req.Header.Set(k, v)
}
req.Header = c.Headers
c.req = req
// TODO anything else really?
@@ -250,7 +190,6 @@ func WithWriter(w io.Writer) CallOpt {
// GetCall builds a Call that can be used to submit jobs to the agent.
//
// TODO we could make this package level just moving the cache around. meh.
// TODO where to put this? async and sync both call this
func (a *agent) GetCall(opts ...CallOpt) (Call, error) {
var c call
@@ -291,7 +230,13 @@ func (a *agent) GetCall(opts ...CallOpt) (Call, error) {
c.execDeadline = execDeadline
execDeadlineStr := strfmt.DateTime(execDeadline).String()
c.EnvVars["FN_DEADLINE"] = execDeadlineStr
// these 2 headers buckets are the same but for posterity!
if c.Headers == nil {
c.Headers = make(http.Header)
c.req.Header = c.Headers
}
c.Headers.Set("FN_DEADLINE", execDeadlineStr)
c.req.Header.Set("FN_DEADLINE", execDeadlineStr)
return &c, nil
@@ -386,11 +331,3 @@ func (c *call) End(ctx context.Context, errIn error) error {
return errIn // original error, important for use in sync call returns
}
func toEnvName(envtype, name string) string {
name = strings.Replace(name, "-", "_", -1)
if envtype == "" {
return name
}
return fmt.Sprintf("%s_%s", envtype, name)
}