diff --git a/pkg/apis/flows/v1alpha1/flow_defaults.go b/pkg/apis/flows/v1alpha1/flow_defaults.go index 7cf54d7c183..d02436f284d 100644 --- a/pkg/apis/flows/v1alpha1/flow_defaults.go +++ b/pkg/apis/flows/v1alpha1/flow_defaults.go @@ -26,9 +26,9 @@ func (fs *FlowSpec) SetDefaults() { } func (fa *FlowAction) SetDefaults() { - // TODO anything? + // nothing to do } func (et *EventTrigger) SetDefaults() { - // TODO anything? + // // nothing to do } diff --git a/pkg/apis/flows/v1alpha1/flow_types.go b/pkg/apis/flows/v1alpha1/flow_types.go index 59ec3bcb2e8..bbb6882ac18 100644 --- a/pkg/apis/flows/v1alpha1/flow_types.go +++ b/pkg/apis/flows/v1alpha1/flow_types.go @@ -42,10 +42,9 @@ type Flow struct { Status FlowStatus `json:"status"` } -// Check that Flow can be validated, can be defaulted, and has immutable fields. +// Check that Flow can be validated and can be defaulted. var _ apis.Validatable = (*Flow)(nil) var _ apis.Defaultable = (*Flow)(nil) -var _ apis.Immutable = (*Flow)(nil) var _ runtime.Object = (*Flow)(nil) var _ webhook.GenericCRD = (*Flow)(nil) diff --git a/pkg/apis/flows/v1alpha1/flow_validation.go b/pkg/apis/flows/v1alpha1/flow_validation.go index b5a4bbd9d08..e11025aa5e4 100644 --- a/pkg/apis/flows/v1alpha1/flow_validation.go +++ b/pkg/apis/flows/v1alpha1/flow_validation.go @@ -19,15 +19,19 @@ package v1alpha1 import ( "github.com/knative/pkg/apis" "k8s.io/apimachinery/pkg/api/equality" + "k8s.io/apimachinery/pkg/util/validation" + "net/url" ) +// Validate validates the Flow resource. func (f *Flow) Validate() *apis.FieldError { return f.Spec.Validate().ViaField("spec") } +// Validate validates the flow spec for a valid action and trigger. func (fs *FlowSpec) Validate() *apis.FieldError { if equality.Semantic.DeepEqual(fs, &FlowSpec{}) { - return apis.ErrMissingField(apis.CurrentField) + return apis.ErrMissingField("trigger", "action") } if err := fs.Trigger.Validate(); err != nil { @@ -39,17 +43,39 @@ func (fs *FlowSpec) Validate() *apis.FieldError { return nil } +// Validate validates event trigger has both eventType and resource set. func (et *EventTrigger) Validate() *apis.FieldError { - // TODO(n3wscott): Implement this. - return nil -} -func (fa *FlowAction) Validate() *apis.FieldError { - // TODO(n3wscott): Implement this. + if et.EventType == "" { + return apis.ErrMissingField("eventType") + } + if errs := validation.IsQualifiedName(et.EventType); len(errs) > 0 { + return apis.ErrInvalidValue(et.EventType, "eventType") + } + + if et.Resource == "" { + return apis.ErrMissingField("resource") + } + return nil } -func (current *Flow) CheckImmutableFields(og apis.Immutable) *apis.FieldError { - // TODO(n3wscott): Anything Immutable? - return nil +// Validate validates flow action has a valid formed target or targetURI, but not both. +func (fa *FlowAction) Validate() *apis.FieldError { + switch { + case fa.Target != nil && fa.TargetURI != nil: + return apis.ErrMultipleOneOf("target", "targetURI") + case fa.Target != nil: + if errs := validation.IsQualifiedName(fa.Target.Name); len(errs) > 0 { + return apis.ErrInvalidValue(fa.Target.Name, "name").ViaField("target") + } + return nil + case fa.TargetURI != nil: + if _, err := url.ParseRequestURI(*fa.TargetURI); err != nil { + return apis.ErrInvalidValue(*fa.TargetURI, "targetURI") + } + return nil + default: + return apis.ErrMissingOneOf("target", "targetURI") + } } diff --git a/pkg/apis/flows/v1alpha1/flow_validation_test.go b/pkg/apis/flows/v1alpha1/flow_validation_test.go new file mode 100644 index 00000000000..d0b0eb8ff01 --- /dev/null +++ b/pkg/apis/flows/v1alpha1/flow_validation_test.go @@ -0,0 +1,185 @@ +/* +Copyright 2018 The Knative 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 v1alpha1 + +import ( + "github.com/google/go-cmp/cmp" + "github.com/google/go-cmp/cmp/cmpopts" + "github.com/knative/pkg/apis" + "k8s.io/api/core/v1" + "testing" +) + +func TestFlowSpecValidation(t *testing.T) { + tests := []struct { + name string + f *FlowSpec + want *apis.FieldError + }{{ + name: "valid with URI", + f: &FlowSpec{ + Action: FlowAction{ + TargetURI: stringPtr("http://example.com/action"), + }, + Trigger: EventTrigger{ + EventType: "foo", + Resource: "bar", + }, + }, + }, { + name: "valid with Target", + f: &FlowSpec{ + Action: FlowAction{ + Target: &v1.ObjectReference{ + Name: "foo", + }, + }, + Trigger: EventTrigger{ + EventType: "foo", + Resource: "bar", + }, + }, + }, { + name: "invalid with URI", + f: &FlowSpec{ + Action: FlowAction{ + TargetURI: stringPtr("http//example.com/action"), + }, + Trigger: EventTrigger{ + EventType: "foo", + Resource: "bar", + }, + }, + want: &apis.FieldError{ + Message: `invalid value "http//example.com/action"`, + Paths: []string{"action.targetURI"}, + }, + }, { + name: "invalid with Target", + f: &FlowSpec{ + Action: FlowAction{ + Target: &v1.ObjectReference{ + Name: "f@o", + }, + }, + Trigger: EventTrigger{ + EventType: "foo", + Resource: "bar", + }, + }, + want: &apis.FieldError{ + Message: `invalid value "f@o"`, + Paths: []string{"action.target.name"}, + }, + }, { + name: "invalid with both target and targetURI", + f: &FlowSpec{ + Action: FlowAction{ + TargetURI: stringPtr("http://example.com/action"), + Target: &v1.ObjectReference{ + Name: "foo", + }, + }, + Trigger: EventTrigger{ + EventType: "foo", + Resource: "bar", + }, + }, + want: &apis.FieldError{ + Message: `expected exactly one, got both`, + Paths: []string{"action.target", "action.targetURI"}, + }, + }, { + name: "invalid, no target or targetURI", + f: &FlowSpec{ + Action: FlowAction{}, + Trigger: EventTrigger{ + EventType: "foo", + Resource: "bar", + }, + }, + want: &apis.FieldError{ + Message: `expected exactly one, got neither`, + Paths: []string{"action.target", "action.targetURI"}, + }, + }, { + name: "invalid, missing event type", + f: &FlowSpec{ + Action: FlowAction{ + TargetURI: stringPtr("http://example.com/action"), + }, + Trigger: EventTrigger{ + Resource: "bar", + }, + }, + want: &apis.FieldError{ + Message: "missing field(s)", + Paths: []string{"trigger.eventType"}, + }, + }, { + name: "invalid event type", + f: &FlowSpec{ + Action: FlowAction{ + TargetURI: stringPtr("http://example.com/action"), + }, + Trigger: EventTrigger{ + EventType: "f@o", + Resource: "bar", + }, + }, + want: &apis.FieldError{ + Message: `invalid value "f@o"`, + Paths: []string{"trigger.eventType"}, + }, + }, { + name: "invalid, missing resource", + f: &FlowSpec{ + Action: FlowAction{ + TargetURI: stringPtr("http://example.com/action"), + }, + Trigger: EventTrigger{ + EventType: "foo", + }, + }, + want: &apis.FieldError{ + Message: "missing field(s)", + Paths: []string{"trigger.resource"}, + }, + }, { + name: "empty", + f: &FlowSpec{}, + want: &apis.FieldError{ + Message: "missing field(s)", + Paths: []string{"trigger", "action"}, + }, + }} + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + got := test.f.Validate() + ignoreArguments := cmpopts.IgnoreFields(apis.FieldError{}, "Details") + if diff := cmp.Diff(test.want, got, ignoreArguments); diff != "" { + t.Errorf("validateFlow (-want, +got) = %v", diff) + } + }) + } +} + +// stringPtr returns a pointer to the passed string. +func stringPtr(s string) *string { + return &s +}