Insist trigger sources are prefixed (#1184)

* Insist trigger sources are prefixed

All trigger sources must have a '/' prefix to be allowed into the datastore.

* Adding condition to novelValue for gen tests

NovelValue was failing to detect same Config values correctly. This
adds a specific check for Config, like the one for Annotation, to
ensure a novel value is indeed generated.
This commit is contained in:
Tom Coupland
2018-08-23 12:24:56 +01:00
committed by GitHub
parent df94333cf4
commit fc3f54d2da
5 changed files with 32 additions and 15 deletions

View File

@@ -1286,7 +1286,7 @@ func RunTriggersTest(t *testing.T, dsf DataStoreFunc, rp ResourceProvider) {
for i := 0; i < 10; i++ { for i := 0; i < 10; i++ {
trigger := rp.ValidTrigger(testApp.ID, testFn.ID) trigger := rp.ValidTrigger(testApp.ID, testFn.ID)
trigger.Source = fmt.Sprintf("src_%v", i) trigger.Source = fmt.Sprintf("/src_%v", i)
storedTriggers = append(storedTriggers, h.GivenTriggerInDb(trigger)) storedTriggers = append(storedTriggers, h.GivenTriggerInDb(trigger))
} }
@@ -1397,12 +1397,12 @@ func RunTriggersTest(t *testing.T, dsf DataStoreFunc, rp ResourceProvider) {
for i := 0; i < 10; i++ { for i := 0; i < 10; i++ {
trigger := rp.ValidTrigger(testApp.ID, testFn.ID) trigger := rp.ValidTrigger(testApp.ID, testFn.ID)
trigger.Source = fmt.Sprintf("src_%v", i) trigger.Source = fmt.Sprintf("/src_%v", i)
storedTriggers = append(storedTriggers, h.GivenTriggerInDb(trigger)) storedTriggers = append(storedTriggers, h.GivenTriggerInDb(trigger))
} }
trigger := rp.ValidTrigger(testApp.ID, testFn2.ID) trigger := rp.ValidTrigger(testApp.ID, testFn2.ID)
trigger.Source = fmt.Sprintf("src_%v", 11) trigger.Source = fmt.Sprintf("/src_%v", 11)
trigger = h.GivenTriggerInDb(trigger) trigger = h.GivenTriggerInDb(trigger)
storedTriggers = append(storedTriggers, trigger) storedTriggers = append(storedTriggers, trigger)
@@ -1458,7 +1458,7 @@ func RunTriggersTest(t *testing.T, dsf DataStoreFunc, rp ResourceProvider) {
testTrigger := h.GivenTriggerInDb(rp.ValidTrigger(testApp.ID, testFn.ID)) testTrigger := h.GivenTriggerInDb(rp.ValidTrigger(testApp.ID, testFn.ID))
testTrigger.Name = "newName" testTrigger.Name = "newName"
testTrigger.Source = "newSource" testTrigger.Source = "/newSource"
time.Sleep(10 * time.Millisecond) time.Sleep(10 * time.Millisecond)
gotTrigger, err := ds.UpdateTrigger(ctx, testTrigger) gotTrigger, err := ds.UpdateTrigger(ctx, testTrigger)
@@ -1557,7 +1557,7 @@ func RunTriggerBySourceTests(t *testing.T, dsf DataStoreFunc, rp ResourceProvide
ds := dsf(t) ds := dsf(t)
ctx := rp.DefaultCtx() ctx := rp.DefaultCtx()
t.Run("get_non_existant_trigger", func(t *testing.T) { t.Run("get_non_existant_trigger", func(t *testing.T) {
_, err := ds.GetTriggerBySource(ctx, "none", "http", "source") _, err := ds.GetTriggerBySource(ctx, "none", "http", "/source")
if err != models.ErrTriggerNotFound { if err != models.ErrTriggerNotFound {
t.Fatalf("Expecting trigger not found, got %s", err) t.Fatalf("Expecting trigger not found, got %s", err)
} }

View File

@@ -20,7 +20,9 @@ func appReflectType() reflect.Type {
} }
func configGenerator() gopter.Gen { func configGenerator() gopter.Gen {
return gen.MapOf(gen.AlphaString(), gen.AlphaString()) return gen.MapOf(gen.AlphaString(), gen.AlphaString()).Map(func(m map[string]string) Config {
return Config(m)
})
} }
func annotationGenerator() gopter.Gen { func annotationGenerator() gopter.Gen {
@@ -75,6 +77,10 @@ func novelValue(t *testing.T, originalInstance reflect.Value, fieldName string,
if !newValue.(Annotations).Equals(currentValue.(Annotations)) { if !newValue.(Annotations).Equals(currentValue.(Annotations)) {
break break
} }
} else if fieldName == "Config" {
if !newValue.(Config).Equals(currentValue.(Config)) {
break
}
} else { } else {
if newValue != currentValue { if newValue != currentValue {
break break

View File

@@ -4,6 +4,7 @@ import (
"errors" "errors"
"fmt" "fmt"
"net/http" "net/http"
"strings"
"time" "time"
"unicode" "unicode"
@@ -119,6 +120,10 @@ var (
ErrTriggerMissingSource = err{ ErrTriggerMissingSource = err{
code: http.StatusBadRequest, code: http.StatusBadRequest,
error: errors.New("Missing Trigger Source")} error: errors.New("Missing Trigger Source")}
//ErrTriggerMissingSourcePrefix - source does not have a / prefix
ErrTriggerMissingSourcePrefix = err{
code: http.StatusBadRequest,
error: errors.New("Missing Trigger Source Prefix '/'")}
//ErrTriggerNotFound - trigger not found //ErrTriggerNotFound - trigger not found
ErrTriggerNotFound = err{ ErrTriggerNotFound = err{
code: http.StatusNotFound, code: http.StatusNotFound,
@@ -164,6 +169,10 @@ func (t *Trigger) Validate() error {
return ErrTriggerMissingSource return ErrTriggerMissingSource
} }
if !strings.HasPrefix(t.Source, "/") {
return ErrTriggerMissingSourcePrefix
}
err := t.Annotations.Validate() err := t.Annotations.Validate()
if err != nil { if err != nil {
return err return err

View File

@@ -44,8 +44,8 @@ func TestTriggerListJsonMarshalling(t *testing.T) {
} }
} }
var httpTrigger = &Trigger{Name: "name", AppID: "foo", FnID: "bar", Type: "http", Source: "baz"} var httpTrigger = &Trigger{Name: "name", AppID: "foo", FnID: "bar", Type: "http", Source: "/baz"}
var invalidTrigger = &Trigger{Name: "name", AppID: "foo", FnID: "bar", Type: "error", Source: "baz"} var invalidTrigger = &Trigger{Name: "name", AppID: "foo", FnID: "bar", Type: "error", Source: "/baz"}
var triggerValidateCases = []struct { var triggerValidateCases = []struct {
val *Trigger val *Trigger

View File

@@ -51,20 +51,21 @@ func TestTriggerCreate(t *testing.T) {
{commonDS, logs.NewMock(), BaseRoute, `{"app_id":"appid", "fn_id":"fnid", "name": "Test" }`, http.StatusBadRequest, models.ErrTriggerTypeUnknown}, {commonDS, logs.NewMock(), BaseRoute, `{"app_id":"appid", "fn_id":"fnid", "name": "Test" }`, http.StatusBadRequest, models.ErrTriggerTypeUnknown},
{commonDS, logs.NewMock(), BaseRoute, `{ "name": "Test", "app_id": "appid", "fn_id": "fnid", "type":"http"}`, http.StatusBadRequest, models.ErrTriggerMissingSource}, {commonDS, logs.NewMock(), BaseRoute, `{ "name": "Test", "app_id": "appid", "fn_id": "fnid", "type":"http"}`, http.StatusBadRequest, models.ErrTriggerMissingSource},
{commonDS, logs.NewMock(), BaseRoute, `{ "name": "Test", "app_id": "appid", "fn_id": "fnid", "type":"http", "source":"src"}`, http.StatusBadRequest, models.ErrTriggerMissingSourcePrefix},
{commonDS, logs.NewMock(), BaseRoute, `{ "name": "1234567890123456789012345678901", "app_id": "appid", "fn_id": "fnid", "type":"http"}`, http.StatusBadRequest, models.ErrTriggerTooLongName}, {commonDS, logs.NewMock(), BaseRoute, `{ "name": "1234567890123456789012345678901", "app_id": "appid", "fn_id": "fnid", "type":"http"}`, http.StatusBadRequest, models.ErrTriggerTooLongName},
{commonDS, logs.NewMock(), BaseRoute, `{ "name": "&&%@!#$#@$","app_id": "appid", "fn_id": "fnid", "type":"http" }`, http.StatusBadRequest, models.ErrTriggerInvalidName}, {commonDS, logs.NewMock(), BaseRoute, `{ "name": "&&%@!#$#@$","app_id": "appid", "fn_id": "fnid", "type":"http" }`, http.StatusBadRequest, models.ErrTriggerInvalidName},
{commonDS, logs.NewMock(), BaseRoute, `{ "name": "trigger", "app_id": "appid", "fn_id": "fnid", "type": "http", "source": "src", "annotations" : { "":"val" }}`, http.StatusBadRequest, models.ErrInvalidAnnotationKey}, {commonDS, logs.NewMock(), BaseRoute, `{ "name": "trigger", "app_id": "appid", "fn_id": "fnid", "type": "http", "source": "/src", "annotations" : { "":"val" }}`, http.StatusBadRequest, models.ErrInvalidAnnotationKey},
{commonDS, logs.NewMock(), BaseRoute, `{ "id": "asdasca", "name": "trigger", "app_id": "appid", "fn_id": "fnid", "type": "http", "source": "src"}`, http.StatusBadRequest, models.ErrTriggerIDProvided}, {commonDS, logs.NewMock(), BaseRoute, `{ "id": "asdasca", "name": "trigger", "app_id": "appid", "fn_id": "fnid", "type": "http", "source": "/src"}`, http.StatusBadRequest, models.ErrTriggerIDProvided},
{commonDS, logs.NewMock(), BaseRoute, `{ "name": "trigger", "app_id": "appid", "fn_id": "fnid", "type": "unsupported", "source": "src"}`, http.StatusBadRequest, models.ErrTriggerTypeUnknown}, {commonDS, logs.NewMock(), BaseRoute, `{ "name": "trigger", "app_id": "appid", "fn_id": "fnid", "type": "unsupported", "source": "/src"}`, http.StatusBadRequest, models.ErrTriggerTypeUnknown},
{commonDS, logs.NewMock(), BaseRoute, `{ "name": "trigger", "app_id": "appid2", "fn_id": "fnid", "type": "http", "source": "src"}`, http.StatusBadRequest, models.ErrTriggerFnIDNotSameApp}, {commonDS, logs.NewMock(), BaseRoute, `{ "name": "trigger", "app_id": "appid2", "fn_id": "fnid", "type": "http", "source": "/src"}`, http.StatusBadRequest, models.ErrTriggerFnIDNotSameApp},
// // success // // success
{commonDS, logs.NewMock(), BaseRoute, `{ "name": "trigger", "app_id": "appid", "fn_id": "fnid", "type": "http", "source": "src"}`, http.StatusOK, nil}, {commonDS, logs.NewMock(), BaseRoute, `{ "name": "trigger", "app_id": "appid", "fn_id": "fnid", "type": "http", "source": "/src"}`, http.StatusOK, nil},
//repeated name //repeated name
{commonDS, logs.NewMock(), BaseRoute, `{ "name": "trigger", "app_id": "appid", "fn_id": "fnid", "type": "http", "source": "src"}`, http.StatusConflict, nil}, {commonDS, logs.NewMock(), BaseRoute, `{ "name": "trigger", "app_id": "appid", "fn_id": "fnid", "type": "http", "source": "/src"}`, http.StatusConflict, nil},
} { } {
rnr, cancel := testRunner(t) rnr, cancel := testRunner(t)
@@ -402,7 +403,7 @@ func TestTriggerUpdate(t *testing.T) {
AppID: "appid", AppID: "appid",
FnID: "fnid", FnID: "fnid",
Type: "http", Type: "http",
Source: "source"} Source: "/source"}
commonDS := datastore.NewMockInit([]*models.App{a}, []*models.Fn{fn}, []*models.Trigger{trig}) commonDS := datastore.NewMockInit([]*models.App{a}, []*models.Fn{fn}, []*models.Trigger{trig})
@@ -419,6 +420,7 @@ func TestTriggerUpdate(t *testing.T) {
{commonDS, logs.NewMock(), BaseRoute + "/notexist", `{"id": "notexist", "name":"changed"}`, "", http.StatusNotFound, nil}, {commonDS, logs.NewMock(), BaseRoute + "/notexist", `{"id": "notexist", "name":"changed"}`, "", http.StatusNotFound, nil},
{commonDS, logs.NewMock(), BaseRoute + "/triggerid", `{"id": "nonmatching", "name":"changed}`, "", http.StatusBadRequest, models.ErrTriggerIDMismatch}, {commonDS, logs.NewMock(), BaseRoute + "/triggerid", `{"id": "nonmatching", "name":"changed}`, "", http.StatusBadRequest, models.ErrTriggerIDMismatch},
{commonDS, logs.NewMock(), BaseRoute + "/triggerid", `{"id": "triggerid", "name":"changed"}`, "changed", http.StatusOK, nil}, {commonDS, logs.NewMock(), BaseRoute + "/triggerid", `{"id": "triggerid", "name":"changed"}`, "changed", http.StatusOK, nil},
{commonDS, logs.NewMock(), BaseRoute + "/triggerid", `{"id": "triggerid", "source":"noprefix"}`, "changed", http.StatusBadRequest, nil},
{commonDS, logs.NewMock(), BaseRoute + "/triggerid", `{"name":"again"}`, "again", http.StatusOK, nil}, {commonDS, logs.NewMock(), BaseRoute + "/triggerid", `{"name":"again"}`, "again", http.StatusOK, nil},
} { } {
rnr, cancel := testRunner(t) rnr, cancel := testRunner(t)