increase app/fn/trigger length caps (#1465)

rdallman · web-flow · commit cdfc3603d9de · 2019-04-09T12:43:49.000-07:00
was 30, now 255. the service validates the same value, via a different method,
for each of these already. sql uses varchar 256 so we're good. can't think of
any other ways this might break, so going for it.
diff --git a/api/models/app.go b/api/models/app.go
@@ -31,7 +31,7 @@ var (
 	}
 	ErrAppsTooLongName = err{
 		code:  http.StatusBadRequest,
-		error: fmt.Errorf("App name must be %v characters or less", maxAppName),
+		error: fmt.Errorf("App name must be %v characters or less", MaxLengthAppName),
 	}
 	ErrAppsInvalidName = err{
 		code:  http.StatusBadRequest,
@@ -98,7 +98,7 @@ func (a *App) ValidateName() error {
 		return ErrMissingName
 	}
 
-	if len(a.Name) > maxAppName {
+	if len(a.Name) > MaxLengthAppName {
 		return ErrAppsTooLongName
 	}
 
diff --git a/api/models/app_test.go b/api/models/app_test.go
@@ -149,12 +149,17 @@ func TestAppEquality(t *testing.T) {
 }
 
 func TestValidateAppName(t *testing.T) {
+	tooLongName := "7"
+	for i := 0; i < MaxLengthAppName+1; i++ {
+		tooLongName += "7"
+	}
+
 	testCases := []struct {
 		Name string
 		Want error
 	}{
 		{"valid_name-101", nil},
-		{"an_app_with_a_name_that_is_too_long", ErrAppsTooLongName},
+		{tooLongName, ErrAppsTooLongName},
 		{"", ErrMissingName},
 		{"invalid.character", ErrAppsInvalidName},
 	}
diff --git a/api/models/error.go b/api/models/error.go
@@ -6,11 +6,13 @@ import (
 	"net/http"
 )
 
-// TODO we can put constants all in this file too
 const (
-	maxAppName     = 30
-	maxFnName      = 30
-	MaxTriggerName = 30
+	// MaxLengthAppName is the max length for an app name
+	MaxLengthAppName = 255
+	// MaxLengthFnName is the max length for an fn name
+	MaxLengthFnName = 255
+	// MaxLengthTriggerName is the max length for a trigger name
+	MaxLengthTriggerName = 255
 )
 
 var (
diff --git a/api/models/fn.go b/api/models/fn.go
@@ -44,7 +44,7 @@ var (
 	}
 	ErrFnsTooLongName = err{
 		code:  http.StatusBadRequest,
-		error: fmt.Errorf("Fn name must be %v characters or less", maxFnName),
+		error: fmt.Errorf("Fn name must be %v characters or less", MaxLengthFnName),
 	}
 	ErrFnsMissingAppID = err{
 		code:  http.StatusBadRequest,
@@ -178,7 +178,7 @@ func (f *Fn) ValidateName() error {
 		return ErrFnsMissingName
 	}
 
-	if len(f.Name) > maxFnName {
+	if len(f.Name) > MaxLengthFnName {
 		return ErrFnsTooLongName
 	}
 
diff --git a/api/models/fn_test.go b/api/models/fn_test.go
@@ -108,13 +108,18 @@ func TestFnEquality(t *testing.T) {
 }
 
 func TestValidateFnName(t *testing.T) {
+	tooLongName := "7"
+	for i := 0; i < MaxLengthFnName+1; i++ {
+		tooLongName += "7"
+	}
+
 	testCases := []struct {
 		Name string
 		Want error
 	}{
 		{"valid_name-101", nil},
 		{"unescaped/path", ErrFnsInvalidName},
-		{"a_function_with_a_name_that_is_too_long", ErrFnsTooLongName},
+		{tooLongName, ErrFnsTooLongName},
 		{"", ErrFnsMissingName},
 	}
 
diff --git a/api/models/trigger.go b/api/models/trigger.go
@@ -95,7 +95,7 @@ var (
 	//ErrTriggerTooLongName - name exceeds maximum permitted name
 	ErrTriggerTooLongName = err{
 		code:  http.StatusBadRequest,
-		error: fmt.Errorf("Trigger name must be %v characters or less", MaxTriggerName)}
+		error: fmt.Errorf("Trigger name must be %v characters or less", MaxLengthTriggerName)}
 	//ErrTriggerInvalidName - name does not comply with naming spec
 	ErrTriggerInvalidName = err{
 		code:  http.StatusBadRequest,
@@ -177,7 +177,7 @@ func (t *Trigger) ValidateName() error {
 		return ErrTriggerMissingName
 	}
 
-	if len(t.Name) > MaxTriggerName {
+	if len(t.Name) > MaxLengthTriggerName {
 		return ErrTriggerTooLongName
 	}
 
diff --git a/api/models/trigger_test.go b/api/models/trigger_test.go
@@ -149,12 +149,17 @@ func TestTriggerEquality(t *testing.T) {
 }
 
 func TestValidateTriggerName(t *testing.T) {
+	tooLongName := "7"
+	for i := 0; i < MaxLengthTriggerName+1; i++ {
+		tooLongName += "7"
+	}
+
 	testCases := []struct {
 		Name string
 		Want error
 	}{
 		{"valid_name-101", nil},
-		{"a_trigger_with_a_name_that_is_too_long", ErrTriggerTooLongName},
+		{tooLongName, ErrTriggerTooLongName},
 		{"", ErrTriggerMissingName},
 		{"invalid.character", ErrTriggerInvalidName},
 	}
diff --git a/api/server/apps_test.go b/api/server/apps_test.go
@@ -5,13 +5,13 @@ import (
 	"encoding/base64"
 	"encoding/json"
 	"errors"
+	"fmt"
 	"log"
 	"net/http"
 	"strings"
 	"testing"
 	"time"
 
-	"fmt"
 	"github.com/fnproject/fn/api/datastore"
 	"github.com/fnproject/fn/api/models"
 	"github.com/gin-gonic/gin"
@@ -36,6 +36,11 @@ func TestAppCreate(t *testing.T) {
 		}
 	}()
 
+	tooLongName := "7"
+	for i := 0; i < models.MaxLengthAppName+1; i++ {
+		tooLongName += "7"
+	}
+
 	for i, test := range []struct {
 		mock          models.Datastore
 		path          string
@@ -48,7 +53,7 @@ func TestAppCreate(t *testing.T) {
 		{datastore.NewMock(), "/v2/apps", `{}`, http.StatusBadRequest, models.ErrMissingName},
 		{datastore.NewMock(), "/v2/apps", `{"name": "app", "id":"badId"}`, http.StatusBadRequest, models.ErrAppIDProvided},
 		{datastore.NewMock(), "/v2/apps", `{ "name": "" }`, http.StatusBadRequest, models.ErrMissingName},
-		{datastore.NewMock(), "/v2/apps", `{"name": "1234567890123456789012345678901" }`, http.StatusBadRequest, models.ErrAppsTooLongName},
+		{datastore.NewMock(), "/v2/apps", fmt.Sprintf(`{"name": "%s" }`, tooLongName), http.StatusBadRequest, models.ErrAppsTooLongName},
 		{datastore.NewMock(), "/v2/apps", `{ "name": "&&%@!#$#@$" }`, http.StatusBadRequest, models.ErrAppsInvalidName},
 		{datastore.NewMock(), "/v2/apps", `{ "name": "app", "annotations" : { "":"val" }}`, http.StatusBadRequest, models.ErrInvalidAnnotationKey},
 		{datastore.NewMock(), "/v2/apps", `{"name": "app", "annotations" : { "key":"" }}`, http.StatusBadRequest, models.ErrInvalidAnnotationValue},
diff --git a/api/server/fns_test.go b/api/server/fns_test.go
@@ -83,6 +83,11 @@ func (test *funcTestCase) run(t *testing.T, i int, buf *bytes.Buffer) {
 func TestFnCreate(t *testing.T) {
 	buf := setLogBuffer()
 
+	tooLongName := "7"
+	for i := 0; i < models.MaxLengthAppName+1; i++ {
+		tooLongName += "7"
+	}
+
 	a := &models.App{Name: "a", ID: "aid"}
 	ds := datastore.NewMockInit([]*models.App{a})
 	for i, test := range []funcTestCase{
@@ -92,6 +97,7 @@ func TestFnCreate(t *testing.T) {
 		{ds, http.MethodPost, "/v2/fns", fmt.Sprintf(`{ "app_id": "%s" }`, a.ID), http.StatusBadRequest, models.ErrFnsMissingName},
 		{ds, http.MethodPost, "/v2/fns", fmt.Sprintf(`{ "app_id": "%s", "name": "a" }`, a.ID), http.StatusBadRequest, models.ErrFnsMissingImage},
 		{ds, http.MethodPost, "/v2/fns", fmt.Sprintf(`{ "app_id": "%s", "name": " ", "image": "fnproject/fn-test-utils" }`, a.ID), http.StatusBadRequest, models.ErrFnsInvalidName},
+		{ds, http.MethodPost, "/v2/fns", fmt.Sprintf(`{ "app_id": "%s", "name": "%s", "image": "fnproject/fn-test-utils" }`, a.ID, tooLongName), http.StatusBadRequest, models.ErrFnsTooLongName},
 		{ds, http.MethodPost, "/v2/fns", fmt.Sprintf(`{ "app_id": "%s", "name": "a", "image": "fnproject/fn-test-utils", "timeout": 3601 }`, a.ID), http.StatusBadRequest, models.ErrFnsInvalidTimeout},
 		{ds, http.MethodPost, "/v2/fns", fmt.Sprintf(`{ "app_id": "%s", "name": "a", "image": "fnproject/fn-test-utils", "idle_timeout": 3601 }`, a.ID), http.StatusBadRequest, models.ErrFnsInvalidIdleTimeout},
 		{ds, http.MethodPost, "/v2/fns", fmt.Sprintf(`{ "app_id": "%s", "name": "a", "image": "fnproject/fn-test-utils", "memory": 100000000000000 }`, a.ID), http.StatusBadRequest, models.ErrInvalidMemory},
diff --git a/api/server/trigger_test.go b/api/server/trigger_test.go
@@ -4,6 +4,7 @@ import (
 	"bytes"
 	"encoding/base64"
 	"encoding/json"
+	"fmt"
 	"net/http"
 	"strings"
 	"testing"
@@ -33,6 +34,11 @@ func TestTriggerCreate(t *testing.T) {
 	fn.SetDefaults()
 	commonDS := datastore.NewMockInit([]*models.App{a, a2}, []*models.Fn{fn})
 
+	tooLongName := "7"
+	for i := 0; i < models.MaxLengthTriggerName+1; i++ {
+		tooLongName += "7"
+	}
+
 	for i, test := range []struct {
 		mock          models.Datastore
 		path          string
@@ -50,7 +56,7 @@ func TestTriggerCreate(t *testing.T) {
 		{commonDS, BaseRoute, `{ "name": "Test", "app_id": "appid", "fn_id": "fnid", "type":"http"}`, http.StatusBadRequest, models.ErrTriggerMissingSource},
 		{commonDS, BaseRoute, `{ "name": "Test", "app_id": "appid", "fn_id": "fnid", "type":"http", "source":"src"}`, http.StatusBadRequest, models.ErrTriggerMissingSourcePrefix},
 
-		{commonDS, BaseRoute, `{ "name": "1234567890123456789012345678901", "app_id": "appid", "fn_id": "fnid", "type":"http"}`, http.StatusBadRequest, models.ErrTriggerTooLongName},
+		{commonDS, BaseRoute, fmt.Sprintf(`{ "name": "%s", "app_id": "appid", "fn_id": "fnid", "type":"http"}`, tooLongName), http.StatusBadRequest, models.ErrTriggerTooLongName},
 		{commonDS, BaseRoute, `{ "name": "&&%@!#$#@$","app_id": "appid", "fn_id": "fnid", "type":"http" }`, http.StatusBadRequest, models.ErrTriggerInvalidName},
 		{commonDS, BaseRoute, `{ "name": "trigger", "app_id": "appid", "fn_id": "fnid", "type": "http", "source": "/src", "annotations" : { "":"val" }}`, http.StatusBadRequest, models.ErrInvalidAnnotationKey},
 		{commonDS, BaseRoute, `{ "id": "asdasca", "name": "trigger", "app_id": "appid", "fn_id": "fnid", "type": "http", "source": "/src"}`, http.StatusBadRequest, models.ErrTriggerIDProvided},

Original file line number	Diff line number	Diff line change
`@@ -31,7 +31,7 @@ var (`
`31`	`31`	`}`
`32`	`32`	`ErrAppsTooLongName = err{`
`33`	`33`	`code: http.StatusBadRequest,`
`34`		`- error: fmt.Errorf("App name must be %v characters or less", maxAppName),`
	`34`	`+ error: fmt.Errorf("App name must be %v characters or less", MaxLengthAppName),`
`35`	`35`	`}`
`36`	`36`	`ErrAppsInvalidName = err{`
`37`	`37`	`code: http.StatusBadRequest,`
`@@ -98,7 +98,7 @@ func (a *App) ValidateName() error {`
`98`	`98`	`return ErrMissingName`
`99`	`99`	`}`
`100`	`100`
`101`		`- if len(a.Name) > maxAppName {`
	`101`	`+ if len(a.Name) > MaxLengthAppName {`
`102`	`102`	`return ErrAppsTooLongName`
`103`	`103`	`}`
`104`	`104`
Original file line number	Diff line number	Diff line change
`@@ -44,7 +44,7 @@ var (`
`44`	`44`	`}`
`45`	`45`	`ErrFnsTooLongName = err{`
`46`	`46`	`code: http.StatusBadRequest,`
`47`		`- error: fmt.Errorf("Fn name must be %v characters or less", maxFnName),`
	`47`	`+ error: fmt.Errorf("Fn name must be %v characters or less", MaxLengthFnName),`
`48`	`48`	`}`
`49`	`49`	`ErrFnsMissingAppID = err{`
`50`	`50`	`code: http.StatusBadRequest,`
`@@ -178,7 +178,7 @@ func (f *Fn) ValidateName() error {`
`178`	`178`	`return ErrFnsMissingName`
`179`	`179`	`}`
`180`	`180`
`181`		`- if len(f.Name) > maxFnName {`
	`181`	`+ if len(f.Name) > MaxLengthFnName {`
`182`	`182`	`return ErrFnsTooLongName`
`183`	`183`	`}`
`184`	`184`