From 49d60b19f3310c79f3c6eb9e626a2b11d0c9f5af Mon Sep 17 00:00:00 2001 From: Ta-Ching Chen Date: Thu, 26 Sep 2019 16:54:16 +0800 Subject: [PATCH] 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/*" --- go.mod | 1 - pkg/apis/fission.io/v1/types.go | 31 +- pkg/apis/fission.io/v1/validation.go | 54 ++ pkg/fission-cli/cmd/httptrigger/parse.go | 118 +++++ pkg/fission-cli/cmd/httptrigger/parse_test.go | 442 ++++++++++++++++ pkg/fission-cli/cmd/spec/spec.go | 5 + pkg/fission-cli/httptrigger.go | 18 +- pkg/fission-cli/main.go | 9 +- pkg/router/ingress.go | 113 ++-- pkg/router/util/util.go | 85 +++ pkg/router/util/util_test.go | 499 ++++++++++++++++++ test/test_utils.sh | 61 ++- test/tests/test_ingress.sh | 103 +++- test/utils.sh | 20 +- 14 files changed, 1421 insertions(+), 138 deletions(-) create mode 100644 pkg/fission-cli/cmd/httptrigger/parse.go create mode 100644 pkg/fission-cli/cmd/httptrigger/parse_test.go create mode 100644 pkg/router/util/util.go create mode 100644 pkg/router/util/util_test.go diff --git a/go.mod b/go.mod index 0e45360a..bd9572a7 100644 --- a/go.mod +++ b/go.mod @@ -74,7 +74,6 @@ require ( google.golang.org/appengine v1.6.1 // indirect gopkg.in/check.v1 v1.0.0-20180628173108-788fd7840127 // indirect gopkg.in/inf.v0 v0.9.1 // indirect - gopkg.in/yaml.v2 v2.2.2 k8s.io/api v0.0.0-20190620084959-7cf5895f2711 k8s.io/apiextensions-apiserver v0.0.0-20190620085554-14e95df34f1f k8s.io/apimachinery v0.0.0-20190612205821-1799e75a0719 diff --git a/pkg/apis/fission.io/v1/types.go b/pkg/apis/fission.io/v1/types.go index 3ade51c5..4a11fded 100644 --- a/pkg/apis/fission.io/v1/types.go +++ b/pkg/apis/fission.io/v1/types.go @@ -547,20 +547,41 @@ type ( // HTTPTriggerSpec is for router to expose user functions at the given URL path. HTTPTriggerSpec struct { - // NOT USED NOW - Host string `json:"-"` //`json:"host"` + // TODO: remove this field since we have IngressConfig already + // Deprecated: the original idea of this field is not for setting Ingress. + // Since we have IngressConfig now, remove Host after couple releases. + Host string `json:"host"` // RelativeURL is the exposed URL for external client to access a function with. RelativeURL string `json:"relativeurl"` - // If CreateIngress is true, router will create a ingress definition. - CreateIngress bool `json:"createingress"` - // HTTP method to access a function. Method string `json:"method"` // FunctionReference is a reference to the target function. FunctionReference FunctionReference `json:"functionref"` + + // If CreateIngress is true, router will create a ingress definition. + CreateIngress bool `json:"createingress"` + + // TODO: make IngressConfig a independent Fission resource + // IngressConfig for router to set up Ingress. + IngressConfig IngressConfig `json:"ingressconfig"` + } + + // IngressConfig is for router to set up Ingress. + IngressConfig struct { + // Annotations will be add to metadata when creating Ingress. + Annotations map[string]string `json:"annotations"` + + // Path is for path matching. The format of path + // depends on what ingress controller you used. + Path string `json:"path"` + + // Host is for ingress controller to apply rules. If + // host is empty or "*", the rule applies to all + // inbound HTTP traffic. + Host string `json:"host"` } // KubernetesWatchTriggerSpec diff --git a/pkg/apis/fission.io/v1/validation.go b/pkg/apis/fission.io/v1/validation.go index 8db61ff1..351e9a49 100644 --- a/pkg/apis/fission.io/v1/validation.go +++ b/pkg/apis/fission.io/v1/validation.go @@ -32,6 +32,8 @@ const ( ErrorUnsupportedType = iota ErrorInvalidValue ErrorInvalidObject + + totalAnnotationSizeLimitB int = 256 * (1 << 10) // 256 kB ) var ( @@ -444,6 +446,58 @@ func (spec HTTPTriggerSpec) Validate() error { } } + result = multierror.Append(result, spec.IngressConfig.Validate()) + + return result.ErrorOrNil() +} + +func (config IngressConfig) Validate() error { + result := &multierror.Error{} + + // Details for how to validate Ingress host rule, + // see https://github.com/kubernetes/kubernetes/blob/release-1.16/pkg/apis/networking/validation/validation.go + + if len(config.Path) > 0 { + if !strings.HasPrefix(config.Path, "/") { + result = multierror.Append(result, MakeValidationErr(ErrorInvalidValue, "HTTPTriggerSpec.IngressConfig.IngressRule.Path", config.Path, "must be an absolute path")) + } + + _, err := regexp.CompilePOSIX(config.Path) + if err != nil { + result = multierror.Append(result, MakeValidationErr(ErrorInvalidValue, "HTTPTriggerSpec.IngressConfig.IngressRule.Path", config.Path, "must be a valid regex")) + } + } + + // In Ingress, to accept requests from all host, the host field will + // be an empty string instead of "*" shown in kubectl. The router replaces + // the asterisk with "" when creating/updateing the Ingress, so here we + // skip the check if the Host is equal to "*". + if len(config.Host) > 0 && config.Host != "*" { + if strings.Contains(config.Host, "*") { + for _, msg := range validation.IsWildcardDNS1123Subdomain(config.Host) { + result = multierror.Append(result, MakeValidationErr(ErrorInvalidValue, "HTTPTriggerSpec.IngressConfig.IngressRule.Host", config.Host, msg)) + } + } + for _, msg := range validation.IsDNS1123Subdomain(config.Host) { + result = multierror.Append(result, MakeValidationErr(ErrorInvalidValue, "HTTPTriggerSpec.IngressConfig.IngressRule.Host", config.Host, msg)) + } + } + + // Details for how to validate annotations, + // see https://github.com/kubernetes/kubernetes/blob/512eccac1f1d72d6d1cb304bc565c50d1f2e295e/staging/src/k8s.io/apimachinery/pkg/api/validation/objectmeta.go#L46 + + var totalSize int64 + for k, v := range config.Annotations { + for _, msg := range validation.IsQualifiedName(strings.ToLower(k)) { + result = multierror.Append(result, MakeValidationErr(ErrorInvalidValue, "HTTPTriggerSpec.IngressConfig.Annotations.key", k, msg)) + } + totalSize += (int64)(len(k)) + (int64)(len(v)) + } + if totalSize > (int64)(totalAnnotationSizeLimitB) { + msg := fmt.Sprintf("must have at most %v characters", totalSize) + result = multierror.Append(result, MakeValidationErr(ErrorInvalidValue, "HTTPTriggerSpec.IngressConfig.Annotations.value", totalAnnotationSizeLimitB, msg)) + } + return result.ErrorOrNil() } diff --git a/pkg/fission-cli/cmd/httptrigger/parse.go b/pkg/fission-cli/cmd/httptrigger/parse.go new file mode 100644 index 00000000..6ce0eded --- /dev/null +++ b/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 +} diff --git a/pkg/fission-cli/cmd/httptrigger/parse_test.go b/pkg/fission-cli/cmd/httptrigger/parse_test.go new file mode 100644 index 00000000..c92a678e --- /dev/null +++ b/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) + } + }) + } +} diff --git a/pkg/fission-cli/cmd/spec/spec.go b/pkg/fission-cli/cmd/spec/spec.go index 83729b54..9b6e4ac6 100644 --- a/pkg/fission-cli/cmd/spec/spec.go +++ b/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 { diff --git a/pkg/fission-cli/httptrigger.go b/pkg/fission-cli/httptrigger.go index 40a11f3f..cabf4f87 100644 --- a/pkg/fission-cli/httptrigger.go +++ b/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) diff --git a/pkg/fission-cli/main.go b/pkg/fission-cli/main.go index 400acef5..3973555f 100644 --- a/pkg/fission-cli/main.go +++ b/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}, } diff --git a/pkg/router/ingress.go b/pkg/router/ingress.go index 7302a5e9..fca28620 100644 --- a/pkg/router/ingress.go +++ b/pkg/router/ingress.go @@ -18,16 +18,15 @@ package router import ( "os" + "reflect" "go.uber.org/zap" - "k8s.io/api/extensions/v1beta1" k8serrors "k8s.io/apimachinery/pkg/api/errors" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" v1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "k8s.io/apimachinery/pkg/util/intstr" "k8s.io/client-go/kubernetes" fv1 "github.com/fission/fission/pkg/apis/fission.io/v1" + "github.com/fission/fission/pkg/router/util" ) var podNamespace string @@ -43,61 +42,14 @@ func createIngress(logger *zap.Logger, trigger *fv1.HTTPTrigger, kubeClient *kub if !trigger.Spec.CreateIngress { return } - _, err := kubeClient.ExtensionsV1beta1().Ingresses(podNamespace).Get(trigger.Metadata.Name, v1.GetOptions{}) - if err == nil { - return - } - - ing := &v1beta1.Ingress{ - ObjectMeta: metav1.ObjectMeta{ - Labels: getDeployLabels(trigger), - Name: trigger.Metadata.Name, - // The Ingress NS MUST be same as Router NS, check long discussion: - // https://github.com/kubernetes/kubernetes/issues/17088 - // We need to revisit this in future, once Kubernetes supports cross namespace ingress - Namespace: podNamespace, - }, - Spec: v1beta1.IngressSpec{ - Rules: []v1beta1.IngressRule{ - { - Host: trigger.Spec.Host, - IngressRuleValue: v1beta1.IngressRuleValue{ - HTTP: &v1beta1.HTTPIngressRuleValue{ - Paths: []v1beta1.HTTPIngressPath{ - { - Backend: v1beta1.IngressBackend{ - ServiceName: "router", - ServicePort: intstr.IntOrString{ - Type: intstr.Int, - IntVal: 80, - }, - }, - Path: trigger.Spec.RelativeURL, - }, - }, - }, - }, - }, - }, - }, - } - - _, err = kubeClient.ExtensionsV1beta1().Ingresses(podNamespace).Create(ing) - if err != nil { + _, err := kubeClient.ExtensionsV1beta1().Ingresses(podNamespace).Create(util.GetIngressSpec(podNamespace, trigger)) + if err != nil && !k8serrors.IsAlreadyExists(err) { logger.Error("failed to create ingress", zap.Error(err)) return } logger.Debug("created ingress successfully for trigger", zap.String("trigger", trigger.Metadata.Name)) } -func getDeployLabels(trigger *fv1.HTTPTrigger) map[string]string { - return map[string]string{ - "triggerName": trigger.Metadata.Name, - "functionName": trigger.Spec.FunctionReference.Name, - "triggerNamespace": trigger.Metadata.Namespace, - } -} - func deleteIngress(logger *zap.Logger, trigger *fv1.HTTPTrigger, kubeClient *kubernetes.Clientset) { if !trigger.Spec.CreateIngress { return @@ -130,25 +82,46 @@ func updateIngress(logger *zap.Logger, oldT *fv1.HTTPTrigger, newT *fv1.HTTPTrig return } - if newT.Spec.Host != oldT.Spec.Host || newT.Spec.RelativeURL != oldT.Spec.RelativeURL { - ingress, err := kubeClient.ExtensionsV1beta1().Ingresses(podNamespace).Get(oldT.Metadata.Name, v1.GetOptions{}) + oldIngress, err := kubeClient.ExtensionsV1beta1().Ingresses(podNamespace).Get(oldT.Metadata.Name, v1.GetOptions{}) + if err != nil { + if k8serrors.IsNotFound(err) { + createIngress(logger, newT, kubeClient) + } + logger.Error("failed to get ingress when updating trigger", + zap.Error(err), + zap.String("trigger", oldT.Metadata.Name)) + return + } + newIngress := util.GetIngressSpec(podNamespace, newT) + + changes := false + + if !reflect.DeepEqual(oldIngress.Annotations, newIngress.Annotations) { + logger.Debug("ingress annotation", + zap.Any("old_trigger", oldIngress.Annotations), zap.Any("new_trigger", newIngress.Annotations)) + + if oldIngress.Annotations == nil || newIngress.Annotations == nil { + oldIngress.Annotations = newIngress.Annotations + } else { + for k, v := range newIngress.Annotations { + oldIngress.Annotations[k] = v + } + } + changes = true + } + + if !reflect.DeepEqual(oldIngress.Spec, newIngress.Spec) { + logger.Debug("ingress spec", + zap.Any("old_trigger", oldIngress.Spec), zap.Any("new_trigger", newIngress.Spec)) + + oldIngress.Spec = newIngress.Spec + changes = true + } + + if changes { + _, err = kubeClient.ExtensionsV1beta1().Ingresses(podNamespace).Update(oldIngress) if err != nil { - logger.Error("failed to get ingress when updating trigger", - zap.Error(err), - zap.String("trigger", oldT.Metadata.Name)) - } - - if newT.Spec.Host != oldT.Spec.Host { - ingress.Spec.Rules[0].Host = newT.Spec.Host - } - - if newT.Spec.RelativeURL != oldT.Spec.RelativeURL { - ingress.Spec.Rules[0].HTTP.Paths[0].Path = newT.Spec.RelativeURL - } - - _, err = kubeClient.ExtensionsV1beta1().Ingresses(podNamespace).Update(ingress) - if err != nil { - logger.Error("failed to update ingress for trigger", zap.String("trigger", oldT.Metadata.Name)) + logger.Error("failed to update ingress for trigger", zap.Error(err), zap.String("trigger", oldT.Metadata.Name)) return } diff --git a/pkg/router/util/util.go b/pkg/router/util/util.go new file mode 100644 index 00000000..d27def58 --- /dev/null +++ b/pkg/router/util/util.go @@ -0,0 +1,85 @@ +/* +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 util + +import ( + "k8s.io/api/extensions/v1beta1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/util/intstr" + + fv1 "github.com/fission/fission/pkg/apis/fission.io/v1" +) + +func GetIngressSpec(namespace string, trigger *fv1.HTTPTrigger) *v1beta1.Ingress { + // TODO: remove backward compatibility + host, path := trigger.Spec.Host, trigger.Spec.RelativeURL + if len(trigger.Spec.IngressConfig.Host) > 0 && len(trigger.Spec.IngressConfig.Path) > 0 { + host, path = trigger.Spec.IngressConfig.Host, trigger.Spec.IngressConfig.Path + } + + // In Ingress, to accept requests from all host, the host field will + // be an empty string instead of "*" shown in kubectl. So replace it + // with empty string + if host == "*" { + host = "" // wildcard Ingress host + } + + ing := &v1beta1.Ingress{ + ObjectMeta: metav1.ObjectMeta{ + Labels: GetDeployLabels(trigger), + Name: trigger.Metadata.Name, + // The Ingress NS MUST be same as Router NS, check long discussion: + // https://github.com/kubernetes/kubernetes/issues/17088 + // We need to revisit this in future, once Kubernetes supports cross namespace ingress + Namespace: namespace, + Annotations: trigger.Spec.IngressConfig.Annotations, + }, + Spec: v1beta1.IngressSpec{ + Rules: []v1beta1.IngressRule{ + { + Host: host, + IngressRuleValue: v1beta1.IngressRuleValue{ + HTTP: &v1beta1.HTTPIngressRuleValue{ + Paths: []v1beta1.HTTPIngressPath{ + { + Backend: v1beta1.IngressBackend{ + ServiceName: "router", + ServicePort: intstr.IntOrString{ + Type: intstr.Int, + IntVal: 80, + }, + }, + Path: path, + }, + }, + }, + }, + }, + }, + }, + } + return ing +} + +func GetDeployLabels(trigger *fv1.HTTPTrigger) map[string]string { + // TODO: support function weight + return map[string]string{ + "triggerName": trigger.Metadata.Name, + "functionName": trigger.Spec.FunctionReference.Name, + "triggerNamespace": trigger.Metadata.Namespace, + } +} diff --git a/pkg/router/util/util_test.go b/pkg/router/util/util_test.go new file mode 100644 index 00000000..ac1c8524 --- /dev/null +++ b/pkg/router/util/util_test.go @@ -0,0 +1,499 @@ +/* +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 util + +import ( + "reflect" + "testing" + + "k8s.io/api/extensions/v1beta1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/util/intstr" + + fv1 "github.com/fission/fission/pkg/apis/fission.io/v1" +) + +func TestGetIngressSpec(t *testing.T) { + type args struct { + ingressNS string + trigger *fv1.HTTPTrigger + } + tests := []struct { + name string + args args + want *v1beta1.Ingress + }{ + { + name: "host-backward-compatibility", + args: args{ + ingressNS: "foobarNS", + trigger: &fv1.HTTPTrigger{ + Metadata: metav1.ObjectMeta{ + Name: "foo", + Namespace: "bar", + }, + Spec: fv1.HTTPTriggerSpec{ + Host: "test.com", + RelativeURL: "/foo/bar", + FunctionReference: fv1.FunctionReference{ + Name: "foofunc", + }, + IngressConfig: fv1.IngressConfig{ + Annotations: nil, + }, + }, + }, + }, + want: &v1beta1.Ingress{ + ObjectMeta: metav1.ObjectMeta{ + Labels: map[string]string{ + "triggerName": "foo", + "functionName": "foofunc", + "triggerNamespace": "bar", + }, + Name: "foo", + Namespace: "foobarNS", + Annotations: nil, + }, + Spec: v1beta1.IngressSpec{ + Rules: []v1beta1.IngressRule{ + { + Host: "test.com", + IngressRuleValue: v1beta1.IngressRuleValue{ + HTTP: &v1beta1.HTTPIngressRuleValue{ + Paths: []v1beta1.HTTPIngressPath{ + { + Backend: v1beta1.IngressBackend{ + ServiceName: "router", + ServicePort: intstr.IntOrString{ + Type: intstr.Int, + IntVal: 80, + }, + }, + Path: "/foo/bar", + }, + }, + }, + }, + }, + }, + }, + }, + }, + { + name: "create-ingress-with-only-annotations", + args: args{ + ingressNS: "foobarNS", + trigger: &fv1.HTTPTrigger{ + Metadata: metav1.ObjectMeta{ + Name: "foo", + Namespace: "bar", + }, + Spec: fv1.HTTPTriggerSpec{ + RelativeURL: "/foo/bar", + FunctionReference: fv1.FunctionReference{ + Name: "foofunc", + }, + IngressConfig: fv1.IngressConfig{ + Annotations: map[string]string{ + "key": "value", + }, + }, + }, + }, + }, + want: &v1beta1.Ingress{ + ObjectMeta: metav1.ObjectMeta{ + Labels: map[string]string{ + "triggerName": "foo", + "functionName": "foofunc", + "triggerNamespace": "bar", + }, + Name: "foo", + Namespace: "foobarNS", + Annotations: map[string]string{ + "key": "value", + }, + }, + Spec: v1beta1.IngressSpec{ + Rules: []v1beta1.IngressRule{ + { + Host: "", + IngressRuleValue: v1beta1.IngressRuleValue{ + HTTP: &v1beta1.HTTPIngressRuleValue{ + Paths: []v1beta1.HTTPIngressPath{ + { + Backend: v1beta1.IngressBackend{ + ServiceName: "router", + ServicePort: intstr.IntOrString{ + Type: intstr.Int, + IntVal: 80, + }, + }, + Path: "/foo/bar", + }, + }, + }, + }, + }, + }, + }, + }, + }, + { + name: "create-ingress-with-only-rule", + args: args{ + ingressNS: "foobarNS", + trigger: &fv1.HTTPTrigger{ + Metadata: metav1.ObjectMeta{ + Name: "foo", + Namespace: "bar", + }, + Spec: fv1.HTTPTriggerSpec{ + RelativeURL: "/foo/{bar}", + FunctionReference: fv1.FunctionReference{ + Name: "foofunc", + }, + IngressConfig: fv1.IngressConfig{ + Annotations: nil, + Path: "/foo/bar", + Host: "test.com", + }, + }, + }, + }, + want: &v1beta1.Ingress{ + ObjectMeta: metav1.ObjectMeta{ + Labels: map[string]string{ + "triggerName": "foo", + "functionName": "foofunc", + "triggerNamespace": "bar", + }, + Name: "foo", + Namespace: "foobarNS", + Annotations: nil, + }, + Spec: v1beta1.IngressSpec{ + Rules: []v1beta1.IngressRule{ + { + Host: "test.com", + IngressRuleValue: v1beta1.IngressRuleValue{ + HTTP: &v1beta1.HTTPIngressRuleValue{ + Paths: []v1beta1.HTTPIngressPath{ + { + Backend: v1beta1.IngressBackend{ + ServiceName: "router", + ServicePort: intstr.IntOrString{ + Type: intstr.Int, + IntVal: 80, + }, + }, + Path: "/foo/bar", + }, + }, + }, + }, + }, + }, + }, + }, + }, + { + name: "create-ingress-with-empty-rule-host", + args: args{ + ingressNS: "foobarNS", + trigger: &fv1.HTTPTrigger{ + Metadata: metav1.ObjectMeta{ + Name: "foo", + Namespace: "bar", + }, + Spec: fv1.HTTPTriggerSpec{ + RelativeURL: "/foo/{bar}", + FunctionReference: fv1.FunctionReference{ + Name: "foofunc", + }, + IngressConfig: fv1.IngressConfig{ + Annotations: nil, + Path: "/foo/bar", + Host: "", + }, + }, + }, + }, + want: &v1beta1.Ingress{ + ObjectMeta: metav1.ObjectMeta{ + Labels: map[string]string{ + "triggerName": "foo", + "functionName": "foofunc", + "triggerNamespace": "bar", + }, + Name: "foo", + Namespace: "foobarNS", + Annotations: nil, + }, + Spec: v1beta1.IngressSpec{ + Rules: []v1beta1.IngressRule{ + { + Host: "", + IngressRuleValue: v1beta1.IngressRuleValue{ + HTTP: &v1beta1.HTTPIngressRuleValue{ + Paths: []v1beta1.HTTPIngressPath{ + { + Backend: v1beta1.IngressBackend{ + ServiceName: "router", + ServicePort: intstr.IntOrString{ + Type: intstr.Int, + IntVal: 80, + }, + }, + Path: "/foo/{bar}", + }, + }, + }, + }, + }, + }, + }, + }, + }, + { + name: "create-ingress-with-empty-rule-path", + args: args{ + ingressNS: "foobarNS", + trigger: &fv1.HTTPTrigger{ + Metadata: metav1.ObjectMeta{ + Name: "foo", + Namespace: "bar", + }, + Spec: fv1.HTTPTriggerSpec{ + RelativeURL: "/foo/{bar}", + FunctionReference: fv1.FunctionReference{ + Name: "foofunc", + }, + IngressConfig: fv1.IngressConfig{ + Annotations: nil, + Path: "", + Host: "test.com", + }, + }, + }, + }, + want: &v1beta1.Ingress{ + ObjectMeta: metav1.ObjectMeta{ + Labels: map[string]string{ + "triggerName": "foo", + "functionName": "foofunc", + "triggerNamespace": "bar", + }, + Name: "foo", + Namespace: "foobarNS", + Annotations: nil, + }, + Spec: v1beta1.IngressSpec{ + Rules: []v1beta1.IngressRule{ + { + Host: "", + IngressRuleValue: v1beta1.IngressRuleValue{ + HTTP: &v1beta1.HTTPIngressRuleValue{ + Paths: []v1beta1.HTTPIngressPath{ + { + Backend: v1beta1.IngressBackend{ + ServiceName: "router", + ServicePort: intstr.IntOrString{ + Type: intstr.Int, + IntVal: 80, + }, + }, + Path: "/foo/{bar}", + }, + }, + }, + }, + }, + }, + }, + }, + }, + { + name: "create-ingress-with-host-and-rule", + args: args{ + ingressNS: "foobarNS", + trigger: &fv1.HTTPTrigger{ + Metadata: metav1.ObjectMeta{ + Name: "foo", + Namespace: "bar", + }, + Spec: fv1.HTTPTriggerSpec{ + Host: "example.com", + RelativeURL: "/foo/{bar}", + FunctionReference: fv1.FunctionReference{ + Name: "foofunc", + }, + IngressConfig: fv1.IngressConfig{ + Annotations: nil, + Path: "/foo/bar", + Host: "test.com", + }, + }, + }, + }, + want: &v1beta1.Ingress{ + ObjectMeta: metav1.ObjectMeta{ + Labels: map[string]string{ + "triggerName": "foo", + "functionName": "foofunc", + "triggerNamespace": "bar", + }, + Name: "foo", + Namespace: "foobarNS", + Annotations: nil, + }, + Spec: v1beta1.IngressSpec{ + Rules: []v1beta1.IngressRule{ + { + Host: "test.com", + IngressRuleValue: v1beta1.IngressRuleValue{ + HTTP: &v1beta1.HTTPIngressRuleValue{ + Paths: []v1beta1.HTTPIngressPath{ + { + Backend: v1beta1.IngressBackend{ + ServiceName: "router", + ServicePort: intstr.IntOrString{ + Type: intstr.Int, + IntVal: 80, + }, + }, + Path: "/foo/bar", + }, + }, + }, + }, + }, + }, + }, + }, + }, + { + name: "create-ingress-with-wildecard-rule-host", + args: args{ + ingressNS: "foobarNS", + trigger: &fv1.HTTPTrigger{ + Metadata: metav1.ObjectMeta{ + Name: "foo", + Namespace: "bar", + }, + Spec: fv1.HTTPTriggerSpec{ + RelativeURL: "/foo/{bar}", + FunctionReference: fv1.FunctionReference{ + Name: "foofunc", + }, + IngressConfig: fv1.IngressConfig{ + Annotations: nil, + Path: "/foo/bar", + Host: "*", + }, + }, + }, + }, + want: &v1beta1.Ingress{ + ObjectMeta: metav1.ObjectMeta{ + Labels: map[string]string{ + "triggerName": "foo", + "functionName": "foofunc", + "triggerNamespace": "bar", + }, + Name: "foo", + Namespace: "foobarNS", + Annotations: nil, + }, + Spec: v1beta1.IngressSpec{ + Rules: []v1beta1.IngressRule{ + { + Host: "", + IngressRuleValue: v1beta1.IngressRuleValue{ + HTTP: &v1beta1.HTTPIngressRuleValue{ + Paths: []v1beta1.HTTPIngressPath{ + { + Backend: v1beta1.IngressBackend{ + ServiceName: "router", + ServicePort: intstr.IntOrString{ + Type: intstr.Int, + IntVal: 80, + }, + }, + Path: "/foo/bar", + }, + }, + }, + }, + }, + }, + }, + }, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if got := GetIngressSpec(tt.args.ingressNS, tt.args.trigger); !reflect.DeepEqual(got, tt.want) { + t.Errorf("GetIngressSpec() = %v, want %v", got, tt.want) + } + }) + } +} + +func TestGetDeployLabels(t *testing.T) { + type args struct { + trigger *fv1.HTTPTrigger + } + // TODO: support function weight + tests := []struct { + name string + args args + want map[string]string + }{ + { + name: "getdeploylabels", + args: args{ + trigger: &fv1.HTTPTrigger{ + Metadata: metav1.ObjectMeta{ + Name: "foo", + Namespace: "bar", + }, + Spec: fv1.HTTPTriggerSpec{ + FunctionReference: fv1.FunctionReference{ + Type: "name", + Name: "foobar", + FunctionWeights: nil, + }, + }, + }, + }, + want: map[string]string{ + "triggerName": "foo", + "functionName": "foobar", + "triggerNamespace": "bar", + }, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if got := GetDeployLabels(tt.args.trigger); !reflect.DeepEqual(got, tt.want) { + t.Errorf("GetDeployLabels() = %v, want %v", got, tt.want) + } + }) + } +} diff --git a/test/test_utils.sh b/test/test_utils.sh index 2af521f3..b48f9bc6 100755 --- a/test/test_utils.sh +++ b/test/test_utils.sh @@ -65,6 +65,20 @@ setupCIBuildEnv() { export PRE_UPGRADE_CHECK_IMAGE=$REPO/pre-upgrade-checks } +setupIngressController() { + # set up NGINX ingress controller + kubectl create clusterrolebinding cluster-admin-binding --clusterrole cluster-admin --user $(gcloud config get-value account) || true + kubectl apply -f https://raw.githubusercontent.com/kubernetes/ingress-nginx/nginx-0.25.1/deploy/static/mandatory.yaml || true + kubectl apply -f https://raw.githubusercontent.com/kubernetes/ingress-nginx/nginx-0.25.1/deploy/static/provider/cloud-generic.yaml || true +} + +removeIngressController() { + # set up NGINX ingress controller + kubectl delete clusterrolebinding cluster-admin-binding || true + kubectl delete -f https://raw.githubusercontent.com/kubernetes/ingress-nginx/nginx-0.25.1/deploy/static/mandatory.yaml || true + kubectl delete -f https://raw.githubusercontent.com/kubernetes/ingress-nginx/nginx-0.25.1/deploy/static/provider/cloud-generic.yaml || true +} + build_and_push_go_mod_cache_image() { image_tag=$1 travis_fold_start go_mod_cache_image $image_tag @@ -195,9 +209,13 @@ set_environment() { id=$1 ns=f-$id + # fission env export FISSION_URL=http://$(kubectl -n $ns get svc controller -o jsonpath='{...ip}') export FISSION_ROUTER=$(kubectl -n $ns get svc router -o jsonpath='{...ip}') export FISSION_NATS_STREAMING_URL="http://defaultFissionAuthToken@$(kubectl -n $ns get svc nats-streaming -o jsonpath='{...ip}:{.spec.ports[0].port}')" + + # ingress controller env + export INGRESS_CONTROLLER=$(kubectl -n ingress-nginx get svc ingress-nginx -o jsonpath='{...ip}') } generate_test_id() { @@ -273,10 +291,9 @@ dump_tiller_logs() { export -f dump_tiller_logs wait_for_service() { - id=$1 + ns=$1 svc=$2 - ns=f-$id while true do ip=$(kubectl -n $ns get svc $svc -o jsonpath='{...ip}') @@ -291,9 +308,11 @@ export -f wait_for_service wait_for_services() { id=$1 + ns=f-$id - wait_for_service $id controller - wait_for_service $id router + wait_for_service $ns controller + wait_for_service $ns router + wait_for_service "ingress-nginx" ingress-nginx echo Waiting for service is routable... sleep 30 @@ -339,23 +358,6 @@ port_forward_services() { xargs -I{} kubectl port-forward {} $port:$port -n $ns & } -wait_for_service() { - id=$1 - svc=$2 - - ns=f-$id - while true - do - ip=$(kubectl -n $ns get svc $svc -o jsonpath='{...ip}') - if [ ! -z $ip ] - then - break - fi - echo Waiting for service $svc... - sleep 1 - done - } - dump_builder_pod_logs() { bns=$1 builderPods=$(kubectl -n $bns get pod -o name) @@ -600,6 +602,8 @@ install_and_test() { exit 1 fi + setupIngressController + timeout 150 bash -c "wait_for_services $id" timeout 120 bash -c "check_gitcommit_version" set_environment $id @@ -607,14 +611,15 @@ install_and_test() { run_all_tests $id $imageTag dump_logs $id + removeIngressController -# Commented out due to Travis-CI log length limit -# if [ $FAILURES -ne 0 ] -# then -# # describe each pod in fission ns and function namespace -# describe_all_pods $id -# exit 1 -# fi + if [ $FAILURES -ne 0 ] + then + # Commented out due to Travis-CI log length limit + # describe each pod in fission ns and function namespace + # describe_all_pods $id + exit 1 + fi } diff --git a/test/tests/test_ingress.sh b/test/tests/test_ingress.sh index 801033a9..f57133b7 100755 --- a/test/tests/test_ingress.sh +++ b/test/tests/test_ingress.sh @@ -7,50 +7,105 @@ echo "TEST_ID = $TEST_ID" ROOT=$(dirname $0)/../.. +env=nodejs-$TEST_ID relativeUrl="/itest-$TEST_ID" functionName="hellotest-$TEST_ID" hostName="test-$TEST_ID.com" +routeName="ingress-$TEST_ID" cleanup() { clean_resource_by_id $TEST_ID } +checkIngress() { + local route=$1 + local host=$2 + local path=$3 + local annotations=$4 + + log "Ingresses matching this trigger:" + kubectl get ing -l 'functionName='$functionName',triggerName='$route --all-namespaces -o=json + + log "Verifying to route value in ingress" + actual_path=$(kubectl get ing -l "functionName=$functionName,triggerName=$route" --all-namespaces -o=jsonpath='{.items[0].spec.rules[0].http.paths[0].path}') + + if [ "$path" != "$actual_path" ] + then + log "Provided route ($path) and route ($actual_path) in ingress don't match" + exit 1 + fi + + actual_host=$(kubectl get ing -l "functionName=$functionName,triggerName=$route" --all-namespaces -o=jsonpath='{.items[0].spec.rules[0].host}') + + if [ "$host" != "$actual_host" ] + then + log "Provided host ($host) and host ($actual_host) in ingress don't match" + exit 1 + fi + + actual_ann=$(kubectl get ing -l "functionName=$functionName,triggerName=$route" --all-namespaces -o jsonpath="{.items[0].metadata.annotations}") + if [ "$annotations" != "$actual_ann" ] + then + log "Provided annotations ($annotations) and annotations ($actual_ann) in ingress don't match" + exit 1 + fi +} + +createFn() { + # Create a hello world function in nodejs, test it with an http trigger + log "Creating nodejs env" + fission env create --name $env --image $NODE_RUNTIME_IMAGE + + log "Creating function" + fission fn create --name $functionName --env $env --code $ROOT/examples/nodejs/hello.js + + log "Doing an HTTP GET on the function's route" + response=$(fission fn test --name $functionName) + + log "Checking for valid response" + echo $response | grep -i hello +} + if [ -z "${TEST_NOCLEANUP:-}" ]; then trap cleanup EXIT else log "TEST_NOCLEANUP is set; not cleaning up test artifacts afterwards." fi +createFn + log "Creating route for URL $relativeUrl" -route_name=$(fission route create --url $relativeUrl --function $functionName --createingress| grep trigger| cut -d" " -f 2|cut -d"'" -f 2) +fission route create --name $routeName --url $relativeUrl --function $functionName --createingress -log "Route $route_name created" - -sleep 5 - -log "Ingresses matching this trigger:" -kubectl get ing -l 'functionName='$functionName',triggerName='$route_name --all-namespaces -o=json - -log "Verifying to route value in ingress" -actual_route=$(kubectl -n fission get ing -l 'functionName='$functionName',triggerName='$route_name --all-namespaces -o=jsonpath='{.items[0].spec.rules[0].http.paths[0].path}') - -if [ $actual_route != $relativeUrl ] -then - log "Provided route and route in ingress don't match" - exit 1 -fi +sleep 3 +checkIngress $routeName "" $relativeUrl "" log "Modifying the route by adding host" -fission route update --name $route_name --host $hostName --function $functionName +fission route update --name $routeName --function $functionName --ingressannotation "foo=bar" --ingressrule "$hostName=/foo/bar" -sleep 2 +sleep 3 +checkIngress $routeName $hostName "/foo/bar" "map[foo:bar]" -actual_host=$(kubectl get ing -l 'functionName='$functionName',triggerName='$route_name --all-namespaces -o=jsonpath='{.items[0].spec.rules[0].host}') +log "Remove ingress annotations, host and rule" +fission route update --name $routeName --function $functionName --ingressannotation "-" --ingressrule "-" -if [ $hostName != $actual_host ] -then - log "Provided host and host in ingress don't match" - exit 1 -fi +sleep 3 +checkIngress $routeName "" $relativeUrl "" + +fission route delete --name $routeName + +relativeUrl="/itest-$TEST_ID/{url}" +wildcardPath="/itest-$TEST_ID/*" +realPath="itest-$TEST_ID/test" + +log "Creating route for wildcard URL $relativeUrl" +fission route create --name $routeName --url $relativeUrl --function $functionName --createingress \ + --ingressannotation "nginx.ingress.kubernetes.io/ssl-redirect=false" \ + --ingressannotation "nginx.ingress.kubernetes.io/use-regex=true" \ + --ingressrule "*=$wildcardPath" + +sleep 3 +checkIngress $routeName "" $wildcardPath "map[nginx.ingress.kubernetes.io/ssl-redirect:false nginx.ingress.kubernetes.io/use-regex:true]" +timeout 10 bash -c "test_ingress $realPath 'hello, world!'" log "Test PASSED" diff --git a/test/utils.sh b/test/utils.sh index 78d53b4a..cb04b7cb 100755 --- a/test/utils.sh +++ b/test/utils.sh @@ -44,9 +44,25 @@ clean_resource_by_id() { } test_fn() { + url="http://$FISSION_ROUTER/$1" + expect=$2 + test_response $url $expect +} +export -f test_fn + +test_ingress() { + url="http://$INGRESS_CONTROLLER/$1" + expect=$2 + + echo $url + test_response $url $expect +} +export -f test_ingress + +test_response() { # Doing an HTTP GET on the function's route # Checking for valid response - url="http://$FISSION_ROUTER/$1" + url=$1 expect=$2 set +e @@ -69,7 +85,7 @@ test_fn() { done set -e } -export -f test_fn +export -f test_response test_post_route() { # Doing an HTTP POST on the function's route