Skip to content

Commit

Permalink
pkg/cvo/updatepayload: Event when forcing through a sig-verification …
Browse files Browse the repository at this point in the history
…failure

For unforced updates, when signature, etc. verification fails,
RetrievePayload returns an error, and we have emitted events for
RetrievePayload errors since 475e71f (emit events for each new
payload, 2020-07-21, openshift#411).  However, when the user forces the update,
we log but do not error out on verification failures.  With this
commit, we will also emit a warning event with an error message, which
will make it easier to understand how signature verification failed,
even when we don't have access to the logs of the outgoing
cluster-version operator [1].

[1]: https://bugzilla.redhat.com/show_bug.cgi?id=2071998#c2
  • Loading branch information
wking committed Apr 21, 2022
1 parent ff254ee commit f05c290
Show file tree
Hide file tree
Showing 2 changed files with 21 additions and 6 deletions.
24 changes: 19 additions & 5 deletions pkg/cvo/sync_worker.go
Expand Up @@ -2,6 +2,7 @@ package cvo

import (
"context"
"errors"
"fmt"
"math/rand"
"reflect"
Expand All @@ -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"
Expand Down Expand Up @@ -276,6 +277,19 @@ func (w *SyncWorker) syncPayload(ctx context.Context, work *SyncWork,
})
return nil, err
}
if info.VerificationError != nil {
msg := info.VerificationError.Error()
if err := errors.Unwrap(info.VerificationError); err != nil {
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,
Expand Down Expand Up @@ -1062,12 +1076,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 {
Expand Down Expand Up @@ -1162,7 +1176,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),
Expand Down Expand Up @@ -1209,7 +1223,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* ")),
}
Expand Down
3 changes: 2 additions & 1 deletion pkg/cvo/updatepayload.go
Expand Up @@ -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
Expand Down

0 comments on commit f05c290

Please sign in to comment.