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 <sanketsudake@gmail.com>
Co-authored-by: Sanket Sudake <sanketsudake@gmail.com>

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)
 }

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