Skip to content

Commit

Permalink
Merge pull request #1 from hectorj2f/v0.6.3-improv_auth_validation
Browse files Browse the repository at this point in the history
change validation to avoid only setting rfc3161timestamp
  • Loading branch information
hectorj2f committed Feb 6, 2023
2 parents 9d1900d + b912dbe commit e7c6cfb
Show file tree
Hide file tree
Showing 7 changed files with 44 additions and 21 deletions.
7 changes: 3 additions & 4 deletions pkg/apis/policy/v1alpha1/clusterimagepolicy_validation.go
Expand Up @@ -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
Expand Down
10 changes: 5 additions & 5 deletions pkg/apis/policy/v1alpha1/clusterimagepolicy_validation_test.go
Expand Up @@ -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{
Expand All @@ -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{
Expand All @@ -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{
Expand All @@ -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{
Expand All @@ -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{
Expand Down
7 changes: 3 additions & 4 deletions pkg/apis/policy/v1beta1/clusterimagepolicy_validation.go
Expand Up @@ -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
Expand Down
12 changes: 6 additions & 6 deletions pkg/apis/policy/v1beta1/clusterimagepolicy_validation_test.go
Expand Up @@ -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{
Expand All @@ -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{
Expand All @@ -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{
Expand All @@ -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{
Expand All @@ -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{
Expand Down Expand Up @@ -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{
Expand Down
25 changes: 25 additions & 0 deletions 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
Expand Up @@ -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:
Expand Down
Expand Up @@ -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:
Expand Down

0 comments on commit e7c6cfb

Please sign in to comment.