Changes to verify package in preparation for move to library-go

The CVO verify package will be moved to library-go for reuse by 'oc adm
release mirror' and CVO. Changes are being made here in CVO before the
move to provide a cleaner move to library-go.

Logic to create verifier was broken out into a new configmap method
NewFromManifests. This method uses k8s encoding and as a result
existing test cases had to be changed from creating manifests from
from declarations within the test cases itself to creating them from
config map files. In this manner the lib/Manifest object is properly
created with its Raw bytes field populated.

Author: jottofar

pkg/cvo/cvo.go

@@ -12,11 +12,9 @@ import (
 
 	"github.com/blang/semver/v4"
 	"github.com/google/uuid"
-	"github.com/pkg/errors"
 	corev1 "k8s.io/api/core/v1"
 	apierrors "k8s.io/apimachinery/pkg/api/errors"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
-	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
 	utilruntime "k8s.io/apimachinery/pkg/util/runtime"
 	"k8s.io/apimachinery/pkg/util/wait"
 	informerscorev1 "k8s.io/client-go/informers/core/v1"
@@ -281,32 +279,20 @@ func (optr *Operator) InitializeFromPayload(restConfig *rest.Config, burstRestCo
 // package for more details on the algorithm for verification. If the annotation is set, a verifier or error
 // is always returned.
 func loadConfigMapVerifierDataFromUpdate(update *payload.Update, clientBuilder sigstore.HTTPClient, configMapClient coreclientsetv1.ConfigMapsGetter) (verify.Interface, *verify.StorePersister, error) {
-	configMapGVK := corev1.SchemeGroupVersion.WithKind("ConfigMap")
-	for _, manifest := range update.Manifests {
-		if manifest.GVK != configMapGVK {
-			continue
-		}
-		if _, ok := manifest.Obj.GetAnnotations()[verify.ReleaseAnnotationConfigMapVerifier]; !ok {
-			continue
-		}
-		src := fmt.Sprintf("the config map %s/%s", manifest.Obj.GetNamespace(), manifest.Obj.GetName())
-		data, _, err := unstructured.NestedStringMap(manifest.Obj.Object, "data")
-		if err != nil {
-			return nil, nil, errors.Wrapf(err, "%s is not valid: %v", src, err)
-		}
-		verifier, err := verify.NewFromConfigMapData(src, data, clientBuilder)
-		if err != nil {
-			return nil, nil, err
-		}
-
-		// allow the verifier to consult the cluster for signature data, and also configure
-		// a process that writes signatures back to that store
-		signatureStore := configmap.NewStore(configMapClient, nil)
-		verifier.Store = &serial.Store{Stores: []store.Store{signatureStore, verifier.Store}}
-		persister := verify.NewSignatureStorePersister(signatureStore, verifier)
-		return verifier, persister, nil
+	verifier, err := verify.NewFromManifests(update.Manifests, clientBuilder)
+	if err != nil {
+		return nil, nil, err
 	}
-	return nil, nil, nil
+	if verifier == nil {
+		return nil, nil, nil
+	}
+
+	// allow the verifier to consult the cluster for signature data, and also configure
+	// a process that writes signatures back to that store
+	signatureStore := configmap.NewStore(configMapClient, nil)
+	verifier.Store = &serial.Store{Stores: []store.Store{signatureStore, verifier.Store}}
+	persister := verify.NewSignatureStorePersister(signatureStore, verifier)
+	return verifier, persister, nil
 }
 
 // Run runs the cluster version operator until stopCh is completed. Workers is ignored for now.

pkg/cvo/cvo_test.go

@@ -1,6 +1,7 @@
 package cvo
 
 import (
+	"bytes"
 	"context"
 	"fmt"
 	"io/ioutil"
@@ -20,7 +21,6 @@ import (
 	apiextclientv1 "k8s.io/apiextensions-apiserver/pkg/client/clientset/clientset/typed/apiextensions/v1beta1"
 	"k8s.io/apimachinery/pkg/api/errors"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
-	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
 	"k8s.io/apimachinery/pkg/labels"
 	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/apimachinery/pkg/runtime/schema"
@@ -3272,190 +3272,97 @@ func fakeClientsetWithUpdates(obj *configv1.ClusterVersion) *fake.Clientset {
 }
 
 func Test_loadReleaseVerifierFromConfigMap(t *testing.T) {
-	redhatData, err := ioutil.ReadFile(filepath.Join("..", "verify", "testdata", "keyrings", "redhat.txt"))
-	if err != nil {
-		t.Fatal(err)
-	}
+	const (
+		ExpectedError    = "the config map openshift-config-managed/release-verification did not provide any signature stores to read from and cannot be used"
+		ExpectedVerifier = "All release image digests must have GPG signatures from verifier-public-key-redhat (567E347AD0044ADE55BA8A5F199E2F91FD431D51: Red Hat, Inc. (release key 2) <[email protected]>) - will check for signatures in containers/image format at serial signature store wrapping config maps in openshift-config-managed with label \"release.openshift.io/verification-signatures\", parallel signature store wrapping file:///verify/testdata/signatures"
+	)
 
 	tests := []struct {
-		name          string
-		update        *payload.Update
-		want          bool
-		wantErr       bool
-		wantVerifiers int
+		name             string
+		fileName         string
+		update           *payload.Update
+		expectedError    string
+		expectedVerifier string
+		expectStore      bool
 	}{
 		{
-			name:   "is a no-op when no objects are found",
-			update: &payload.Update{},
+			name:     "is a no-op when no objects are found",
+			fileName: "",
+			update:   &payload.Update{},
 		},
 		{
-			name: "requires data",
-			update: &payload.Update{
-				Manifests: []lib.Manifest{
-					{
-						GVK: schema.GroupVersionKind{Version: "v1", Kind: "ConfigMap"},
-						Obj: &unstructured.Unstructured{
-							Object: map[string]interface{}{
-								"metadata": map[string]interface{}{
-									"name":      "release-verification",
-									"namespace": "openshift-config-managed",
-									"annotations": map[string]interface{}{
-										"release.openshift.io/verification-config-map": "",
-									},
-								},
-							},
-						},
-					},
-				},
-			},
-			wantErr: true,
+			name:          "requires data",
+			fileName:      "requires-data.yaml",
+			update:        &payload.Update{},
+			expectedError: ExpectedError,
 		},
 		{
-			name: "requires stores",
-			update: &payload.Update{
-				Manifests: []lib.Manifest{
-					{
-						GVK: schema.GroupVersionKind{Version: "v1", Kind: "ConfigMap"},
-						Obj: &unstructured.Unstructured{
-							Object: map[string]interface{}{
-								"metadata": map[string]interface{}{
-									"name":      "verification",
-									"namespace": "openshift-config",
-									"annotations": map[string]interface{}{
-										"release.openshift.io/verification-config-map": "",
-									},
-								},
-								"data": map[string]interface{}{
-									"verifier-public-key-redhat": string(redhatData),
-								},
-							},
-						},
-					},
-				},
-			},
-			wantErr: true,
+			name:          "requires stores",
+			fileName:      "requires-stores.yaml",
+			update:        &payload.Update{},
+			expectedError: ExpectedError,
 		},
 		{
-			name: "requires verifiers",
-			update: &payload.Update{
-				Manifests: []lib.Manifest{
-					{
-						GVK: schema.GroupVersionKind{Version: "v1", Kind: "ConfigMap"},
-						Obj: &unstructured.Unstructured{
-							Object: map[string]interface{}{
-								"metadata": map[string]interface{}{
-									"name":      "release-verification",
-									"namespace": "openshift-config-managed",
-									"annotations": map[string]interface{}{
-										"release.openshift.io/verification-config-map": "",
-									},
-								},
-								"data": map[string]interface{}{
-									"store-local": "file://../verify/testdata/signatures",
-								},
-							},
-						},
-					},
-				},
-			},
-			wantErr: true,
+			name:          "requires verifiers",
+			fileName:      "requires-verifiers.yaml",
+			update:        &payload.Update{},
+			expectedError: ExpectedError,
 		},
 		{
-			name: "loads valid configuration",
-			update: &payload.Update{
-				Manifests: []lib.Manifest{
-					{
-						GVK: schema.GroupVersionKind{Version: "v1", Kind: "ConfigMap"},
-						Obj: &unstructured.Unstructured{
-							Object: map[string]interface{}{
-								"metadata": map[string]interface{}{
-									"name":      "release-verification",
-									"namespace": "openshift-config-managed",
-									"annotations": map[string]interface{}{
-										"release.openshift.io/verification-config-map": "",
-									},
-								},
-								"data": map[string]interface{}{
-									"verifier-public-key-redhat": string(redhatData),
-									"store-local":                "file://../verify/testdata/signatures",
-								},
-							},
-						},
-					},
-				},
-			},
-			want:          true,
-			wantVerifiers: 1,
+			name:             "loads valid configuration",
+			fileName:         "loads-valid.yaml",
+			update:           &payload.Update{},
+			expectedVerifier: ExpectedVerifier,
+			expectStore:      true,
 		},
 		{
-			name: "only the first valid configuration is used",
-			update: &payload.Update{
-				Manifests: []lib.Manifest{
-					{
-						GVK: schema.GroupVersionKind{Version: "v1", Kind: "ConfigMap"},
-						Obj: &unstructured.Unstructured{
-							Object: map[string]interface{}{
-								"metadata": map[string]interface{}{
-									"name":      "release-verification",
-									"namespace": "openshift-config-managed",
-									"annotations": map[string]interface{}{
-										"release.openshift.io/verification-config-map": "",
-									},
-								},
-								"data": map[string]interface{}{
-									"verifier-public-key-redhat": string(redhatData),
-									"store-local":                "\nfile://../verify/testdata/signatures\n",
-								},
-							},
-						},
-					},
-					{
-						GVK: schema.GroupVersionKind{Version: "v1", Kind: "ConfigMap"},
-						Obj: &unstructured.Unstructured{
-							Object: map[string]interface{}{
-								"metadata": map[string]interface{}{
-									"name":      "release-verificatio-2n",
-									"namespace": "openshift-config-managed",
-									"annotations": map[string]interface{}{
-										"release.openshift.io/verification-config-map": "",
-									},
-								},
-								"data": map[string]interface{}{
-									"verifier-public-key-redhat":   string(redhatData),
-									"verifier-public-key-redhat-2": string(redhatData),
-									"store-local":                  "file://../verify/testdata/signatures",
-								},
-							},
-						},
-					},
-				},
-			},
-			want:          true,
-			wantVerifiers: 1,
+			name:             "only the first valid configuration is used",
+			fileName:         "only-first-used.yaml",
+			update:           &payload.Update{},
+			expectedVerifier: ExpectedVerifier,
+			expectStore:      true,
 		},
 	}
 	for _, tt := range tests {
+		if tt.fileName != "" {
+			raw, err := ioutil.ReadFile(filepath.Join("..", "verify", "testdata", "manifests", tt.fileName))
+			if err != nil {
+				t.Fatal(err)
+			}
+			ms, err := lib.ParseManifests(bytes.NewReader(raw))
+			if err != nil {
+				t.Fatalf("failed to parse file %s as a manifest, error = %v", tt.fileName, err)
+			}
+			tt.update.Manifests = ms
+		}
 		t.Run(tt.name, func(t *testing.T) {
 			f := kfake.NewSimpleClientset()
 			got, store, err := loadConfigMapVerifierDataFromUpdate(tt.update, sigstore.DefaultClient, f.CoreV1())
-			if (err != nil) != tt.wantErr {
-				t.Fatalf("loadReleaseVerifierFromPayload() error = %v, wantErr %v", err, tt.wantErr)
-			}
-			if (got != nil) != tt.want {
-				t.Fatal(got)
-			}
-			if tt.want && store == nil {
-				t.Fatalf("expected valid store")
-			}
-			if err != nil {
-				return
+			if err == nil {
+				if tt.expectedError != "" {
+					t.Fatalf("loadReleaseVerifierFromPayload succeeded when we expected error \"%s\"", tt.expectedError)
+				}
+			} else if tt.expectedError == "" {
+				t.Fatalf("loadReleaseVerifierFromPayload failed when we expected success: %v", err)
+			} else if tt.expectedError != err.Error() {
+				t.Fatalf("loadReleaseVerifierFromPayload failed with \"%v\" (expected \"%s\")", err, tt.expectedError)
 			}
+
 			if got == nil {
-				return
+				if tt.expectedVerifier != "" {
+					t.Fatalf("loadReleaseVerifierFromPayload did not return a verifier when expected")
+				}
+			} else if tt.expectedVerifier == "" {
+				t.Fatalf("loadReleaseVerifierFromPayload returned a verifer when not expected")
+			} else {
+				rvString := got.(*verify.ReleaseVerifier).String()
+				if rvString != tt.expectedVerifier {
+					t.Fatalf("loadReleaseVerifierFromPayload returned \"%v\" when we expected \"%v\"", rvString, tt.expectedVerifier)
+				}
 			}
-			rv := got.(*verify.ReleaseVerifier)
-			if len(rv.Verifiers()) != tt.wantVerifiers {
-				t.Fatalf("unexpected release verifier: %#v", rv)
+
+			if tt.expectStore && store == nil {
+				t.Fatalf("loadReleaseVerifierFromPayload did not return a store when expected")
 			}
 		})
 	}