Skip to content

Commit

Permalink
merge #556 into opencontainers/umoci:main
Browse files Browse the repository at this point in the history
Aleksa Sarai (9):
  pkg: fmtcompat: remove now that there are no users
  errors: drop remaining errors.Wrap-like fmtcompat.Errorf users
  errors: drop fmtcompat.Errorf for complicated err != nil checks
  errors: drop fmtcompat.Errorf for explicit err != nil checks
  errors: drop fmtcompat.Errorf for non-%w errors
  *: migrate to Go stdlib error wrapping
  pkg: fmtcompat: add a compatibility shim for fmt.Errorf
  fmt: use %q for quoting
  *: fix new golint warnings

LGTMs: tych0 cyphar
  • Loading branch information
cyphar committed Nov 29, 2024
2 parents cddb884 + e9fff47 commit 8f807a3
Show file tree
Hide file tree
Showing 59 changed files with 818 additions and 730 deletions.
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,8 @@ and this project adheres to [Semantic Versioning](http://semver.org/).
- Add the `cgroup` namespace to the default configuration generated by `umoci
unpack` to make sure that our configuration plays nicely with `runc` when on
cgroupv2 systems.
- umoci has been migrated away from `github.com/pkg/errors` to Go stdlib error
wrapping.

### Fixed ###
- In 0.4.7, a performance regression was introduced as part of the
Expand Down
5 changes: 3 additions & 2 deletions api.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,9 +18,10 @@
package umoci

import (
"fmt"

"github.com/opencontainers/umoci/oci/cas/dir"
"github.com/opencontainers/umoci/oci/casext"
"github.com/pkg/errors"
)

// OpenLayout opens an existing OCI image layout, and fails if it does not
Expand All @@ -29,7 +30,7 @@ func OpenLayout(imagePath string) (casext.Engine, error) {
// Get a reference to the CAS.
engine, err := dir.Open(imagePath)
if err != nil {
return casext.Engine{}, errors.Wrap(err, "open CAS")
return casext.Engine{}, fmt.Errorf("open CAS: %w", err)
}

return casext.NewEngine(engine), nil
Expand Down
51 changes: 26 additions & 25 deletions cmd/umoci/config.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
/*
* umoci: Umoci Modifies Open Containers' Images
* Copyright (C) 2016-2020 SUSE LLC
* Copyright (C) 2016-2024 SUSE LLC
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
Expand All @@ -19,6 +19,8 @@ package main

import (
"context"
"errors"
"fmt"
"strings"
"time"

Expand All @@ -28,7 +30,6 @@ import (
"github.com/opencontainers/umoci/oci/cas/dir"
"github.com/opencontainers/umoci/oci/casext"
igen "github.com/opencontainers/umoci/oci/config/generate"
"github.com/pkg/errors"
"github.com/urfave/cli"
)

Expand All @@ -52,13 +53,13 @@ image.`,
// Verify the metadata.
Before: func(ctx *cli.Context) error {
if ctx.NArg() != 0 {
return errors.Errorf("invalid number of positional arguments: expected none")
return errors.New("invalid number of positional arguments: expected none")
}
if _, ok := ctx.App.Metadata["--image-path"]; !ok {
return errors.Errorf("missing mandatory argument: --image")
return errors.New("missing mandatory argument: --image")
}
if _, ok := ctx.App.Metadata["--image-tag"]; !ok {
return errors.Errorf("missing mandatory argument: --image")
return errors.New("missing mandatory argument: --image")
}
return nil
},
Expand Down Expand Up @@ -126,12 +127,12 @@ func fromImage(image ispec.Image) (ispec.ImageConfig, mutate.Meta) {
func parseKV(input string) (string, string, error) {
parts := strings.SplitN(input, "=", 2)
if len(parts) != 2 {
return "", "", errors.Errorf("must contain '=': %s", input)
return "", "", fmt.Errorf("must contain '=': %s", input)
}

name, value := parts[0], parts[1]
if name == "" {
return "", "", errors.Errorf("must have non-empty name: %s", input)
return "", "", fmt.Errorf("must have non-empty name: %s", input)
}
return name, value, nil
}
Expand All @@ -149,46 +150,46 @@ func config(ctx *cli.Context) error {
// Get a reference to the CAS.
engine, err := dir.Open(imagePath)
if err != nil {
return errors.Wrap(err, "open CAS")
return fmt.Errorf("open CAS: %w", err)
}
engineExt := casext.NewEngine(engine)
defer engine.Close()

fromDescriptorPaths, err := engineExt.ResolveReference(context.Background(), fromName)
if err != nil {
return errors.Wrap(err, "get descriptor")
return fmt.Errorf("get descriptor: %w", err)
}
if len(fromDescriptorPaths) == 0 {
return errors.Errorf("tag not found: %s", fromName)
return fmt.Errorf("tag not found: %s", fromName)
}
if len(fromDescriptorPaths) != 1 {
// TODO: Handle this more nicely.
return errors.Errorf("tag is ambiguous: %s", fromName)
return fmt.Errorf("tag is ambiguous: %s", fromName)
}

mutator, err := mutate.New(engine, fromDescriptorPaths[0])
if err != nil {
return errors.Wrap(err, "create mutator for manifest")
return fmt.Errorf("create mutator for manifest: %w", err)
}

config, err := mutator.Config(context.Background())
if err != nil {
return errors.Wrap(err, "get base config")
return fmt.Errorf("get base config: %w", err)
}

imageMeta, err := mutator.Meta(context.Background())
if err != nil {
return errors.Wrap(err, "get base metadata")
return fmt.Errorf("get base metadata: %w", err)
}

annotations, err := mutator.Annotations(context.Background())
if err != nil {
return errors.Wrap(err, "get base annotations")
return fmt.Errorf("get base annotations: %w", err)
}

g, err := igen.NewFromImage(toImage(config.Config, imageMeta))
if err != nil {
return errors.Wrap(err, "create new generator")
return fmt.Errorf("create new generator: %w", err)
}

if ctx.IsSet("clear") {
Expand All @@ -206,13 +207,13 @@ func config(ctx *cli.Context) error {
g.ClearConfigVolumes()
case "rootfs.diffids":
//g.ClearRootfsDiffIDs()
return errors.Errorf("--clear=rootfs.diffids is not safe")
return errors.New("--clear=rootfs.diffids is not safe")
case "config.cmd":
g.ClearConfigCmd()
case "config.entrypoint":
g.ClearConfigEntrypoint()
default:
return errors.Errorf("unknown key to --clear: %s", key)
return fmt.Errorf("unknown key to --clear: %s", key)
}
}
}
Expand All @@ -221,7 +222,7 @@ func config(ctx *cli.Context) error {
// How do we handle other formats?
created, err := time.Parse(igen.ISO8601, ctx.String("created"))
if err != nil {
return errors.Wrap(err, "parse --created")
return fmt.Errorf("parse --created: %w", err)
}
g.SetCreated(created)
}
Expand Down Expand Up @@ -252,7 +253,7 @@ func config(ctx *cli.Context) error {
for _, env := range ctx.StringSlice("config.env") {
name, value, err := parseKV(env)
if err != nil {
return errors.Wrap(err, "config.env")
return fmt.Errorf("config.env: %w", err)
}
g.AddConfigEnv(name, value)
}
Expand All @@ -274,7 +275,7 @@ func config(ctx *cli.Context) error {
for _, label := range ctx.StringSlice("config.label") {
name, value, err := parseKV(label)
if err != nil {
return errors.Wrap(err, "config.label")
return fmt.Errorf("config.label: %w", err)
}
g.AddConfigLabel(name, value)
}
Expand Down Expand Up @@ -309,7 +310,7 @@ func config(ctx *cli.Context) error {
if ctx.IsSet("history.created") {
created, err := time.Parse(igen.ISO8601, ctx.String("history.created"))
if err != nil {
return errors.Wrap(err, "parsing --history.created")
return fmt.Errorf("parsing --history.created: %w", err)
}
history.Created = &created
}
Expand All @@ -320,18 +321,18 @@ func config(ctx *cli.Context) error {

newConfig, newMeta := fromImage(g.Image())
if err := mutator.Set(context.Background(), newConfig, newMeta, annotations, history); err != nil {
return errors.Wrap(err, "set modified configuration")
return fmt.Errorf("set modified configuration: %w", err)
}

newDescriptorPath, err := mutator.Commit(context.Background())
if err != nil {
return errors.Wrap(err, "commit mutated image")
return fmt.Errorf("commit mutated image: %w", err)
}

log.Infof("new image manifest created: %s->%s", newDescriptorPath.Root().Digest, newDescriptorPath.Descriptor().Digest)

if err := engineExt.UpdateReference(context.Background(), tagName, newDescriptorPath.Root()); err != nil {
return errors.Wrap(err, "add new tag")
return fmt.Errorf("add new tag: %w", err)
}

log.Infof("created new tag for image manifest: %s", tagName)
Expand Down
16 changes: 10 additions & 6 deletions cmd/umoci/gc.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
/*
* umoci: Umoci Modifies Open Containers' Images
* Copyright (C) 2016-2020 SUSE LLC
* Copyright (C) 2016-2024 SUSE LLC
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
Expand All @@ -19,10 +19,11 @@ package main

import (
"context"
"errors"
"fmt"

"github.com/opencontainers/umoci/oci/cas/dir"
"github.com/opencontainers/umoci/oci/casext"
"github.com/pkg/errors"
"github.com/urfave/cli"
)

Expand All @@ -42,10 +43,10 @@ root set of references. All other blobs will be removed.`,

Before: func(ctx *cli.Context) error {
if ctx.NArg() != 0 {
return errors.Errorf("invalid number of positional arguments: expected none")
return errors.New("invalid number of positional arguments: expected none")
}
if _, ok := ctx.App.Metadata["--image-path"]; !ok {
return errors.Errorf("missing mandatory argument: --layout")
return errors.New("missing mandatory argument: --layout")
}
return nil
},
Expand All @@ -59,11 +60,14 @@ func gc(ctx *cli.Context) error {
// Get a reference to the CAS.
engine, err := dir.Open(imagePath)
if err != nil {
return errors.Wrap(err, "open CAS")
return fmt.Errorf("open CAS: %w", err)
}
engineExt := casext.NewEngine(engine)
defer engine.Close()

// Run the GC.
return errors.Wrap(engineExt.GC(context.Background()), "gc")
if err := engineExt.GC(context.Background()); err != nil {
return fmt.Errorf("gc: %w", err)
}
return nil
}
12 changes: 6 additions & 6 deletions cmd/umoci/init.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
/*
* umoci: Umoci Modifies Open Containers' Images
* Copyright (C) 2016-2020 SUSE LLC
* Copyright (C) 2016-2024 SUSE LLC
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
Expand All @@ -18,12 +18,12 @@
package main

import (
"errors"
"fmt"
"os"

"github.com/apex/log"
"github.com/opencontainers/umoci/oci/cas/dir"
"github.com/pkg/errors"
"github.com/urfave/cli"
)

Expand All @@ -43,7 +43,7 @@ commands.`,

Before: func(ctx *cli.Context) error {
if ctx.NArg() != 0 {
return errors.Errorf("invalid number of positional arguments: expected none")
return errors.New("invalid number of positional arguments: expected none")
}
return nil
},
Expand All @@ -54,15 +54,15 @@ commands.`,
func initLayout(ctx *cli.Context) error {
imagePath := ctx.App.Metadata["--image-path"].(string)

if _, err := os.Stat(imagePath); !os.IsNotExist(err) {
if _, err := os.Stat(imagePath); !errors.Is(err, os.ErrNotExist) {
if err == nil {
err = fmt.Errorf("path already exists: %s", imagePath)
}
return errors.Wrap(err, "image layout creation")
return fmt.Errorf("image layout creation: %w", err)
}

if err := dir.Create(imagePath); err != nil {
return errors.Wrap(err, "image layout creation")
return fmt.Errorf("image layout creation: %w", err)
}

log.Infof("created new OCI image: %s", imagePath)
Expand Down
Loading

0 comments on commit 8f807a3

Please sign in to comment.