Adding validatable to bus types.

pkg/apis/channels/v1alpha1/bus_defaults.go

@@ -0,0 +1,33 @@
+/*
+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
+
+// TODO(n3wscott): This is staging work, the plan is another pass to bring up
+// the test coverage, then remove unused after each type is stubbed.
+// This is all prep for new serving style webhook integration.
+
+func (b *Bus) SetDefaults() {
+	b.Spec.SetDefaults()
+}
+
+func (bs *BusSpec) SetDefaults() {
+	bs.Parameters.SetDefaults()
+}
+
+func (bp *BusParameters) SetDefaults() {
+	// TODO anything?
+}

pkg/apis/channels/v1alpha1/bus_types.go

@@ -19,6 +19,7 @@ package v1alpha1
 import (
 	"encoding/json"
 
+	"github.com/knative/pkg/apis"
 	kapi "k8s.io/api/core/v1"
 	meta_v1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/runtime"
@@ -145,4 +146,9 @@ type GenericBus interface {
 	meta_v1.ObjectMetaAccessor
 	BacksChannel(channel *Channel) bool
 	GetSpec() *BusSpec
+
+	// Needed for generic webhook support
+	apis.Defaultable
+	apis.Immutable
+	apis.Validatable
 }

pkg/apis/channels/v1alpha1/bus_validation.go

@@ -0,0 +1,63 @@
+/*
+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 (
+	"fmt"
+	"github.com/knative/pkg/apis"
+	"k8s.io/apimachinery/pkg/util/validation"
+)
+
+// TODO(n3wscott): This is staging work, the plan is another pass to bring up
+// the test coverage, then remove unused after each type is stubbed.
+// This is all prep for new serving style webhook integration.
+
+func (b *Bus) Validate() *apis.FieldError {
+	return b.Spec.Validate().ViaField("spec")
+}
+
+func (bs *BusSpec) Validate() *apis.FieldError {
+	if bs.Parameters != nil {
+		return bs.Parameters.Validate().ViaField("parameters")
+	}
+	return nil
+}
+
+func (bp *BusParameters) Validate() *apis.FieldError {
+	if bp.Channel != nil {
+		for i, p := range *bp.Channel {
+			errs := validation.IsConfigMapKey(p.Name)
+			if len(errs) > 0 {
+				return apis.ErrInvalidKeyName(p.Name, "name", errs...).ViaField(fmt.Sprintf("channel[%d]", i))
+			}
+		}
+	}
+	if bp.Subscription != nil {
+		for i, p := range *bp.Subscription {
+			errs := validation.IsConfigMapKey(p.Name)
+			if len(errs) > 0 {
+				return apis.ErrInvalidKeyName(p.Name, "name", errs...).ViaField(fmt.Sprintf("subscription[%d]", i))
+			}
+		}
+	}
+	return nil
+}
+
+func (current *Bus) CheckImmutableFields(og apis.Immutable) *apis.FieldError {
+	// TODO(n3wscott): Anything to check?
+	return nil
+}

pkg/apis/channels/v1alpha1/bus_validation_test.go

@@ -0,0 +1,140 @@
+/*
+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 (
+	"fmt"
+	"github.com/google/go-cmp/cmp"
+	"github.com/knative/pkg/apis"
+	"strings"
+	"testing"
+)
+
+var longName = strings.Repeat("A", 255)
+
+// TODO: add the following tests:
+//  1. Multiple parameters to the same Bus.
+//  2. Two parameters with the same Name in the same list.
+//  3. Multiple errors in the same object.
+
+func TestBusSpecValidation(t *testing.T) {
+	tests := []struct {
+		name string
+		bs   *BusSpec
+		want *apis.FieldError
+	}{{
+		name: "valid",
+		bs: &BusSpec{
+			Parameters: &BusParameters{
+				Channel: &[]Parameter{
+					{
+						Name:        "foo",
+						Description: "bar",
+					},
+				},
+				Subscription: &[]Parameter{
+					{
+						Name:        "foo",
+						Description: "bar",
+					},
+				},
+			},
+		},
+	}, {
+		name: "valid no description",
+		bs: &BusSpec{
+			Parameters: &BusParameters{
+				Channel: &[]Parameter{
+					{
+						Name: "foo",
+					},
+				},
+				Subscription: &[]Parameter{
+					{
+						Name: "foo",
+					},
+				},
+			},
+		},
+	}, {
+		name: "invalid channel parameter",
+		bs: &BusSpec{
+			Parameters: &BusParameters{
+				Channel: &[]Parameter{
+					{
+						Name: "foo@bar",
+					},
+				},
+			},
+		},
+		want: &apis.FieldError{
+			Message: `invalid key name "foo@bar"`,
+			Paths: []string{
+				"parameters.channel[0].name",
+			},
+			Details: "a valid config key must consist of alphanumeric characters, '-', '_' or '.' (e.g. 'key.name',  or 'KEY_NAME',  or 'key-name', regex used for validation is '[-._a-zA-Z0-9]+')",
+		},
+	}, {
+		name: "invalid subscription parameter",
+		bs: &BusSpec{
+			Parameters: &BusParameters{
+				Subscription: &[]Parameter{
+					{
+						Name: "foo@bar",
+					},
+				},
+			},
+		},
+		want: &apis.FieldError{
+			Message: `invalid key name "foo@bar"`,
+			Paths: []string{
+				"parameters.subscription[0].name",
+			},
+			Details: "a valid config key must consist of alphanumeric characters, '-', '_' or '.' (e.g. 'key.name',  or 'KEY_NAME',  or 'key-name', regex used for validation is '[-._a-zA-Z0-9]+')",
+		},
+	}, {
+		name: "invalid channel too long",
+		bs: &BusSpec{
+			Parameters: &BusParameters{
+				Channel: &[]Parameter{
+					{
+						Name: longName,
+					},
+				},
+			},
+		},
+		want: &apis.FieldError{
+			Message: fmt.Sprintf("invalid key name %q", longName),
+			Paths: []string{
+				"parameters.channel[0].name",
+			},
+			Details: "must be no more than 253 characters",
+		},
+	}, {
+		name: "empty bus",
+		bs:   &BusSpec{},
+	}}
+
+	for _, test := range tests {
+		t.Run(test.name, func(t *testing.T) {
+			got := test.bs.Validate()
+			if diff := cmp.Diff(test.want, got); diff != "" {
+				t.Errorf("validateBus (-want, +got) = %v", diff)
+			}
+		})
+	}
+}

pkg/apis/channels/v1alpha1/clusterbus_defaults.go

@@ -0,0 +1,21 @@
+/*
+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
+
+func (b *ClusterBus) SetDefaults() {
+	b.Spec.SetDefaults()
+}

pkg/apis/channels/v1alpha1/clusterbus_validation.go

@@ -0,0 +1,30 @@
+/*
+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/knative/pkg/apis"
+)
+
+func (b *ClusterBus) Validate() *apis.FieldError {
+	return b.Spec.Validate().ViaField("spec")
+}
+
+func (current *ClusterBus) CheckImmutableFields(og apis.Immutable) *apis.FieldError {
+	// TODO(n3wscott): Anything to check?
+	return nil
+}

pkg/webhook/bus.go

@@ -19,13 +19,10 @@ package webhook
 import (
 	"context"
 	"errors"
-	"fmt"
-	"strings"
 
 	"github.com/golang/glog"
 	"github.com/knative/eventing/pkg/apis/channels/v1alpha1"
 	"github.com/mattbaird/jsonpatch"
-	"k8s.io/apimachinery/pkg/util/validation"
 )
 
 var (
@@ -46,25 +43,11 @@ func ValidateBus(ctx context.Context) ResourceCallback {
 }
 
 func validateBus(old, new v1alpha1.GenericBus) error {
-	if new.GetSpec().Parameters != nil {
-		if new.GetSpec().Parameters.Channel != nil {
-			for _, p := range *new.GetSpec().Parameters.Channel {
-				errs := validation.IsConfigMapKey(p.Name)
-				if len(errs) > 0 {
-					return fmt.Errorf("invalid parameter name Spec.Parameters.Channel.%s: %s", p.Name,
-						strings.Join(errs, ", "))
-				}
-			}
-		}
-		if new.GetSpec().Parameters.Subscription != nil {
-			for _, p := range *new.GetSpec().Parameters.Subscription {
-				errs := validation.IsConfigMapKey(p.Name)
-				if len(errs) > 0 {
-					return fmt.Errorf("invalid parameter name Spec.Parameters.Subscription.%s: %s", p.Name,
-						strings.Join(errs, ", "))
-				}
-			}
-		}
+	if err := new.Validate(); err != nil {
+		return err
+	}
+	if err := new.CheckImmutableFields(old); err != nil {
+		return err
 	}
 	return nil
 }

pkg/webhook/webhook_channels_test.go

@@ -75,7 +75,7 @@ func TestInvalidBusParameterNameFails(t *testing.T) {
 		t.Fatalf("Failed to marshal bus: %s", err)
 	}
 	req.Object.Raw = marshaled
-	expectFailsWith(t, ac.admit(testCtx, req), "invalid parameter name Spec.Parameters.Subscription.paramètre")
+	expectFailsWith(t, ac.admit(testCtx, req), `invalid key name "paramètre": spec.parameters.subscription[0].name`)
 
 	invalidName = "param/name"
 	bus = createBus(testBusName, "foobar/dispatcher")
@@ -85,7 +85,7 @@ func TestInvalidBusParameterNameFails(t *testing.T) {
 		t.Fatalf("Failed to marshal bus: %s", err)
 	}
 	req.Object.Raw = marshaled
-	expectFailsWith(t, ac.admit(testCtx, req), "invalid parameter name Spec.Parameters.Channel.param/name")
+	expectFailsWith(t, ac.admit(testCtx, req), `invalid key name "param/name": spec.parameters.channel[0].name`)
 }
 
 func TestInvalidClusterBusParameterNameFails(t *testing.T) {
@@ -102,7 +102,7 @@ func TestInvalidClusterBusParameterNameFails(t *testing.T) {
 		t.Fatalf("Failed to marshal bus: %s", err)
 	}
 	req.Object.Raw = marshaled
-	expectFailsWith(t, ac.admit(testCtx, req), "invalid parameter name Spec.Parameters.Subscription.paramètre")
+	expectFailsWith(t, ac.admit(testCtx, req), `invalid key name "paramètre": spec.parameters.subscription[0].name`)
 
 	invalidName = "param/name"
 	bus = createClusterBus(testBusName, "foobar/dispatcher")
@@ -112,7 +112,7 @@ func TestInvalidClusterBusParameterNameFails(t *testing.T) {
 		t.Fatalf("Failed to marshal bus: %s", err)
 	}
 	req.Object.Raw = marshaled
-	expectFailsWith(t, ac.admit(testCtx, req), "invalid parameter name Spec.Parameters.Channel.param/name")
+	expectFailsWith(t, ac.admit(testCtx, req), `invalid key name "param/name": spec.parameters.channel[0].name`)
 }
 
 func TestInvalidNewChannelNameFails(t *testing.T) {