Set shortcodes in trigger response entities based on config or request URL (#1099)

* adding trigger short code injection

* more annotation provider stuff

* fixed up tests

* Fix validator

api/datastore/internal/datastoreutil/validator.go

@@ -31,10 +31,6 @@ func (v *validator) GetAppByID(ctx context.Context, appID string) (*models.App,
 	return v.Datastore.GetAppByID(ctx, appID)
 }
 
-func (v *validator) GetApps(ctx context.Context, appFilter *models.AppFilter) (*models.AppList, error) {
-	return v.Datastore.GetApps(ctx, appFilter)
-}
-
 // app and app.Name will never be nil/empty.
 func (v *validator) InsertApp(ctx context.Context, app *models.App) (*models.App, error) {
 	if app == nil {
@@ -147,10 +143,6 @@ func (v *validator) InsertTrigger(ctx context.Context, t *models.Trigger) (*mode
 	return v.Datastore.InsertTrigger(ctx, t)
 }
 
-func (v *validator) UpdateTrigger(ctx context.Context, trigger *models.Trigger) (*models.Trigger, error) {
-	return v.Datastore.UpdateTrigger(ctx, trigger)
-}
-
 func (v *validator) GetTriggers(ctx context.Context, filter *models.TriggerFilter) (*models.TriggerList, error) {
 
 	if filter.AppID == "" {
@@ -183,10 +175,6 @@ func (v *validator) InsertFn(ctx context.Context, fn *models.Fn) (*models.Fn, er
 	return v.Datastore.InsertFn(ctx, fn)
 }
 
-func (v *validator) UpdateFn(ctx context.Context, fn *models.Fn) (*models.Fn, error) {
-	return v.Datastore.UpdateFn(ctx, fn)
-}
-
 func (v *validator) GetFnByID(ctx context.Context, fnID string) (*models.Fn, error) {
 	if fnID == "" {
 		return nil, models.ErrDatastoreEmptyFnID

api/models/trigger.go

@@ -10,6 +10,9 @@ import (
 	"github.com/fnproject/fn/api/common"
 )
 
+// For want of a better place to put this it's here
+const TriggerHTTPEndpointAnnotation = "fnproject.io/trigger/httpEndpoint"
+
 type Trigger struct {
 	ID          string          `json:"id" db:"id"`
 	Name        string          `json:"name" db:"name"`
@@ -50,7 +53,9 @@ func (t *Trigger) EqualsWithAnnotationSubset(t2 *Trigger) bool {
 	return eq
 }
 
-var triggerTypes = []string{"http"}
+const TriggerTypeHTTP = "http"
+
+var triggerTypes = []string{TriggerTypeHTTP}
 
 func ValidTriggerTypes() []string {
 	return triggerTypes

api/server/server.go

@@ -43,13 +43,15 @@ import (
 )
 
 const (
-	EnvLogLevel        = "FN_LOG_LEVEL"
+	EnvLogLevel              = "FN_LOG_LEVEL"
-	EnvLogDest         = "FN_LOG_DEST"
+	EnvLogDest               = "FN_LOG_DEST"
-	EnvLogPrefix       = "FN_LOG_PREFIX"
+	EnvLogPrefix             = "FN_LOG_PREFIX"
-	EnvMQURL           = "FN_MQ_URL"
+	EnvMQURL                 = "FN_MQ_URL"
-	EnvDBURL           = "FN_DB_URL"
+	EnvDBURL                 = "FN_DB_URL"
-	EnvLOGDBURL        = "FN_LOGSTORE_URL"
+	EnvLOGDBURL              = "FN_LOGSTORE_URL"
-	EnvRunnerURL       = "FN_RUNNER_API_URL"
+	EnvRunnerURL             = "FN_RUNNER_API_URL"
+	EnvPublicLoadBalancerURL = "FN_PUBLIC_LB_URL"
+
 	EnvRunnerAddresses = "FN_RUNNER_ADDRESSES"
 	EnvNodeType        = "FN_NODE_TYPE"
 	EnvPort            = "FN_PORT" // be careful, Gin expects this variable to be "port"
@@ -123,6 +125,7 @@ type Server struct {
 	rootMiddlewares  []fnext.Middleware
 	apiMiddlewares   []fnext.Middleware
 	promExporter     *prometheus.Exporter
+	triggerAnnotator TriggerAnnotator
 	// Extensions can append to this list of contexts so that cancellations are properly handled.
 	extraCtxs []context.Context
 }
@@ -172,6 +175,14 @@ func NewFromEnv(ctx context.Context, opts ...ServerOption) *Server {
 	opts = append(opts, WithNodeCertKey(getEnv(EnvCertKey, "")))
 	opts = append(opts, WithNodeCertAuthority(getEnv(EnvCertAuth, "")))
 
+	publicLbUrl := getEnv(EnvPublicLoadBalancerURL, "")
+	if publicLbUrl != "" {
+		logrus.Infof("using LB Base URL: '%s'", publicLbUrl)
+		opts = append(opts, WithTriggerAnnotator(NewStaticURLTriggerAnnotator(publicLbUrl)))
+	} else {
+		opts = append(opts, WithTriggerAnnotator(NewRequestBasedTriggerAnnotator()))
+	}
+
 	// Agent handling depends on node type and several other options so it must be the last processed option.
 	// Also we only need to create an agent if this is not an API node.
 	if nodeType != ServerTypeAPI {
@@ -495,6 +506,14 @@ func WithExtraCtx(extraCtx context.Context) ServerOption {
 	}
 }
 
+//WithTriggerAnnotator adds a trigggerEndpoint provider to the server
+func WithTriggerAnnotator(provider TriggerAnnotator) ServerOption {
+	return func(ctx context.Context, s *Server) error {
+		s.triggerAnnotator = provider
+		return nil
+	}
+}
+
 // WithAdminServer starts the admin server on the specified port.
 func WithAdminServer(port int) ServerOption {
 	return func(ctx context.Context, s *Server) error {
@@ -538,6 +557,9 @@ func New(ctx context.Context, opts ...ServerOption) *Server {
 		if s.agent != nil {
 			log.Fatal("Incorrect configuration, API nodes must not have an agent initialized.")
 		}
+		if s.triggerAnnotator == nil {
+			log.Fatal("No trigger annotatator  set ")
+		}
 	default:
 		if s.agent == nil {
 			log.Fatal("Incorrect configuration, non-API nodes must have an agent initialized.")

api/server/server_test.go

@@ -34,6 +34,7 @@ func testServer(ds models.Datastore, mq models.MessageQueue, logDB models.LogSto
 		WithLogstore(logDB),
 		WithAgent(rnr),
 		WithType(nodeType),
+		WithTriggerAnnotator(NewRequestBasedTriggerAnnotator()),
 	)
 }
 

api/server/trigger_annotator.go

@@ -0,0 +1,63 @@
+package server
+
+import (
+	"fmt"
+	"github.com/fnproject/fn/api/models"
+	"github.com/gin-gonic/gin"
+	"strings"
+)
+
+//TriggerAnnotator Is used to inject trigger context (such as request URLs) into outbound trigger resources
+type TriggerAnnotator interface {
+	// Annotates a trigger on read
+	AnnotateTrigger(ctx *gin.Context, a *models.App, t *models.Trigger) (*models.Trigger, error)
+}
+
+type requestBasedTriggerAnnotator struct{}
+
+func annotateTriggerWithBaseUrl(baseURL string, app *models.App, t *models.Trigger) (*models.Trigger, error) {
+	if t.Type != models.TriggerTypeHTTP {
+		return t, nil
+	}
+
+	baseURL = strings.TrimSuffix(baseURL, "/")
+	src := strings.TrimPrefix(t.Source, "/")
+	triggerPath := fmt.Sprintf("%s/t/%s/%s", baseURL, app.Name, src)
+
+	newT := t.Clone()
+	newAnnotations, err := newT.Annotations.With(models.TriggerHTTPEndpointAnnotation, triggerPath)
+	if err != nil {
+		return nil, err
+	}
+	newT.Annotations = newAnnotations
+	return newT, nil
+}
+
+func (tp *requestBasedTriggerAnnotator) AnnotateTrigger(ctx *gin.Context, app *models.App, t *models.Trigger) (*models.Trigger, error) {
+
+	//No, I don't feel good about myself either
+	scheme := "http"
+	if ctx.Request.TLS != nil {
+		scheme = "https"
+	}
+
+	return annotateTriggerWithBaseUrl(fmt.Sprintf("%s://%s", scheme, ctx.Request.Host), app, t)
+}
+
+func NewRequestBasedTriggerAnnotator() TriggerAnnotator {
+	return &requestBasedTriggerAnnotator{}
+}
+
+type staticUrlTriggerAnnotator struct {
+	urlBase string
+}
+
+func NewStaticURLTriggerAnnotator(baseUrl string) TriggerAnnotator {
+
+	return &staticUrlTriggerAnnotator{urlBase: baseUrl}
+}
+
+func (s *staticUrlTriggerAnnotator) AnnotateTrigger(ctx *gin.Context, app *models.App, trigger *models.Trigger) (*models.Trigger, error) {
+	return annotateTriggerWithBaseUrl(s.urlBase, app, trigger)
+
+}

api/server/trigger_annotator_test.go

@@ -0,0 +1,160 @@
+package server
+
+import (
+	bytes2 "bytes"
+	"crypto/tls"
+	"encoding/json"
+	"github.com/fnproject/fn/api/models"
+	"github.com/gin-gonic/gin"
+	"net/http/httptest"
+	"testing"
+)
+
+func TestAnnotateTriggerDefaultProvider(t *testing.T) {
+
+	app := &models.App{
+		ID:   "app_id",
+		Name: "myApp",
+	}
+
+	tr := &models.Trigger{
+		Name:   "myTrigger",
+		Type:   "http",
+		AppID:  app.ID,
+		Source: "/url/to/somewhere",
+	}
+
+	// defaults the trigger endpoint to the base URL if it's not already set
+	tep := NewRequestBasedTriggerAnnotator()
+
+	c, _ := gin.CreateTestContext(httptest.NewRecorder())
+	c.Request = httptest.NewRequest("GET", "/v2/foo/bar", bytes2.NewBuffer([]byte{}))
+	c.Request.Host = "my-server.com:8192"
+	newT, err := tep.AnnotateTrigger(c, app, tr)
+
+	if err != nil {
+		t.Fatalf("expected no error, got %s", err)
+	}
+
+	bytes, got := newT.Annotations.Get(models.TriggerHTTPEndpointAnnotation)
+	if !got {
+		t.Fatalf("Expecting annotation to be present but got %v", newT.Annotations)
+	}
+
+	var annot string
+	err = json.Unmarshal(bytes, &annot)
+	if err != nil {
+		t.Fatalf("Couldn't get annotation")
+	}
+
+	expected := "http://my-server.com:8192/t/myApp/url/to/somewhere"
+	if annot != expected {
+		t.Errorf("expected annotation to be %s but was %s", expected, annot)
+	}
+}
+
+func TestNonHttpTrigger(t *testing.T) {
+	tep := NewRequestBasedTriggerAnnotator()
+
+	c, _ := gin.CreateTestContext(httptest.NewRecorder())
+	c.Request = httptest.NewRequest("GET", "http://foo.com", bytes2.NewBuffer([]byte{}))
+
+	tr := &models.Trigger{
+		Name:   "myTrigger",
+		Type:   "other",
+		AppID:  "",
+		Source: "/url/to/somewhere",
+	}
+
+	newT, err := tep.AnnotateTrigger(c, nil, tr.Clone())
+
+	if err != nil {
+		t.Fatalf("error annotating trigger %s", err)
+	}
+
+	if !newT.Equals(tr) {
+		t.Errorf("expecting non-http  trigger to be ignored")
+	}
+
+}
+
+func TestHttpsTrigger(t *testing.T) {
+
+	app := &models.App{
+		ID:   "app_id",
+		Name: "myApp",
+	}
+
+	tr := &models.Trigger{
+		Name:   "myTrigger",
+		Type:   "http",
+		AppID:  app.ID,
+		Source: "/url/to/somewhere",
+	}
+
+	// defaults the trigger endpoint to the base URL if it's not already set
+	tep := NewRequestBasedTriggerAnnotator()
+
+	c, _ := gin.CreateTestContext(httptest.NewRecorder())
+	c.Request = httptest.NewRequest("GET", "/v2/foo/bar", bytes2.NewBuffer([]byte{}))
+	c.Request.Host = "my-server.com:8192"
+	c.Request.TLS = &tls.ConnectionState{}
+
+	newT, err := tep.AnnotateTrigger(c, app, tr)
+
+	if err != nil {
+		t.Fatalf("expected no error, got %s", err)
+	}
+
+	bytes, got := newT.Annotations.Get(models.TriggerHTTPEndpointAnnotation)
+	if !got {
+		t.Fatalf("Expecting annotation  to be present but got %v", newT.Annotations)
+	}
+	var annot string
+	err = json.Unmarshal(bytes, &annot)
+	if err != nil {
+		t.Fatalf("Couldn't get annotation")
+	}
+
+	expected := "https://my-server.com:8192/t/myApp/url/to/somewhere"
+	if annot != expected {
+		t.Errorf("expected annotation to be %s but was %s", expected, annot)
+	}
+}
+
+func TestStaticUrlTriggerAnnotator(t *testing.T) {
+	a := NewStaticURLTriggerAnnotator("http://foo.bar.com/somewhere")
+
+	app := &models.App{
+		ID:   "app_id",
+		Name: "myApp",
+	}
+
+	tr := &models.Trigger{
+		Name:   "myTrigger",
+		Type:   "http",
+		AppID:  app.ID,
+		Source: "/url/to/somewhere",
+	}
+
+	newT, err := a.AnnotateTrigger(nil, app, tr)
+	if err != nil {
+		t.Fatalf("failed when should hae succeeded: %s", err)
+	}
+
+	bytes, got := newT.Annotations.Get(models.TriggerHTTPEndpointAnnotation)
+	if !got {
+		t.Fatalf("Expecting annotation to be present but got %v", newT.Annotations)
+	}
+	var annot string
+	err = json.Unmarshal(bytes, &annot)
+	if err != nil {
+		t.Fatalf("Couldn't get annotation")
+	}
+
+	expected := "http://foo.bar.com/somewhere/t/myApp/url/to/somewhere"
+	if annot != expected {
+		t.Errorf("expected annotation to be %s but was %s", expected, annot)
+	}
+
+}

api/server/trigger_get.go

@@ -3,6 +3,7 @@ package server
 import (
 	"net/http"
 
+	"fmt"
 	"github.com/fnproject/fn/api"
 	"github.com/gin-gonic/gin"
 )
@@ -11,10 +12,18 @@ func (s *Server) handleTriggerGet(c *gin.Context) {
 	ctx := c.Request.Context()
 
 	trigger, err := s.datastore.GetTriggerByID(ctx, c.Param(api.ParamTriggerID))
+
 	if err != nil {
 		handleErrorResponse(c, err)
 		return
 	}
+	app, err := s.datastore.GetAppByID(ctx, trigger.AppID)
+
+	if err != nil {
+		handleErrorResponse(c, fmt.Errorf("unexpected error - trigger app not available: %s", err))
+	}
+
+	s.triggerAnnotator.AnnotateTrigger(c, app, trigger)
 
 	c.JSON(http.StatusOK, trigger)
 }

api/server/trigger_list.go

@@ -3,6 +3,7 @@ package server
 import (
 	"net/http"
 
+	"fmt"
 	"github.com/fnproject/fn/api/models"
 	"github.com/gin-gonic/gin"
 )
@@ -28,5 +29,32 @@ func (s *Server) handleTriggerList(c *gin.Context) {
 		return
 	}
 
+	// Annotate the outbound triggers
+
+	// this is fairly cludgy bit hard to do in datastore middleware confidently
+	appCache := make(map[string]*models.App)
+	newTriggers := make([]*models.Trigger, len(triggers.Items))
+
+	for idx, t := range triggers.Items {
+		app, ok := appCache[t.AppID]
+		if !ok {
+			gotApp, err := s.Datastore().GetAppByID(ctx, t.AppID)
+			if err != nil {
+				handleErrorResponse(c, fmt.Errorf("failed to get app for trigger %s", err))
+				return
+			}
+			app = gotApp
+			appCache[app.ID] = gotApp
+		}
+
+		newT, err := s.triggerAnnotator.AnnotateTrigger(c, app, t)
+		if err != nil {
+			handleErrorResponse(c, err)
+			return
+		}
+		newTriggers[idx] = newT
+	}
+
+	triggers.Items = newTriggers
 	c.JSON(http.StatusOK, triggers)
 }

api/server/trigger_test.go

@@ -275,10 +275,10 @@ func TestTriggerGet(t *testing.T) {
 
 	a := &models.App{ID: "appid"}
 
-	fn := &models.Fn{ID: "fnid"}
+	fn := &models.Fn{ID: "fnid", AppID: a.ID}
 	fn.SetDefaults()
 
-	trig := &models.Trigger{ID: "triggerid"}
+	trig := &models.Trigger{ID: "triggerid", FnID: fn.ID, AppID: a.ID}
 	commonDS := datastore.NewMockInit([]*models.App{a}, []*models.Fn{fn}, []*models.Trigger{trig})
 
 	for i, test := range []struct {

api/triggers/triggers.go

@@ -0,0 +1 @@
+package triggers

test/fn-system-tests/system_test.go

@@ -184,6 +184,7 @@ func SetUpAPINode(ctx context.Context) (*server.Server, error) {
 	opts = append(opts, server.WithMQURL(getEnv(server.EnvMQURL, defaultMQ)))
 	opts = append(opts, server.WithLogURL(""))
 	opts = append(opts, server.WithLogstoreFromDatastore())
+	opts = append(opts, server.WithTriggerAnnotator(server.NewStaticURLTriggerAnnotator("http://localhost:8081")))
 	opts = append(opts, server.EnableShutdownEndpoint(ctx, func() {})) // TODO: do it properly
 	return server.New(ctx, opts...), nil
 }