From a94e8126686b05407ba5d8fe211dbca7e6fbde63 Mon Sep 17 00:00:00 2001 From: Derek Wang Date: Thu, 3 Dec 2020 14:44:19 -0800 Subject: [PATCH] feat: Validate dependencies duplication. Closes #971 (#972) * feat: Validate dependencies duplication. Closes #971 Signed-off-by: Derek Wang * fix test case Signed-off-by: Derek Wang Co-authored-by: Vaibhav --- controllers/sensor/validate.go | 17 +++++--- controllers/sensor/validate_test.go | 48 +++++++++++++++++++++++ examples/sensors/time-filter-webhook.yaml | 2 +- sensors/dependencies/filter_test.go | 12 +++--- 4 files changed, 67 insertions(+), 12 deletions(-) diff --git a/controllers/sensor/validate.go b/controllers/sensor/validate.go index 2fca3e89d4..3ae333cad9 100644 --- a/controllers/sensor/validate.go +++ b/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 } diff --git a/controllers/sensor/validate_test.go b/controllers/sensor/validate_test.go index 79a798cda4..c5a44cca4b 100644 --- a/controllers/sensor/validate_test.go +++ b/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")) + }) +} diff --git a/examples/sensors/time-filter-webhook.yaml b/examples/sensors/time-filter-webhook.yaml index 696a590de6..48382d790f 100644 --- a/examples/sensors/time-filter-webhook.yaml +++ b/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, diff --git a/sensors/dependencies/filter_test.go b/sensors/dependencies/filter_test.go index e6c14e4224..ed6a1d8283 100644 --- a/sensors/dependencies/filter_test.go +++ b/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: "!=", }, },