Support for multiple HTTP verbs in routes/HTTPTrigger (#2064)

* Support for multiple HTTP verbs in routes/HTTPTrigger

Signed-off-by: Sanket Sudake <sanketsudake@gmail.com>

* Update pkg/apis/core/v1/types.go

Co-authored-by: Harsh Thakur <harshthakur9030@gmail.com>

* Fix fallbackurl for ingress

Signed-off-by: Sanket Sudake <sanketsudake@gmail.com>

Co-authored-by: Harsh Thakur <harshthakur9030@gmail.com>

cmd/builder/Dockerfile.fission-builder

@@ -1,4 +1,4 @@
-FROM golang:1.14-alpine as godep
+FROM golang:1.15-alpine as godep
 RUN apk add bash ca-certificates git gcc g++ libc-dev
 
 ARG GOPKG=github.com/fission/fission

crds/v1/fission.io_httptriggers.yaml

@@ -77,8 +77,13 @@ spec:
                     type: string
                 type: object
               method:
-                description: HTTP method to access a function.
+                description: 'Deprecated: Use Methods instead of Method. HTTP method to access a function.'
                 type: string
+              methods:
+                description: HTTP methods to access a function
+                items:
+                  type: string
+                type: array
               prefix:
                 description: 'Prefix with which functions are exposed. NOTE: Prefix takes precedence over URL/RelativeURL. Note that it does not treat slashes specially ("/foobar/" will be matched by the prefix "/foobar").'
                 type: string

pkg/apis/core/v1/types.go

@@ -658,10 +658,15 @@ type (
 		// +optional
 		Prefix *string `json:"prefix,omitempty"`
 
+		// Deprecated: Use Methods instead of Method.
 		// HTTP method to access a function.
 		// +optional
 		Method string `json:"method"`
 
+		// HTTP methods to access a function
+		// +optional
+		Methods []string `json:"methods,omitempty"`
+
 		// FunctionReference is a reference to the target function.
 		FunctionReference FunctionReference `json:"functionref"`
 

pkg/apis/core/v1/validation.go

@@ -400,12 +400,23 @@ func (spec EnvironmentSpec) Validate() error {
 
 func (spec HTTPTriggerSpec) Validate() error {
 	result := &multierror.Error{}
+	checkMethod := func(method string, result *multierror.Error) *multierror.Error {
+		switch method {
+		case http.MethodGet, http.MethodHead, http.MethodPost, http.MethodPut, http.MethodPatch,
+			http.MethodDelete, http.MethodConnect, http.MethodOptions, http.MethodTrace: // no op
+		default:
+			result = multierror.Append(result, MakeValidationErr(ErrorUnsupportedType, "HTTPTriggerSpec.Method", spec.Method, "not a valid HTTP method"))
+		}
+		return result
+	}
+	if len(spec.Methods) > 0 {
+		for _, method := range spec.Methods {
+			result = checkMethod(method, result)
+		}
+	}
 
-	switch spec.Method {
-	case http.MethodGet, http.MethodHead, http.MethodPost, http.MethodPut, http.MethodPatch,
-		http.MethodDelete, http.MethodConnect, http.MethodOptions, http.MethodTrace: // no op
-	default:
-		result = multierror.Append(result, MakeValidationErr(ErrorUnsupportedType, "HTTPTriggerSpec.Method", spec.Method, "not a valid HTTP method"))
+	if len(spec.Method) > 0 {
+		result = checkMethod(spec.Method, result)
 	}
 
 	result = multierror.Append(result, spec.FunctionReference.Validate())

pkg/apis/core/v1/zz_generated.deepcopy.go

@@ -512,6 +512,11 @@ func (in *HTTPTriggerSpec) DeepCopyInto(out *HTTPTriggerSpec) {
 		*out = new(string)
 		**out = **in
 	}
+	if in.Methods != nil {
+		in, out := &in.Methods, &out.Methods
+		*out = make([]string, len(*in))
+		copy(*out, *in)
+	}
 	in.FunctionReference.DeepCopyInto(&out.FunctionReference)
 	in.IngressConfig.DeepCopyInto(&out.IngressConfig)
 	return

pkg/canaryconfigmgr/canaryConfigMgr.go

@@ -282,9 +282,21 @@ func (canaryCfgMgr *canaryConfigMgr) RollForwardOrBack(canaryConfig *fv1.CanaryC
 		} else {
 			urlPath = triggerObj.Spec.RelativeURL
 		}
-		failurePercent, err := canaryCfgMgr.promClient.GetFunctionFailurePercentage(urlPath, triggerObj.Spec.Method,
+		methods := triggerObj.Spec.Methods
+		if len(triggerObj.Spec.Method) > 0 {
+			present := false
+			for _, m := range triggerObj.Spec.Methods {
+				if m == triggerObj.Spec.Method {
+					present = true
+					break
+				}
+			}
+			if !present {
+				methods = append(methods, triggerObj.Spec.Method)
+			}
+		}
+		failurePercent, err := canaryCfgMgr.promClient.GetFunctionFailurePercentage(urlPath, methods,
 			canaryConfig.Spec.NewFunction, canaryConfig.ObjectMeta.Namespace, canaryConfig.Spec.WeightIncrementDuration)
-
 		if err != nil {
 			// silently ignore. wait for next window to increment weight
 			canaryCfgMgr.logger.Error("error calculating failure percentage",

pkg/canaryconfigmgr/prometheusClient.go

@@ -51,21 +51,28 @@ func MakePrometheusClient(logger *zap.Logger, prometheusSvc string) (*Prometheus
 	}, nil
 }
 
-func (promApiClient *PrometheusApiClient) GetFunctionFailurePercentage(path, method, funcName, funcNs string, window string) (float64, error) {
+func (promApiClient *PrometheusApiClient) GetFunctionFailurePercentage(path string, methods []string, funcName, funcNs string, window string) (float64, error) {
+	var reqs, failedReqs float64
 	// first get a total count of requests to this url in a time window
-	reqs, err := promApiClient.GetRequestsToFuncInWindow(path, method, funcName, funcNs, window)
-	if err != nil {
-		return 0, err
+	for _, method := range methods {
+		mreqs, err := promApiClient.GetRequestsToFuncInWindow(path, method, funcName, funcNs, window)
+		if err != nil {
+			return 0, err
+		}
+		reqs += mreqs
 	}
 
 	if reqs <= 0 {
-		return -1, fmt.Errorf("no requests to this url %v and method %v in the window: %v", path, method, window)
+		return -1, fmt.Errorf("no requests to this url %v and method %v in the window: %v", path, methods, window)
 	}
 
 	// next, get a total count of errored out requests to this function in the same window
-	failedReqs, err := promApiClient.GetTotalFailedRequestsToFuncInWindow(funcName, funcNs, path, method, window)
-	if err != nil {
-		return 0, err
+	for _, method := range methods {
+		mfailedReqs, err := promApiClient.GetTotalFailedRequestsToFuncInWindow(funcName, funcNs, path, method, window)
+		if err != nil {
+			return 0, err
+		}
+		failedReqs += mfailedReqs
 	}
 
 	// calculate the failure percentage of the function

pkg/controller/api_test.go

@@ -158,7 +158,7 @@ func TestHTTPTriggerApi(t *testing.T) {
 			Namespace: testNS,
 		},
 		Spec: fv1.HTTPTriggerSpec{
-			Method:      http.MethodGet,
+			Methods:     []string{http.MethodGet},
 			RelativeURL: "/hello",
 			FunctionReference: fv1.FunctionReference{
 				Type: fv1.FunctionReferenceTypeFunctionName,
@@ -181,7 +181,7 @@ func TestHTTPTriggerApi(t *testing.T) {
 
 	tr, err := g.Client().V1().HTTPTrigger().Get(m)
 	panicIf(err)
-	assert(testTrigger.Spec.Method == tr.Spec.Method &&
+	assert(len(testTrigger.Spec.Methods) == len(tr.Spec.Methods) &&
 		testTrigger.Spec.RelativeURL == tr.Spec.RelativeURL &&
 		testTrigger.Spec.FunctionReference.Type == tr.Spec.FunctionReference.Type &&
 		testTrigger.Spec.FunctionReference.Name == tr.Spec.FunctionReference.Name, "trigger should match after reading")

pkg/controller/httpTriggerApi.go

@@ -22,6 +22,7 @@ import (
 	"fmt"
 	"io/ioutil"
 	"net/http"
+	"sort"
 
 	"github.com/emicklei/go-restful"
 	restfulspec "github.com/emicklei/go-restful-openapi"
@@ -136,7 +137,18 @@ func (a *API) checkHTTPTriggerDuplicates(t *fv1.HTTPTrigger) error {
 		if ht.Spec.RelativeURL == t.Spec.RelativeURL || (ht.Spec.Prefix != nil && t.Spec.Prefix != nil && *ht.Spec.Prefix != "" && *ht.Spec.Prefix == *t.Spec.Prefix) {
 			urlMatch = true
 		}
-		if urlMatch && ht.Spec.Method == t.Spec.Method && ht.Spec.Host == t.Spec.Host {
+		methodMatch := false
+		if ht.Spec.Method == t.Spec.Method && len(ht.Spec.Methods) == len(t.Spec.Methods) {
+			methodMatch = true
+			sort.Strings(ht.Spec.Methods)
+			sort.Strings(t.Spec.Methods)
+			for i, m1 := range ht.Spec.Methods {
+				if m1 != t.Spec.Methods[i] {
+					methodMatch = false
+				}
+			}
+		}
+		if urlMatch && methodMatch && ht.Spec.Method == t.Spec.Method && ht.Spec.Host == t.Spec.Host {
 			return ferror.MakeError(ferror.ErrorNameExists,
 				fmt.Sprintf("HTTPTrigger with same Host, URL & method already exists (%v)",
 					ht.ObjectMeta.Name))

pkg/fission-cli/cmd/function/create.go

@@ -345,9 +345,17 @@ func (opts *CreateSubCommand) run(input cli.Input) error {
 	if len(prefix) != 0 && len(triggerUrl) > 0 {
 		console.Warn("Prefix will take precedence over URL/RelativeURL")
 	}
-	method, err := httptrigger.GetMethod(input.String(flagkey.HtMethod))
-	if err != nil {
-		return errors.Wrap(err, "error getting HTTP trigger method")
+
+	methods := input.StringSlice(flagkey.HtMethod)
+	if len(methods) == 0 {
+		return errors.New("HTTP methods not mentioned")
+	}
+
+	for _, method := range methods {
+		_, err := httptrigger.GetMethod(method)
+		if err != nil {
+			return err
+		}
 	}
 
 	triggerName := uuid.NewV4().String()
@@ -359,7 +367,7 @@ func (opts *CreateSubCommand) run(input cli.Input) error {
 		Spec: fv1.HTTPTriggerSpec{
 			RelativeURL: triggerUrl,
 			Prefix:      &prefix,
-			Method:      method,
+			Methods:     methods,
 			FunctionReference: fv1.FunctionReference{
 				Type: fv1.FunctionReferenceTypeFunctionName,
 				Name: opts.function.ObjectMeta.Name,
@@ -371,7 +379,7 @@ func (opts *CreateSubCommand) run(input cli.Input) error {
 		return errors.Wrap(err, "error creating HTTP trigger")
 	}
 
-	fmt.Printf("route created: %v %v -> %v\n", method, triggerUrl, opts.function.ObjectMeta.Name)
+	fmt.Printf("route created: %v %v -> %v\n", methods, triggerUrl, opts.function.ObjectMeta.Name)
 	return nil
 }
 

pkg/fission-cli/cmd/function/test.go

@@ -109,9 +109,19 @@ func (opts *TestSubCommand) do(input cli.Input) error {
 		defer closeCtx()
 	}
 
+	methods := input.StringSlice(flagkey.HtMethod)
+	if len(methods) == 0 {
+		return errors.New("HTTP method not mentioned")
+	} else if len(methods) > 1 {
+		return errors.New("More than one HTTP method not supported")
+	}
+	method, err := httptrigger.GetMethod(methods[0])
+	if err != nil {
+		return err
+	}
 	resp, err := doHTTPRequest(ctx, functionUrl.String(),
 		input.StringSlice(flagkey.FnTestHeader),
-		input.String(flagkey.HtMethod),
+		method,
 		input.String(flagkey.FnTestBody))
 	if err != nil {
 		return err

pkg/fission-cli/cmd/httptrigger/create.go

@@ -89,6 +89,7 @@ func (opts *CreateSubCommand) complete(input cli.Input) error {
 
 	triggerUrl := input.String(flagkey.HtUrl)
 	prefix := input.String(flagkey.HtPrefix)
+	fallbackURL := ""
 
 	if triggerUrl == "" && prefix == "" {
 		console.Error("You need to supply either Prefix or URL/RelativeURL")
@@ -108,10 +109,22 @@ func (opts *CreateSubCommand) complete(input cli.Input) error {
 	if prefix != "" && !strings.HasPrefix(prefix, "/") {
 		prefix = "/" + prefix
 	}
+	if prefix != "" {
+		fallbackURL = prefix
+	} else {
+		fallbackURL = triggerUrl
+	}
 
-	method, err := GetMethod(input.String(flagkey.HtMethod))
-	if err != nil {
-		return err
+	methods := input.StringSlice(flagkey.HtMethod)
+	if len(methods) == 0 {
+		return errors.New("HTTP methods not mentioned")
+	}
+
+	for _, method := range methods {
+		_, err := GetMethod(method)
+		if err != nil {
+			return err
+		}
 	}
 
 	// For Specs, the spec validate checks for function reference
@@ -146,7 +159,7 @@ func (opts *CreateSubCommand) complete(input cli.Input) error {
 	createIngress := input.Bool(flagkey.HtIngress)
 	ingressConfig, err := GetIngressConfig(
 		input.StringSlice(flagkey.HtIngressAnnotation), input.String(flagkey.HtIngressRule),
-		input.String(flagkey.HtIngressTLS), triggerUrl, nil)
+		input.String(flagkey.HtIngressTLS), fallbackURL, nil)
 	if err != nil {
 		return errors.Wrap(err, "error parsing ingress configuration")
 	}
@@ -161,7 +174,7 @@ func (opts *CreateSubCommand) complete(input cli.Input) error {
 		Spec: fv1.HTTPTriggerSpec{
 			Host:              host,
 			RelativeURL:       triggerUrl,
-			Method:            method,
+			Methods:           methods,
 			FunctionReference: *functionRef,
 			CreateIngress:     createIngress,
 			IngressConfig:     *ingressConfig,
@@ -220,7 +233,7 @@ func GetMethod(method string) (string, error) {
 	case http.MethodTrace:
 		return http.MethodTrace, nil
 	default:
-		return "", fmt.Errorf("invalid or unsupported HTTP Method %v", method)
+		return "", fmt.Errorf("invalid or unsupported HTTP Method '%v'", method)
 	}
 }
 

pkg/fission-cli/cmd/httptrigger/get.go

@@ -86,8 +86,15 @@ func printHtSummary(triggers []fv1.HTTPTrigger) {
 		}
 		ann := strings.Join(msg, ", ")
 
+		methods := []string{}
+		if len(trigger.Spec.Method) > 0 {
+			methods = append(methods, trigger.Spec.Method)
+		}
+		if len(trigger.Spec.Methods) > 0 {
+			methods = trigger.Spec.Methods
+		}
 		fmt.Fprintf(w, "%v\t%v\t%v\t%v\t%v\t%v\t%v\t%v\t%v\n",
-			trigger.ObjectMeta.Name, trigger.Spec.Method, trigger.Spec.RelativeURL, function, trigger.Spec.CreateIngress, host, path, trigger.Spec.IngressConfig.TLS, ann)
+			trigger.ObjectMeta.Name, methods, trigger.Spec.RelativeURL, function, trigger.Spec.CreateIngress, host, path, trigger.Spec.IngressConfig.TLS, ann)
 	}
 	w.Flush()
 }

pkg/fission-cli/cmd/httptrigger/update.go

@@ -80,8 +80,15 @@ func (opts *UpdateSubCommand) complete(input cli.Input) error {
 	ht.Spec.RelativeURL = triggerUrl
 	ht.Spec.Prefix = &prefix
 
-	if input.IsSet(flagkey.HtMethod) {
-		ht.Spec.Method = input.String(flagkey.HtMethod)
+	methods := input.StringSlice(flagkey.HtMethod)
+	if len(methods) > 0 {
+		for _, method := range methods {
+			_, err := GetMethod(method)
+			if err != nil {
+				return err
+			}
+		}
+		ht.Spec.Methods = methods
 	}
 
 	if input.IsSet(flagkey.HtFnName) {

pkg/fission-cli/cmd/spec/list.go

@@ -329,8 +329,16 @@ func ShowHTTPTriggers(hts []fv1.HTTPTrigger) {
 			}
 			ann := strings.Join(msg, ", ")
 
+			methods := []string{}
+			if len(trigger.Spec.Method) > 0 {
+				methods = append(methods, trigger.Spec.Method)
+			}
+			if len(trigger.Spec.Methods) > 0 {
+				methods = trigger.Spec.Methods
+			}
+
 			fmt.Fprintf(w, "%v\t%v\t%v\t%v\t%v\t%v\t%v\t%v\t%v\n",
-				trigger.ObjectMeta.Name, trigger.Spec.Method, trigger.Spec.RelativeURL, function, trigger.Spec.CreateIngress, host, path, trigger.Spec.IngressConfig.TLS, ann)
+				trigger.ObjectMeta.Name, methods, trigger.Spec.RelativeURL, function, trigger.Spec.CreateIngress, host, path, trigger.Spec.IngressConfig.TLS, ann)
 		}
 		fmt.Fprintf(w, "\n")
 		w.Flush()

pkg/fission-cli/cmd/spec/spec.go

@@ -443,6 +443,9 @@ func (fr *FissionResources) Validate(input cli.Input) ([]string, error) {
 		if len(t.Spec.Host) > 0 {
 			warnings = append(warnings, "Host in HTTPTrigger spec.Host is now marked as deprecated, see 'help' for details")
 		}
+		if len(t.Spec.Method) > 0 {
+			warnings = append(warnings, "Method in HTTTPTrigger spec.Method is deprecated, use spec.Methods instead")
+		}
 		result = multierror.Append(result, t.Validate())
 	}
 	for _, t := range fr.KubernetesWatchTriggers {

pkg/fission-cli/flag/flag.go

@@ -116,7 +116,7 @@ var (
 	FnSubPath               = Flag{Type: String, Name: flagkey.FnSubPath, Usage: "Sub Path to check if function internally supports routing"}
 
 	HtName              = Flag{Type: String, Name: flagkey.HtName, Usage: "HTTP trigger name"}
-	HtMethod            = Flag{Type: String, Name: flagkey.HtMethod, Usage: "HTTP Method: GET|POST|PUT|DELETE|HEAD", DefaultValue: http.MethodGet}
+	HtMethod            = Flag{Type: StringSlice, Name: flagkey.HtMethod, Usage: "HTTP Methods: GET,POST,PUT,DELETE,HEAD. To mention single method: --method GET and for multiple methods --method GET --method POST.", DefaultValue: []string{http.MethodGet}}
 	HtUrl               = Flag{Type: String, Name: flagkey.HtUrl, Usage: "URL pattern (See gorilla/mux supported patterns)"}
 	HtHost              = Flag{Type: String, Name: flagkey.HtHost, Usage: "Use --ingressrule instead", Deprecated: true, Substitute: flagkey.HtIngressRule}
 	HtIngress           = Flag{Type: Bool, Name: flagkey.HtIngress, Usage: "Creates ingress with same URL"}

pkg/router/httpTriggers.go

@@ -173,11 +173,25 @@ func (ts *HTTPTriggerSet) getRouter(fnTimeoutMap map[types.UID]int) *mux.Router
 		} else {
 			ht = muxRouter.HandleFunc(trigger.Spec.RelativeURL, fh.handler)
 		}
-		ht.Methods(trigger.Spec.Method)
+		methods := trigger.Spec.Methods
+		if len(trigger.Spec.Method) > 0 {
+			present := false
+			for _, m := range trigger.Spec.Methods {
+				if m == trigger.Spec.Method {
+					present = true
+					break
+				}
+			}
+			if !present {
+				methods = append(methods, trigger.Spec.Method)
+			}
+		}
+		ht.Methods(methods...)
 		if trigger.Spec.Host != "" {
 			ht.Host(trigger.Spec.Host)
 		}
-		if trigger.Spec.Prefix == nil && trigger.Spec.RelativeURL == "/" && trigger.Spec.Method == "GET" {
+
+		if trigger.Spec.Prefix == nil && trigger.Spec.RelativeURL == "/" && len(methods) == 1 && methods[0] == http.MethodGet {
 			homeHandled = true
 		}
 	}