Skip to content

Commit

Permalink
change: Improve Usage of ImageAllowRules (acorn-io#2067 + acorn-io#2064
Browse files Browse the repository at this point in the history
… + acorn-io#2069) (acorn-io#2074)

- clarify output of  `acorn image sign`
- fix: properly handle images with missing required signature
- fix: DO NOT try to delete signature from remote registry
  • Loading branch information
iwilltry42 authored and cloudnautique committed Sep 28, 2023
1 parent b5ba255 commit affee3a
Show file tree
Hide file tree
Showing 6 changed files with 37 additions and 35 deletions.
2 changes: 1 addition & 1 deletion integration/client/signatures/rules_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -137,7 +137,7 @@ func TestImageAllowRules(t *testing.T) {

// try to run - expect success
app, err := c.AppRun(ctx, tagName, nil)
assert.NoError(t, err, "should not error since image is covered by images scope of IAR and there are not other rules")
assert.NoError(t, err, "should not error since image `%s` is covered by images scope `%+v` of IAR and there are not other rules", tagName, iar.Images)

// remove app
_, err = c.AppDelete(ctx, app.Name)
Expand Down
2 changes: 1 addition & 1 deletion pkg/cli/images_sign.go
Original file line number Diff line number Diff line change
Expand Up @@ -135,7 +135,7 @@ func (a *ImageSign) Run(cmd *cobra.Command, args []string) error {
return err
}

pterm.Success.Printf("Done: Pushed signature %s\n", sig.SignatureDigest)
pterm.Success.Printf("Created signature %s\n", sig.SignatureDigest)

return nil
}
Expand Down
3 changes: 2 additions & 1 deletion pkg/imageallowrules/imageallowrules.go
Original file line number Diff line number Diff line change
Expand Up @@ -115,7 +115,8 @@ iarLoop:
// Any verification error or failed verification issue will skip on to the next IAR
for _, rule := range imageAllowRule.Signatures.Rules {
if err := cosign.EnsureReferences(ctx, c, image, namespace, &verifyOpts); err != nil {
return err
logrus.Infof("failed checking image %s against %s/%s.signatures: %v", image, imageAllowRule.Namespace, imageAllowRule.Name, err)
continue iarLoop
}
verifyOpts.AnnotationRules = rule.Annotations

Expand Down
10 changes: 1 addition & 9 deletions pkg/server/registry/apigroups/acorn/apps/validator.go
Original file line number Diff line number Diff line change
Expand Up @@ -178,7 +178,7 @@ func (s *Validator) Validate(ctx context.Context, obj runtime.Object) (result fi
}

if !disableCheckImageAllowRules {
if err := s.checkImageAllowed(ctx, app.Namespace, checkImage, remote.WithTransport(s.transport)); err != nil {
if err := imageallowrules.CheckImageAllowed(ctx, s.client, app.Namespace, checkImage, imageDetails.AppImage.Digest, remote.WithTransport(s.transport)); err != nil {
result = append(result, field.Forbidden(field.NewPath("spec", "image"), fmt.Sprintf("%s not allowed to run: %s", app.Spec.Image, err.Error())))
return
}
Expand Down Expand Up @@ -833,11 +833,3 @@ func (s *Validator) getImageDetails(ctx context.Context, namespace string, profi

return details, nil
}

func (s *Validator) checkImageAllowed(ctx context.Context, namespace, image string, opts ...remote.Option) error {
digest, _, err := s.resolveLocalImage(ctx, namespace, image)
if err != nil {
return err
}
return imageallowrules.CheckImageAllowed(ctx, s.client, namespace, image, digest, opts...)
}
23 changes: 14 additions & 9 deletions pkg/server/registry/apigroups/acorn/images/sign.go
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,7 @@ func (t *ImageSignStrategy) ImageSign(ctx context.Context, namespace string, sig
mutateOpts = append(mutateOpts, mutate.WithDupeDetector(dupeDetector))
}

ociEntity, err := ociremote.SignedEntity(ref, ociremote.WithRemoteOptions(remoteOpts...))
targetEntity, err := ociremote.SignedEntity(ref, ociremote.WithRemoteOptions(remoteOpts...))
if err != nil {
return "", fmt.Errorf("accessing entity: %w", err)
}
Expand All @@ -102,22 +102,27 @@ func (t *ImageSignStrategy) ImageSign(ctx context.Context, namespace string, sig
return "", err
}

sigOCIDigest, err := signatureOCI.Digest()
signedEntity, err := mutate.AttachSignatureToEntity(targetEntity, signatureOCI, mutateOpts...)
if err != nil {
return "", err
}

mutatedOCIEntity, err := mutate.AttachSignatureToEntity(ociEntity, signatureOCI, mutateOpts...)
if err != nil {
targetRepo := ref.Context()

if err := ociremote.WriteSignatures(targetRepo, signedEntity, ociremote.WithRemoteOptions(remoteOpts...)); err != nil {
return "", err
}

targetRepo := ref.Context()
if err := ociremote.WriteSignatures(targetRepo, mutatedOCIEntity, ociremote.WithRemoteOptions(remoteOpts...)); err != nil {
// Get the digest of the signature artifact we just wrote
se, err := signedEntity.Signatures()
if err != nil {
return "", err
}
sigDigest, err := se.Digest()
if err != nil {
return "", err
}
logrus.Infof("Wrote signatures artifact %s to %s", sigDigest, targetRepo.Name())

logrus.Debugf("Wrote signature %s to %s", sigOCIDigest, targetRepo.Name())

return sigOCIDigest.String(), nil
return sigDigest.String(), nil
}
32 changes: 18 additions & 14 deletions pkg/server/registry/apigroups/acorn/images/strategy.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import (
v1 "github.com/acorn-io/runtime/pkg/apis/internal.acorn.io/v1"
acornsign "github.com/acorn-io/runtime/pkg/cosign"
"github.com/acorn-io/runtime/pkg/images"
"github.com/acorn-io/runtime/pkg/imagesystem"
"github.com/acorn-io/runtime/pkg/publicname"
"github.com/acorn-io/runtime/pkg/tables"
"github.com/google/go-containerregistry/pkg/name"
Expand Down Expand Up @@ -175,24 +176,27 @@ func (s *Strategy) Delete(ctx context.Context, obj types.Object) (types.Object,
return nil, err
}

// Prune signatures
ref, err := images.GetImageReference(ctx, s.client, image.Namespace, image.Name)
if err != nil {
return nil, err
}

remoteOpts := []remote.Option{remote.WithTransport(s.transport)}
// Prune signatures - from cluster (internal) registry only
if !image.Remote && image.Repo == "" {
remoteOpts := []remote.Option{remote.WithTransport(s.transport)}

sigTag, sigDigest, err := acornsign.FindSignature(ref.Context().Digest(image.Digest), remoteOpts...)
if err != nil {
return nil, err
}
// make sure we're only searching in the internal registry
repo, _, err := imagesystem.GetInternalRepoForNamespace(ctx, s.client, image.Namespace)
if err != nil {
return nil, err
}

if sigDigest.Hex != "" {
logrus.Debugf("Deleting signature artifact %s (digest %s) from registry", sigTag.Name(), sigDigest.String())
if err := remote.Delete(sigTag.Context().Digest(sigDigest.String()), remoteOpts...); err != nil {
sigTag, sigDigest, err := acornsign.FindSignature(repo.Digest(image.Digest), remoteOpts...)
if err != nil {
return nil, err
}

if sigDigest.Hex != "" {
logrus.Debugf("Deleting signature artifact %s (digest %s) from registry", sigTag.Name(), sigDigest.String())
if err := remote.Delete(sigTag.Context().Digest(sigDigest.String()), remoteOpts...); err != nil {
return nil, err
}
}
}

return image, s.client.Delete(ctx, imageToDelete)
Expand Down

0 comments on commit affee3a

Please sign in to comment.