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

linters: enable contextcheck #1790

Merged
merged 1 commit into from
Jan 9, 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
6 changes: 5 additions & 1 deletion .golangci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,6 @@ linters:
- godox
- lll
- nestif
- contextcheck
- cyclop
- depguard
- errchkjson
Expand Down Expand Up @@ -84,3 +83,8 @@ issues:
- path: _test\.go
linters:
- dupl
# Exclude "should pass the context parameter" for libimage.LookupImage because of backward compatibility.
- path: "libimage"
text: "LookupImage"
linters:
- contextcheck
2 changes: 1 addition & 1 deletion libimage/corrupted_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ func TestCorruptedLayers(t *testing.T) {
_, err = image.Inspect(ctx, nil)
require.Error(t, err, "inspecting corrupted image should fail")

err = image.isCorrupted(imageName)
err = image.isCorrupted(ctx, imageName)
require.Error(t, err, "image is corrupted")

exists, err = runtime.Exists(imageName)
Expand Down
4 changes: 2 additions & 2 deletions libimage/disk_usage.go
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ func (r *Runtime) DiskUsage(ctx context.Context) ([]ImageDiskUsage, int64, error
return nil, -1, err
}

layerTree, err := r.layerTree(images)
layerTree, err := r.layerTree(ctx, images)
if err != nil {
return nil, -1, err
}
Expand Down Expand Up @@ -79,7 +79,7 @@ func (r *Runtime) DiskUsage(ctx context.Context) ([]ImageDiskUsage, int64, error

// diskUsageForImage returns the disk-usage baseistics for the specified image.
func diskUsageForImage(ctx context.Context, image *Image, tree *layerTree) ([]ImageDiskUsage, error) {
if err := image.isCorrupted(""); err != nil {
if err := image.isCorrupted(ctx, ""); err != nil {
return nil, err
}

Expand Down
8 changes: 4 additions & 4 deletions libimage/filters.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ import (
type filterFunc func(*Image) (bool, error)

// Apply the specified filters. At least one filter of each key must apply.
func (i *Image) applyFilters(filters map[string][]filterFunc) (bool, error) {
func (i *Image) applyFilters(ctx context.Context, filters map[string][]filterFunc) (bool, error) {
matches := false
for key := range filters { // and
matches = false
Expand All @@ -32,7 +32,7 @@ func (i *Image) applyFilters(filters map[string][]filterFunc) (bool, error) {
// Some images may have been corrupted in the
// meantime, so do an extra check and make the
// error non-fatal (see containers/podman/issues/12582).
if errCorrupted := i.isCorrupted(""); errCorrupted != nil {
if errCorrupted := i.isCorrupted(ctx, ""); errCorrupted != nil {
logrus.Errorf(errCorrupted.Error())
return false, nil
}
Expand Down Expand Up @@ -62,7 +62,7 @@ func (r *Runtime) filterImages(ctx context.Context, images []*Image, options *Li
}
result := []*Image{}
for i := range images {
match, err := images[i].applyFilters(filters)
match, err := images[i].applyFilters(ctx, filters)
if err != nil {
return nil, err
}
Expand All @@ -83,7 +83,7 @@ func (r *Runtime) compileImageFilters(ctx context.Context, options *ListImagesOp
var tree *layerTree
getTree := func() (*layerTree, error) {
if tree == nil {
t, err := r.layerTree(nil)
t, err := r.layerTree(ctx, nil)
if err != nil {
return nil, err
}
Expand Down
2 changes: 1 addition & 1 deletion libimage/history.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ func (i *Image) History(ctx context.Context) ([]ImageHistory, error) {
return nil, err
}

layerTree, err := i.runtime.layerTree(nil)
layerTree, err := i.runtime.layerTree(ctx, nil)
if err != nil {
return nil, err
}
Expand Down
10 changes: 5 additions & 5 deletions libimage/image.go
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,7 @@ func (i *Image) reload() error {
}

// isCorrupted returns an error if the image may be corrupted.
func (i *Image) isCorrupted(name string) error {
func (i *Image) isCorrupted(ctx context.Context, name string) error {
// If it's a manifest list, we're good for now.
if _, err := i.getManifestList(); err == nil {
return nil
Expand All @@ -95,7 +95,7 @@ func (i *Image) isCorrupted(name string) error {
return err
}

img, err := ref.NewImage(context.Background(), nil)
img, err := ref.NewImage(ctx, nil)
if err != nil {
if name == "" {
name = i.ID()[:12]
Expand Down Expand Up @@ -257,7 +257,7 @@ func (i *Image) TopLayer() string {

// Parent returns the parent image or nil if there is none
func (i *Image) Parent(ctx context.Context) (*Image, error) {
tree, err := i.runtime.layerTree(nil)
tree, err := i.runtime.layerTree(ctx, nil)
if err != nil {
return nil, err
}
Expand Down Expand Up @@ -291,7 +291,7 @@ func (i *Image) Children(ctx context.Context) ([]*Image, error) {
// created for this invocation only.
func (i *Image) getChildren(ctx context.Context, all bool, tree *layerTree) ([]*Image, error) {
if tree == nil {
t, err := i.runtime.layerTree(nil)
t, err := i.runtime.layerTree(ctx, nil)
if err != nil {
return nil, err
}
Expand Down Expand Up @@ -1030,7 +1030,7 @@ func getImageID(ctx context.Context, src types.ImageReference, sys *types.System
// - 2) a bool indicating whether architecture, os or variant were set (some callers need that to decide whether they need to throw an error)
// - 3) a fatal error that occurred prior to check for matches (e.g., storage errors etc.)
func (i *Image) matchesPlatform(ctx context.Context, os, arch, variant string) (error, bool, error) {
if err := i.isCorrupted(""); err != nil {
if err := i.isCorrupted(ctx, ""); err != nil {
return err, false, nil
}
inspectInfo, err := i.inspectInfo(ctx)
Expand Down
3 changes: 2 additions & 1 deletion libimage/image_tree.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
package libimage

import (
"context"
"fmt"
"strings"

Expand Down Expand Up @@ -37,7 +38,7 @@ func (i *Image) Tree(traverseChildren bool) (string, error) {
fmt.Fprintf(sb, "No Image Layers")
}

layerTree, err := i.runtime.layerTree(nil)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wouldn't it make sense to force users of libimage to pass in a context

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That would be breaking API change, and so far I do not see the need of the context so this would juts make our live harder when vendoring into podman/buildah.

layerTree, err := i.runtime.layerTree(context.Background(), nil)
if err != nil {
return "", err
}
Expand Down
4 changes: 2 additions & 2 deletions libimage/layer_tree.go
Original file line number Diff line number Diff line change
Expand Up @@ -91,14 +91,14 @@ func (l *layerNode) repoTags() ([]string, error) {

// layerTree extracts a layerTree from the layers in the local storage and
// relates them to the specified images.
func (r *Runtime) layerTree(images []*Image) (*layerTree, error) {
func (r *Runtime) layerTree(ctx context.Context, images []*Image) (*layerTree, error) {
layers, err := r.store.Layers()
if err != nil {
return nil, err
}

if images == nil {
images, err = r.ListImages(context.Background(), nil, nil)
images, err = r.ListImages(ctx, nil, nil)
if err != nil {
return nil, err
}
Expand Down
2 changes: 1 addition & 1 deletion libimage/pull.go
Original file line number Diff line number Diff line change
Expand Up @@ -510,7 +510,7 @@ func (r *Runtime) copySingleImageFromRegistry(ctx context.Context, imageName str

// If the local image is corrupted, we need to repull it.
if localImage != nil {
if err := localImage.isCorrupted(imageName); err != nil {
if err := localImage.isCorrupted(ctx, imageName); err != nil {
logrus.Error(err)
localImage = nil
}
Expand Down
4 changes: 2 additions & 2 deletions libimage/runtime.go
Original file line number Diff line number Diff line change
Expand Up @@ -171,7 +171,7 @@ func (r *Runtime) Exists(name string) (bool, error) {
if image == nil {
return false, nil
}
if err := image.isCorrupted(name); err != nil {
if err := image.isCorrupted(context.Background(), name); err != nil {
logrus.Error(err)
return false, nil
}
Expand Down Expand Up @@ -608,7 +608,7 @@ func (r *Runtime) ListImages(ctx context.Context, names []string, options *ListI
// as the layer tree will computed once for all instead of once for
// each individual image (see containers/podman/issues/17828).

tree, err := r.layerTree(images)
tree, err := r.layerTree(ctx, images)
if err != nil {
return nil, err
}
Expand Down