feat: Validate dependencies duplication. Closes #971 (#972)

* feat: Validate dependencies duplication. Closes #971

Signed-off-by: Derek Wang <whynowy@gmail.com>

* fix test case

Signed-off-by: Derek Wang <whynowy@gmail.com>

Co-authored-by: Vaibhav <vaibhav.page@gmail.com>

controllers/sensor/validate.go

@@ -17,6 +17,7 @@ limitations under the License.
 package sensor
 
 import (
+	"fmt"
 	"net/http"
 	"strings"
 	"time"
@@ -33,7 +34,7 @@ import (
 // Exporting this function so that external APIs can use this to validate sensor resource.
 func ValidateSensor(s *v1alpha1.Sensor) error {
 	if err := validateDependencies(s.Spec.Dependencies); err != nil {
-		s.Status.MarkDependenciesNotProvided("InvalidDependencies", "Faild to validate dependencies.")
+		s.Status.MarkDependenciesNotProvided("InvalidDependencies", err.Error())
 		return err
 	}
 	// DEPRECATED.
@@ -394,7 +395,7 @@ func validateCustomTrigger(trigger *v1alpha1.CustomTrigger) error {
 	return nil
 }
 
-// validateTriggerParameters validates resource and template parameters if any
+// validateTriggerTemplateParameters validates resource and template parameters if any
 func validateTriggerTemplateParameters(trigger *v1alpha1.Trigger) error {
 	if trigger.Parameters != nil {
 		for i, parameter := range trigger.Parameters {
@@ -435,18 +436,24 @@ func validateDependencies(eventDependencies []v1alpha1.EventDependency) error {
 	if len(eventDependencies) < 1 {
 		return errors.New("no event dependencies found")
 	}
+	comboKeys := make(map[string]bool)
 	for _, dep := range eventDependencies {
 		if dep.Name == "" {
 			return errors.New("event dependency must define a name")
 		}
 		if dep.EventSourceName == "" {
-			return errors.New("event dependency must define the EventSource name")
+			return errors.New("event dependency must define the EventSourceName")
 		}
 
 		if dep.EventName == "" {
-			return errors.New("event dependency must define the event name")
+			return errors.New("event dependency must define the EventName")
 		}
-
+		// EventSourceName + EventName can not be referenced more than once in one Sensor object.
+		comboKey := fmt.Sprintf("%s-$$$-%s", dep.EventSourceName, dep.EventName)
+		if _, existing := comboKeys[comboKey]; existing {
+			return errors.Errorf("%s and %s are referenced more than once in this Sensor object", dep.EventSourceName, dep.EventName)
+		}
+		comboKeys[comboKey] = true
 		if err := validateEventFilter(dep.Filters); err != nil {
 			return err
 		}

controllers/sensor/validate_test.go

@@ -19,6 +19,7 @@ package sensor
 import (
 	"fmt"
 	"io/ioutil"
+	"strings"
 	"testing"
 
 	"github.com/argoproj/argo-events/pkg/apis/sensor/v1alpha1"
@@ -40,3 +41,50 @@ func TestValidateSensor(t *testing.T) {
 		assert.Nil(t, err)
 	}
 }
+
+func TestValidDepencies(t *testing.T) {
+	t.Run("test duplicate deps", func(t *testing.T) {
+		sObj := sensorObj.DeepCopy()
+		sObj.Spec.Dependencies = append(sObj.Spec.Dependencies, v1alpha1.EventDependency{
+			Name:            "fake-dep2",
+			EventSourceName: "fake-source",
+			EventName:       "fake-one",
+		})
+		err := ValidateSensor(sObj)
+		assert.NotNil(t, err)
+		assert.Equal(t, true, strings.Contains(err.Error(), "more than once"))
+	})
+
+	t.Run("test empty event source name", func(t *testing.T) {
+		sObj := sensorObj.DeepCopy()
+		sObj.Spec.Dependencies = append(sObj.Spec.Dependencies, v1alpha1.EventDependency{
+			Name:      "fake-dep2",
+			EventName: "fake-one",
+		})
+		err := ValidateSensor(sObj)
+		assert.NotNil(t, err)
+		assert.Equal(t, true, strings.Contains(err.Error(), "must define the EventSourceName"))
+	})
+
+	t.Run("test empty event name", func(t *testing.T) {
+		sObj := sensorObj.DeepCopy()
+		sObj.Spec.Dependencies = append(sObj.Spec.Dependencies, v1alpha1.EventDependency{
+			Name:            "fake-dep2",
+			EventSourceName: "fake-source",
+		})
+		err := ValidateSensor(sObj)
+		assert.NotNil(t, err)
+		assert.Equal(t, true, strings.Contains(err.Error(), "must define the EventName"))
+	})
+
+	t.Run("test empty event name", func(t *testing.T) {
+		sObj := sensorObj.DeepCopy()
+		sObj.Spec.Dependencies = append(sObj.Spec.Dependencies, v1alpha1.EventDependency{
+			EventSourceName: "fake-source2",
+			EventName:       "fake-one2",
+		})
+		err := ValidateSensor(sObj)
+		assert.NotNil(t, err)
+		assert.Equal(t, true, strings.Contains(err.Error(), "must define a name"))
+	})
+}

examples/sensors/time-filter-webhook.yaml

@@ -15,7 +15,7 @@ spec:
           stop: "23:04:05"
     - name: test-another-dep
       eventSourceName: webhook
-      eventName: example
+      eventName: example1
       filters:
         time:
           # start > stop, stop is treated as next day of start,

sensors/dependencies/filter_test.go

@@ -160,9 +160,9 @@ func TestFilterData(t *testing.T) {
 			args: args{
 				data: []v1alpha1.DataFilter{
 					{
-						Path:  "k",
-						Type:  v1alpha1.JSONTypeString,
-						Value: []string{"v"},
+						Path:       "k",
+						Type:       v1alpha1.JSONTypeString,
+						Value:      []string{"v"},
 						Comparator: "=",
 					},
 				},
@@ -181,9 +181,9 @@ func TestFilterData(t *testing.T) {
 			args: args{
 				data: []v1alpha1.DataFilter{
 					{
-						Path:  "k",
-						Type:  v1alpha1.JSONTypeString,
-						Value: []string{"b"},
+						Path:       "k",
+						Type:       v1alpha1.JSONTypeString,
+						Value:      []string{"b"},
 						Comparator: "!=",
 					},
 				},