Merge pull request #399 from hectorj2f/source_secrets

feat: accept source without setting any oci repository

Author: hectorj2f

pkg/apis/policy/v1alpha1/clusterimagepolicy_validation.go

@@ -221,10 +221,10 @@ func (keyless *KeylessRef) Validate(ctx context.Context) *apis.FieldError {
 
 func (source *Source) Validate(ctx context.Context) *apis.FieldError {
 	var errs *apis.FieldError
-	if source.OCI == "" {
-		errs = errs.Also(apis.ErrMissingField("oci"))
-	} else if err := common.ValidateOCI(source.OCI); err != nil {
-		errs = errs.Also(apis.ErrInvalidValue(source.OCI, "oci", err.Error()))
+	if source.OCI != "" {
+		if err := common.ValidateOCI(source.OCI); err != nil {
+			errs = errs.Also(apis.ErrInvalidValue(source.OCI, "oci", err.Error()))
+		}
 	}
 
 	if len(source.SignaturePullSecrets) > 0 {

pkg/apis/policy/v1alpha1/clusterimagepolicy_validation_test.go

@@ -752,20 +752,6 @@ func TestAuthoritiesValidation(t *testing.T) {
 				},
 			},
 		},
-	}, {
-		name:        "Should fail when source oci is empty",
-		errorString: "missing field(s): spec.authorities[0].source[0].oci",
-		policy: ClusterImagePolicy{
-			Spec: ClusterImagePolicySpec{
-				Images: []ImagePattern{{Glob: "gcr.io/*"}},
-				Authorities: []Authority{
-					{
-						Key:     &KeyRef{KMS: "kms://key/path"},
-						Sources: []Source{{OCI: ""}},
-					},
-				},
-			},
-		},
 	}, {
 		name: "Should pass with multiple source oci is present",
 		policy: ClusterImagePolicy{
@@ -909,6 +895,19 @@ func TestAuthoritiesValidation(t *testing.T) {
 				},
 			},
 		},
+	}, {
+		name: "Should pass when source oci is empty",
+		policy: ClusterImagePolicy{
+			Spec: ClusterImagePolicySpec{
+				Images: []ImagePattern{{Glob: "gcr.io/*"}},
+				Authorities: []Authority{
+					{
+						Key:     &KeyRef{KMS: "kms://key/path"},
+						Sources: []Source{{OCI: ""}},
+					},
+				},
+			},
+		},
 	},
 	}
 

pkg/apis/policy/v1beta1/clusterimagepolicy_validation.go

@@ -222,10 +222,10 @@ func (keyless *KeylessRef) Validate(ctx context.Context) *apis.FieldError {
 
 func (source *Source) Validate(ctx context.Context) *apis.FieldError {
 	var errs *apis.FieldError
-	if source.OCI == "" {
-		errs = errs.Also(apis.ErrMissingField("oci"))
-	} else if err := common.ValidateOCI(source.OCI); err != nil {
-		errs = errs.Also(apis.ErrInvalidValue(source.OCI, "oci", err.Error()))
+	if source.OCI != "" {
+		if err := common.ValidateOCI(source.OCI); err != nil {
+			errs = errs.Also(apis.ErrInvalidValue(source.OCI, "oci", err.Error()))
+		}
 	}
 
 	if len(source.SignaturePullSecrets) > 0 {

pkg/apis/policy/v1beta1/clusterimagepolicy_validation_test.go

@@ -704,20 +704,6 @@ func TestAuthoritiesValidation(t *testing.T) {
 				},
 			},
 		},
-	}, {
-		name:        "Should fail when source oci is empty",
-		errorString: "missing field(s): spec.authorities[0].source[0].oci",
-		policy: ClusterImagePolicy{
-			Spec: ClusterImagePolicySpec{
-				Images: []ImagePattern{{Glob: "*"}},
-				Authorities: []Authority{
-					{
-						Key:     &KeyRef{KMS: "kms://key/path"},
-						Sources: []Source{{OCI: ""}},
-					},
-				},
-			},
-		},
 	}, {
 		name: "Should pass with multiple source oci is present",
 		policy: ClusterImagePolicy{
@@ -942,6 +928,19 @@ func TestAuthoritiesValidation(t *testing.T) {
 				},
 			},
 		},
+	}, {
+		name: "Should pass when source oci is empty",
+		policy: ClusterImagePolicy{
+			Spec: ClusterImagePolicySpec{
+				Images: []ImagePattern{{Glob: "*"}},
+				Authorities: []Authority{
+					{
+						Key:     &KeyRef{KMS: "kms://key/path"},
+						Sources: []Source{{OCI: ""}},
+					},
+				},
+			},
+		},
 	},
 	}
 

pkg/reconciler/clusterimagepolicy/clusterimagepolicy_test.go

@@ -93,6 +93,8 @@ RCTfQ5s1kD+hGMSE1rH7s46hmXEeyhnlRnaGF8eMU/SBJE/2NKPnxE7WzQ==
 
 	replaceCIPKeySourcePatch = `[{"op":"replace","path":"/data/test-cip","value":"{\"uid\":\"test-uid\",\"resourceVersion\":\"0123456789\",\"images\":[{\"glob\":\"ghcr.io/example/*\"}],\"authorities\":[{\"name\":\"authority-0\",\"key\":{\"data\":\"-----BEGIN PUBLIC KEY-----\\nMFkwEwYHKoZIzj0CAQYIKoZIzj0DAQcDQgAExB6+H6054/W1SJgs5JR6AJr6J35J\\nRCTfQ5s1kD+hGMSE1rH7s46hmXEeyhnlRnaGF8eMU/SBJE/2NKPnxE7WzQ==\\n-----END PUBLIC KEY-----\",\"hashAlgorithm\":\"sha256\"},\"source\":[{\"oci\":\"example.com/alternative/signature\",\"signaturePullSecrets\":[{\"name\":\"signaturePullSecretName\"}]}]}],\"mode\":\"enforce\"}"}]`
 
+	replaceCIPKeySourceWithoutOCIPatch = `[{"op":"replace","path":"/data/test-cip","value":"{\"uid\":\"test-uid\",\"resourceVersion\":\"0123456789\",\"images\":[{\"glob\":\"ghcr.io/example/*\"}],\"authorities\":[{\"name\":\"authority-0\",\"key\":{\"data\":\"-----BEGIN PUBLIC KEY-----\\nMFkwEwYHKoZIzj0CAQYIKoZIzj0DAQcDQgAExB6+H6054/W1SJgs5JR6AJr6J35J\\nRCTfQ5s1kD+hGMSE1rH7s46hmXEeyhnlRnaGF8eMU/SBJE/2NKPnxE7WzQ==\\n-----END PUBLIC KEY-----\",\"hashAlgorithm\":\"sha256\"},\"source\":[{\"signaturePullSecrets\":[{\"name\":\"signaturePullSecretName\"}]}]}],\"mode\":\"enforce\"}"}]`
+
 	resourceVersion = "0123456789"
 	uid             = "test-uid"
 )
@@ -605,6 +607,35 @@ func TestReconcile(t *testing.T) {
 			WantPatches: []clientgotesting.PatchActionImpl{
 				makePatch(replaceCIPKeySourcePatch),
 			},
+		}, {
+			Name: "Key with data, source, no oci but signature pull secrets",
+			Key:  testKey,
+
+			SkipNamespaceValidation: true, // Cluster scoped
+			Objects: []runtime.Object{
+				NewClusterImagePolicy(cipName,
+					WithUID(uid),
+					WithResourceVersion(resourceVersion),
+					WithFinalizer,
+					WithImagePattern(v1alpha1.ImagePattern{
+						Glob: glob,
+					}),
+					WithAuthority(v1alpha1.Authority{
+						Key: &v1alpha1.KeyRef{
+							Data: validPublicKeyData,
+						},
+						Sources: []v1alpha1.Source{{
+							SignaturePullSecrets: []corev1.LocalObjectReference{
+								{Name: "signaturePullSecretName"},
+							},
+						}},
+					}),
+				),
+				makeConfigMap(),
+			},
+			WantPatches: []clientgotesting.PatchActionImpl{
+				makePatch(replaceCIPKeySourceWithoutOCIPatch),
+			},
 		}, {
 			Name: "ClusterImagePolicy with glob and KMS key, for unsupported KMS provider",
 			Key:  cipKMSName,

pkg/webhook/clusterimagepolicy/clusterimagepolicy_types.go

@@ -193,10 +193,12 @@ func (a *Authority) UnmarshalJSON(data []byte) error {
 	// Determine additional RemoteOpts
 	if len(rawAuthority.Sources) > 0 {
 		for _, source := range rawAuthority.Sources {
-			if targetRepoOverride, err := name.NewRepository(source.OCI); err != nil {
-				return fmt.Errorf("failed to determine source: %w", err)
-			} else if (targetRepoOverride != name.Repository{}) {
-				rawAuthority.RemoteOpts = append(rawAuthority.RemoteOpts, ociremote.WithTargetRepository(targetRepoOverride))
+			if source.OCI != "" {
+				if targetRepoOverride, err := name.NewRepository(source.OCI); err != nil {
+					return fmt.Errorf("failed to determine source: %w", err)
+				} else if (targetRepoOverride != name.Repository{}) {
+					rawAuthority.RemoteOpts = append(rawAuthority.RemoteOpts, ociremote.WithTargetRepository(targetRepoOverride))
+				}
 			}
 		}
 	}

test/testdata/policy-controller/valid/valid-keyless-with-source.yaml

@@ -0,0 +1,30 @@
+# 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.
+---
+apiVersion: policy.sigstore.dev/v1alpha1
+kind: ClusterImagePolicy
+metadata:
+  name: image-policy
+spec:
+  images:
+  - glob: images.*
+  authorities:
+  - keyless:
+      ca-cert:
+        secretRef:
+          name: ca-cert-secret
+          namespace: some-namespacemak
+    source:
+      - signaturePullSecrets:
+        - name: examplePullSecret