diff --git a/pkg/reconciler/clusterimagepolicy/clusterimagepolicy.go b/pkg/reconciler/clusterimagepolicy/clusterimagepolicy.go index 620f90c1066..50fc4022d5e 100644 --- a/pkg/reconciler/clusterimagepolicy/clusterimagepolicy.go +++ b/pkg/reconciler/clusterimagepolicy/clusterimagepolicy.go @@ -23,6 +23,7 @@ import ( "github.com/sigstore/cosign/pkg/apis/utils" clusterimagepolicyreconciler "github.com/sigstore/cosign/pkg/client/injection/reconciler/cosigned/v1alpha1/clusterimagepolicy" "github.com/sigstore/cosign/pkg/reconciler/clusterimagepolicy/resources" + corev1 "k8s.io/api/core/v1" apierrs "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/types" @@ -53,9 +54,22 @@ var _ clusterimagepolicyreconciler.Finalizer = (*Reconciler)(nil) // ReconcileKind implements Interface.ReconcileKind. func (r *Reconciler) ReconcileKind(ctx context.Context, cip *v1alpha1.ClusterImagePolicy) reconciler.Event { - cipCopy, err := r.inlineSecrets(ctx, cip) - if err != nil { - return err + cipCopy, cipErr := r.inlineSecrets(ctx, cip) + if cipErr != nil { + // The CIP is invalid, try to remove it from the configmap. + existing, err := r.configmaplister.ConfigMaps(system.Namespace()).Get(config.ImagePoliciesConfigName) + if err != nil { + if !apierrs.IsNotFound(err) { + logging.FromContext(ctx).Errorf("Failed to get configmap: %v", err) + } + // Note that we return the error about the Invalid cip here to make + // sure that it's surfaced. + return cipErr + } + if err := r.removeCIPEntry(ctx, existing, cip); err != nil { + logging.FromContext(ctx).Errorf("Failed to get configmap: %v", err) + } + return cipErr } // See if the CM holding configs exists existing, err := r.configmaplister.ConfigMaps(system.Namespace()).Get(config.ImagePoliciesConfigName) @@ -105,16 +119,7 @@ func (r *Reconciler) FinalizeKind(ctx context.Context, cip *v1alpha1.ClusterImag return nil } // CM exists, so remove our entry from it. - patchBytes, err := resources.CreateRemovePatch(system.Namespace(), config.ImagePoliciesConfigName, existing.DeepCopy(), cip) - if err != nil { - logging.FromContext(ctx).Errorf("Failed to create remove patch: %v", err) - return err - } - if len(patchBytes) > 0 { - _, err = r.kubeclient.CoreV1().ConfigMaps(system.Namespace()).Patch(ctx, config.ImagePoliciesConfigName, types.JSONPatchType, patchBytes, metav1.PatchOptions{}) - return err - } - return nil + return r.removeCIPEntry(ctx, existing, cip) } // inlineSecrets will go through the CIP and try to read the referenced @@ -178,3 +183,17 @@ func (r *Reconciler) inlineAndTrackSecret(ctx context.Context, cip *v1alpha1.Clu } return nil } + +// removeCIPEntry removes an entry from a CM. If no entry exists, it's a nop. +func (r *Reconciler) removeCIPEntry(ctx context.Context, cm *corev1.ConfigMap, cip *v1alpha1.ClusterImagePolicy) error { + patchBytes, err := resources.CreateRemovePatch(system.Namespace(), config.ImagePoliciesConfigName, cm.DeepCopy(), cip) + if err != nil { + logging.FromContext(ctx).Errorf("Failed to create remove patch: %v", err) + return err + } + if len(patchBytes) > 0 { + _, err = r.kubeclient.CoreV1().ConfigMaps(system.Namespace()).Patch(ctx, config.ImagePoliciesConfigName, types.JSONPatchType, patchBytes, metav1.PatchOptions{}) + return err + } + return nil +} diff --git a/pkg/reconciler/clusterimagepolicy/clusterimagepolicy_test.go b/pkg/reconciler/clusterimagepolicy/clusterimagepolicy_test.go index e808fb36455..e1cb10ead1d 100644 --- a/pkg/reconciler/clusterimagepolicy/clusterimagepolicy_test.go +++ b/pkg/reconciler/clusterimagepolicy/clusterimagepolicy_test.go @@ -65,8 +65,12 @@ RCTfQ5s1kD+hGMSE1rH7s46hmXEeyhnlRnaGF8eMU/SBJE/2NKPnxE7WzQ== removeDataPatch = `[{"op":"remove","path":"/data"}]` // This is the patch for removing only a single entry from a map that has - // two entries but only one is being removed. - removeSingleEntryPatch = `[{"op":"remove","path":"/data/test-cip-2"}]` + // two entries but only one is being removed. For key entry + removeSingleEntryKeyPatch = `[{"op":"remove","path":"/data/test-cip"}]` + + // This is the patch for removing only a single entry from a map that has + // two entries but only one is being removed. For keyless entry. + removeSingleEntryKeylessPatch = `[{"op":"remove","path":"/data/test-cip-2"}]` // This is the patch for inlined secret for key ref data inlinedSecretKeyPatch = `[{"op":"replace","path":"/data/test-cip","value":"{\"images\":[{\"glob\":\"ghcr.io/example/*\",\"regex\":\"\"}],\"authorities\":[{\"key\":{\"data\":\"-----BEGIN PUBLIC KEY-----\\nMFkwEwYHKoZIzj0CAQYIKoZIzj0DAQcDQgAExB6+H6054/W1SJgs5JR6AJr6J35J\\nRCTfQ5s1kD+hGMSE1rH7s46hmXEeyhnlRnaGF8eMU/SBJE/2NKPnxE7WzQ==\\n-----END PUBLIC KEY-----\"}}]}"}]` @@ -219,13 +223,40 @@ func TestReconcile(t *testing.T) { }, WantPatches: []clientgotesting.PatchActionImpl{ patchRemoveFinalizers(system.Namespace(), cipName2), - makePatch(removeSingleEntryPatch), + makePatch(removeSingleEntryKeylessPatch), }, WantEvents: []string{ Eventf(corev1.EventTypeNormal, "FinalizerUpdate", `Updated "test-cip-2" finalizers`), }, }, { - Name: "Key with secret, secret does not exist.", + Name: "Key with secret, secret does not exist, no entry in configmap", + Key: testKey, + + SkipNamespaceValidation: true, // Cluster scoped + Objects: []runtime.Object{ + NewClusterImagePolicy(cipName, + WithFinalizer, + WithImagePattern(v1alpha1.ImagePattern{ + Glob: glob, + }), + WithAuthority(v1alpha1.Authority{ + Key: &v1alpha1.KeyRef{ + SecretRef: &corev1.SecretReference{ + Name: keySecretName, + }, + }}), + ), + makeEmptyConfigMap(), + }, + WantErr: true, + WantEvents: []string{ + Eventf(corev1.EventTypeWarning, "InternalError", `secret "publickey-key" not found`), + }, + PostConditions: []func(*testing.T, *TableRow){ + AssertTrackingSecret(system.Namespace(), keySecretName), + }, + }, { + Name: "Key with secret, secret does not exist, entry removed from configmap", Key: testKey, SkipNamespaceValidation: true, // Cluster scoped @@ -248,6 +279,35 @@ func TestReconcile(t *testing.T) { WantEvents: []string{ Eventf(corev1.EventTypeWarning, "InternalError", `secret "publickey-key" not found`), }, + WantPatches: []clientgotesting.PatchActionImpl{ + makePatch(removeSingleEntryKeyPatch), + }, + PostConditions: []func(*testing.T, *TableRow){ + AssertTrackingSecret(system.Namespace(), keySecretName), + }, + }, { + Name: "Key with secret, secret does not exist, cm does not exist", + Key: testKey, + + SkipNamespaceValidation: true, // Cluster scoped + Objects: []runtime.Object{ + NewClusterImagePolicy(cipName, + WithFinalizer, + WithImagePattern(v1alpha1.ImagePattern{ + Glob: glob, + }), + WithAuthority(v1alpha1.Authority{ + Key: &v1alpha1.KeyRef{ + SecretRef: &corev1.SecretReference{ + Name: keySecretName, + }, + }}), + ), + }, + WantErr: true, + WantEvents: []string{ + Eventf(corev1.EventTypeWarning, "InternalError", `secret "publickey-key" not found`), + }, PostConditions: []func(*testing.T, *TableRow){ AssertTrackingSecret(system.Namespace(), keySecretName), }, @@ -277,6 +337,9 @@ func TestReconcile(t *testing.T) { WantEvents: []string{ Eventf(corev1.EventTypeWarning, "InternalError", `secret "publickey-keyless" not found`), }, + WantPatches: []clientgotesting.PatchActionImpl{ + makePatch(removeSingleEntryKeyPatch), + }, PostConditions: []func(*testing.T, *TableRow){ AssertTrackingSecret(system.Namespace(), keylessSecretName), }, @@ -304,7 +367,7 @@ func TestReconcile(t *testing.T) { Name: keySecretName, }, }, - makeConfigMapWithTwoEntries(), + makeEmptyConfigMap(), }, WantErr: true, WantEvents: []string{ @@ -341,7 +404,7 @@ func TestReconcile(t *testing.T) { "second": []byte("second data"), }, }, - makeConfigMapWithTwoEntries(), + makeEmptyConfigMap(), }, WantErr: true, WantEvents: []string{ @@ -368,7 +431,7 @@ func TestReconcile(t *testing.T) { }, }}), ), - makeConfigMapWithTwoEntries(), + makeEmptyConfigMap(), makeSecret(keySecretName, "garbage secret value, not a public key"), }, WantErr: true, @@ -465,6 +528,15 @@ func makeSecret(name, secret string) *corev1.Secret { } } +func makeEmptyConfigMap() *corev1.ConfigMap { + return &corev1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: system.Namespace(), + Name: config.ImagePoliciesConfigName, + }, + } +} + func makeConfigMap() *corev1.ConfigMap { return &corev1.ConfigMap{ ObjectMeta: metav1.ObjectMeta{