From 2882e0d3e72e1a690483fa2888b123659778f567 Mon Sep 17 00:00:00 2001 From: neha_gupta Date: Tue, 13 Sep 2022 13:30:00 +0530 Subject: [PATCH] allow two HTTP triggers with no URLs and different prefix (#2540) * allow two HTTP triggers with no URL and different prefix * update dependency * Fix controller existing tests * Ensure namespace cleanup in API test * update test cases * handle error conditions in test Signed-off-by: Sanket Sudake Co-authored-by: Sanket Sudake --- pkg/controller/api_test.go | 162 +++++++++++++++++++++++++++---- pkg/controller/httpTriggerApi.go | 2 +- 2 files changed, 144 insertions(+), 20 deletions(-) diff --git a/pkg/controller/api_test.go b/pkg/controller/api_test.go index 9eb2297b..e9627184 100644 --- a/pkg/controller/api_test.go +++ b/pkg/controller/api_test.go @@ -31,7 +31,6 @@ import ( uuid "github.com/satori/go.uuid" "go.uber.org/zap" - "go.uber.org/zap/zapcore" v1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -41,6 +40,7 @@ import ( "github.com/fission/fission/pkg/crd" ferror "github.com/fission/fission/pkg/error" "github.com/fission/fission/pkg/fission-cli/cmd" + "github.com/fission/fission/pkg/utils/loggerfactory" ) var ( @@ -130,7 +130,10 @@ func TestFunctionApi(t *testing.T) { testFunc.ObjectMeta.Name = "bar" m2, err := g.Client().V1().Function().Create(testFunc) panicIf(err) - defer panicIf(g.Client().V1().Function().Delete(m2)) + defer func() { + err := g.Client().V1().Function().Delete(m2) + panicIf(err) + }() funcs, err := g.Client().V1().Function().List(testNS) panicIf(err) @@ -174,7 +177,10 @@ func TestHTTPTriggerApi(t *testing.T) { m, err := g.Client().V1().HTTPTrigger().Create(testTrigger) panicIf(err) - defer panicIf(g.Client().V1().HTTPTrigger().Delete(m)) + defer func() { + err := g.Client().V1().HTTPTrigger().Delete(m) + panicIf(err) + }() _, err = g.Client().V1().HTTPTrigger().Create(testTrigger) assertNameReuseFailure(err, "httptrigger") @@ -199,13 +205,111 @@ func TestHTTPTriggerApi(t *testing.T) { testTrigger.Spec.RelativeURL = "/hi2" m2, err := g.Client().V1().HTTPTrigger().Create(testTrigger) panicIf(err) - defer panicIf(g.Client().V1().HTTPTrigger().Delete(m2)) + defer func() { + err = g.Client().V1().HTTPTrigger().Delete(m2) + panicIf(err) + }() ts, err := g.Client().V1().HTTPTrigger().List(testNS) panicIf(err) assert(len(ts) == 2, fmt.Sprintf("created two triggers, but found %v", len(ts))) } +func createFissionFnForMultipleTrigger() { + testFunc := &fv1.Function{ + ObjectMeta: metav1.ObjectMeta{ + Name: "foo1", + Namespace: testNS, + }, + Spec: fv1.FunctionSpec{ + Environment: fv1.EnvironmentReference{ + Name: "nodejs", + Namespace: testNS, + }, + Package: fv1.FunctionPackageRef{ + FunctionName: "xxx", + PackageRef: fv1.PackageRef{ + Namespace: testNS, + Name: "xxx", + ResourceVersion: "12345", + }, + }, + }, + } + + _, err := g.Client().V1().Function().Create(testFunc) + panicIf(err) + defer func() { + panicIf(err) + }() + + testFunc.Name = "foo2" + + _, err = g.Client().V1().Function().Create(testFunc) + panicIf(err) + defer func() { + panicIf(err) + }() +} + +func TestHTTPTriggerCreateMultipleTrigger(t *testing.T) { + logger := loggerfactory.GetLogger() + createFissionFnForMultipleTrigger() + prefix := "url_new" + testTrigger := &fv1.HTTPTrigger{ + ObjectMeta: metav1.ObjectMeta{ + Name: "foo1", + Namespace: testNS, + }, + + Spec: fv1.HTTPTriggerSpec{ + Methods: []string{http.MethodGet}, + Prefix: &prefix, + FunctionReference: fv1.FunctionReference{ + Type: fv1.FunctionReferenceTypeFunctionName, + Name: "foo1", + }, + }, + } + + m, err := g.Client().V1().HTTPTrigger().Create(testTrigger) + panicIf(err) + defer panicIf(g.Client().V1().HTTPTrigger().Delete(m)) + + prefix_2 := "url_another" + testTrigger2 := &fv1.HTTPTrigger{ + ObjectMeta: metav1.ObjectMeta{ + Name: "foo2", + Namespace: testNS, + }, + + Spec: fv1.HTTPTriggerSpec{ + Methods: []string{http.MethodGet}, + Prefix: &prefix_2, + FunctionReference: fv1.FunctionReference{ + Type: fv1.FunctionReferenceTypeFunctionName, + Name: "foo2", + }, + }, + } + + m2, err := g.Client().V1().HTTPTrigger().Create(testTrigger2) + + if err != nil { + t.Fatal() + } + + defer func() { + if m2 != nil { + err := g.Client().V1().HTTPTrigger().Delete(m2) + if err != nil { + logger.Error("Error deleting http trigger", zap.String("name", m2.Name), zap.Error(err)) + } + } + }() + +} + func TestEnvironmentApi(t *testing.T) { testEnv := &fv1.Environment{ ObjectMeta: metav1.ObjectMeta{ @@ -228,7 +332,10 @@ func TestEnvironmentApi(t *testing.T) { m, err := g.Client().V1().Environment().Create(testEnv) panicIf(err) - defer panicIf(g.Client().V1().Environment().Delete(m)) + defer func() { + err := g.Client().V1().Environment().Delete(m) + panicIf(err) + }() _, err = g.Client().V1().Environment().Create(testEnv) assertNameReuseFailure(err, "environment") @@ -247,7 +354,10 @@ func TestEnvironmentApi(t *testing.T) { m2, err := g.Client().V1().Environment().Create(testEnv) panicIf(err) - defer panicIf(g.Client().V1().Environment().Delete(m2)) + defer func() { + err := g.Client().V1().Environment().Delete(m2) + panicIf(err) + }() ts, err := g.Client().V1().Environment().List(testNS) panicIf(err) @@ -277,7 +387,10 @@ func TestWatchApi(t *testing.T) { m, err := g.Client().V1().KubeWatcher().Create(testWatch) panicIf(err) - defer panicIf(g.Client().V1().KubeWatcher().Delete(m)) + defer func() { + err := g.Client().V1().KubeWatcher().Delete(m) + panicIf(err) + }() _, err = g.Client().V1().KubeWatcher().Create(testWatch) assertNameReuseFailure(err, "watch") @@ -292,7 +405,10 @@ func TestWatchApi(t *testing.T) { testWatch.ObjectMeta.Name = "yyy" m2, err := g.Client().V1().KubeWatcher().Create(testWatch) panicIf(err) - defer panicIf(g.Client().V1().KubeWatcher().Delete(m2)) + defer func() { + err := g.Client().V1().KubeWatcher().Delete(m2) + panicIf(err) + }() ws, err := g.Client().V1().KubeWatcher().List(testNS) panicIf(err) @@ -318,7 +434,10 @@ func TestTimeTriggerApi(t *testing.T) { m, err := g.Client().V1().TimeTrigger().Create(testTrigger) panicIf(err) - defer panicIf(g.Client().V1().TimeTrigger().Delete(m)) + defer func() { + err := g.Client().V1().TimeTrigger().Delete(m) + panicIf(err) + }() _, err = g.Client().V1().TimeTrigger().Create(testTrigger) assertNameReuseFailure(err, "trigger") @@ -348,10 +467,15 @@ func TestTimeTriggerApi(t *testing.T) { func TestMain(m *testing.M) { flag.Parse() + ctx, cancel := context.WithCancel(context.Background()) + + logger := loggerfactory.GetLogger() + // skip test if no cluster available for testing kubeconfig := os.Getenv("KUBECONFIG") if len(kubeconfig) == 0 { log.Println("Skipping test, no kubernetes cluster") + cancel() return } @@ -362,21 +486,13 @@ func TestMain(m *testing.M) { id, err := uuid.NewV4() panicIf(err) testNS = id.String() - _, err = kubeClient.CoreV1().Namespaces().Create(context.TODO(), &v1.Namespace{ + _, err = kubeClient.CoreV1().Namespaces().Create(ctx, &v1.Namespace{ ObjectMeta: metav1.ObjectMeta{ Name: testNS, }, }, metav1.CreateOptions{}) panicIf(err) - defer panicIf(kubeClient.CoreV1().Namespaces().Delete(context.TODO(), testNS, metav1.DeleteOptions{})) - config := zap.NewDevelopmentConfig() - config.EncoderConfig.EncodeTime = zapcore.ISO8601TimeEncoder - logger, err := config.Build() - - panicIf(err) - - ctx := context.Background() go Start(ctx, logger, 8888, true) time.Sleep(5 * time.Second) @@ -400,5 +516,13 @@ func TestMain(m *testing.M) { _, err = io.ReadAll(resp.Body) panicIf(err) - os.Exit(m.Run()) + exitVal := m.Run() + logger.Info("Deleting test namespace", zap.String("namespace", testNS)) + gracePeriod := int64(0) + err = kubeClient.CoreV1().Namespaces().Delete(context.TODO(), testNS, metav1.DeleteOptions{GracePeriodSeconds: &gracePeriod}) + if err != nil { + logger.Error("error deleting test namespace", zap.String("namespace", testNS), zap.Error(err)) + } + cancel() + os.Exit(exitVal) } diff --git a/pkg/controller/httpTriggerApi.go b/pkg/controller/httpTriggerApi.go index 1f5449d4..6a5a97ec 100644 --- a/pkg/controller/httpTriggerApi.go +++ b/pkg/controller/httpTriggerApi.go @@ -134,7 +134,7 @@ func (a *API) checkHTTPTriggerDuplicates(ctx context.Context, t *fv1.HTTPTrigger continue } urlMatch := false - if ht.Spec.RelativeURL == t.Spec.RelativeURL || (ht.Spec.Prefix != nil && t.Spec.Prefix != nil && *ht.Spec.Prefix != "" && *ht.Spec.Prefix == *t.Spec.Prefix) { + if (ht.Spec.RelativeURL != "" && ht.Spec.RelativeURL == t.Spec.RelativeURL) || (ht.Spec.Prefix != nil && t.Spec.Prefix != nil && *ht.Spec.Prefix != "" && *ht.Spec.Prefix == *t.Spec.Prefix) { urlMatch = true } methodMatch := false