FN_ prefix env vars

this adds `FN_` in front of env vars that we are injecting into calls, for
namespacing reasons. this will break code relying on the current variables but
if we want to do this, the chance is now really. alternatively, we could
maintain both the old and new for a short period of time to ease the
adjustment (speak now...). updated the docs, as well.

this also adds tests for the notoriously finicky configuration of the env vars
and headers when setting up a call. this won't test the container / request
for the call is actually receiving them, but it's a decent start and will yell
loudly enough upon formatting breakage.

added back FXLB_WAIT to a couple places so the lb can ride again

one thing for feedback:

headers are a bit confusing at the moment (not from this change, but that
behavior is kept here for now), we've a chance to fix them. currently, headers
in the request __are not__ prefixed with `FN_HEADER_`, i.e. 'hot'+sync containers
will receive `Content-Length` in the http request headers, yet a 'cold'
container from the same request would receive `FN_HEADER_Content-Length` in
its environment. This is additionally confusing because if this function were
hot+async, it would receive `FN_HEADER_Content-Length` in the headers, where
just changing it to sync goes back to `Content-Length`. If that was confusing,
then point made ;)

I propose to remove the `FN_HEADER_` prefix for request headers in the
environment, so that the request headers and env will match, as request
headers already are of this format (not prefixed). please lmk thoughts here

Would be fine with going back to the 'plain' vars too, then this patch will
mostly just be adding tests and changing `FN_FORMAT` to `FORMAT`. obviously,
from the examples, it's a bit ingrained now. anyway, entirely up to y'all.

api/agent/agent.go

@@ -30,9 +30,7 @@ import (
 // TODO handle timeouts / no response in sync & async (sync is json+503 atm, not 504, async is empty log+status)
 // see also: server/runner.go wrapping the response writer there, but need to handle async too (push down?)
 // TODO herd launch prevention part deux
-// TODO plumb FXLB-WAIT back - can we use headers now? maybe let's use api
-// TODO none of the Datastore methods actually use the ctx for timeouts :(
-// TODO not adding padding if call times out to store appropriately (ctx timed out, happenstance it works now cuz of ^)
+// TODO storing logs / call can push call over the timeout
 // TODO all Datastore methods need to take unit of tenancy (app or route) at least (e.g. not just call id)
 // TODO limit the request body length when making calls
 // TODO discuss concrete policy for hot launch or timeout / timeout vs time left
@@ -50,7 +48,6 @@ import (
 // dies). need coordination w/ db.
 // TODO if a cold call times out but container is created but hasn't replied, could
 // end up that the client doesn't get a reply until long after the timeout (b/c of container removal, async it?)
-// TODO we should prob not be logging all async output to the logs by default...
 // TODO the call api should fill in all the fields
 // TODO the log api should be plaintext (or at least offer it)
 // TODO func logger needs to be hanged, dragged and quartered. in reverse order.

api/agent/agent_test.go

@@ -0,0 +1,268 @@
+package agent
+
+import (
+	"bytes"
+	"io"
+	"net/http"
+	"net/http/httptest"
+	"strconv"
+	"strings"
+	"testing"
+	"time"
+
+	"github.com/fnproject/fn/api/datastore"
+	"github.com/fnproject/fn/api/models"
+	"github.com/fnproject/fn/api/mqs"
+)
+
+func TestCallConfigurationRequest(t *testing.T) {
+	appName := "myapp"
+	path := "/sleeper"
+	image := "fnproject/sleeper"
+	const timeout = 1
+	const idleTimeout = 20
+	const memory = 256
+
+	cfg := models.Config{"APP_VAR": "FOO"}
+	rCfg := models.Config{"ROUTE_VAR": "BAR"}
+
+	ds := datastore.NewMockInit(
+		[]*models.App{
+			{Name: appName, Config: cfg},
+		},
+		[]*models.Route{
+			{
+				Config:      rCfg,
+				Path:        path,
+				AppName:     appName,
+				Image:       image,
+				Type:        "sync",
+				Format:      "default",
+				Timeout:     timeout,
+				IdleTimeout: idleTimeout,
+				Memory:      memory,
+			},
+		}, nil,
+	)
+
+	a := New(ds, new(mqs.Mock))
+	defer a.Close()
+
+	w := httptest.NewRecorder()
+
+	method := "GET"
+	url := "http://127.0.0.1:8080/r/" + appName + path
+	payload := "payload"
+	contentLength := strconv.Itoa(len(payload))
+	req, err := http.NewRequest(method, url, strings.NewReader(payload))
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	req.Header.Add("MYREALHEADER", "FOOLORD")
+	req.Header.Add("MYREALHEADER", "FOOPEASANT")
+	req.Header.Add("Content-Length", contentLength)
+
+	call, err := a.GetCall(
+		WithWriter(w), // XXX (reed): order matters [for now]
+		FromRequest(appName, path, req),
+	)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	model := call.Model()
+
+	// make sure the values are all set correctly
+	if model.ID == "" {
+		t.Fatal("model does not have id, GetCall should assign id")
+	}
+	if model.AppName != appName {
+		t.Fatal("app name mismatch", model.AppName, appName)
+	}
+	if model.Path != path {
+		t.Fatal("path mismatch", model.Path, path)
+	}
+	if model.Image != image {
+		t.Fatal("image mismatch", model.Image, image)
+	}
+	if model.Type != "sync" {
+		t.Fatal("route type mismatch", model.Type)
+	}
+	if model.Priority == nil {
+		t.Fatal("GetCall should make priority non-nil so that async works because for whatever reason some clowns plumbed it all over the mqs even though the user can't specify it gg")
+	}
+	if model.Timeout != timeout {
+		t.Fatal("timeout mismatch", model.Timeout, timeout)
+	}
+	if model.IdleTimeout != idleTimeout {
+		t.Fatal("idle timeout mismatch", model.IdleTimeout, idleTimeout)
+	}
+	if time.Time(model.CreatedAt).IsZero() {
+		t.Fatal("GetCall should stamp CreatedAt, got nil timestamp")
+	}
+	if model.URL != url {
+		t.Fatal("url mismatch", model.URL, url)
+	}
+	if model.Method != method {
+		t.Fatal("method mismatch", model.Method, method)
+	}
+	if model.Payload != "" { // NOTE: this is expected atm
+		t.Fatal("GetCall FromRequest should not fill payload, got non-empty payload", model.Payload)
+	}
+
+	expectedBase := map[string]string{
+		"FN_FORMAT":    "default",
+		"FN_APP_NAME":  appName,
+		"FN_ROUTE":     path,
+		"FN_MEMORY_MB": strconv.Itoa(memory),
+		"APP_VAR":      "FOO",
+		"ROUTE_VAR":    "BAR",
+	}
+
+	expectedEnv := make(map[string]string)
+	for k, v := range expectedBase {
+		expectedEnv[k] = v
+	}
+
+	for k, v := range expectedBase {
+		if v2 := model.BaseEnv[k]; v2 != v {
+			t.Fatal("base var mismatch", k, v, v2, model.BaseEnv)
+		}
+		delete(expectedBase, k)
+	}
+
+	if len(expectedBase) > 0 {
+		t.Fatal("got extra vars in base env set, add me to tests ;)", expectedBase)
+	}
+
+	expectedEnv["FN_CALL_ID"] = model.ID
+	expectedEnv["FN_METHOD"] = method
+	expectedEnv["FN_REQUEST_URL"] = url
+
+	// do this before the "real" headers get sucked in cuz they are formatted differently
+	expectedHeaders := make(http.Header)
+	for k, v := range expectedEnv {
+		expectedHeaders.Add(k, v)
+	}
+
+	// from the request headers (look different in env than in req.Header, idk, up to user anger)
+	// req headers down cases things
+	expectedEnv["FN_HEADER_Myrealheader"] = "FOOLORD, FOOPEASANT"
+	expectedEnv["FN_HEADER_Content_Length"] = contentLength
+
+	for k, v := range expectedEnv {
+		if v2 := model.EnvVars[k]; v2 != v {
+			t.Fatal("env var mismatch", k, v, v2, model.EnvVars)
+		}
+		delete(expectedEnv, k)
+	}
+
+	if len(expectedEnv) > 0 {
+		t.Fatal("got extra vars in base env set, add me to tests ;)", expectedBase)
+	}
+
+	expectedHeaders.Add("MYREALHEADER", "FOOLORD")
+	expectedHeaders.Add("MYREALHEADER", "FOOPEASANT")
+	expectedHeaders.Add("Content-Length", contentLength)
+
+	for k, vs := range req.Header {
+		for i, v := range expectedHeaders[k] {
+			if i >= len(vs) || vs[i] != v {
+				t.Fatal("header mismatch", k, vs)
+			}
+		}
+		delete(expectedHeaders, k)
+	}
+
+	if len(expectedHeaders) > 0 {
+		t.Fatal("got extra headers, bad")
+	}
+
+	if w.Header()["Fn_call_id"][0] != model.ID {
+		t.Fatal("response writer should have the call id, or else")
+	}
+
+	// TODO check response writer for route headers
+
+	// TODO idk what param even is or how to get them, but need to test those
+	// TODO we should double check the things we're rewriting defaults of, like type, format, timeout, idle_timeout
+}
+
+func TestCallConfigurationModel(t *testing.T) {
+	appName := "myapp"
+	path := "/sleeper"
+	image := "fnproject/sleeper"
+	const timeout = 1
+	const idleTimeout = 20
+	const memory = 256
+	method := "GET"
+	url := "http://127.0.0.1:8080/r/" + appName + path
+	payload := "payload"
+	env := map[string]string{
+		"FN_FORMAT":    "default",
+		"FN_APP_NAME":  appName,
+		"FN_ROUTE":     path,
+		"FN_MEMORY_MB": strconv.Itoa(memory),
+		"APP_VAR":      "FOO",
+		"ROUTE_VAR":    "BAR",
+		"DOUBLE_VAR":   "BIZ, BAZ",
+	}
+
+	cm := &models.Call{
+		BaseEnv:     env,
+		EnvVars:     env,
+		AppName:     appName,
+		Path:        path,
+		Image:       image,
+		Type:        "sync",
+		Format:      "default",
+		Timeout:     timeout,
+		IdleTimeout: idleTimeout,
+		Memory:      memory,
+		Payload:     payload,
+		URL:         url,
+		Method:      method,
+	}
+
+	// FromModel doesn't need a datastore, for now...
+	ds := datastore.NewMockInit(nil, nil, nil)
+
+	a := New(ds, new(mqs.Mock))
+	defer a.Close()
+
+	callI, err := a.GetCall(FromModel(cm))
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	// make sure headers seem reasonable
+	req := callI.(*call).req
+
+	// NOTE these are added as is atm, and if the env vars were comma joined
+	// they are not again here comma separated.
+	expectedHeaders := make(http.Header)
+	for k, v := range env {
+		expectedHeaders.Add(k, v)
+	}
+
+	for k, vs := range req.Header {
+		for i, v := range expectedHeaders[k] {
+			if i >= len(vs) || vs[i] != v {
+				t.Fatal("header mismatch", k, vs)
+			}
+		}
+		delete(expectedHeaders, k)
+	}
+
+	if len(expectedHeaders) > 0 {
+		t.Fatal("got extra headers, bad")
+	}
+
+	var b bytes.Buffer
+	io.Copy(&b, req.Body)
+
+	if b.String() != payload {
+		t.Fatal("expected payload to match, but it was a lie")
+	}
+}

api/agent/call.go

@@ -66,9 +66,9 @@ func FromRequest(appName, path string, req *http.Request) CallOpt {
 		// 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)
 		baseVars["FN_FORMAT"] = route.Format
-		baseVars["APP_NAME"] = appName
-		baseVars["ROUTE"] = route.Path
-		baseVars["MEMORY_MB"] = fmt.Sprintf("%d", route.Memory)
+		baseVars["FN_APP_NAME"] = appName
+		baseVars["FN_ROUTE"] = route.Path
+		baseVars["FN_MEMORY_MB"] = fmt.Sprintf("%d", route.Memory)
 
 		// app config
 		for k, v := range app.Config {
@@ -87,24 +87,31 @@ func FromRequest(appName, path string, req *http.Request) CallOpt {
 			envVars[k] = v
 		}
 
-		envVars["CALL_ID"] = id
-		envVars["METHOD"] = req.Method
-		envVars["REQUEST_URL"] = fmt.Sprintf("%v://%v%v", func() string {
-			if req.TLS == nil {
-				return "http"
+		envVars["FN_CALL_ID"] = id
+		envVars["FN_METHOD"] = req.Method
+		envVars["FN_REQUEST_URL"] = func() string {
+			if req.URL.Scheme == "" {
+				if req.TLS == nil {
+					req.URL.Scheme = "http"
+				} else {
+					req.URL.Scheme = "https"
+				}
 			}
-			return "https"
-		}(), req.Host, req.URL.String())
+			if req.URL.Host == "" {
+				req.URL.Host = req.Host
+			}
+			return req.URL.String()
+		}()
 
 		// params
 		for _, param := range params {
-			envVars[toEnvName("PARAM", param.Key)] = param.Value
+			envVars[toEnvName("FN_PARAM", param.Key)] = param.Value
 		}
 
 		headerVars := make(map[string]string, len(req.Header))
 
 		for k, v := range req.Header {
-			headerVars[toEnvName("HEADER", k)] = strings.Join(v, ", ")
+			headerVars[toEnvName("FN_HEADER", k)] = strings.Join(v, ", ")
 		}
 
 		// add all the env vars we build to the request headers
@@ -118,6 +125,7 @@ func FromRequest(appName, path string, req *http.Request) CallOpt {
 		}
 
 		// 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 {
 			rw.Header().Add("FN_CALL_ID", id)
 			for k, vs := range route.Headers {
@@ -246,6 +254,10 @@ func (c *call) Start(ctx context.Context) error {
 	c.StartedAt = strfmt.DateTime(time.Now())
 	c.Status = "running"
 
+	if rw, ok := c.w.(http.ResponseWriter); ok { // TODO need to figure out better way to wire response headers in
+		rw.Header().Set("XXX-FXLB-WAIT", time.Time(c.StartedAt).Sub(time.Time(c.CreatedAt)).String())
+	}
+
 	if c.Type == models.TypeAsync {
 		// XXX (reed): make sure MQ reservation is lengthy. to skirt MQ semantics,
 		// we could add a new message to MQ w/ delay of call.Timeout and delete the

api/server/runner.go

@@ -6,6 +6,7 @@ import (
 	"net/http"
 	"path"
 	"strings"
+	"time"
 
 	"github.com/fnproject/fn/api"
 	"github.com/fnproject/fn/api/agent"
@@ -58,8 +59,9 @@ func (s *Server) serve(c *gin.Context, appName, path string) {
 	}
 
 	// TODO we could add FireBeforeDispatch right here with Call in hand
+	model := call.Model()
 
-	if model := call.Model(); model.Type == "async" {
+	if model.Type == "async" {
 		// TODO we should push this into GetCall somehow (CallOpt maybe) or maybe agent.Queue(Call) ?
 		buf := bytes.NewBuffer(make([]byte, 0, c.Request.ContentLength)) // TODO sync.Pool me
 		_, err := buf.ReadFrom(c.Request.Body)
@@ -86,6 +88,10 @@ func (s *Server) serve(c *gin.Context, appName, path string) {
 		// we could filter that error out here too as right now it yells a little
 
 		if err == context.DeadlineExceeded {
+			// TODO maneuver
+			// add this, since it means that start may not have been called [and it's relevant]
+			c.Writer.Header().Add("XXX-FXLB-WAIT", time.Now().Sub(time.Time(model.CreatedAt)).String())
+
 			err = models.ErrCallTimeout // 504 w/ friendly note
 		}
 		// NOTE: if the task wrote the headers already then this will fail to write

docs/writing.md

@@ -31,14 +31,15 @@ To read in the function body, just read from STDIN.
 
 You will also have access to a set of environment variables.
 
-* REQUEST_URL - the full URL for the request ([parsing example](https://github.com/fnproject/fn/tree/master/examples/tutorial/params))
-* APP_NAME - the name of the application that matched this route, eg: `myapp`
-* ROUTE - the matched route, eg: `/hello`
-* METHOD - the HTTP method for the request, eg: `GET` or `POST`
-* CALL_ID - a unique ID for each function execution.
-* FORMAT - a string representing one of the [function formats](function-format.md), currently either `default` or `http`. Default is `default`. 
-* HEADER_X - the HTTP headers that were set for this request. Replace X with the upper cased name of the header and replace dashes in the header with underscores.
-* X - any [configuration values](https://gitlab.oracledx.com/odx/functions/blob/master/fn/README.md#application-level-configuration) you've set
+* `FN_REQUEST_URL` - the full URL for the request ([parsing example](https://github.com/fnproject/fn/tree/master/examples/tutorial/params))
+* `FN_APP_NAME` - the name of the application that matched this route, eg: `myapp`
+* `FN_ROUTE` - the matched route, eg: `/hello`
+* `FN_METHOD` - the HTTP method for the request, eg: `GET` or `POST`
+* `FN_CALL_ID` - a unique ID for each function execution.
+* `FN_FORMAT` - a string representing one of the [function formats](function-format.md), currently either `default` or `http`. Default is `default`. 
+* `FN_MEMORY` - a number representing the amount of memory available to the call, in MB
+* `FN_HEADER_$X` - the HTTP headers that were set for this request. Replace $X with the upper cased name of the header and replace dashes in the header with underscores.
+  * `$X` - any [configuration values](https://gitlab.oracledx.com/odx/functions/blob/master/fn/README.md#application-level-configuration) you've set
   for the Application or the Route. Replace X with the upper cased name of the config variable you set. Ex: `minio_secret=secret` will be exposed via MINIO_SECRET env var.
 
 Warning: these may change before release.

examples/blog/function.go

@@ -13,7 +13,7 @@ import (
 var noAuth = map[string]interface{}{}
 
 func main() {
-	request := fmt.Sprintf("%s %s", os.Getenv("METHOD"), os.Getenv("ROUTE"))
+	request := fmt.Sprintf("%s %s", os.Getenv("FN_METHOD"), os.Getenv("FN_ROUTE"))
 
 	dbURI := os.Getenv("DB")
 	if dbURI == "" {
@@ -36,7 +36,7 @@ func main() {
 	}
 
 	// GETTING TOKEN
-	if os.Getenv("ROUTE") == "/token" {
+	if os.Getenv("FN_ROUTE") == "/token" {
 		route.HandleToken(db)
 		return
 	}

examples/blog/routes/post_read.go

@@ -7,7 +7,7 @@ import (
 )
 
 func HandlePostRead(db *database.Database, auth map[string]interface{}) {
-	id := os.Getenv("PARAM_ID")
+	id := os.Getenv("FN_PARAM_ID")
 
 	if id == "" {
 		SendError("Missing post ID")

examples/blog/routes/server.go

@@ -63,7 +63,7 @@ func HandleToken(db *database.Database) {
 }
 
 func Authentication() (map[string]interface{}, bool) {
-	authorization := os.Getenv("HEADER_AUTHORIZATION")
+	authorization := os.Getenv("FN_HEADER_AUTHORIZATION")
 
 	p := strings.Split(authorization, " ")
 	if len(p) <= 1 {

examples/blog/test.sh

@@ -9,8 +9,8 @@ docker rm test-mongo-func
 
 docker run -p 27017:27017 --name test-mongo-func -d mongo
 
-echo '{ "title": "My New Post", "body": "Hello world!", "user": "test" }' | docker run --rm -i -e METHOD=POST -e ROUTE=/posts -e DB=mongo:27017 --link test-mongo-func:mongo -e TEST=1 username/func-blog
-docker run --rm -i -e METHOD=GET -e ROUTE=/posts -e DB=mongo:27017 --link test-mongo-func:mongo -e TEST=1 username/func-blog
+echo '{ "title": "My New Post", "body": "Hello world!", "user": "test" }' | docker run --rm -i -e FN_METHOD=POST -e FN_ROUTE=/posts -e DB=mongo:27017 --link test-mongo-func:mongo -e TEST=1 username/func-blog
+docker run --rm -i -e FN_METHOD=GET -e FN_ROUTE=/posts -e DB=mongo:27017 --link test-mongo-func:mongo -e TEST=1 username/func-blog
 
 docker stop test-mongo-func
 docker rm test-mongo-func

examples/checker/function.rb

@@ -21,21 +21,21 @@ if payload != ""
 end
 
 # Also check for expected env vars: https://gitlab.oracledx.com/odx/functions/blob/master/docs/writing.md#inputs
-e = ENV["REQUEST_URL"]
+e = ENV["FN_REQUEST_URL"]
 puts e
 uri = URI.parse(e)
 if !uri.scheme.start_with?('http') 
   raise "invalid REQUEST_URL, does not start with http"
 end
-e = ENV["METHOD"]
+e = ENV["FN_METHOD"]
 if !(e == "GET" || e == "POST" || e == "DELETE" || e == "PATCH" || e == "PUT")
   raise "Invalid METHOD: #{e}"
 end
-e = ENV["APP_NAME"]
+e = ENV["FN_APP_NAME"]
 if e == nil || e == ''
   raise "No APP_NAME found"
 end
-e = ENV["ROUTE"]
+e = ENV["FN_ROUTE"]
 if e == nil || e == ''
   raise "No ROUTE found"
 end

examples/postgres/func.go

@@ -16,11 +16,11 @@ import (
 
 var (
 	// command to execute, 'SELECT' or 'INSERT'
-	command = os.Getenv("HEADER_COMMAND")
+	command = os.Getenv("FN_HEADER_COMMAND")
 	// postgres host:port, e.g. 'postgres:5432'
-	server = os.Getenv("HEADER_SERVER")
+	server = os.Getenv("FN_HEADER_SERVER")
 	// postgres table name
-	table = os.Getenv("HEADER_TABLE")
+	table = os.Getenv("FN_HEADER_TABLE")
 )
 
 func main() {

examples/testframework/local/func.go

@@ -6,9 +6,9 @@ import (
 )
 
 func main() {
-	envvar := os.Getenv("HEADER_ENVVAR")
+	envvar := os.Getenv("FN_HEADER_ENVVAR")
 	if envvar != "" {
-		fmt.Println("HEADER_ENVVAR:", envvar)
+		fmt.Println("FN_HEADER_ENVVAR:", envvar)
 	}
 	fmt.Println("hw")
 }

examples/testframework/local/func.yaml

@@ -9,7 +9,7 @@ tests:
     hw
 - name: envvar
   out: |
-    HEADER_ENVVAR: trololo
+    FN_HEADER_ENVVAR: trololo
     hw
   env:
     envvar: trololo

examples/testframework/remote/func.yaml

@@ -9,7 +9,7 @@ tests:
     hw
 - name: envvar
   out: |
-    HEADER_ENVVAR: trololo
+    FN_HEADER_ENVVAR: trololo
     hw
   env:
     envvar: trololo

examples/tutorial/params/func.go

@@ -8,9 +8,9 @@ import (
 )
 
 func main() {
-	s := os.Getenv("REQUEST_URL")
+	s := os.Getenv("FN_REQUEST_URL")
 
-	fmt.Printf("REQUEST_URL --> %v\n\n", s)
+	fmt.Printf("FN_REQUEST_URL --> %v\n\n", s)
 
 	u, err := url.Parse(s)
 	if err != nil {

test/fnlb-test-harness/primes-func/func.go

@@ -30,7 +30,7 @@ func main() {
 	numLoops := 1
 
 	// Parse the query string
-	s := strings.Split(os.Getenv("REQUEST_URL"), "?")
+	s := strings.Split(os.Getenv("FN_REQUEST_URL"), "?")
 	if len(s) > 1 {
 		for _, pair := range strings.Split(s[1], "&") {
 			kv := strings.Split(pair, "=")