Use pkg/webhook along with build webhook implementation

cmd/webhook/main.go

@@ -28,7 +28,12 @@ import (
 	"github.com/knative/build/pkg"
 	onclusterbuilder "github.com/knative/build/pkg/builder/cluster"
 	buildclientset "github.com/knative/build/pkg/client/clientset/versioned"
+
+	"github.com/knative/build/pkg/apis/build/v1alpha1"
 	"github.com/knative/build/pkg/webhook"
+	pkgwebhook "github.com/knative/pkg/webhook"
+	"k8s.io/apimachinery/pkg/runtime/schema"
+
 	"github.com/knative/pkg/configmap"
 	"github.com/knative/pkg/logging"
 	"github.com/knative/pkg/logging/logkey"
@@ -78,12 +83,27 @@ func main() {
 	kubeInformerFactory := kubeinformers.NewSharedInformerFactory(kubeClient, time.Second*30)
 	bldr := onclusterbuilder.NewBuilder(kubeClient, kubeInformerFactory, logger)
 
-	options := webhook.ControllerOptions{
-		ServiceName:      "build-webhook",
-		ServiceNamespace: pkg.GetBuildSystemNamespace(),
-		Port:             443,
-		SecretName:       "build-webhook-certs",
-		WebhookName:      "webhook.build.knative.dev",
+	pkgoptions := pkgwebhook.ControllerOptions{
+		ServiceName:    "build-webhook",
+		DeploymentName: "build-webhook",
+		Namespace:      pkg.GetBuildSystemNamespace(),
+		Port:           443,
+		SecretName:     "build-webhook-certs",
+		WebhookName:    "webhook.build.knative.dev",
 	}
-	webhook.NewAdmissionController(kubeClient, buildClient, bldr, options, logger).Run(stopCh)
+
+	pkgcontroller := pkgwebhook.AdmissionController{
+		Client:  kubeClient,
+		Options: pkgoptions,
+		Handlers: map[schema.GroupVersionKind]pkgwebhook.GenericCRD{
+			v1alpha1.SchemeGroupVersion.WithKind("Build"):                &v1alpha1.Build{},
+			v1alpha1.SchemeGroupVersion.WithKind("ClusterBuildTemplate"): &v1alpha1.ClusterBuildTemplate{},
+			v1alpha1.SchemeGroupVersion.WithKind("BuildTemplate"):        &v1alpha1.BuildTemplate{},
+		},
+		Logger: logger,
+	}
+	go pkgcontroller.Run(stopCh)
+
+	buildWebhookController := webhook.NewAdmissionController(kubeClient, buildClient, bldr, pkgoptions, logger)
+	go buildWebhookController.Run(stopCh)
 }

pkg/apis/build/v1alpha1/build_defaults.go

@@ -0,0 +1,20 @@
+package v1alpha1
+
+import (
+	"time"
+
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+)
+
+// DefaultTime is 10min
+const DefaultTimeout = 10 * time.Minute
+
+// SetDefaults for build
+func (b *Build) SetDefaults() {
+	if b.Spec.ServiceAccountName == "" {
+		b.Spec.ServiceAccountName = "default"
+	}
+	if b.Spec.Timeout.Duration == 0 {
+		b.Spec.Timeout = metav1.Duration{Duration: DefaultTimeout}
+	}
+}

pkg/apis/build/v1alpha1/build_defaults_test.go

@@ -0,0 +1,16 @@
+package v1alpha1
+
+import (
+	"testing"
+)
+
+func TestSetDefault(t *testing.T) {
+	emptyBuild := &Build{}
+	emptyBuild.SetDefaults()
+	if emptyBuild.Spec.ServiceAccountName != "default" {
+		t.Errorf("Expect default to be the serviceaccount name but got %s", emptyBuild.Spec.ServiceAccountName)
+	}
+	if emptyBuild.Spec.Timeout.Duration != DefaultTimeout {
+		t.Errorf("Expect build timeout to be set")
+	}
+}

pkg/apis/build/v1alpha1/build_template_defaults.go

@@ -0,0 +1,6 @@
+package v1alpha1
+
+// SetDefaults for build template
+func (b *BuildTemplate) SetDefaults() {
+	return
+}

pkg/apis/build/v1alpha1/build_template_types.go

@@ -19,6 +19,7 @@ package v1alpha1
 import (
 	"encoding/json"
 
+	"github.com/knative/pkg/apis"
 	corev1 "k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/runtime/schema"
@@ -48,6 +49,10 @@ type BuildTemplate struct {
 var _ kmeta.OwnerRefable = (*BuildTemplate)(nil)
 var _ Template = (*BuildTemplate)(nil)
 
+// Check that BuildTemplate may be validated and defaulted.
+var _ apis.Validatable = (*BuildTemplate)(nil)
+var _ apis.Defaultable = (*BuildTemplate)(nil)
+
 // BuildTemplateSpec is the spec for a BuildTemplate.
 type BuildTemplateSpec struct {
 	// TODO: Generation does not work correctly with CRD. They are scrubbed

pkg/apis/build/v1alpha1/build_template_types_test.go

@@ -47,3 +47,15 @@ func TestBuildTemplateGroupVersionKind(t *testing.T) {
 		t.Errorf("GetGroupVersionKind mismatch; expected: %v got: %v", expectedKind, c.GetGroupVersionKind().Kind)
 	}
 }
+
+func TestBuildTemplateGeneration(t *testing.T) {
+	c := BuildTemplate{}
+	if a := c.GetGeneration(); a != 0 {
+		t.Errorf("empty build template generation should be 0 but got: %d", a)
+	}
+
+	c.SetGeneration(5)
+	if e, a := int64(5), c.GetGeneration(); e != a {
+		t.Errorf("getgeneration mismatch; expected: %d got: %d", e, a)
+	}
+}

pkg/apis/build/v1alpha1/build_template_validation.go

@@ -0,0 +1,91 @@
+package v1alpha1
+
+import (
+	"strings"
+
+	"github.com/knative/pkg/apis"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+
+	corev1 "k8s.io/api/core/v1"
+)
+
+// Validate build template
+func (b *BuildTemplate) Validate() *apis.FieldError {
+	return validateObjectMetadata(b.GetObjectMeta()).ViaField("metadata").Also(b.Spec.Validate().ViaField("spec"))
+}
+
+// Validate Build Template
+func (b *BuildTemplateSpec) Validate() *apis.FieldError {
+	if err := validateSteps(b.Steps); err != nil {
+		return err
+	}
+	if err := validateVolumes(b.Volumes); err != nil {
+		return err
+	}
+	if err := validateParameters(b.Parameters); err != nil {
+		return err
+	}
+	return nil
+}
+
+func validateObjectMetadata(meta metav1.Object) *apis.FieldError {
+	name := meta.GetName()
+
+	if strings.Contains(name, ".") {
+		return &apis.FieldError{
+			Message: "Invalid resource name: special character . must not be present",
+			Paths:   []string{"name"},
+		}
+	}
+
+	if len(name) > 63 {
+		return &apis.FieldError{
+			Message: "Invalid resource name: length must be no more than 63 characters",
+			Paths:   []string{"name"},
+		}
+	}
+	return nil
+}
+
+func validateParameters(params []ParameterSpec) *apis.FieldError {
+	// Template must not duplicate parameter names.
+	seen := map[string]struct{}{}
+	for _, p := range params {
+		if _, ok := seen[p.Name]; ok {
+			return apis.ErrMultipleOneOf("ParamName")
+		}
+		seen[p.Name] = struct{}{}
+	}
+	return nil
+}
+
+func validateVolumes(volumes []corev1.Volume) *apis.FieldError {
+	// Build must not duplicate volume names.
+	vols := map[string]struct{}{}
+	for _, v := range volumes {
+		if _, ok := vols[v.Name]; ok {
+			return apis.ErrMultipleOneOf("volumeName")
+		}
+		vols[v.Name] = struct{}{}
+	}
+	return nil
+}
+
+func validateSteps(steps []corev1.Container) *apis.FieldError {
+	// Build must not duplicate step names.
+	names := map[string]struct{}{}
+	for _, s := range steps {
+		if s.Image == "" {
+			return apis.ErrMissingField("Image")
+		}
+
+		if s.Name == "" {
+			continue
+		}
+		if _, ok := names[s.Name]; ok {
+			return apis.ErrMultipleOneOf("stepName")
+		}
+		names[s.Name] = struct{}{}
+	}
+	return nil
+}

pkg/apis/build/v1alpha1/build_template_validation_test.go

@@ -0,0 +1,127 @@
+package v1alpha1
+
+import (
+	"testing"
+
+	corev1 "k8s.io/api/core/v1"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+)
+
+func TestValidateTemplate(t *testing.T) {
+	hasDefault := "has-default"
+	for _, c := range []struct {
+		desc   string
+		tmpl   *BuildTemplate
+		reason string // if "", expect success.
+	}{{
+		desc: "Single named step",
+		tmpl: &BuildTemplate{
+			Spec: BuildTemplateSpec{
+				Steps: []corev1.Container{{
+					Name:  "foo",
+					Image: "gcr.io/foo-bar/baz:latest",
+				}},
+			},
+		},
+	}, {
+		desc:   "long name",
+		reason: "long name",
+		tmpl: &BuildTemplate{
+			ObjectMeta: metav1.ObjectMeta{
+				Name: "*********************************************************************************************",
+			},
+		},
+	}, {
+		desc:   "invalid name",
+		reason: "invalid name",
+		tmpl: &BuildTemplate{
+			ObjectMeta: metav1.ObjectMeta{
+				Name: "b.c",
+			},
+		},
+	}, {
+		desc: "Multiple unnamed steps",
+		tmpl: &BuildTemplate{
+			Spec: BuildTemplateSpec{
+				Steps: []corev1.Container{{
+					Image: "gcr.io/foo-bar/baz:latest",
+				}, {
+					Image: "gcr.io/foo-bar/baz:latest",
+				}, {
+					Image: "gcr.io/foo-bar/baz:latest",
+				}},
+			},
+		},
+	}, {
+		tmpl: &BuildTemplate{
+			Spec: BuildTemplateSpec{
+				Steps: []corev1.Container{{
+					Name:  "foo",
+					Image: "gcr.io/foo-bar/baz:latest",
+				}, {
+					Name:  "foo",
+					Image: "gcr.io/foo-bar/baz:oops",
+				}},
+			},
+		},
+		reason: "DuplicateStepName",
+	}, {
+		tmpl: &BuildTemplate{
+			Spec: BuildTemplateSpec{
+				Steps: []corev1.Container{{
+					Name:  "foo",
+					Image: "gcr.io/foo-bar/baz:latest",
+				}},
+				Volumes: []corev1.Volume{{
+					Name: "foo",
+					VolumeSource: corev1.VolumeSource{
+						EmptyDir: &corev1.EmptyDirVolumeSource{},
+					},
+				}, {
+					Name: "foo",
+					VolumeSource: corev1.VolumeSource{
+						EmptyDir: &corev1.EmptyDirVolumeSource{},
+					},
+				}},
+			},
+		},
+		reason: "DuplicateVolumeName",
+	}, {
+		tmpl: &BuildTemplate{
+			Spec: BuildTemplateSpec{
+				Parameters: []ParameterSpec{{
+					Name: "foo",
+				}, {
+					Name: "foo",
+				}},
+			},
+		},
+		reason: "DuplicateParamName",
+	}, {
+		tmpl: &BuildTemplate{
+			Spec: BuildTemplateSpec{
+				Steps: []corev1.Container{{
+					Name: "step-name-${FOO${BAR}}",
+				}},
+				Parameters: []ParameterSpec{{
+					Name: "FOO",
+				}, {
+					Name:    "BAR",
+					Default: &hasDefault,
+				}},
+			},
+		},
+		reason: "NestedPlaceholder",
+	}} {
+		name := c.desc
+		if c.reason != "" {
+			name = "invalid-" + c.reason
+		}
+		t.Run(name, func(t *testing.T) {
+			verr := c.tmpl.Validate()
+			if gotErr, wantErr := verr != nil, c.reason != ""; gotErr != wantErr {
+				t.Errorf("validateBuildTemplate(%s); got %v, want %q", name, verr, c.reason)
+			}
+		})
+	}
+}

pkg/apis/build/v1alpha1/build_types.go

@@ -23,6 +23,7 @@ import (
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/runtime/schema"
 
+	"github.com/knative/pkg/apis"
 	duckv1alpha1 "github.com/knative/pkg/apis/duck/v1alpha1"
 	"github.com/knative/pkg/kmeta"
 )
@@ -45,6 +46,10 @@ type Build struct {
 // Check that our resource implements several interfaces.
 var _ kmeta.OwnerRefable = (*Build)(nil)
 
+// Check that Build may be validated and defaulted.
+var _ apis.Validatable = (*Build)(nil)
+var _ apis.Defaultable = (*Build)(nil)
+
 // BuildSpec is the spec for a Build resource.
 type BuildSpec struct {
 	// TODO: Generation does not work correctly with CRD. They are scrubbed

pkg/apis/build/v1alpha1/build_types_test.go

@@ -76,6 +76,11 @@ func TestBuildConditions(t *testing.T) {
 		t.Errorf("Unexpected build condition type; want %v got %v", want, b.Status.GetConditions())
 	}
 
+	fooStatus := b.Status.GetCondition(foo.Type)
+	if cmp.Diff(fooStatus, foo, ignoreVolatileTime) != "" {
+		t.Errorf("Unexpected build condition type; want %v got %v", fooStatus, foo)
+	}
+
 	// Add a second condition.
 	b.Status.SetCondition(bar)