Skip to content

Commit

Permalink
snapshot/containerd: fix wrong errdefs package import
Browse files Browse the repository at this point in the history
I noticed that we were importing the nydus errdefs package here, and
looking at [f044e0a][1] (v0.12.0-rc1),
which introduced this import, this very likely was meant to be containerd's
errdefs package.

The only function consumed from the package is `errdefs.IsNotFound` which at
the time of the commit was not compatible with containerd's `errdefs.IsNotFound`
as it was [checking for the nydus error specifically][2].

Nydus-snapshotter v0.8.0 fixed this incompatibility by aliasing the error to
[containerd's `ErrNotFound`][3] and was updated through [483e877][4].

Ironically, the original commit [f044e0a][1]
broke vendoring, because the Nydus errdefs was no longer vendored. This was
fixed in [75dd88e][4], but failed to notice
that the missing vendor was due to an incorrect import.

So it looks like things were broken _twice_ in the chain of events (once
because the wrong errdefs package did not match the expected error; once
because the errdefs package was missing), but all of them landed in v0.12.0-rc1,
so nothing broke in a release ':-)

This PR;

- fixes the wrong import
- adds a depguard rule to prevent accidental importing of this package

[1]: f044e0a
[2]: https://github.com/moby/buildkit/blob/f044e0a9468639559db93fe30ee826ce502ac481/vendor/github.com/containerd/nydus-snapshotter/pkg/errdefs/errors.go#L22-L33
[3]: https://github.com/moby/buildkit/blob/f044e0a9468639559db93fe30ee826ce502ac481/vendor/github.com/containerd/nydus-snapshotter/pkg/errdefs/errors.go#L22-L33
[4]: 483e877
[5]: 75dd88e

Signed-off-by: Sebastiaan van Stijn <[email protected]>
(cherry picked from commit c6745c3)
Signed-off-by: Sebastiaan van Stijn <[email protected]>
  • Loading branch information
thaJeztah committed Jul 29, 2024
1 parent 979542e commit f3ed463
Show file tree
Hide file tree
Showing 4 changed files with 7 additions and 54 deletions.
4 changes: 4 additions & 0 deletions .golangci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,10 @@ linters-settings:
desc: The containerd log package was migrated to a separate module. Use github.com/containerd/log instead.
- pkg: "github.com/containerd/containerd/platforms"
desc: The containerd platforms package was migrated to a separate module. Use github.com/containerd/platforms instead.
- pkg: "github.com/containerd/nydus-snapshotter/pkg/errdefs"
desc: You probably meant to use github.com/containerd/errdefs
- pkg: "github.com/opencontainers/runc/libcontainer/userns"
desc: Use github.com/moby/sys/user/userns instead.
- pkg: "io/ioutil"
desc: The io/ioutil package has been deprecated.
forbidigo:
Expand Down
6 changes: 3 additions & 3 deletions snapshot/containerd/content.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ import (

"github.com/containerd/containerd/content"
"github.com/containerd/containerd/namespaces"
"github.com/containerd/nydus-snapshotter/pkg/errdefs"
cerrdefs "github.com/containerd/errdefs"
digest "github.com/opencontainers/go-digest"
ocispecs "github.com/opencontainers/image-spec/specs-go/v1"
"github.com/pkg/errors"
Expand Down Expand Up @@ -103,7 +103,7 @@ var _ content.Store = &nsFallbackStore{}
func (c *nsFallbackStore) Info(ctx context.Context, dgst digest.Digest) (content.Info, error) {
info, err := c.main.Info(ctx, dgst)
if err != nil {
if errdefs.IsNotFound(err) {
if cerrdefs.IsNotFound(err) {
return c.fb.Info(ctx, dgst)
}
}
Expand Down Expand Up @@ -137,7 +137,7 @@ func (c *nsFallbackStore) Abort(ctx context.Context, ref string) error {
func (c *nsFallbackStore) ReaderAt(ctx context.Context, desc ocispecs.Descriptor) (content.ReaderAt, error) {
ra, err := c.main.ReaderAt(ctx, desc)
if err != nil {
if errdefs.IsNotFound(err) {
if cerrdefs.IsNotFound(err) {
return c.fb.ReaderAt(ctx, desc)
}
}
Expand Down

This file was deleted.

1 change: 0 additions & 1 deletion vendor/modules.txt
Original file line number Diff line number Diff line change
Expand Up @@ -383,7 +383,6 @@ github.com/containerd/log
## explicit; go 1.19
github.com/containerd/nydus-snapshotter/pkg/converter
github.com/containerd/nydus-snapshotter/pkg/converter/tool
github.com/containerd/nydus-snapshotter/pkg/errdefs
github.com/containerd/nydus-snapshotter/pkg/label
# github.com/containerd/platforms v0.2.1
## explicit; go 1.20
Expand Down

0 comments on commit f3ed463

Please sign in to comment.