Merge pull request #5180 from dmcgowan/lint-enforce-comments

Fix exported comments enforcer in CI

.golangci.yml

@@ -14,6 +14,10 @@ linters:
   disable:
     - errcheck
 
+issues:
+  include:
+    - EXC0002
+
 run:
   timeout: 3m
   skip-dirs:

client.go

@@ -720,10 +720,12 @@ func (c *Client) Version(ctx context.Context) (Version, error) {
 	}, nil
 }
 
+// ServerInfo represents the introspected server information
 type ServerInfo struct {
 	UUID string
 }
 
+// Server returns server information from the introspection service
 func (c *Client) Server(ctx context.Context) (ServerInfo, error) {
 	c.connMu.Lock()
 	if c.conn == nil {
@@ -789,6 +791,8 @@ func CheckRuntime(current, expected string) bool {
 	return true
 }
 
+// GetSnapshotterSupportedPlatforms returns a platform matchers which represents the
+// supported platforms for the given snapshotters
 func (c *Client) GetSnapshotterSupportedPlatforms(ctx context.Context, snapshotterName string) (platforms.MatchComparer, error) {
 	filters := []string{fmt.Sprintf("type==%s, id==%s", plugin.SnapshotPlugin, snapshotterName)}
 	in := c.IntrospectionService()

cmd/ctr/commands/content/fetch.go

@@ -198,6 +198,8 @@ func Fetch(ctx context.Context, client *containerd.Client, ref string, config *F
 	return img, nil
 }
 
+// ShowProgress continuously updates the output with job progress
+// by checking status in the content store.
 func ShowProgress(ctx context.Context, ongoing *Jobs, cs content.Store, out io.Writer) {
 	var (
 		ticker   = time.NewTicker(100 * time.Millisecond)
@@ -328,6 +330,7 @@ type Jobs struct {
 	resolved bool
 }
 
+// NewJobs creates a new instance of the job status tracker
 func NewJobs(name string) *Jobs {
 	return &Jobs{
 		name:  name,
@@ -335,6 +338,7 @@ func NewJobs(name string) *Jobs {
 	}
 }
 
+// Add adds a descriptor to be tracked
 func (j *Jobs) Add(desc ocispec.Descriptor) {
 	j.mu.Lock()
 	defer j.mu.Unlock()
@@ -347,6 +351,7 @@ func (j *Jobs) Add(desc ocispec.Descriptor) {
 	j.added[desc.Digest] = struct{}{}
 }
 
+// Jobs returns a list of all tracked descriptors
 func (j *Jobs) Jobs() []ocispec.Descriptor {
 	j.mu.Lock()
 	defer j.mu.Unlock()
@@ -355,6 +360,7 @@ func (j *Jobs) Jobs() []ocispec.Descriptor {
 	return append(descs, j.descs...)
 }
 
+// IsResolved checks whether a descriptor has been resolved
 func (j *Jobs) IsResolved() bool {
 	j.mu.Lock()
 	defer j.mu.Unlock()

cmd/ctr/commands/oci/oci.go

@@ -25,6 +25,7 @@ import (
 	"github.com/containerd/containerd/oci"
 )
 
+// Command is the parent for all OCI related tools under 'oci'
 var Command = cli.Command{
 	Name:  "oci",
 	Usage: "OCI tools",

content/adaptor.go

@@ -22,7 +22,7 @@ import (
 	"github.com/containerd/containerd/filters"
 )
 
-// AdoptInfo returns `filters.Adaptor` that handles `content.Info`.
+// AdaptInfo returns `filters.Adaptor` that handles `content.Info`.
 func AdaptInfo(info Info) filters.Adaptor {
 	return filters.AdapterFunc(func(fieldpath []string) (string, bool) {
 		if len(fieldpath) == 0 {

contrib/apparmor/apparmor.go

@@ -81,7 +81,7 @@ func LoadDefaultProfile(name string) error {
 	return nil
 }
 
-// DumpDefaultProfiles dumps the default profile with the given name.
+// DumpDefaultProfile dumps the default profile with the given name.
 func DumpDefaultProfile(name string) (string, error) {
 	p, err := loadData(name)
 	if err != nil {

diff/stream.go

@@ -168,6 +168,10 @@ func (c *compressedProcessor) Close() error {
 	return c.rc.Close()
 }
 
+// BinaryHandler creates a new stream processor handler which calls out to the given binary.
+// The id is used to identify the stream processor and allows the caller to send
+// payloads specific for that stream processor (i.e. decryption keys for decrypt stream processor).
+// The binary will be called for the provided mediaTypes and return the given media type.
 func BinaryHandler(id, returnsMediaType string, mediaTypes []string, path string, args, env []string) Handler {
 	set := make(map[string]struct{}, len(mediaTypes))
 	for _, m := range mediaTypes {

images/converter/default.go

@@ -348,6 +348,7 @@ func clearDockerV1DummyID(cfg DualConfig) (bool, error) {
 	return modified, nil
 }
 
+// ObjectWithMediaType represents an object with a MediaType field
 type ObjectWithMediaType struct {
 	// MediaType appears on Docker manifests and manifest lists.
 	// MediaType does not appear on OCI manifests and index

images/converter/uncompress/uncompress.go

@@ -91,6 +91,8 @@ func LayerConvertFunc(ctx context.Context, cs content.Store, desc ocispec.Descri
 	return &newDesc, nil
 }
 
+// IsUncompressedType returns whether the provided media type is considered
+// an uncompressed layer type
 func IsUncompressedType(mt string) bool {
 	switch mt {
 	case

integration/remote/remote_runtime.go

@@ -551,6 +551,7 @@ func (r *RuntimeService) ContainerStats(containerID string) (*runtimeapi.Contain
 	return resp.GetStats(), nil
 }
 
+// ListContainerStats lists all container stats given the provided filter
 func (r *RuntimeService) ListContainerStats(filter *runtimeapi.ContainerStatsFilter) ([]*runtimeapi.ContainerStats, error) {
 	klog.V(10).Infof("[RuntimeService] ListContainerStats (filter=%v)", filter)
 	// Do not set timeout, because writable layer stats collection takes time.
@@ -570,6 +571,7 @@ func (r *RuntimeService) ListContainerStats(filter *runtimeapi.ContainerStatsFil
 	return resp.GetStats(), nil
 }
 
+// ReopenContainerLog reopens the container log for the given container ID
 func (r *RuntimeService) ReopenContainerLog(containerID string) error {
 	klog.V(10).Infof("[RuntimeService] ReopenContainerLog (containerID=%v, timeout=%v)", containerID, r.timeout)
 	ctx, cancel := getContextWithTimeout(r.timeout)

mount/losetup_linux.go

@@ -186,7 +186,7 @@ func removeLoop(loopdev string) error {
 	return err
 }
 
-// Attach a specified backing file to a loop device
+// AttachLoopDevice attaches a specified backing file to a loop device
 func AttachLoopDevice(backingFile string) (string, error) {
 	file, err := setupLoop(backingFile, LoopParams{})
 	if err != nil {
@@ -196,7 +196,7 @@ func AttachLoopDevice(backingFile string) (string, error) {
 	return file.Name(), nil
 }
 
-// Detach a loop device
+// DetachLoopDevice detaches the provided loop devices
 func DetachLoopDevice(devices ...string) error {
 	for _, dev := range devices {
 		if err := removeLoop(dev); err != nil {

oci/spec_opts.go

@@ -1172,8 +1172,6 @@ func WithLinuxDevices(devices []specs.LinuxDevice) SpecOpts {
 	}
 }
 
-var ErrNotADevice = errors.New("not a device node")
-
 // WithLinuxDevice adds the device specified by path to the spec
 func WithLinuxDevice(path, permissions string) SpecOpts {
 	return func(_ context.Context, _ Client, _ *containers.Container, s *Spec) error {

oci/spec_opts_linux.go

@@ -27,6 +27,7 @@ import (
 	"github.com/containerd/containerd/containers"
 	"github.com/containerd/containerd/pkg/cap"
 	specs "github.com/opencontainers/runtime-spec/specs-go"
+	"github.com/pkg/errors"
 	"golang.org/x/sys/unix"
 )
 
@@ -42,6 +43,8 @@ func WithHostDevices(_ context.Context, _ Client, _ *containers.Container, s *Sp
 	return nil
 }
 
+var errNotADevice = errors.New("not a device node")
+
 func getDevices(path string) ([]specs.LinuxDevice, error) {
 	files, err := ioutil.ReadDir(path)
 	if err != nil {
@@ -70,7 +73,7 @@ func getDevices(path string) ([]specs.LinuxDevice, error) {
 		}
 		device, err := deviceFromPath(filepath.Join(path, f.Name()), "rwm")
 		if err != nil {
-			if err == ErrNotADevice {
+			if err == errNotADevice {
 				continue
 			}
 			if os.IsNotExist(err) {
@@ -96,7 +99,7 @@ func deviceFromPath(path, permissions string) (*specs.LinuxDevice, error) {
 		minor     = unix.Minor(devNumber)
 	)
 	if major == 0 {
-		return nil, ErrNotADevice
+		return nil, errNotADevice
 	}
 
 	var (

oci/spec_opts_unix.go

@@ -26,6 +26,7 @@ import (
 
 	"github.com/containerd/containerd/containers"
 	specs "github.com/opencontainers/runtime-spec/specs-go"
+	"github.com/pkg/errors"
 	"golang.org/x/sys/unix"
 )
 
@@ -41,6 +42,8 @@ func WithHostDevices(_ context.Context, _ Client, _ *containers.Container, s *Sp
 	return nil
 }
 
+var errNotADevice = errors.New("not a device node")
+
 func getDevices(path string) ([]specs.LinuxDevice, error) {
 	files, err := ioutil.ReadDir(path)
 	if err != nil {
@@ -69,7 +72,7 @@ func getDevices(path string) ([]specs.LinuxDevice, error) {
 		}
 		device, err := deviceFromPath(filepath.Join(path, f.Name()), "rwm")
 		if err != nil {
-			if err == ErrNotADevice {
+			if err == errNotADevice {
 				continue
 			}
 			if os.IsNotExist(err) {
@@ -94,7 +97,7 @@ func deviceFromPath(path, permissions string) (*specs.LinuxDevice, error) {
 		minor     = unix.Minor(devNumber)
 	)
 	if major == 0 {
-		return nil, ErrNotADevice
+		return nil, errNotADevice
 	}
 
 	var (

pkg/apparmor/apparmor.go

@@ -1,5 +1,3 @@
-// +build linux
-
 /*
    Copyright The containerd Authors.
 
@@ -18,31 +16,12 @@
 
 package apparmor
 
-import (
+// HostSupports returns true if apparmor is enabled for the host, // On non-Linux returns false
-	"io/ioutil"
+// On Linux returns true if apparmor_parser is enabled, and if we
-	"os"
+//  are not running docker-in-docker.
-	"sync"
-)
-
-var (
-	appArmorSupported bool
-	checkAppArmor     sync.Once
-)
-
-// HostSupports returns true if apparmor is enabled for the host, if
-// apparmor_parser is enabled, and if we are not running docker-in-docker.
 //
-// It is a modified version of libcontainer/apparmor.IsEnabled(), which does not
+//  It is a modified version of libcontainer/apparmor.IsEnabled(), which does not
-// check for apparmor_parser to be present, or if we're running docker-in-docker.
+//  check for apparmor_parser to be present, or if we're running docker-in-docker.
 func HostSupports() bool {
-	checkAppArmor.Do(func() {
+	return hostSupports()
-		// see https://github.com/docker/docker/commit/de191e86321f7d3136ff42ff75826b8107399497
-		if _, err := os.Stat("/sys/kernel/security/apparmor"); err == nil && os.Getenv("container") == "" {
-			if _, err = os.Stat("/sbin/apparmor_parser"); err == nil {
-				buf, err := ioutil.ReadFile("/sys/module/apparmor/parameters/enabled")
-				appArmorSupported = err == nil && len(buf) > 1 && buf[0] == 'Y'
-			}
-		}
-	})
-	return appArmorSupported
 }

pkg/apparmor/apparmor_linux.go

@@ -0,0 +1,48 @@
+// +build linux
+
+/*
+   Copyright The containerd Authors.
+
+   Licensed under the Apache License, Version 2.0 (the "License");
+   you may not use this file except in compliance with the License.
+   You may obtain a copy of the License at
+
+       http://www.apache.org/licenses/LICENSE-2.0
+
+   Unless required by applicable law or agreed to in writing, software
+   distributed under the License is distributed on an "AS IS" BASIS,
+   WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+   See the License for the specific language governing permissions and
+   limitations under the License.
+*/
+
+package apparmor
+
+import (
+	"io/ioutil"
+	"os"
+	"sync"
+)
+
+var (
+	appArmorSupported bool
+	checkAppArmor     sync.Once
+)
+
+// hostSupports returns true if apparmor is enabled for the host, if
+// apparmor_parser is enabled, and if we are not running docker-in-docker.
+//
+// It is a modified version of libcontainer/apparmor.IsEnabled(), which does not
+// check for apparmor_parser to be present, or if we're running docker-in-docker.
+func hostSupports() bool {
+	checkAppArmor.Do(func() {
+		// see https://github.com/docker/docker/commit/de191e86321f7d3136ff42ff75826b8107399497
+		if _, err := os.Stat("/sys/kernel/security/apparmor"); err == nil && os.Getenv("container") == "" {
+			if _, err = os.Stat("/sbin/apparmor_parser"); err == nil {
+				buf, err := ioutil.ReadFile("/sys/module/apparmor/parameters/enabled")
+				appArmorSupported = err == nil && len(buf) > 1 && buf[0] == 'Y'
+			}
+		}
+	})
+	return appArmorSupported
+}

pkg/apparmor/apparmor_unsupported.go

@@ -18,7 +18,6 @@
 
 package apparmor
 
-//nolint: deadcode, unused
+func hostSupports() bool {
-func HostSupports() bool {
 	return false
 }

pkg/cap/cap_linux.go

@@ -65,7 +65,7 @@ type Type int
 const (
 	// Effective is CapEff
 	Effective Type = 1 << iota
-	// Effective is CapPrm
+	// Permitted is CapPrm
 	Permitted
 	// Inheritable is CapInh
 	Inheritable

pkg/cri/store/label/label.go

@@ -22,6 +22,7 @@ import (
 	"github.com/opencontainers/selinux/go-selinux"
 )
 
+// Store is used to store SELinux process labels
 type Store struct {
 	sync.Mutex
 	levels   map[string]int
@@ -29,6 +30,7 @@ type Store struct {
 	Reserver func(string)
 }
 
+// NewStore creates a new SELinux process label store
 func NewStore() *Store {
 	return &Store{
 		levels:   map[string]int{},
@@ -37,6 +39,8 @@ func NewStore() *Store {
 	}
 }
 
+// Reserve reserves the MLS/MCS level component of the specified label
+// and prevents multiple reserves for the same level
 func (s *Store) Reserve(label string) error {
 	s.Lock()
 	defer s.Unlock()
@@ -60,6 +64,9 @@ func (s *Store) Reserve(label string) error {
 	return nil
 }
 
+// Release un-reserves the MLS/MCS level component of the specified label,
+// allowing it to be used by another process once labels with the same
+// level have been released.
 func (s *Store) Release(label string) {
 	s.Lock()
 	defer s.Unlock()

pkg/seccomp/seccomp.go

@@ -0,0 +1,25 @@
+/*
+   Copyright The containerd Authors.
+
+   Licensed under the Apache License, Version 2.0 (the "License");
+   you may not use this file except in compliance with the License.
+   You may obtain a copy of the License at
+
+       http://www.apache.org/licenses/LICENSE-2.0
+
+   Unless required by applicable law or agreed to in writing, software
+   distributed under the License is distributed on an "AS IS" BASIS,
+   WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+   See the License for the specific language governing permissions and
+   limitations under the License.
+*/
+
+package seccomp
+
+// IsEnabled returns whether seccomp support is enabled
+// On Linux returns if the kernel has been configured to support seccomp.
+//  From https://github.com/opencontainers/runc/blob/v1.0.0-rc91/libcontainer/seccomp/seccomp_linux.go#L86-L102
+// On non-Linux returns false
+func IsEnabled() bool {
+	return isEnabled()
+}

pkg/seccomp/seccomp_linux.go

@@ -40,9 +40,9 @@ import (
 	"golang.org/x/sys/unix"
 )
 
-// IsEnabled returns if the kernel has been configured to support seccomp.
+// isEnabled returns if the kernel has been configured to support seccomp.
 // From https://github.com/opencontainers/runc/blob/v1.0.0-rc91/libcontainer/seccomp/seccomp_linux.go#L86-L102
-func IsEnabled() bool {
+func isEnabled() bool {
 	// Try to read from /proc/self/status for kernels > 3.8
 	s, err := parseStatusFile("/proc/self/status")
 	if err != nil {

pkg/seccomp/seccomp_unsupported.go

@@ -18,6 +18,6 @@
 
 package seccomp
 
-func IsEnabled() bool {
+func isEnabled() bool {
 	return false
 }

remotes/docker/config/hosts.go

@@ -14,7 +14,7 @@
    limitations under the License.
 */
 
-// config package containers utilities for helping configure the Docker resolver
+// Package config contains utilities for helping configure the Docker resolver
 package config
 
 import (

remotes/docker/registry.go

@@ -56,6 +56,7 @@ const (
 	// Reserved for future capabilities (i.e. search, catalog, remove)
 )
 
+// Has checks whether the capabilities list has the provide capability
 func (c HostCapabilities) Has(t HostCapabilities) bool {
 	return c&t == t
 }

runtime/v2/shim/publisher.go

@@ -41,6 +41,7 @@ type item struct {
 	count int
 }
 
+// NewPublisher creates a new remote events publisher
 func NewPublisher(address string) (*RemoteEventsPublisher, error) {
 	client, err := ttrpcutil.NewClient(address)
 	if err != nil {
@@ -57,6 +58,7 @@ func NewPublisher(address string) (*RemoteEventsPublisher, error) {
 	return l, nil
 }
 
+// RemoteEventsPublisher forwards events to a ttrpc server
 type RemoteEventsPublisher struct {
 	client  *ttrpcutil.Client
 	closed  chan struct{}
@@ -64,10 +66,12 @@ type RemoteEventsPublisher struct {
 	requeue chan *item
 }
 
+// Done returns a channel which closes when done
 func (l *RemoteEventsPublisher) Done() <-chan struct{} {
 	return l.closed
 }
 
+// Close closes the remote connection and closes the done channel
 func (l *RemoteEventsPublisher) Close() (err error) {
 	err = l.client.Close()
 	l.closer.Do(func() {
@@ -100,6 +104,7 @@ func (l *RemoteEventsPublisher) queue(i *item) {
 	}()
 }
 
+// Publish publishes the event by forwarding it to the configured ttrpc server
 func (l *RemoteEventsPublisher) Publish(ctx context.Context, topic string, event events.Event) error {
 	ns, err := namespaces.NamespaceRequired(ctx)
 	if err != nil {

runtime/v2/shim/util_unix.go

@@ -87,6 +87,7 @@ func AnonDialer(address string, timeout time.Duration) (net.Conn, error) {
 	return dialer.Dialer(socket(address).path(), timeout)
 }
 
+// AnonReconnectDialer returns a dialer for an existing socket on reconnection
 func AnonReconnectDialer(address string, timeout time.Duration) (net.Conn, error) {
 	return AnonDialer(address, timeout)
 }

script/setup/install-dev-tools

@@ -34,4 +34,4 @@ GO111MODULE=off go get -d github.com/gogo/googleapis || true
 GO111MODULE=off go get -d github.com/gogo/protobuf || true
 
 GO111MODULE=on go get github.com/cpuguy83/go-md2man/v2@v2.0.0
-GO111MODULE=on go get github.com/golangci/golangci-lint/cmd/golangci-lint@v1.23.8
+GO111MODULE=on go get github.com/golangci/golangci-lint/cmd/golangci-lint@v1.38.0

services/introspection/introspection.go

@@ -25,6 +25,7 @@ import (
 	ptypes "github.com/gogo/protobuf/types"
 )
 
+// Service defines the instrospection service interface
 type Service interface {
 	Plugins(context.Context, []string) (*api.PluginsResponse, error)
 	Server(context.Context, *ptypes.Empty) (*api.ServerResponse, error)
@@ -36,6 +37,7 @@ type introspectionRemote struct {
 
 var _ = (Service)(&introspectionRemote{})
 
+// NewIntrospectionServiceFromClient creates a new introspection service from an API client
 func NewIntrospectionServiceFromClient(c api.IntrospectionClient) Service {
 	return &introspectionRemote{client: c}
 }

services/introspection/local.go

@@ -54,6 +54,7 @@ func init() {
 	})
 }
 
+// Local is a local implementation of the introspection service
 type Local struct {
 	mu      sync.Mutex
 	plugins []api.Plugin
@@ -62,6 +63,7 @@ type Local struct {
 
 var _ = (api.IntrospectionClient)(&Local{})
 
+// UpdateLocal updates the local introspection service
 func (l *Local) UpdateLocal(root string, plugins []api.Plugin) {
 	l.mu.Lock()
 	defer l.mu.Unlock()
@@ -69,6 +71,7 @@ func (l *Local) UpdateLocal(root string, plugins []api.Plugin) {
 	l.plugins = plugins
 }
 
+// Plugins returns the locally defined plugins
 func (l *Local) Plugins(ctx context.Context, req *api.PluginsRequest, _ ...grpc.CallOption) (*api.PluginsResponse, error) {
 	filter, err := filters.ParseAll(req.Filters...)
 	if err != nil {
@@ -96,6 +99,7 @@ func (l *Local) getPlugins() []api.Plugin {
 	return l.plugins
 }
 
+// Server returns the local server information
 func (l *Local) Server(ctx context.Context, _ *ptypes.Empty, _ ...grpc.CallOption) (*api.ServerResponse, error) {
 	u, err := l.getUUID()
 	if err != nil {

snapshots/devmapper/pool_device.go

@@ -395,6 +395,7 @@ func (p *PoolDevice) SuspendDevice(ctx context.Context, deviceName string) error
 	return nil
 }
 
+// ResumeDevice resumes IO for the given device
 func (p *PoolDevice) ResumeDevice(ctx context.Context, deviceName string) error {
 	if err := p.transition(ctx, deviceName, Resuming, Resumed, func() error {
 		return dmsetup.ResumeDevice(deviceName)

snapshots/devmapper/snapshotter.go

@@ -482,6 +482,7 @@ func (s *Snapshotter) withTransaction(ctx context.Context, writable bool, fn fun
 	return nil
 }
 
+// Cleanup cleans up all removed and unused resources
 func (s *Snapshotter) Cleanup(ctx context.Context) error {
 	var removedDevices []*DeviceInfo
 

snapshots/snapshotter.go

@@ -28,7 +28,8 @@ import (
 const (
 	// UnpackKeyPrefix is the beginning of the key format used for snapshots that will have
 	// image content unpacked into them.
-	UnpackKeyPrefix       = "extract"
+	UnpackKeyPrefix = "extract"
+	// UnpackKeyFormat is the format for the snapshotter keys used for extraction
 	UnpackKeyFormat       = UnpackKeyPrefix + "-%s %s"
 	inheritedLabelsPrefix = "containerd.io/snapshot/"
 	labelSnapshotRef      = "containerd.io/snapshot.ref"