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