deprecate notary support in v1alpha2

buildpacks-community · Sep 28, 2021 · bf6b013 · bf6b013
1 parent 949cbfe
commit bf6b013
Show file tree

Hide file tree

Showing 4 changed files with 81 additions and 49 deletions.
diff --git a/pkg/apis/build/v1alpha2/build_validation.go b/pkg/apis/build/v1alpha2/build_validation.go
@@ -34,7 +34,8 @@ func (bs *BuildSpec) Validate(ctx context.Context) *apis.FieldError {
 		Also(bs.LastBuild.Validate(ctx).ViaField("lastBuild")).
 		Also(bs.validateImmutableFields(ctx)).
 		Also(bs.validateCnbBindings(ctx).ViaField("cnbBindings")).
-		Also(bs.validateNodeSelector(ctx))
+		Also(bs.validateNodeSelector(ctx)).
+		Also(bs.validateNotary(ctx).ViaField("notary"))
 }
 
 func (bs *BuildSpec) validateCnbBindings(ctx context.Context) *apis.FieldError {
@@ -46,6 +47,15 @@ func (bs *BuildSpec) validateCnbBindings(ctx context.Context) *apis.FieldError {
 	return bs.CNBBindings.Validate(ctx)
 }
 
+func (bs *BuildSpec) validateNotary(ctx context.Context) *apis.FieldError {
+	//only allow the kpack controller to create resources with notary
+	if !resourceCreatedByKpackController(apis.GetUserInfo(ctx)) && bs.Notary != nil {
+		return apis.ErrGeneric("use of this field has been deprecated and cannot be set", "")
+	}
+
+	return bs.Notary.Validate(ctx)
+}
+
 func resourceCreatedByKpackController(info *authv1.UserInfo) bool {
 	if info == nil {
 		return false

diff --git a/pkg/apis/build/v1alpha2/build_validation_test.go b/pkg/apis/build/v1alpha2/build_validation_test.go
@@ -275,6 +275,48 @@ func testBuildValidation(t *testing.T, when spec.G, it spec.S) {
 			})
 		})
 
+		it("validates notary config has not been set by user", func() {
+			build.Spec.Notary = &corev1alpha1.NotaryConfig{
+				V1: &corev1alpha1.NotaryV1Config{
+					URL: "",
+					SecretRef: corev1alpha1.NotarySecretRef{
+						Name: "some-secret-name",
+					},
+				},
+			}
+			assertValidationError(build, context.TODO(), apis.ErrGeneric("use of this field has been deprecated and cannot be set", "spec.notary"))
+
+		})
+
+		when("validating notary if build is created by kpack controller", func() {
+			ctx := apis.WithUserInfo(context.TODO(), &authv1.UserInfo{Username: kpackControllerServiceAccountUsername})
+			it("handles an empty notary url", func() {
+				build.Spec.Notary = &corev1alpha1.NotaryConfig{
+					V1: &corev1alpha1.NotaryV1Config{
+						URL: "",
+						SecretRef: corev1alpha1.NotarySecretRef{
+							Name: "some-secret-name",
+						},
+					},
+				}
+				err := build.Validate(ctx)
+				assert.EqualError(t, err, "missing field(s): spec.notary.v1.url")
+			})
+
+			it("handles an empty notary secret ref", func() {
+				build.Spec.Notary = &corev1alpha1.NotaryConfig{
+					V1: &corev1alpha1.NotaryV1Config{
+						URL: "some-url",
+						SecretRef: corev1alpha1.NotarySecretRef{
+							Name: "",
+						},
+					},
+				}
+				err := build.Validate(ctx)
+				assert.EqualError(t, err, "missing field(s): spec.notary.v1.secretRef.name")
+			})
+		})
+
 		it("validates not registry AND volume cache are both specified", func() {
 			build.Spec.Cache = &BuildCacheConfig{
 				Volume:   &BuildPersistentVolumeCache{ClaimName: "pvc"},

diff --git a/pkg/apis/build/v1alpha2/image_validation.go b/pkg/apis/build/v1alpha2/image_validation.go
@@ -5,12 +5,12 @@ import (
 	"fmt"
 	"strings"
 
+	"github.com/google/go-containerregistry/pkg/name"
 	v1 "k8s.io/api/core/v1"
 	"k8s.io/apimachinery/pkg/api/resource"
 	"k8s.io/apimachinery/pkg/util/validation"
 	"knative.dev/pkg/apis"
 
-	"github.com/google/go-containerregistry/pkg/name"
 	corev1alpha1 "github.com/pivotal/kpack/pkg/apis/core/v1alpha1"
 	"github.com/pivotal/kpack/pkg/apis/validate"
 )
@@ -87,10 +87,18 @@ func (is *ImageSpec) ValidateSpec(ctx context.Context) *apis.FieldError {
 		Also(is.Build.Validate(ctx).ViaField("build")).
 		Also(is.Cache.Validate(ctx).ViaField("cache")).
 		Also(is.validateVolumeCache(ctx)).
-		Also(is.Notary.Validate(ctx).ViaField("notary")).
+		Also(validateNotaryUnset(is.Notary).ViaField("notary")).
 		Also(is.Cosign.Validate(ctx).ViaField("cosign"))
 }
 
+func validateNotaryUnset(notaryConfig *corev1alpha1.NotaryConfig) *apis.FieldError {
+	if notaryConfig != nil {
+		return apis.ErrGeneric("notary support has been deprecated in v1alpha2, please use v1alpha1 for notary image signing", "")
+	}
+
+	return nil
+}
+
 func (is *ImageSpec) validateTag(ctx context.Context) *apis.FieldError {
 	if apis.IsInUpdate(ctx) {
 		original := apis.GetBaseline(ctx).(*Image)
@@ -169,7 +177,7 @@ func (ib *ImageBuild) Validate(ctx context.Context) *apis.FieldError {
 
 func validateCnbBindingsEmpty(bindings corev1alpha1.CNBBindings) *apis.FieldError {
 	if len(bindings) > 0 {
-		return apis.ErrDisallowedFields("")
+		return apis.ErrGeneric("CNB binding support has been deprecated in v1alpha2, please use v1alpha1 for CNB bindings", "")
 	}
 
 	return nil

diff --git a/pkg/apis/build/v1alpha2/image_validation_test.go b/pkg/apis/build/v1alpha2/image_validation_test.go
@@ -267,7 +267,7 @@ func testImageValidation(t *testing.T, when spec.G, it spec.S) {
 				},
 			}
 
-			assertValidationError(image, ctx, apis.ErrDisallowedFields("spec.build.cnbBindings"))
+			assertValidationError(image, ctx, apis.ErrGeneric("CNB binding support has been deprecated in v1alpha2, please use v1alpha1 for CNB bindings", "spec.build.cnbBindings"))
 		})
 
 		it("image name is too long", func() {
@@ -383,54 +383,26 @@ func testImageValidation(t *testing.T, when spec.G, it spec.S) {
 			assert.Nil(t, image.Validate(ctx))
 		})
 
-		when("validating the notary config", func() {
-			it("handles a valid notary config", func() {
-				image.Spec.Notary = &corev1alpha1.NotaryConfig{
-					V1: &corev1alpha1.NotaryV1Config{
-						URL: "some-url",
-						SecretRef: corev1alpha1.NotarySecretRef{
-							Name: "some-secret-name",
-						},
-					},
-				}
-				assert.Nil(t, image.Validate(ctx))
-			})
-
-			it("handles an empty notary url", func() {
-				image.Spec.Notary = &corev1alpha1.NotaryConfig{
-					V1: &corev1alpha1.NotaryV1Config{
-						URL: "",
-						SecretRef: corev1alpha1.NotarySecretRef{
-							Name: "some-secret-name",
-						},
-					},
-				}
-				err := image.Validate(ctx)
-				assert.EqualError(t, err, "missing field(s): spec.notary.v1.url")
-			})
-
-			it("handles an empty notary secret ref", func() {
-				image.Spec.Notary = &corev1alpha1.NotaryConfig{
-					V1: &corev1alpha1.NotaryV1Config{
-						URL: "some-url",
-						SecretRef: corev1alpha1.NotarySecretRef{
-							Name: "",
-						},
+		it("validates that notary config is unset", func() {
+			image.Spec.Notary = &corev1alpha1.NotaryConfig{
+				V1: &corev1alpha1.NotaryV1Config{
+					URL: "some-url",
+					SecretRef: corev1alpha1.NotarySecretRef{
+						Name: "some-secret-name",
 					},
-				}
-				err := image.Validate(ctx)
-				assert.EqualError(t, err, "missing field(s): spec.notary.v1.secretRef.name")
-			})
-
-			it("validates not registry AND volume cache are both specified", func() {
-				original := image.DeepCopy()
+				},
+			}
+			err := image.Validate(ctx)
+			assert.EqualError(t, err, "notary support has been deprecated in v1alpha2, please use v1alpha1 for notary image signing: spec.notary")
+		})
 
-				image.Spec.Cache.Registry = &RegistryCache{Tag: "test"}
+		it("validates not registry AND volume cache are both specified", func() {
+			original := image.DeepCopy()
 
-				err := image.Validate(apis.WithinUpdate(ctx, original))
-				assert.EqualError(t, err, "only one type of cache can be specified: spec.cache.registry, spec.cache.volume")
-			})
+			image.Spec.Cache.Registry = &RegistryCache{Tag: "test"}
 
+			err := image.Validate(apis.WithinUpdate(ctx, original))
+			assert.EqualError(t, err, "only one type of cache can be specified: spec.cache.registry, spec.cache.volume")
 		})
 
 		it("validates kubernetes.io/os node selector is unset", func() {