Revert "Fix for knative#779"

This reverts commit 60665a2.

cmd/webhook/main.go

@@ -18,10 +18,8 @@ package main
 import (
 	"flag"
 	"log"
-	"time"
 
-	channeldefaulter "github.com/knative/eventing/pkg/webhook/defaulters"
-	"github.com/knative/eventing/pkg/webhook/validators"
+	"github.com/knative/eventing/pkg/channeldefaulter"
 
 	"go.uber.org/zap"
 
@@ -35,12 +33,9 @@ import (
 	"github.com/knative/eventing/pkg/logconfig"
 	"github.com/knative/pkg/system"
 
-	eventingclient "github.com/knative/eventing/pkg/client/clientset/versioned"
-	"k8s.io/apimachinery/pkg/fields"
 	"k8s.io/apimachinery/pkg/runtime/schema"
 	"k8s.io/client-go/kubernetes"
 	"k8s.io/client-go/rest"
-	"k8s.io/client-go/tools/cache"
 )
 
 func main() {
@@ -88,22 +83,6 @@ func main() {
 		logger.Fatalf("failed to start webhook configmap watcher: %v", err)
 	}
 
-	// Create an eventing client that will be used to create a shared informer
-	evtClient, err := eventingclient.NewForConfig(clusterConfig)
-	if err != nil {
-		logger.Fatal("Failed to get the eventing client set", zap.Error(err))
-	}
-
-	sInformer := newClusterChannelProvisionerInformer(evtClient)
-	go sInformer.Run(stopCh)
-	logger.Info("Started shared informer for clusterchannnelprovisioners")
-
-	eventingv1alpha1.GlobalChannelValidator, err = validators.NewChannelValidator(sInformer, logger.Desugar())
-	if err != nil {
-		logger.Fatalf("failed to setup Channel validator. %v", err)
-	}
-	logger.Info("Configured global channel validator")
-
 	options := webhook.ControllerOptions{
 		ServiceName:    "webhook",
 		DeploymentName: "webhook",
@@ -128,21 +107,3 @@ func main() {
 	}
 	controller.Run(stopCh)
 }
-
-func newClusterChannelProvisionerInformer(client *eventingclient.Clientset) cache.SharedIndexInformer {
-
-	watchlist := cache.NewListWatchFromClient(
-		client.EventingV1alpha1().RESTClient(),
-		"clusterchannelprovisioners",
-		"",
-		fields.Everything())
-
-	informer := cache.NewSharedIndexInformer(
-		watchlist,
-		&eventingv1alpha1.ClusterChannelProvisioner{},
-		time.Hour*8,
-		cache.Indexers{},
-	)
-
-	return informer
-}

pkg/apis/eventing/v1alpha1/channel_validation.go

@@ -17,26 +17,34 @@ limitations under the License.
 package v1alpha1
 
 import (
+	"fmt"
+
 	"github.com/google/go-cmp/cmp"
 	"github.com/google/go-cmp/cmp/cmpopts"
 	"github.com/knative/pkg/apis"
 )
 
-type ChannelValidator interface {
-	Validate(c *Channel) *apis.FieldError
+func (c *Channel) Validate() *apis.FieldError {
+	return c.Spec.Validate().ViaField("spec")
 }
 
-// ChannelValidator is initialized by the webhook (or the host)
-var (
-	GlobalChannelValidator ChannelValidator
-)
+func (cs *ChannelSpec) Validate() *apis.FieldError {
+	var errs *apis.FieldError
+	if cs.Provisioner == nil {
+		errs = errs.Also(apis.ErrMissingField("provisioner"))
+	}
 
-func (c *Channel) Validate() *apis.FieldError {
-	// No validations will run if GlobalChannelValidator is not set. Should we panic is such a case?
-	if GlobalChannelValidator != nil {
-		return GlobalChannelValidator.Validate(c)
+	if cs.Subscribable != nil {
+		for i, subscriber := range cs.Subscribable.Subscribers {
+			if subscriber.ReplyURI == "" && subscriber.SubscriberURI == "" {
+				fe := apis.ErrMissingField("replyURI", "subscriberURI")
+				fe.Details = "expected at least one of, got none"
+				errs = errs.Also(fe.ViaField(fmt.Sprintf("subscriber[%d]", i)).ViaField("subscribable"))
+			}
+		}
 	}
-	return nil
+
+	return errs
 }
 
 func (current *Channel) CheckImmutableFields(og apis.Immutable) *apis.FieldError {

pkg/apis/eventing/v1alpha1/channel_validation_test.go

@@ -20,46 +20,90 @@ import (
 	"testing"
 
 	"github.com/google/go-cmp/cmp"
+	eventingduck "github.com/knative/eventing/pkg/apis/duck/v1alpha1"
 	"github.com/knative/pkg/apis"
 	corev1 "k8s.io/api/core/v1"
 	"k8s.io/apimachinery/pkg/runtime"
 )
 
 var dnsName = "example.com"
 
-// pkg/webhook/validators/channel_validator_test.go covers all test cases exhaustively. Hence just testing integration with a mock Validator
 func TestChannelValidation(t *testing.T) {
-
-	channel := Channel{}
-	tests := []struct {
-		name                      string
-		setGlobalChannelValidator bool
-		expected                  *apis.FieldError
-	}{
-		{
-			name:                      "GlobalChannelValidator not set",
-			setGlobalChannelValidator: false,
-			expected:                  nil,
+	tests := []CRDTest{{
+		name: "valid",
+		cr: &Channel{
+			Spec: ChannelSpec{
+				Provisioner: &corev1.ObjectReference{
+					Name: "foo",
+				},
+			},
 		},
-		{
-			name:                      "Test MockValidator",
-			setGlobalChannelValidator: true,
-			expected:                  &apis.FieldError{Message: "Test Error"},
+		want: nil,
+	}, {
+		name: "empty",
+		cr: &Channel{
+			Spec: ChannelSpec{},
 		},
-	}
-	for _, tc := range tests {
-		t.Run(tc.name, func(t *testing.T) {
-			if tc.setGlobalChannelValidator {
-				GlobalChannelValidator = &MockValidator{}
-			} else {
-				GlobalChannelValidator = nil
-			}
-			actual := channel.Validate()
-			if diff := cmp.Diff(tc.expected.Error(), actual.Error()); diff != "" {
-				t.Errorf("%s: validate (-want, +got) = %v", tc.name, diff)
-			}
-		})
-	}
+		want: apis.ErrMissingField("spec.provisioner"),
+	}, {
+		name: "subscribers array",
+		cr: &Channel{
+			Spec: ChannelSpec{
+				Provisioner: &corev1.ObjectReference{
+					Name: "foo",
+				},
+				Subscribable: &eventingduck.Subscribable{
+					Subscribers: []eventingduck.ChannelSubscriberSpec{{
+						SubscriberURI: "subscriberendpoint",
+						ReplyURI:      "resultendpoint",
+					}},
+				}},
+		},
+		want: nil,
+	}, {
+		name: "empty subscriber at index 1",
+		cr: &Channel{
+			Spec: ChannelSpec{
+				Provisioner: &corev1.ObjectReference{
+					Name: "foo",
+				},
+				Subscribable: &eventingduck.Subscribable{
+					Subscribers: []eventingduck.ChannelSubscriberSpec{{
+						SubscriberURI: "subscriberendpoint",
+						ReplyURI:      "replyendpoint",
+					}, {}},
+				}},
+		},
+		want: func() *apis.FieldError {
+			fe := apis.ErrMissingField("spec.subscribable.subscriber[1].replyURI", "spec.subscribable.subscriber[1].subscriberURI")
+			fe.Details = "expected at least one of, got none"
+			return fe
+		}(),
+	}, {
+		name: "2 empty subscribers",
+		cr: &Channel{
+			Spec: ChannelSpec{
+				Provisioner: &corev1.ObjectReference{
+					Name: "foo",
+				},
+				Subscribable: &eventingduck.Subscribable{
+					Subscribers: []eventingduck.ChannelSubscriberSpec{{}, {}},
+				},
+			},
+		},
+		want: func() *apis.FieldError {
+			var errs *apis.FieldError
+			fe := apis.ErrMissingField("spec.subscribable.subscriber[0].replyURI", "spec.subscribable.subscriber[0].subscriberURI")
+			fe.Details = "expected at least one of, got none"
+			errs = errs.Also(fe)
+			fe = apis.ErrMissingField("spec.subscribable.subscriber[1].replyURI", "spec.subscribable.subscriber[1].subscriberURI")
+			fe.Details = "expected at least one of, got none"
+			errs = errs.Also(fe)
+			return errs
+		}(),
+	}}
+
+	doValidateTest(t, tests)
 }
 
 func TestChannelImmutableFields(t *testing.T) {
@@ -163,10 +207,3 @@ func TestChannelImmutableFields(t *testing.T) {
 		})
 	}
 }
-
-type MockValidator struct {
-}
-
-func (mv *MockValidator) Validate(c *Channel) *apis.FieldError {
-	return &apis.FieldError{Message: "Test Error"}
-}