diff --git a/go.mod b/go.mod index a4de6a771..87154c085 100644 --- a/go.mod +++ b/go.mod @@ -11,7 +11,7 @@ require ( github.com/imdario/mergo v0.3.8 // indirect github.com/openshift/api v0.0.0-20220504105152-6f735e7109c8 github.com/openshift/client-go v0.0.0-20220504114320-6aec01bb0754 - github.com/openshift/library-go v0.0.0-20220512194226-3c66b317b110 + github.com/openshift/library-go v0.0.0-20220523142556-5bcfed822fc6 github.com/pkg/errors v0.9.1 github.com/prometheus/client_golang v1.11.1 github.com/prometheus/client_model v0.2.0 diff --git a/go.sum b/go.sum index 0beda9d50..9cd5bd688 100644 --- a/go.sum +++ b/go.sum @@ -488,8 +488,8 @@ github.com/openshift/api v0.0.0-20220504105152-6f735e7109c8/go.mod h1:F/eU6jgr6Q github.com/openshift/build-machinery-go v0.0.0-20211213093930-7e33a7eb4ce3/go.mod h1:b1BuldmJlbA/xYtdZvKi+7j5YGB44qJUJDZ9zwiNCfE= github.com/openshift/client-go v0.0.0-20220504114320-6aec01bb0754 h1:E/SORtM8rYRq5vp7zlSyxUeH1v71QZAFXW7FkAggw0Q= github.com/openshift/client-go v0.0.0-20220504114320-6aec01bb0754/go.mod h1:0KyRH70L+vAGs8wkOkqbsE9qR4lgjW2ugJsCzl1nj5w= -github.com/openshift/library-go v0.0.0-20220512194226-3c66b317b110 h1:5lgF3YD1XwaJVtasYsWKFTwI3ZUwuVMcMAwJf2x4YSY= -github.com/openshift/library-go v0.0.0-20220512194226-3c66b317b110/go.mod h1:1QSdymJBGXSOgBj7tWhj4cV+i1+AvkQ/Tq78ebWjXCU= +github.com/openshift/library-go v0.0.0-20220523142556-5bcfed822fc6 h1:HSmUjhgHhwxdNPvq8xPf19BV67wf1GIepefPOl7dsIU= +github.com/openshift/library-go v0.0.0-20220523142556-5bcfed822fc6/go.mod h1:1QSdymJBGXSOgBj7tWhj4cV+i1+AvkQ/Tq78ebWjXCU= github.com/opentracing/opentracing-go v1.1.0/go.mod h1:UkNAQd3GIcIGf0SeVgPpRdFStlNbqXla1AfSYxPUl2o= github.com/pascaldekloe/goe v0.0.0-20180627143212-57f6aae5913c/go.mod h1:lzWF7FIEvWOWxwDKqyGYQf6ZUaNfKdP144TG7ZOy1lc= github.com/pborman/uuid v1.2.0/go.mod h1:X/NO0urCmaxf9VXbdlT7C2Yzkj2IKimNn4k+gtPdI/k= diff --git a/pkg/cvo/sync_worker.go b/pkg/cvo/sync_worker.go index 27d82c816..edc65f093 100644 --- a/pkg/cvo/sync_worker.go +++ b/pkg/cvo/sync_worker.go @@ -2,6 +2,7 @@ package cvo import ( "context" + "errors" "fmt" "math/rand" "reflect" @@ -14,7 +15,7 @@ import ( "github.com/prometheus/client_golang/prometheus" "golang.org/x/time/rate" corev1 "k8s.io/api/core/v1" - "k8s.io/apimachinery/pkg/util/errors" + apierrors "k8s.io/apimachinery/pkg/util/errors" utilruntime "k8s.io/apimachinery/pkg/util/runtime" "k8s.io/apimachinery/pkg/util/wait" "k8s.io/client-go/tools/record" @@ -283,6 +284,19 @@ func (w *SyncWorker) syncPayload(ctx context.Context, work *SyncWork, }) return nil, err } + if info.VerificationError != nil { + msg := "" + for err := info.VerificationError; err != nil; err = errors.Unwrap(err) { + details := err.Error() + if !strings.Contains(msg, details) { + // library-go/pkg/verify wraps the details, but does not include them + // in the top-level error string. If we have an error like that, + // include the details here. + msg = fmt.Sprintf("%s\n%s", msg, details) + } + } + w.eventRecorder.Eventf(cvoObjectRef, corev1.EventTypeWarning, "RetrievePayload", msg) + } w.eventRecorder.Eventf(cvoObjectRef, corev1.EventTypeNormal, "LoadPayload", "Loading payload version=%q image=%q", desired.Version, desired.Image) payloadUpdate, err := payload.LoadUpdate(info.Directory, desired.Image, w.exclude, w.includeTechPreview, w.clusterProfile, @@ -1081,12 +1095,12 @@ func summarizeTaskGraphErrors(errs []error) error { // we ignore context errors (canceled or timed out) since they don't // provide good feedback to users and are an internal detail of the // server - err := errors.FilterOut(errors.NewAggregate(errs), isContextError) + err := apierrors.FilterOut(apierrors.NewAggregate(errs), isContextError) if err == nil { klog.V(2).Infof("All errors were context errors: %v", errs) return nil } - agg, ok := err.(errors.Aggregate) + agg, ok := err.(apierrors.Aggregate) if !ok { errs = []error{err} } else { @@ -1181,7 +1195,7 @@ func newClusterOperatorsNotAvailable(errs []error) error { sort.Strings(names) name := strings.Join(names, ", ") return &payload.UpdateError{ - Nested: errors.NewAggregate(errs), + Nested: apierrors.NewAggregate(errs), UpdateEffect: updateEffect, Reason: "ClusterOperatorsNotAvailable", Message: fmt.Sprintf("Some cluster operators are still updating: %s", name), @@ -1228,7 +1242,7 @@ func newMultipleError(errs []error) error { return errs[0] } return &payload.UpdateError{ - Nested: errors.NewAggregate(errs), + Nested: apierrors.NewAggregate(errs), Reason: "MultipleErrors", Message: fmt.Sprintf("Multiple errors are preventing progress:\n* %s", strings.Join(messages, "\n* ")), } diff --git a/pkg/cvo/updatepayload.go b/pkg/cvo/updatepayload.go index 4d1e34dc3..f68b026f4 100644 --- a/pkg/cvo/updatepayload.go +++ b/pkg/cvo/updatepayload.go @@ -113,7 +113,8 @@ func (r *payloadRetriever) RetrievePayload(ctx context.Context, update configv1. if !update.Force { return PayloadInfo{}, vErr } - klog.Warningf("An image was retrieved from %q that failed verification: %v", update.Image, vErr) + vErr.Message = fmt.Sprintf("Target release version=%q image=%q cannot be verified, but continuing anyway because the update was forced: %v", update.Version, update.Image, err) + klog.Warning(vErr) info.VerificationError = vErr } else { info.Verified = true diff --git a/pkg/payload/task.go b/pkg/payload/task.go index 7902c6a55..5d914f431 100644 --- a/pkg/payload/task.go +++ b/pkg/payload/task.go @@ -178,10 +178,20 @@ func (e *UpdateError) Error() string { return e.Message } +// Cause supports github.com/pkg/errors.Cause [1]. +// +// [1]: https://pkg.go.dev/github.com/pkg/errors#readme-retrieving-the-cause-of-an-error func (e *UpdateError) Cause() error { return e.Nested } +// Unwrap supports errors.Unwrap [1]. +// +// [1]: https://pkg.go.dev/errors#Unwrap +func (e *UpdateError) Unwrap() error { + return e.Nested +} + // reasonForUpdateError provides a succint explanation of a known error type for use in a human readable // message during update. Since all objects in the image should be successfully applied, messages // should direct the reader (likely a cluster administrator) to a possible cause in their own config. diff --git a/vendor/github.com/openshift/library-go/pkg/verify/store/configmap/configmap.go b/vendor/github.com/openshift/library-go/pkg/verify/store/configmap/configmap.go index f1c374f46..95706ae14 100644 --- a/vendor/github.com/openshift/library-go/pkg/verify/store/configmap/configmap.go +++ b/vendor/github.com/openshift/library-go/pkg/verify/store/configmap/configmap.go @@ -84,8 +84,8 @@ func (s *Store) mostRecentConfigMaps() []corev1.ConfigMap { return s.last } -// Signatures returns a list of signatures that match the request -// digest out of config maps labelled with ReleaseLabelConfigMap in the +// Signatures fetches signatures for the provided digest +// out of config maps labelled with ReleaseLabelConfigMap in the // NamespaceLabelConfigMap namespace. func (s *Store) Signatures(ctx context.Context, name string, digest string, fn store.Callback) error { // avoid repeatedly reloading config maps @@ -122,6 +122,12 @@ func (s *Store) Signatures(ctx context.Context, name string, digest string, fn s } } } + if done, err := fn(ctx, nil, fmt.Errorf("prefix %s in config map %s: %w", prefix, cm.ObjectMeta.Name, store.ErrNotFound)); err != nil || done { + return err + } + if err := ctx.Err(); err != nil { + return err + } } return nil } diff --git a/vendor/github.com/openshift/library-go/pkg/verify/store/parallel/parallel.go b/vendor/github.com/openshift/library-go/pkg/verify/store/parallel/parallel.go index 4d3161ad9..f5737a327 100644 --- a/vendor/github.com/openshift/library-go/pkg/verify/store/parallel/parallel.go +++ b/vendor/github.com/openshift/library-go/pkg/verify/store/parallel/parallel.go @@ -35,7 +35,11 @@ func (s *Store) Signatures(ctx context.Context, name string, digest string, fn s errorChannel <- wrappedStore.Signatures(ctx, name, digest, func(ctx context.Context, signature []byte, errIn error) (done bool, err error) { select { case <-ctx.Done(): - return true, nil + select { + case responses <- signatureResponse{signature: signature, errIn: errIn}: + default: + } + return false, ctx.Err() case responses <- signatureResponse{signature: signature, errIn: errIn}: } return false, nil @@ -74,7 +78,13 @@ func (s *Store) Signatures(ctx context.Context, name string, digest string, fn s if loopError != nil { return loopError } - return ctx.Err() // because we discard context errors from the wrapped stores + + if err := ctx.Err(); err != nil { + return err // because we discard context errors from the wrapped stores + } + + _, err := fn(ctx, nil, fmt.Errorf("%s: %w", s.String(), store.ErrNotFound)) + return err } // String returns a description of where this store finds diff --git a/vendor/github.com/openshift/library-go/pkg/verify/store/serial/serial.go b/vendor/github.com/openshift/library-go/pkg/verify/store/serial/serial.go index 700c001a8..2c47b1ece 100644 --- a/vendor/github.com/openshift/library-go/pkg/verify/store/serial/serial.go +++ b/vendor/github.com/openshift/library-go/pkg/verify/store/serial/serial.go @@ -38,7 +38,8 @@ func (s *Store) Signatures(ctx context.Context, name string, digest string, fn s } } - return nil + _, err := fn(ctx, nil, fmt.Errorf("%s: %w", s.String(), store.ErrNotFound)) + return err } // String returns a description of where this store finds diff --git a/vendor/github.com/openshift/library-go/pkg/verify/store/sigstore/sigstore.go b/vendor/github.com/openshift/library-go/pkg/verify/store/sigstore/sigstore.go index d61bb07f4..32bbd0b41 100644 --- a/vendor/github.com/openshift/library-go/pkg/verify/store/sigstore/sigstore.go +++ b/vendor/github.com/openshift/library-go/pkg/verify/store/sigstore/sigstore.go @@ -32,8 +32,6 @@ import ( // an attacker was able to take ownership of the store to perform DoS on clusters). const maxSignatureSearch = 10 -var errNotFound = errors.New("no more signatures to check") - // Store provides access to signatures stored in memory. type Store struct { // URI is the base from which signature URIs are constructed. @@ -85,7 +83,7 @@ func checkHTTPSignatures(ctx context.Context, client *http.Client, u url.URL, ma req, err := http.NewRequest("GET", sigURL.String(), nil) if err != nil { - _, err = fn(ctx, nil, fmt.Errorf("could not build request to check signature: %v", err)) + _, err = fn(ctx, nil, fmt.Errorf("could not build request to check signature: %w", err)) return err // even if the callback ate the error, no sense in checking later indexes which will fail the same way } req = req.WithContext(ctx) @@ -110,25 +108,22 @@ func checkHTTPSignatures(ctx context.Context, client *http.Client, u url.URL, ma }() if resp.StatusCode == http.StatusNotFound { - return nil, errNotFound + return nil, store.ErrNotFound } if resp.StatusCode < 200 || resp.StatusCode >= 300 { - if i == 1 { - klog.V(4).Infof("Could not find signature at store location %v", sigURL) - } - return nil, fmt.Errorf("unable to retrieve signature from %v: %d", sigURL, resp.StatusCode) + return nil, fmt.Errorf("response status %d", resp.StatusCode) } return ioutil.ReadAll(resp.Body) }() - if err == errNotFound { - break - } if err != nil { - klog.V(4).Info(err) - done, err := fn(ctx, nil, err) - if done || err != nil { - return err + err := fmt.Errorf("unable to retrieve signature from %s: %w", sigURL.String(), err) + done, err2 := fn(ctx, nil, err) + if done || err2 != nil { + return err2 + } + if errors.Is(err, store.ErrNotFound) { + break } continue } diff --git a/vendor/github.com/openshift/library-go/pkg/verify/store/store.go b/vendor/github.com/openshift/library-go/pkg/verify/store/store.go index 00a19d2be..8c98cbdac 100644 --- a/vendor/github.com/openshift/library-go/pkg/verify/store/store.go +++ b/vendor/github.com/openshift/library-go/pkg/verify/store/store.go @@ -3,6 +3,7 @@ package store import ( "context" + "errors" ) // Callback returns true if an acceptable signature has been found, or @@ -11,13 +12,25 @@ import ( // problem and the function can decide how to handle that error. type Callback func(ctx context.Context, signature []byte, errIn error) (done bool, err error) +// ErrNotFound is a base error for Callback, to be used when the store +// decides one signature-retrieval avenue is exhausted. +var ErrNotFound = errors.New("no more signatures to check") + // Store provides access to signatures by digest. type Store interface { // Signatures fetches signatures for the provided digest, feeding // them into the provided callback until an acceptable signature is - // found or an error occurs. Not finding any acceptable signatures - // is not an error; it is up to the caller to handle that case. + // found or an error occurs. + // + // Not finding additional signatures should result in a callback + // call with an error wrapping ErrNotFound, to allow the caller to + // figure out when and why the store was unable to find a signature. + // When a store has several lookup mechanisms, this may result in + // several callback calls with different ErrNotFound. Signatures + // itself should return nil in this case, because eventually running + // out of signatures is an expected part of any invocation where the + // callback calls never return done=true. Signatures(ctx context.Context, name string, digest string, fn Callback) error // String returns a description of where this store finds diff --git a/vendor/github.com/openshift/library-go/pkg/verify/verify.go b/vendor/github.com/openshift/library-go/pkg/verify/verify.go index a1f8d935a..73bbbd97c 100644 --- a/vendor/github.com/openshift/library-go/pkg/verify/verify.go +++ b/vendor/github.com/openshift/library-go/pkg/verify/verify.go @@ -189,19 +189,19 @@ func (v *releaseVerifier) Verify(ctx context.Context, releaseDigest string) erro err := v.store.Signatures(ctx, "", releaseDigest, func(ctx context.Context, signature []byte, errIn error) (done bool, err error) { if errIn != nil { klog.V(4).Infof("error retrieving signature for %s: %v", releaseDigest, errIn) - errs = append(errs, errIn) + errs = append(errs, fmt.Errorf("%s: %w", time.Now().Format(time.RFC3339), errIn)) return false, nil } for k, keyring := range remaining { content, _, err := verifySignatureWithKeyring(bytes.NewReader(signature), keyring) if err != nil { klog.V(4).Infof("keyring %q could not verify signature for %s: %v", k, releaseDigest, err) - errs = append(errs, err) + errs = append(errs, fmt.Errorf("%s: %w", time.Now().Format(time.RFC3339), err)) continue } if err := verifyAtomicContainerSignature(content, releaseDigest); err != nil { klog.V(4).Infof("signature for %s is not valid: %v", releaseDigest, err) - errs = append(errs, err) + errs = append(errs, fmt.Errorf("%s: %w", time.Now().Format(time.RFC3339), err)) continue } delete(remaining, k) @@ -210,9 +210,8 @@ func (v *releaseVerifier) Verify(ctx context.Context, releaseDigest string) erro return len(remaining) == 0, nil }) if err != nil { - klog.V(4).Infof("Failed to retrieve signatures for %s (should never happen)", releaseDigest) - errs = append(errs, err) - return err + klog.V(4).Infof("Failed to retrieve signatures for %s: %v", releaseDigest, err) + errs = append(errs, fmt.Errorf("%s: %w", time.Now().Format(time.RFC3339), err)) } if len(remaining) > 0 { diff --git a/vendor/modules.txt b/vendor/modules.txt index 8e6e3ec72..9104e1f59 100644 --- a/vendor/modules.txt +++ b/vendor/modules.txt @@ -84,7 +84,7 @@ github.com/openshift/client-go/image/clientset/versioned/scheme github.com/openshift/client-go/image/clientset/versioned/typed/image/v1 github.com/openshift/client-go/security/clientset/versioned/scheme github.com/openshift/client-go/security/clientset/versioned/typed/security/v1 -# github.com/openshift/library-go v0.0.0-20220512194226-3c66b317b110 +# github.com/openshift/library-go v0.0.0-20220523142556-5bcfed822fc6 ## explicit github.com/openshift/library-go/pkg/config/clusterstatus github.com/openshift/library-go/pkg/config/leaderelection