chore: upgrade golangci-lint to v1.44.2

Bump golangci-lint from v1.21.0 to v1.44.2

Removes deprecated linters and adds recommended replacement linters
'revive' and 'exportloopref'.

Marks G306 and G307 as excluded until those errors can be audited
seperately.

Removes an instance of unused variable flagged by 'unused' linter.

Updates few instances of local variable names to be inline with 'revive'
naming standards.

Updates few instances of exported function names to be inline with
'revive' naming standards.

Fixes a couple of instances where 'append' was being used to initialize slice
without using variadic operator to pass slice.

Signed-off-by: Austin Vazquez <[email protected]>

Author: austinvazquez

.golangci.yml

@@ -13,18 +13,16 @@ linters:
     - ineffassign
     - deadcode
     - typecheck
-    - golint
     - gosec
     - unconvert
     - goconst
     - goimports
     - misspell
     - prealloc
-    - scopelint
     - gocritic
-    - interfacer # Suggests narrower interface types
-    - scopelint # Checks for unpinned variables
     - gofmt
+    - revive
+    - exportloopref
 
 issues:
   exclude-use-default: false
@@ -33,3 +31,5 @@ issues:
     - G103 # Use of unsafe calls should be audited
     - G204 # Subprocess launched with variable
     - G304 # Potential file inclusion via variable
+    - G306 # WriteFile permissions 0600 or less to be audited
+    - G307 # Deferring unsafe method "Close" on type "*os.File" to be audited

Makefile

@@ -112,7 +112,7 @@ tidy:
 	go mod tidy
 
 deps:
-	curl -sfL https://raw.githubusercontent.com/golangci/golangci-lint/master/install.sh| sh -s -- -b $(BINPATH) v1.21.0
+	curl -sfL https://raw.githubusercontent.com/golangci/golangci-lint/master/install.sh| sh -s -- -b $(BINPATH) v1.44.2
 	$(BINPATH)/golangci-lint --version
 	GOBIN=$(BINPATH) GO111MODULE=off go get -u github.com/vbatts/git-validation
 	GOBIN=$(BINPATH) GO111MODULE=off go get -u github.com/kunalkushwaha/ltag

agent/service.go

@@ -189,11 +189,11 @@ func (ts *TaskService) Create(requestCtx context.Context, req *taskAPI.CreateTas
 		return nil
 	})
 
-	isVmLocalRootFs := len(req.Rootfs) > 0 && vm.IsLocalMount(req.Rootfs[0])
+	isVMLocalRootFs := len(req.Rootfs) > 0 && vm.IsLocalMount(req.Rootfs[0])
 
 	// If the rootfs is inside the VM, then the DriveMount call didn't happen and therefore
 	// the bundledir was not created. Create it here.
-	if isVmLocalRootFs {
+	if isVMLocalRootFs {
 		if err := os.MkdirAll(bundleDir.RootfsPath(), 0700); err != nil {
 			return nil, errors.Wrapf(err, "Failed to create bundle's rootfs path from inside the vm %q", bundleDir.RootfsPath())
 		}
@@ -219,7 +219,7 @@ func (ts *TaskService) Create(requestCtx context.Context, req *taskAPI.CreateTas
 	// If the rootfs is inside the VM then:
 	// a) the rootfs mount type has a prefix that we used to identify this which needs to be stripped before passing to runc
 	// b) we were not able to inspect the container's rootfs from the client when setting up the spec. Do that here.
-	if isVmLocalRootFs {
+	if isVMLocalRootFs {
 		req.Rootfs[0] = vm.StripLocalMountIdentifier(req.Rootfs[0])
 		rootfsMount := mount.Mount{
 			Type:    req.Rootfs[0].Type,

internal/common_test.go

@@ -70,7 +70,8 @@ func TestGenerateStubContent(t *testing.T) {
 	stubContent, err := GenerateStubContent(driveID)
 	assert.NoError(t, err)
 
-	expected := append(MagicStubBytes, byte(len(driveID)))
+	expected := append([]byte{}, MagicStubBytes...)
+	expected = append(expected, byte(len(driveID)))
 	expected = append(expected, []byte(driveID)...)
 	assert.Equal(t, string(expected), stubContent)
 }
@@ -83,7 +84,8 @@ func TestGenerateStubContent_LongID(t *testing.T) {
 
 func TestParseStubContent(t *testing.T) {
 	expectedDriveID := "foo"
-	contents := append(MagicStubBytes, byte(len(expectedDriveID)))
+	contents := append([]byte{}, MagicStubBytes...)
+	contents = append(contents, byte(len(expectedDriveID)))
 	contents = append(contents, []byte(expectedDriveID)...)
 	contents = append(contents, []byte("junkcontent")...)
 

internal/vm/oci.go

@@ -42,7 +42,7 @@ import (
 )
 
 // UpdateUserInSpec modifies a serialized json spec object with user information from inside the container.
-// If the client used firecrackeroci.WithVmLocalImageConfig or firecrakceroci.WithVmLocalUser, this
+// If the client used firecrackeroci.WithVMLocalImageConfig or firecrackeroci.WithVMLocalUser, this
 // method will do the mapping between username -> uid, group name -> gid, and lookup additional groups for the user.
 // This is used to split where the user configures a spec via containerd's oci methods in the client
 // from where the data is actually present (from the agent inside the VM)

runtime/firecrackeroci/vm.go

@@ -47,11 +47,11 @@ var (
 	}
 )
 
-// WithVmLocalImageConfig configures a spec with the content of the image's config.
+// WithVMLocalImageConfig configures a spec with the content of the image's config.
 // It is similar to containerd's oci.WithImageConfig except that it does not access
 // the image's rootfs on the host. Instead, it configures what it can on the host and
 // passes information to the agent running in the VM to inspect the image's rootfs.
-func WithVmLocalImageConfig(image containerd.Image) oci.SpecOpts {
+func WithVMLocalImageConfig(image containerd.Image) oci.SpecOpts {
 	return func(ctx context.Context, client oci.Client, container *containers.Container, spec *oci.Spec) error {
 		ic, err := image.Config(ctx)
 		if err != nil {
@@ -85,25 +85,26 @@ func WithVmLocalImageConfig(image containerd.Image) oci.SpecOpts {
 		}
 		spec.Process.Env = replaceOrAppendEnvValues(defaults, spec.Process.Env)
 		cmd := config.Cmd
-		spec.Process.Args = append(config.Entrypoint, cmd...)
+		spec.Process.Args = append([]string{}, config.Entrypoint...)
+		spec.Process.Args = append(spec.Process.Args, cmd...)
 
 		cwd := config.WorkingDir
 		if cwd == "" {
 			cwd = "/"
 		}
 		spec.Process.Cwd = cwd
 		if config.User != "" {
-			WithVmLocalUser(config.User)(ctx, client, container, spec)
+			WithVMLocalUser(config.User)(ctx, client, container, spec)
 		}
 		return nil
 	}
 }
 
-// WithVmLocalUser configures the user of the spec.
+// WithVMLocalUser configures the user of the spec.
 // It is similar to oci.WithUser except that it doesn't map
 // username -> uid or group name -> gid. It passes the user to the
 // agent running inside the VM to do that mapping.
-func WithVmLocalUser(user string) oci.SpecOpts {
+func WithVMLocalUser(user string) oci.SpecOpts {
 	return func(ctx context.Context, client oci.Client, container *containers.Container, spec *oci.Spec) error {
 		// This is technically an LCOW specific field, but we piggy back
 		// to get the string user into the VM where will will do the uid/gid mapping

runtime/jailer_integ_test.go

@@ -87,7 +87,7 @@ func TestAttachBlockDevice_Isolated(t *testing.T) {
 }
 
 func fsSafeTestName(tb testing.TB) string {
-	return strings.Replace(tb.Name(), "/", "-", -1)
+	return strings.ReplaceAll(tb.Name(), "/", "-")
 }
 
 func testJailer(t *testing.T, jailerConfig *proto.JailerConfig) {

runtime/service.go

@@ -81,8 +81,6 @@ const (
 	defaultShutdownTimeout     = 5 * time.Second
 	defaultVSockConnectTimeout = 5 * time.Second
 
-	jailerStopTimeout = 3 * time.Second
-
 	// StartEventName is the topic published to when a VM starts
 	StartEventName = "/firecracker-vm/start"
 
@@ -899,7 +897,7 @@ func (s *service) GetBalloonStats(requestCtx context.Context, req *proto.GetBall
 	return resp, nil
 }
 
-//UpdateBalloonStats will update an existing balloon device statistics interval, before or after machine startup.
+// UpdateBalloonStats will update an existing balloon device statistics interval, before or after machine startup.
 func (s *service) UpdateBalloonStats(requestCtx context.Context, req *proto.UpdateBalloonStatsRequest) (*types.Empty, error) {
 	defer logPanicAndDie(s.logger)
 
@@ -1175,11 +1173,11 @@ func (s *service) Create(requestCtx context.Context, request *taskAPI.CreateTask
 	}
 	rootfsMnt := request.Rootfs[0]
 
-	isVmLocalRootfs := vm.IsLocalMount(rootfsMnt)
+	isVMLocalRootfs := vm.IsLocalMount(rootfsMnt)
 
 	// Only mount the container's rootfs as a block device if the mount doesn't
 	// signal that it is only accessible from inside the VM.
-	if !isVmLocalRootfs {
+	if !isVMLocalRootfs {
 		err = s.containerStubHandler.Reserve(requestCtx, request.ID,
 			rootfsMnt.Source, vmBundleDir.RootfsPath(), "ext4", nil, s.driveMountClient, s.machine)
 		if err != nil {
@@ -1216,7 +1214,7 @@ func (s *service) Create(requestCtx context.Context, request *taskAPI.CreateTask
 	// override the request with the bundle dir that should be used inside the VM
 	request.Bundle = vmBundleDir.RootPath()
 
-	if !isVmLocalRootfs {
+	if !isVMLocalRootfs {
 		// If the rootfs is not inside the VM, it is mounted via a MountDrive call,
 		// so unset Rootfs in the request.
 		// We unfortunately can't rely on just having the runc shim inside the VM do