Merge pull request #1 from hectorj2f/v0.6.3-improv_auth_validation

change validation to avoid only setting rfc3161timestamp
sigstore · Feb 6, 2023 · e7c6cfb · e7c6cfb
2 parents 9d1900d + b912dbe
commit e7c6cfb
Show file tree

Hide file tree

Showing 7 changed files with 44 additions and 21 deletions.
diff --git a/pkg/apis/policy/v1alpha1/clusterimagepolicy_validation.go b/pkg/apis/policy/v1alpha1/clusterimagepolicy_validation.go
@@ -87,17 +87,16 @@ func (image *ImagePattern) Validate(ctx context.Context) *apis.FieldError {
 
 func (authority *Authority) Validate(ctx context.Context) *apis.FieldError {
 	var errs *apis.FieldError
-	if authority.Key == nil && authority.Keyless == nil && authority.RFC3161Timestamp == nil && authority.Static == nil {
-		errs = errs.Also(apis.ErrMissingOneOf("key", "keyless", "rfc3161timestamp", "static"))
+	if authority.Key == nil && authority.Keyless == nil && authority.Static == nil {
+		errs = errs.Also(apis.ErrMissingOneOf("key", "keyless", "static"))
 		// Instead of returning all the missing subfields, just return here
 		// to give a more concise and arguably a more meaningful error message.
 		return errs
 	}
 	if (authority.Key != nil && authority.Keyless != nil) ||
-		(authority.RFC3161Timestamp != nil && authority.Static != nil) ||
 		(authority.Key != nil && authority.Static != nil) ||
 		(authority.Keyless != nil && authority.Static != nil) {
-		errs = errs.Also(apis.ErrMultipleOneOf("key", "keyless", "rfc3161timestamp", "static"))
+		errs = errs.Also(apis.ErrMultipleOneOf("key", "keyless", "static"))
 		// Instead of returning all the missing subfields, just return here
 		// to give a more concise and arguably a more meaningful error message.
 		return errs

diff --git a/pkg/apis/policy/v1alpha1/clusterimagepolicy_validation_test.go b/pkg/apis/policy/v1alpha1/clusterimagepolicy_validation_test.go
@@ -534,7 +534,7 @@ func TestAuthoritiesValidation(t *testing.T) {
 		policy      ClusterImagePolicy
 	}{{
 		name:        "Should fail when authority is empty",
-		errorString: "expected exactly one, got neither: spec.authorities[0].key, spec.authorities[0].keyless, spec.authorities[0].rfc3161timestamp, spec.authorities[0].static",
+		errorString: "expected exactly one, got neither: spec.authorities[0].key, spec.authorities[0].keyless, spec.authorities[0].static",
 		policy: ClusterImagePolicy{
 			Spec: ClusterImagePolicySpec{
 				Images: []ImagePattern{
@@ -549,7 +549,7 @@ func TestAuthoritiesValidation(t *testing.T) {
 		},
 	}, {
 		name:        "Should fail when key/keyless specified",
-		errorString: "expected exactly one, got both: spec.authorities[0].key, spec.authorities[0].keyless, spec.authorities[0].rfc3161timestamp, spec.authorities[0].static",
+		errorString: "expected exactly one, got both: spec.authorities[0].key, spec.authorities[0].keyless, spec.authorities[0].static",
 		policy: ClusterImagePolicy{
 			Spec: ClusterImagePolicySpec{
 				Images: []ImagePattern{
@@ -567,7 +567,7 @@ func TestAuthoritiesValidation(t *testing.T) {
 		},
 	}, {
 		name:        "Should fail when key/static specified",
-		errorString: "expected exactly one, got both: spec.authorities[0].key, spec.authorities[0].keyless, spec.authorities[0].rfc3161timestamp, spec.authorities[0].static",
+		errorString: "expected exactly one, got both: spec.authorities[0].key, spec.authorities[0].keyless, spec.authorities[0].static",
 		policy: ClusterImagePolicy{
 			Spec: ClusterImagePolicySpec{
 				Images: []ImagePattern{
@@ -585,7 +585,7 @@ func TestAuthoritiesValidation(t *testing.T) {
 		},
 	}, {
 		name:        "Should fail when keyless/static specified",
-		errorString: "expected exactly one, got both: spec.authorities[0].key, spec.authorities[0].keyless, spec.authorities[0].rfc3161timestamp, spec.authorities[0].static",
+		errorString: "expected exactly one, got both: spec.authorities[0].key, spec.authorities[0].keyless, spec.authorities[0].static",
 		policy: ClusterImagePolicy{
 			Spec: ClusterImagePolicySpec{
 				Images: []ImagePattern{
@@ -603,7 +603,7 @@ func TestAuthoritiesValidation(t *testing.T) {
 		},
 	}, {
 		name:        "Should fail when key/keyless/static specified",
-		errorString: "expected exactly one, got both: spec.authorities[0].key, spec.authorities[0].keyless, spec.authorities[0].rfc3161timestamp, spec.authorities[0].static",
+		errorString: "expected exactly one, got both: spec.authorities[0].key, spec.authorities[0].keyless, spec.authorities[0].static",
 		policy: ClusterImagePolicy{
 			Spec: ClusterImagePolicySpec{
 				Images: []ImagePattern{

diff --git a/pkg/apis/policy/v1beta1/clusterimagepolicy_validation.go b/pkg/apis/policy/v1beta1/clusterimagepolicy_validation.go
@@ -100,17 +100,16 @@ func (matchResource *MatchResource) Validate(ctx context.Context) *apis.FieldErr
 
 func (authority *Authority) Validate(ctx context.Context) *apis.FieldError {
 	var errs *apis.FieldError
-	if authority.Key == nil && authority.Keyless == nil && authority.RFC3161Timestamp == nil && authority.Static == nil {
-		errs = errs.Also(apis.ErrMissingOneOf("key", "keyless", "rfc3161timestamp", "static"))
+	if authority.Key == nil && authority.Keyless == nil && authority.Static == nil {
+		errs = errs.Also(apis.ErrMissingOneOf("key", "keyless", "static"))
 		// Instead of returning all the missing subfields, just return here
 		// to give a more concise and arguably a more meaningful error message.
 		return errs
 	}
 	if (authority.Key != nil && authority.Keyless != nil) ||
 		(authority.Key != nil && authority.Static != nil) ||
-		(authority.RFC3161Timestamp != nil && authority.Static != nil) ||
 		(authority.Keyless != nil && authority.Static != nil) {
-		errs = errs.Also(apis.ErrMultipleOneOf("key", "keyless", "rfc3161timestamp", "static"))
+		errs = errs.Also(apis.ErrMultipleOneOf("key", "keyless", "static"))
 		// Instead of returning all the missing subfields, just return here
 		// to give a more concise and arguably a more meaningful error message.
 		return errs

diff --git a/pkg/apis/policy/v1beta1/clusterimagepolicy_validation_test.go b/pkg/apis/policy/v1beta1/clusterimagepolicy_validation_test.go
@@ -582,7 +582,7 @@ func TestAuthoritiesValidation(t *testing.T) {
 		policy      ClusterImagePolicy
 	}{{
 		name:        "Should fail when authority is empty",
-		errorString: "expected exactly one, got neither: spec.authorities[0].key, spec.authorities[0].keyless, spec.authorities[0].rfc3161timestamp, spec.authorities[0].static",
+		errorString: "expected exactly one, got neither: spec.authorities[0].key, spec.authorities[0].keyless, spec.authorities[0].static",
 		policy: ClusterImagePolicy{
 			Spec: ClusterImagePolicySpec{
 				Images: []ImagePattern{
@@ -597,7 +597,7 @@ func TestAuthoritiesValidation(t *testing.T) {
 		},
 	}, {
 		name:        "Should fail when key/keyless specified",
-		errorString: "expected exactly one, got both: spec.authorities[0].key, spec.authorities[0].keyless, spec.authorities[0].rfc3161timestamp, spec.authorities[0].static",
+		errorString: "expected exactly one, got both: spec.authorities[0].key, spec.authorities[0].keyless, spec.authorities[0].static",
 		policy: ClusterImagePolicy{
 			Spec: ClusterImagePolicySpec{
 				Images: []ImagePattern{
@@ -615,7 +615,7 @@ func TestAuthoritiesValidation(t *testing.T) {
 		},
 	}, {
 		name:        "Should fail when key/static specified",
-		errorString: "expected exactly one, got both: spec.authorities[0].key, spec.authorities[0].keyless, spec.authorities[0].rfc3161timestamp, spec.authorities[0].static",
+		errorString: "expected exactly one, got both: spec.authorities[0].key, spec.authorities[0].keyless, spec.authorities[0].static",
 		policy: ClusterImagePolicy{
 			Spec: ClusterImagePolicySpec{
 				Images: []ImagePattern{
@@ -633,7 +633,7 @@ func TestAuthoritiesValidation(t *testing.T) {
 		},
 	}, {
 		name:        "Should fail when keyless/static specified",
-		errorString: "expected exactly one, got both: spec.authorities[0].key, spec.authorities[0].keyless, spec.authorities[0].rfc3161timestamp, spec.authorities[0].static",
+		errorString: "expected exactly one, got both: spec.authorities[0].key, spec.authorities[0].keyless, spec.authorities[0].static",
 		policy: ClusterImagePolicy{
 			Spec: ClusterImagePolicySpec{
 				Images: []ImagePattern{
@@ -651,7 +651,7 @@ func TestAuthoritiesValidation(t *testing.T) {
 		},
 	}, {
 		name:        "Should fail when key/keyless/static specified",
-		errorString: "expected exactly one, got both: spec.authorities[0].key, spec.authorities[0].keyless, spec.authorities[0].rfc3161timestamp, spec.authorities[0].static",
+		errorString: "expected exactly one, got both: spec.authorities[0].key, spec.authorities[0].keyless, spec.authorities[0].static",
 		policy: ClusterImagePolicy{
 			Spec: ClusterImagePolicySpec{
 				Images: []ImagePattern{
@@ -711,7 +711,7 @@ func TestAuthoritiesValidation(t *testing.T) {
 		},
 	}, {
 		name:        "Should fail when static and sources,attestations, and rfc3161timestamp is specified",
-		errorString: "expected exactly one, got both: spec.authorities[0].key, spec.authorities[0].keyless, spec.authorities[0].rfc3161timestamp, spec.authorities[0].static",
+		errorString: "expected exactly one, got both: spec.authorities[0].attestations, spec.authorities[0].rfc3161timestamp, spec.authorities[0].source, spec.authorities[0].static",
 		policy: ClusterImagePolicy{
 			Spec: ClusterImagePolicySpec{
 				Images: []ImagePattern{

diff --git a/test/testdata/policy-controller/invalid/invalid-authority.yaml b/test/testdata/policy-controller/invalid/invalid-authority.yaml
@@ -0,0 +1,25 @@
+# Copyright 2022 The Sigstore 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.
+---
+# ERROR: expected exactly one, got neither: spec.authorities[0].key, spec.authorities[0].keyless, spec.authorities[0].static
+apiVersion: policy.sigstore.dev/v1beta1
+kind: ClusterImagePolicy
+metadata:
+  name: invalid-authority
+spec:
+  images:
+  - glob: image*
+  authorities:
+  - rfc3161timestamp:
+      trustRootRef: my-sigstore-keys
diff --git a/test/testdata/policy-controller/invalid/v1beta1-valid-keyref-and-keylessref.yaml b/test/testdata/policy-controller/invalid/v1beta1-valid-keyref-and-keylessref.yaml
@@ -12,7 +12,7 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 ---
-# ERROR:expected exactly one, got both: spec.authorities[0].key, spec.authorities[0].keyless, spec.authorities[0].rfc3161timestamp, spec.authorities[0].static
+# ERROR:expected exactly one, got both: spec.authorities[0].key, spec.authorities[0].keyless, spec.authorities[0].static
 apiVersion: policy.sigstore.dev/v1beta1
 kind: ClusterImagePolicy
 metadata:

diff --git a/test/testdata/policy-controller/invalid/valid-keyref-and-keylessref.yaml b/test/testdata/policy-controller/invalid/valid-keyref-and-keylessref.yaml
@@ -12,7 +12,7 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 ---
-# ERROR:expected exactly one, got both: spec.authorities[0].key, spec.authorities[0].keyless, spec.authorities[0].rfc3161timestamp, spec.authorities[0].static
+# ERROR:expected exactly one, got both: spec.authorities[0].key, spec.authorities[0].keyless, spec.authorities[0].static
 apiVersion: policy.sigstore.dev/v1alpha1
 kind: ClusterImagePolicy
 metadata: