Add Ingress host, path and annotations support (#1325)

For each ingress controller, the format of ingress host,
path and annotations are different. To support different
kinds of controller, this PR adds new ingress config field
to http trigger spec. A user can set annotations, host and
path based on the type of underlying ingress controller
with CLI.

Command example:

fission route create --name foo \
    --url /foo/{bar} --function foofn --createingress \
    --ingressannotation "nginx.ingress.kubernetes.io/ssl-redirect=false" \
    --ingressannotation "nginx.ingress.kubernetes.io/use-regex=true" \
    --ingressrule "*=/foo/*"

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

@@ -0,0 +1,118 @@
+/*
+Copyright 2019 The Fission Authors.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package httptrigger
+
+import (
+	"fmt"
+	"strings"
+
+	fv1 "github.com/fission/fission/pkg/apis/fission.io/v1"
+)
+
+// GetIngressConfig returns an IngressConfig based on user inputs; return error if any.
+func GetIngressConfig(annotations []string, rule string, fallbackRelativeURL string, oldIngressConfig *fv1.IngressConfig) (*fv1.IngressConfig, error) {
+
+	removeAnns, anns, err := getIngressAnnotations(annotations)
+	if err != nil {
+		return nil, err
+	}
+	isEmptyRule, host, path, err := getIngressHostRule(rule, fallbackRelativeURL)
+	if err != nil {
+		return nil, err
+	}
+
+	if oldIngressConfig == nil {
+		if isEmptyRule { // assign default value
+			host = "*"
+			path = fallbackRelativeURL
+		}
+		return &fv1.IngressConfig{
+			Annotations: anns,
+			Host:        host,
+			Path:        path,
+		}, nil
+	}
+
+	if removeAnns {
+		oldIngressConfig.Annotations = nil
+	} else if len(anns) > 0 {
+		if oldIngressConfig.Annotations == nil {
+			oldIngressConfig.Annotations = make(map[string]string, len(anns))
+		}
+		for k, v := range anns {
+			oldIngressConfig.Annotations[k] = v
+		}
+	}
+
+	if isEmptyRule {
+		// an empty rule means no new rule was given,
+		// leave host and path intact except when host
+		// or path is empty.
+		if len(oldIngressConfig.Host) == 0 {
+			oldIngressConfig.Host = "*"
+		}
+		if len(oldIngressConfig.Path) == 0 {
+			oldIngressConfig.Path = fallbackRelativeURL
+		}
+	} else {
+		oldIngressConfig.Host = host
+		oldIngressConfig.Path = path
+	}
+
+	return oldIngressConfig, nil
+}
+
+func getIngressAnnotations(annotations []string) (remove bool, anns map[string]string, err error) {
+	if len(annotations) == 0 {
+		return false, nil, nil
+	}
+
+	anns = make(map[string]string)
+	for _, ann := range annotations {
+		if ann == "-" {
+			// remove all annotations
+			return true, nil, nil
+		}
+		v := strings.Split(ann, "=")
+		if len(v) != 2 {
+			return false, nil, fmt.Errorf("illegal ingress annotation: %v", ann)
+		}
+		key, val := v[0], v[1]
+		anns[key] = val
+	}
+	return false, anns, nil
+}
+
+func getIngressHostRule(rule string, fallbackPath string) (empty bool, host string, path string, err error) {
+	if len(fallbackPath) == 0 {
+		return false, "", "", fmt.Errorf("fallback url cannot be empty")
+	}
+	if len(rule) == 0 {
+		return true, "", "", nil
+	}
+	if rule == "-" {
+		return false, "*", fallbackPath, nil
+	}
+	v := strings.Split(rule, "=")
+	if len(v) != 2 {
+		return false, "", "", fmt.Errorf("illegal ingress rule: %v", rule)
+	}
+	if len(v[0]) == 0 || len(v[1]) == 0 {
+		return false, "", "", fmt.Errorf("host (%v) or path (%v) cannot be empty", v[0], v[1])
+	}
+	return false, v[0], v[1], nil
+}

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

@@ -0,0 +1,442 @@
+/*
+Copyright 2019 The Fission Authors.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package httptrigger
+
+import (
+	"reflect"
+	"testing"
+
+	fv1 "github.com/fission/fission/pkg/apis/fission.io/v1"
+)
+
+func Test_GetIngressConfig(t *testing.T) {
+	type args struct {
+		ingressConfig       *fv1.IngressConfig
+		annotations         []string
+		rule                string
+		fallbackRelativeURL string
+	}
+	tests := []struct {
+		name    string
+		args    args
+		want    *fv1.IngressConfig
+		wantErr bool
+	}{
+		{
+			name: "pass-nil-ingressconfig-pointer",
+			args: args{
+				ingressConfig:       nil,
+				annotations:         []string{"foo=bar", "bar=foo"},
+				rule:                "test.com=/foo/bar",
+				fallbackRelativeURL: "/test",
+			},
+			want: &fv1.IngressConfig{
+				Annotations: map[string]string{
+					"foo": "bar",
+					"bar": "foo",
+				},
+				Host: "test.com",
+				Path: "/foo/bar",
+			},
+			wantErr: false,
+		},
+		{
+			name: "pass-non-nil-ingressconfig-pointer",
+			args: args{
+				ingressConfig: &fv1.IngressConfig{
+					Annotations: map[string]string{
+						"hello": "world",
+					},
+					Host: "foo",
+					Path: "bar",
+				},
+				annotations:         []string{"foo=bar", "bar=foo"},
+				rule:                "test.com=/foo/bar",
+				fallbackRelativeURL: "/test",
+			},
+			want: &fv1.IngressConfig{
+				Annotations: map[string]string{
+					"foo":   "bar",
+					"bar":   "foo",
+					"hello": "world",
+				},
+				Host: "test.com",
+				Path: "/foo/bar",
+			},
+			wantErr: false,
+		},
+		{
+			name: "ingressconfig-with-nil-annotations",
+			args: args{
+				ingressConfig: &fv1.IngressConfig{
+					Annotations: nil,
+					Host:        "foo",
+					Path:        "bar",
+				},
+				annotations:         []string{"foo=bar", "bar=foo"},
+				rule:                "test.com=/foo/bar",
+				fallbackRelativeURL: "/test",
+			},
+			want: &fv1.IngressConfig{
+				Annotations: map[string]string{
+					"foo": "bar",
+					"bar": "foo",
+				},
+				Host: "test.com",
+				Path: "/foo/bar",
+			},
+			wantErr: false,
+		},
+		{
+			name: "remove-annotations-from-ingressconfig",
+			args: args{
+				ingressConfig: &fv1.IngressConfig{
+					Annotations: map[string]string{
+						"hello": "world",
+					},
+				},
+				annotations:         []string{"-"},
+				rule:                "test.com=/foo/bar",
+				fallbackRelativeURL: "/test",
+			},
+			want: &fv1.IngressConfig{
+				Annotations: nil,
+				Host:        "test.com",
+				Path:        "/foo/bar",
+			},
+			wantErr: false,
+		},
+		{
+			name: "remove-rule-from-ingressconfig",
+			args: args{
+				ingressConfig: &fv1.IngressConfig{
+					Annotations: map[string]string{
+						"hello": "world",
+					},
+				},
+				annotations:         []string{"-"},
+				rule:                "-",
+				fallbackRelativeURL: "/test",
+			},
+			want: &fv1.IngressConfig{
+				Annotations: nil,
+				Host:        "*",
+				Path:        "/test",
+			},
+			wantErr: false,
+		},
+		{
+			name: "wrong-annotations-value-1",
+			args: args{
+				ingressConfig:       nil,
+				annotations:         []string{"a"},
+				rule:                "-",
+				fallbackRelativeURL: "/test",
+			},
+			want:    nil,
+			wantErr: true,
+		},
+		{
+			name: "wrong-annotations-value-2",
+			args: args{
+				ingressConfig:       nil,
+				annotations:         []string{"a=b=c"},
+				rule:                "-",
+				fallbackRelativeURL: "/test",
+			},
+			want:    nil,
+			wantErr: true,
+		},
+		{
+			name: "wrong-rule-value-1",
+			args: args{
+				ingressConfig: &fv1.IngressConfig{
+					Annotations: map[string]string{
+						"hello": "world",
+					},
+				},
+				annotations:         []string{"a=b"},
+				rule:                "a",
+				fallbackRelativeURL: "/test",
+			},
+			want:    nil,
+			wantErr: true,
+		},
+		{
+			name: "wrong-rule-value-2",
+			args: args{
+				ingressConfig: &fv1.IngressConfig{
+					Annotations: map[string]string{
+						"hello": "world",
+					},
+				},
+				annotations:         []string{"a=b"},
+				rule:                "a=b=c",
+				fallbackRelativeURL: "/test",
+			},
+			want:    nil,
+			wantErr: true,
+		},
+		{
+			name: "ingressconfog-with-only-fallback-rul",
+			args: args{
+				ingressConfig:       nil,
+				annotations:         nil,
+				rule:                "",
+				fallbackRelativeURL: "/test",
+			},
+			want: &fv1.IngressConfig{
+				Annotations: nil,
+				Host:        "*",
+				Path:        "/test",
+			},
+			wantErr: false,
+		},
+		{
+			name: "backward-compatibility-test",
+			args: args{
+				ingressConfig: &fv1.IngressConfig{
+					Annotations: nil,
+					Host:        "",
+					Path:        "",
+				},
+				annotations:         nil,
+				rule:                "",
+				fallbackRelativeURL: "/test",
+			},
+			want: &fv1.IngressConfig{
+				Annotations: nil,
+				Host:        "*",
+				Path:        "/test",
+			},
+			wantErr: false,
+		},
+		{
+			name: "preserve-annotation-if-nothing-change",
+			args: args{
+				ingressConfig: &fv1.IngressConfig{
+					Annotations: map[string]string{
+						"a": "b",
+					},
+					Host: "test.com",
+					Path: "/foo/bar",
+				},
+				annotations:         nil,
+				rule:                "",
+				fallbackRelativeURL: "/test",
+			},
+			want: &fv1.IngressConfig{
+				Annotations: map[string]string{
+					"a": "b",
+				},
+				Host: "test.com",
+				Path: "/foo/bar",
+			},
+			wantErr: false,
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			got, err := GetIngressConfig(tt.args.annotations, tt.args.rule, tt.args.fallbackRelativeURL, tt.args.ingressConfig)
+			if (err != nil) != tt.wantErr {
+				t.Errorf("getIngressConfig() error = %v, wantErr %v", err, tt.wantErr)
+				return
+			}
+			if !reflect.DeepEqual(got, tt.want) {
+				t.Errorf("%v %v %v %v", got.Annotations == nil, tt.want.Annotations == nil, got.Path == tt.want.Path, got.Host == tt.want.Host)
+				t.Errorf("getIngressConfig() got = %v, want %v", got, tt.want)
+			}
+		})
+	}
+}
+
+func Test_getIngressAnnotations(t *testing.T) {
+	type args struct {
+		annotations []string
+	}
+	tests := []struct {
+		name       string
+		args       args
+		wantRemove bool
+		wantAnns   map[string]string
+		wantErr    bool
+	}{
+		{
+			name: "get-annotations",
+			args: args{
+				annotations: []string{"a=b", "c=d"},
+			},
+			wantRemove: false,
+			wantAnns: map[string]string{
+				"a": "b",
+				"c": "d",
+			},
+			wantErr: false,
+		},
+		{
+			name: "remove-all-annotations",
+			args: args{
+				annotations: []string{"-", "c=d"},
+			},
+			wantRemove: true,
+			wantAnns:   nil,
+			wantErr:    false,
+		},
+		{
+			name: "incorrect-annotation",
+			args: args{
+				annotations: []string{"a==b"},
+			},
+			wantRemove: false,
+			wantAnns:   nil,
+			wantErr:    true,
+		},
+		{
+			name: "zero-annotations-1",
+			args: args{
+				annotations: []string{},
+			},
+			wantRemove: false,
+			wantAnns:   nil,
+			wantErr:    false,
+		},
+		{
+			name: "zero-annotations-2",
+			args: args{
+				annotations: nil,
+			},
+			wantRemove: false,
+			wantAnns:   nil,
+			wantErr:    false,
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			gotRemove, gotAnns, err := getIngressAnnotations(tt.args.annotations)
+			if (err != nil) != tt.wantErr {
+				t.Errorf("getIngressAnnotations() error = %v, wantErr %v", err, tt.wantErr)
+				return
+			}
+			if gotRemove != tt.wantRemove {
+				t.Errorf("getIngressAnnotations() gotRemove = %v, want %v", gotRemove, tt.wantRemove)
+			}
+			if !reflect.DeepEqual(gotAnns, tt.wantAnns) {
+				t.Errorf("getIngressAnnotations() gotAnns = %v, want %v", gotAnns, tt.wantAnns)
+			}
+		})
+	}
+}
+
+func Test_getIngressHostRule(t *testing.T) {
+	type args struct {
+		rule         string
+		fallbackPath string
+	}
+	tests := []struct {
+		name      string
+		args      args
+		wantEmpty bool
+		wantHost  string
+		wantPath  string
+		wantErr   bool
+	}{
+		{
+			name: "get-rule",
+			args: args{
+				rule:         "a=b",
+				fallbackPath: "/foo",
+			},
+			wantEmpty: false,
+			wantHost:  "a",
+			wantPath:  "b",
+			wantErr:   false,
+		},
+		{
+			name: "remove-rule",
+			args: args{
+				rule:         "-",
+				fallbackPath: "/foo",
+			},
+			wantEmpty: false,
+			wantHost:  "*",
+			wantPath:  "/foo",
+			wantErr:   false,
+		},
+		{
+			name: "empty-rule",
+			args: args{
+				rule:         "",
+				fallbackPath: "/foo",
+			},
+			wantEmpty: true,
+			wantHost:  "",
+			wantPath:  "",
+			wantErr:   false,
+		},
+		{
+			name: "empty-host",
+			args: args{
+				rule:         "=/aasd",
+				fallbackPath: "/foo",
+			},
+			wantEmpty: false,
+			wantHost:  "",
+			wantPath:  "",
+			wantErr:   true,
+		},
+		{
+			name: "empty-path",
+			args: args{
+				rule:         "test.com=",
+				fallbackPath: "/foo",
+			},
+			wantEmpty: false,
+			wantHost:  "",
+			wantPath:  "",
+			wantErr:   true,
+		},
+		{
+			name: "empty-fallback-url",
+			args: args{
+				rule:         "test.com=",
+				fallbackPath: "",
+			},
+			wantEmpty: false,
+			wantHost:  "",
+			wantPath:  "",
+			wantErr:   true,
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			gotEmpty, gotHost, gotPath, err := getIngressHostRule(tt.args.rule, tt.args.fallbackPath)
+			if (err != nil) != tt.wantErr {
+				t.Errorf("getIngressHostRule() error = %v, wantErr %v", err, tt.wantErr)
+				return
+			}
+			if gotEmpty != tt.wantEmpty {
+				t.Errorf("getIngressHostRule() gotEmpty = %v, want %v", gotEmpty, tt.wantEmpty)
+			}
+			if gotHost != tt.wantHost {
+				t.Errorf("getIngressHostRule() gotHost = %v, want %v", gotHost, tt.wantHost)
+			}
+			if gotPath != tt.wantPath {
+				t.Errorf("getIngressHostRule() gotPath = %v, want %v", gotPath, tt.wantPath)
+			}
+		})
+	}
+}

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

@@ -441,6 +441,11 @@ func (fr *FissionResources) Validate(c *cli.Context) error {
 		if err != nil {
 			result = multierror.Append(result, err)
 		}
+
+		if len(t.Spec.Host) > 0 {
+			log.Warn(fmt.Sprintf("Host in HTTPTrigger spec.Host is now marked as deprecated, see 'help' for details"))
+		}
+
 		result = multierror.Append(result, t.Validate())
 	}
 	for _, t := range fr.KubernetesWatchTriggers {

pkg/fission-cli/httptrigger.go

@@ -29,6 +29,7 @@ import (
 
 	fv1 "github.com/fission/fission/pkg/apis/fission.io/v1"
 	ferror "github.com/fission/fission/pkg/error"
+	"github.com/fission/fission/pkg/fission-cli/cmd/httptrigger"
 	"github.com/fission/fission/pkg/fission-cli/cmd/spec"
 	"github.com/fission/fission/pkg/fission-cli/log"
 	"github.com/fission/fission/pkg/fission-cli/util"
@@ -143,12 +144,14 @@ func htCreate(c *cli.Context) error {
 		}
 	}
 
-	createIngress := false
-	if c.IsSet("createingress") {
-		createIngress = c.Bool("createingress")
-	}
+	createIngress := c.Bool("createingress")
+	ingressConfig, err := httptrigger.GetIngressConfig(c.StringSlice("ingressannotation"), c.String("ingressrule"), triggerUrl, nil)
+	util.CheckErr(err, "parse ingress configuration")
 
 	host := c.String("host")
+	if c.IsSet("host") {
+		log.Warn(fmt.Sprintf("--host is now marked as deprecated, see 'help' for details"))
+	}
 
 	// just name triggers by uuid.
 	if triggerName == "" {
@@ -166,6 +169,7 @@ func htCreate(c *cli.Context) error {
 			Method:            getMethod(method),
 			FunctionReference: *functionRef,
 			CreateIngress:     createIngress,
+			IngressConfig:     *ingressConfig,
 		},
 	}
 
@@ -264,6 +268,12 @@ func htUpdate(c *cli.Context) error {
 
 	if c.IsSet("host") {
 		ht.Spec.Host = c.String("host")
+		log.Warn(fmt.Sprintf("--host is now marked as deprecated, see 'help' for details"))
+	}
+
+	if c.IsSet("ingressrule") || c.IsSet("ingressannotation") {
+		_, err = httptrigger.GetIngressConfig(c.StringSlice("ingressannotation"), c.String("ingressrule"), ht.Spec.RelativeURL, &ht.Spec.IngressConfig)
+		util.CheckErr(err, "parse ingress configuration")
 	}
 
 	_, err = client.HTTPTriggerUpdate(ht)

pkg/fission-cli/main.go

@@ -143,15 +143,16 @@ func NewCliApp() *cli.App {
 
 	// httptriggers
 	htNameFlag := cli.StringFlag{Name: "name", Usage: "HTTP Trigger name"}
-	htHostFlag := cli.StringFlag{Name: "host", Usage: "FQDN of the network host for route"}
+	htHostFlag := cli.StringFlag{Name: "host", Usage: "(DEPRECATED) Use --ingressrule instead"}
 	htIngressFlag := cli.BoolFlag{Name: "createingress", Usage: "Creates ingress with same URL, defaults to false"}
+	htIngressRuleFlag := cli.StringFlag{Name: "ingressrule", Usage: "Host for Ingress rule: --ingressrule host=path (the format of host/path depends on what ingress controller you used)"}
+	htIngressAnnotationFlag := cli.StringSliceFlag{Name: "ingressannotation", Usage: "Annotation for Ingress: --ingressannotation key=value (the format of annotation depends on what ingress controller you used)"}
 	htFnNameFlag := cli.StringSliceFlag{Name: "function", Usage: "Name(s) of the function for this trigger. If 2 functions are supplied with this flag, traffic gets routed to them based on weights supplied with --weight flag."}
 	htFnWeightFlag := cli.IntSliceFlag{Name: "weight", Usage: "Weight for each function supplied with --function flag, in the same order. Used for canary deployment"}
 	htSubcommands := []cli.Command{
-
-		{Name: "create", Aliases: []string{"add"}, Usage: "Create HTTP trigger", Flags: []cli.Flag{htNameFlag, htMethodFlag, htUrlFlag, htFnNameFlag, htHostFlag, htIngressFlag, fnNamespaceFlag, specSaveFlag, htFnWeightFlag}, Action: htCreate},
+		{Name: "create", Aliases: []string{"add"}, Usage: "Create HTTP trigger", Flags: []cli.Flag{htNameFlag, htMethodFlag, htUrlFlag, htFnNameFlag, htIngressRuleFlag, htIngressAnnotationFlag, htIngressFlag, fnNamespaceFlag, specSaveFlag, htFnWeightFlag, htHostFlag}, Action: htCreate},
 		{Name: "get", Usage: "Get HTTP trigger", Flags: []cli.Flag{htNameFlag}, Action: htGet},
-		{Name: "update", Usage: "Update HTTP trigger", Flags: []cli.Flag{htNameFlag, triggerNamespaceFlag, htFnNameFlag, htHostFlag, htIngressFlag, htFnWeightFlag}, Action: htUpdate},
+		{Name: "update", Usage: "Update HTTP trigger", Flags: []cli.Flag{htNameFlag, triggerNamespaceFlag, htFnNameFlag, htIngressRuleFlag, htIngressAnnotationFlag, htIngressFlag, htFnWeightFlag, htHostFlag}, Action: htUpdate},
 		{Name: "delete", Usage: "Delete HTTP trigger", Flags: []cli.Flag{htNameFlag, triggerNamespaceFlag}, Action: htDelete},
 		{Name: "list", Usage: "List HTTP triggers", Flags: []cli.Flag{triggerNamespaceFlag}, Action: htList},
 	}