Skip to content

Commit 247ab9a

Browse files
openshift-merge-robotopenshift-merge-robot
authored
Merge pull request #413 from jottofar/ota-190-verify-reuse-prep
Changes to verify package in preparation for move to library-go
2 parents 71aef74 + 472cc1b commit 247ab9a

File tree

14 files changed

+453
-232
lines changed

14 files changed

+453
-232
lines changed

‎pkg/cvo/cvo.go

Lines changed: 13 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -11,11 +11,9 @@ import (
1111
"time"
1212

1313
"github.com/google/uuid"
14-
"github.com/pkg/errors"
1514
corev1 "k8s.io/api/core/v1"
1615
apierrors "k8s.io/apimachinery/pkg/api/errors"
1716
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
18-
"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
1917
utilruntime "k8s.io/apimachinery/pkg/util/runtime"
2018
"k8s.io/apimachinery/pkg/util/wait"
2119
informerscorev1 "k8s.io/client-go/informers/core/v1"
@@ -274,32 +272,20 @@ func (optr *Operator) InitializeFromPayload(restConfig *rest.Config, burstRestCo
274272
// package for more details on the algorithm for verification. If the annotation is set, a verifier or error
275273
// is always returned.
276274
func loadConfigMapVerifierDataFromUpdate(update *payload.Update, clientBuilder sigstore.HTTPClient, configMapClient coreclientsetv1.ConfigMapsGetter) (verify.Interface, *verify.StorePersister, error) {
277-
configMapGVK := corev1.SchemeGroupVersion.WithKind("ConfigMap")
278-
for _, manifest := range update.Manifests {
279-
if manifest.GVK != configMapGVK {
280-
continue
281-
}
282-
if _, ok := manifest.Obj.GetAnnotations()[verify.ReleaseAnnotationConfigMapVerifier]; !ok {
283-
continue
284-
}
285-
src := fmt.Sprintf("the config map %s/%s", manifest.Obj.GetNamespace(), manifest.Obj.GetName())
286-
data, _, err := unstructured.NestedStringMap(manifest.Obj.Object, "data")
287-
if err != nil {
288-
return nil, nil, errors.Wrapf(err, "%s is not valid: %v", src, err)
289-
}
290-
verifier, err := verify.NewFromConfigMapData(src, data, clientBuilder)
291-
if err != nil {
292-
return nil, nil, err
293-
}
294-
295-
// allow the verifier to consult the cluster for signature data, and also configure
296-
// a process that writes signatures back to that store
297-
signatureStore := configmap.NewStore(configMapClient, nil)
298-
verifier.Store = &serial.Store{Stores: []store.Store{signatureStore, verifier.Store}}
299-
persister := verify.NewSignatureStorePersister(signatureStore, verifier)
300-
return verifier, persister, nil
275+
verifier, err := verify.NewFromManifests(update.Manifests, clientBuilder)
276+
if err != nil {
277+
return nil, nil, err
301278
}
302-
return nil, nil, nil
279+
if verifier == nil {
280+
return nil, nil, nil
281+
}
282+
283+
// allow the verifier to consult the cluster for signature data, and also configure
284+
// a process that writes signatures back to that store
285+
signatureStore := configmap.NewStore(configMapClient, nil)
286+
verifier.Store = &serial.Store{Stores: []store.Store{signatureStore, verifier.Store}}
287+
persister := verify.NewSignatureStorePersister(signatureStore, verifier)
288+
return verifier, persister, nil
303289
}
304290

305291
// Run runs the cluster version operator until stopCh is completed. Workers is ignored for now.

‎pkg/cvo/cvo_test.go

Lines changed: 69 additions & 162 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
package cvo
22

33
import (
4+
"bytes"
45
"context"
56
"fmt"
67
"io/ioutil"
@@ -20,7 +21,6 @@ import (
2021
apiextclientv1 "k8s.io/apiextensions-apiserver/pkg/client/clientset/clientset/typed/apiextensions/v1beta1"
2122
"k8s.io/apimachinery/pkg/api/errors"
2223
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
23-
"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
2424
"k8s.io/apimachinery/pkg/labels"
2525
"k8s.io/apimachinery/pkg/runtime"
2626
"k8s.io/apimachinery/pkg/runtime/schema"
@@ -3396,190 +3396,97 @@ func fakeClientsetWithUpdates(obj *configv1.ClusterVersion) *fake.Clientset {
33963396
}
33973397

33983398
func Test_loadReleaseVerifierFromConfigMap(t *testing.T) {
3399-
redhatData, err := ioutil.ReadFile(filepath.Join("..", "verify", "testdata", "keyrings", "redhat.txt"))
3400-
if err != nil {
3401-
t.Fatal(err)
3402-
}
3399+
const (
3400+
ExpectedError = "the config map openshift-config-managed/release-verification did not provide any signature stores to read from and cannot be used"
3401+
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"
3402+
)
34033403

34043404
tests := []struct {
3405-
name string
3406-
update *payload.Update
3407-
want bool
3408-
wantErr bool
3409-
wantVerifiers int
3405+
name string
3406+
fileName string
3407+
update *payload.Update
3408+
expectedError string
3409+
expectedVerifier string
3410+
expectStore bool
34103411
}{
34113412
{
3412-
name: "is a no-op when no objects are found",
3413-
update: &payload.Update{},
3413+
name: "is a no-op when no objects are found",
3414+
fileName: "",
3415+
update: &payload.Update{},
34143416
},
34153417
{
3416-
name: "requires data",
3417-
update: &payload.Update{
3418-
Manifests: []lib.Manifest{
3419-
{
3420-
GVK: schema.GroupVersionKind{Version: "v1", Kind: "ConfigMap"},
3421-
Obj: &unstructured.Unstructured{
3422-
Object: map[string]interface{}{
3423-
"metadata": map[string]interface{}{
3424-
"name": "release-verification",
3425-
"namespace": "openshift-config-managed",
3426-
"annotations": map[string]interface{}{
3427-
"release.openshift.io/verification-config-map": "",
3428-
},
3429-
},
3430-
},
3431-
},
3432-
},
3433-
},
3434-
},
3435-
wantErr: true,
3418+
name: "requires data",
3419+
fileName: "requires-data.yaml",
3420+
update: &payload.Update{},
3421+
expectedError: ExpectedError,
34363422
},
34373423
{
3438-
name: "requires stores",
3439-
update: &payload.Update{
3440-
Manifests: []lib.Manifest{
3441-
{
3442-
GVK: schema.GroupVersionKind{Version: "v1", Kind: "ConfigMap"},
3443-
Obj: &unstructured.Unstructured{
3444-
Object: map[string]interface{}{
3445-
"metadata": map[string]interface{}{
3446-
"name": "verification",
3447-
"namespace": "openshift-config",
3448-
"annotations": map[string]interface{}{
3449-
"release.openshift.io/verification-config-map": "",
3450-
},
3451-
},
3452-
"data": map[string]interface{}{
3453-
"verifier-public-key-redhat": string(redhatData),
3454-
},
3455-
},
3456-
},
3457-
},
3458-
},
3459-
},
3460-
wantErr: true,
3424+
name: "requires stores",
3425+
fileName: "requires-stores.yaml",
3426+
update: &payload.Update{},
3427+
expectedError: ExpectedError,
34613428
},
34623429
{
3463-
name: "requires verifiers",
3464-
update: &payload.Update{
3465-
Manifests: []lib.Manifest{
3466-
{
3467-
GVK: schema.GroupVersionKind{Version: "v1", Kind: "ConfigMap"},
3468-
Obj: &unstructured.Unstructured{
3469-
Object: map[string]interface{}{
3470-
"metadata": map[string]interface{}{
3471-
"name": "release-verification",
3472-
"namespace": "openshift-config-managed",
3473-
"annotations": map[string]interface{}{
3474-
"release.openshift.io/verification-config-map": "",
3475-
},
3476-
},
3477-
"data": map[string]interface{}{
3478-
"store-local": "file://../verify/testdata/signatures",
3479-
},
3480-
},
3481-
},
3482-
},
3483-
},
3484-
},
3485-
wantErr: true,
3430+
name: "requires verifiers",
3431+
fileName: "requires-verifiers.yaml",
3432+
update: &payload.Update{},
3433+
expectedError: ExpectedError,
34863434
},
34873435
{
3488-
name: "loads valid configuration",
3489-
update: &payload.Update{
3490-
Manifests: []lib.Manifest{
3491-
{
3492-
GVK: schema.GroupVersionKind{Version: "v1", Kind: "ConfigMap"},
3493-
Obj: &unstructured.Unstructured{
3494-
Object: map[string]interface{}{
3495-
"metadata": map[string]interface{}{
3496-
"name": "release-verification",
3497-
"namespace": "openshift-config-managed",
3498-
"annotations": map[string]interface{}{
3499-
"release.openshift.io/verification-config-map": "",
3500-
},
3501-
},
3502-
"data": map[string]interface{}{
3503-
"verifier-public-key-redhat": string(redhatData),
3504-
"store-local": "file://../verify/testdata/signatures",
3505-
},
3506-
},
3507-
},
3508-
},
3509-
},
3510-
},
3511-
want: true,
3512-
wantVerifiers: 1,
3436+
name: "loads valid configuration",
3437+
fileName: "loads-valid.yaml",
3438+
update: &payload.Update{},
3439+
expectedVerifier: ExpectedVerifier,
3440+
expectStore: true,
35133441
},
35143442
{
3515-
name: "only the first valid configuration is used",
3516-
update: &payload.Update{
3517-
Manifests: []lib.Manifest{
3518-
{
3519-
GVK: schema.GroupVersionKind{Version: "v1", Kind: "ConfigMap"},
3520-
Obj: &unstructured.Unstructured{
3521-
Object: map[string]interface{}{
3522-
"metadata": map[string]interface{}{
3523-
"name": "release-verification",
3524-
"namespace": "openshift-config-managed",
3525-
"annotations": map[string]interface{}{
3526-
"release.openshift.io/verification-config-map": "",
3527-
},
3528-
},
3529-
"data": map[string]interface{}{
3530-
"verifier-public-key-redhat": string(redhatData),
3531-
"store-local": "\nfile://../verify/testdata/signatures\n",
3532-
},
3533-
},
3534-
},
3535-
},
3536-
{
3537-
GVK: schema.GroupVersionKind{Version: "v1", Kind: "ConfigMap"},
3538-
Obj: &unstructured.Unstructured{
3539-
Object: map[string]interface{}{
3540-
"metadata": map[string]interface{}{
3541-
"name": "release-verificatio-2n",
3542-
"namespace": "openshift-config-managed",
3543-
"annotations": map[string]interface{}{
3544-
"release.openshift.io/verification-config-map": "",
3545-
},
3546-
},
3547-
"data": map[string]interface{}{
3548-
"verifier-public-key-redhat": string(redhatData),
3549-
"verifier-public-key-redhat-2": string(redhatData),
3550-
"store-local": "file://../verify/testdata/signatures",
3551-
},
3552-
},
3553-
},
3554-
},
3555-
},
3556-
},
3557-
want: true,
3558-
wantVerifiers: 1,
3443+
name: "only the first valid configuration is used",
3444+
fileName: "only-first-used.yaml",
3445+
update: &payload.Update{},
3446+
expectedVerifier: ExpectedVerifier,
3447+
expectStore: true,
35593448
},
35603449
}
35613450
for _, tt := range tests {
3451+
if tt.fileName != "" {
3452+
raw, err := ioutil.ReadFile(filepath.Join("..", "verify", "testdata", "manifests", tt.fileName))
3453+
if err != nil {
3454+
t.Fatal(err)
3455+
}
3456+
ms, err := lib.ParseManifests(bytes.NewReader(raw))
3457+
if err != nil {
3458+
t.Fatalf("failed to parse file %s as a manifest, error = %v", tt.fileName, err)
3459+
}
3460+
tt.update.Manifests = ms
3461+
}
35623462
t.Run(tt.name, func(t *testing.T) {
35633463
f := kfake.NewSimpleClientset()
35643464
got, store, err := loadConfigMapVerifierDataFromUpdate(tt.update, sigstore.DefaultClient, f.CoreV1())
3565-
if (err != nil) != tt.wantErr {
3566-
t.Fatalf("loadReleaseVerifierFromPayload() error = %v, wantErr %v", err, tt.wantErr)
3567-
}
3568-
if (got != nil) != tt.want {
3569-
t.Fatal(got)
3570-
}
3571-
if tt.want && store == nil {
3572-
t.Fatalf("expected valid store")
3573-
}
3574-
if err != nil {
3575-
return
3465+
if err == nil {
3466+
if tt.expectedError != "" {
3467+
t.Fatalf("loadReleaseVerifierFromPayload succeeded when we expected error \"%s\"", tt.expectedError)
3468+
}
3469+
} else if tt.expectedError == "" {
3470+
t.Fatalf("loadReleaseVerifierFromPayload failed when we expected success: %v", err)
3471+
} else if tt.expectedError != err.Error() {
3472+
t.Fatalf("loadReleaseVerifierFromPayload failed with \"%v\" (expected \"%s\")", err, tt.expectedError)
35763473
}
3474+
35773475
if got == nil {
3578-
return
3476+
if tt.expectedVerifier != "" {
3477+
t.Fatalf("loadReleaseVerifierFromPayload did not return a verifier when expected")
3478+
}
3479+
} else if tt.expectedVerifier == "" {
3480+
t.Fatalf("loadReleaseVerifierFromPayload returned a verifer when not expected")
3481+
} else {
3482+
rvString := got.(*verify.ReleaseVerifier).String()
3483+
if rvString != tt.expectedVerifier {
3484+
t.Fatalf("loadReleaseVerifierFromPayload returned \"%v\" when we expected \"%v\"", rvString, tt.expectedVerifier)
3485+
}
35793486
}
3580-
rv := got.(*verify.ReleaseVerifier)
3581-
if len(rv.Verifiers()) != tt.wantVerifiers {
3582-
t.Fatalf("unexpected release verifier: %#v", rv)
3487+
3488+
if tt.expectStore && store == nil {
3489+
t.Fatalf("loadReleaseVerifierFromPayload did not return a store when expected")
35833490
}
35843491
})
35853492
}

0 commit comments

Comments
 (0)