Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[v14] Fixes a bug in the kube-agent-updater breaking private image resolution #44193

Merged
merged 4 commits into from
Jul 15, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 3 additions & 2 deletions integrations/kube-agent-updater/pkg/controller/deployment.go
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,7 @@ func (r *DeploymentVersionUpdater) Reconcile(ctx context.Context, req ctrl.Reque
var (
noNewVersionErr *version.NoNewVersionError
maintenanceErr *MaintenanceNotTriggeredError
trustErr *trace.TrustError
)
switch {
case errors.As(err, &noNewVersionErr):
Expand All @@ -88,9 +89,9 @@ func (r *DeploymentVersionUpdater) Reconcile(ctx context.Context, req ctrl.Reque
// Not logging the error because it provides no other information than its type.
log.Info("No maintenance triggered, not updating.", "currentVersion", currentVersion)
return requeueLater, nil
case trace.IsTrustError(err):
case errors.As(err, &trustErr):
// Logging as error as image verification should not fail under normal use
log.Error(err, "Image verification failed, not updating.")
log.Error(trustErr.OrigError(), "Image verification failed, not updating.")
return requeueLater, nil
case err != nil:
log.Error(err, "Unexpected error, not updating.")
Expand Down
5 changes: 3 additions & 2 deletions integrations/kube-agent-updater/pkg/controller/statefulset.go
Original file line number Diff line number Diff line change
Expand Up @@ -102,6 +102,7 @@ func (r *StatefulSetVersionUpdater) Reconcile(ctx context.Context, req ctrl.Requ
var (
noNewVersionErr *version.NoNewVersionError
maintenanceErr *MaintenanceNotTriggeredError
trustErr *trace.TrustError
)
switch {
case errors.As(err, &noNewVersionErr):
Expand All @@ -117,9 +118,9 @@ func (r *StatefulSetVersionUpdater) Reconcile(ctx context.Context, req ctrl.Requ
// No need to check for blocked rollout because the unhealthy workload
// trigger has not approved the maintenance
return requeueLater, nil
case trace.IsTrustError(err):
case errors.As(err, &trustErr):
// Logging as error as image verification should not fail under normal use
log.Error(err, "Image verification failed, not updating.")
log.Error(trustErr.OrigError(), "Image verification failed, not updating.")
if err := r.unblockStatefulSetRolloutIfStuck(ctx, &obj); err != nil {
log.Error(err, "statefulset unblocking failed, the rollout might get stuck")
}
Expand Down
18 changes: 11 additions & 7 deletions integrations/kube-agent-updater/pkg/img/cosign.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ import (
ociremote "github.com/sigstore/cosign/v2/pkg/oci/remote"
"github.com/sigstore/sigstore/pkg/cryptoutils"
"github.com/sigstore/sigstore/pkg/signature"
ctrllog "sigs.k8s.io/controller-runtime/pkg/log"
)

// hashAlgo is the Digest algorithm for OCI artfiacts:
Expand Down Expand Up @@ -62,28 +63,31 @@ func (v *cosignKeyValidator) Name() string {
// ValidateAndResolveDigest resolves the image digest and validates it was
// signed with cosign using a trusted static key.
func (v *cosignKeyValidator) ValidateAndResolveDigest(ctx context.Context, image reference.NamedTagged) (NamedTaggedDigested, error) {
// No RegistryClientOpts for now, this means no private repo support
// This might also be useful for local testing (running a test registry)
checkOpts := &cosign.CheckOpts{
RegistryClientOpts: v.registryOptions,
Annotations: nil,
ClaimVerifier: cosign.SimpleClaimVerifier,
SigVerifier: v.verifier,
IgnoreTlog: true, // TODO: should we keep this?
}
// Those are debug logs only
log := ctrllog.FromContext(ctx).V(1)
log.Info("Resolving digest", "image", image.String())

ref, err := NamedTaggedToDigest(image)
ref, err := NamedTaggedToDigest(image, v.registryOptions...)
if err != nil {
return nil, trace.Wrap(err)
return nil, trace.Wrap(err, "failed to resolve image digest")
}
log.Info("Resolved digest", "image", image.String(), "digest", ref.Digest, "reference", ref.String())

verified, _, err := cosign.VerifyImageSignatures(ctx, ref, checkOpts)
if err != nil {
return nil, trace.Trust(err, "failed to verify image signature")
return nil, trace.Wrap(err, "failed to verify image signature")
}
if len(verified) == 0 {
return nil, trace.Wrap(&trace.TrustError{Message: "cannot validate image: no valid signature found"})
}
log.Info("Signature validated", "image", image.String(), "digest", ref.Digest, "reference", ref.String())

// There are legitimate use-cases where the signing reference is not the same
// as the img we're pulling from: img promoted to an internal registry,
Expand Down Expand Up @@ -119,11 +123,11 @@ func NewCosignSingleKeyValidator(pem []byte, name string) (Validator, error) {

// NamedTaggedToDigest resolves the digest of a reference.NamedTagged image and
// returns a name.Digest image corresponding to the resolved image.
func NamedTaggedToDigest(image reference.NamedTagged) (name.Digest, error) {
func NamedTaggedToDigest(image reference.NamedTagged, opts ...ociremote.Option) (name.Digest, error) {
ref, err := name.ParseReference(image.String())
if err != nil {
return name.Digest{}, trace.Wrap(err)
}
digested, err := ociremote.ResolveDigest(ref)
digested, err := ociremote.ResolveDigest(ref, opts...)
return digested, trace.Wrap(err)
}
Loading
Loading