Merge pull request #1592 from mtrmac/lint-1.18

Various lint fixes

cmd/skopeo/copy.go

@@ -110,7 +110,7 @@ func parseMultiArch(multiArch string) (copy.ImageListSelection, error) {
 	}
 }
 
-func (opts *copyOptions) run(args []string, stdout io.Writer) error {
+func (opts *copyOptions) run(args []string, stdout io.Writer) (retErr error) {
 	if len(args) != 2 {
 		return errorShouldDisplayUsage{errors.New("Exactly two arguments expected")}
 	}
@@ -125,7 +125,11 @@ func (opts *copyOptions) run(args []string, stdout io.Writer) error {
 	if err != nil {
 		return fmt.Errorf("Error loading trust policy: %v", err)
 	}
-	defer policyContext.Destroy()
+	defer func() {
+		if err := policyContext.Destroy(); err != nil {
+			retErr = fmt.Errorf("(error tearing down policy context: %v): %w", err, retErr)
+		}
+	}()
 
 	srcRef, err := alltransports.ParseImageName(imageNames[0])
 	if err != nil {

cmd/skopeo/proxy.go

@@ -98,7 +98,7 @@ const maxMsgSize = 32 * 1024
 // https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Number/MAX_SAFE_INTEGER
 // We hard error if the input JSON numbers we expect to be
 // integers are above this.
-const maxJSONFloat = float64(1<<53 - 1)
+const maxJSONFloat = float64(uint64(1)<<53 - 1)
 
 // request is the JSON serialization of a function call
 type request struct {
@@ -664,7 +664,14 @@ func proxyCmd(global *globalOptions) *cobra.Command {
 // processRequest dispatches a remote request.
 // replyBuf is the result of the invocation.
 // terminate should be true if processing of requests should halt.
-func (h *proxyHandler) processRequest(req request) (rb replyBuf, terminate bool, err error) {
+func (h *proxyHandler) processRequest(readBytes []byte) (rb replyBuf, terminate bool, err error) {
+	var req request
+
+	// Parse the request JSON
+	if err = json.Unmarshal(readBytes, &req); err != nil {
+		err = fmt.Errorf("invalid request: %v", err)
+		return
+	}
 	// Dispatch on the method
 	switch req.Method {
 	case "Initialize":
@@ -717,18 +724,15 @@ func (opts *proxyOptions) run(args []string, stdout io.Writer) error {
 			}
 			return fmt.Errorf("reading socket: %v", err)
 		}
-		// Parse the request JSON
 		readbuf := buf[0:n]
-		var req request
-		if err := json.Unmarshal(readbuf, &req); err != nil {
-			rb := replyBuf{}
-			rb.send(conn, fmt.Errorf("invalid request: %v", err))
-		}
 
-		rb, terminate, err := handler.processRequest(req)
+		rb, terminate, err := handler.processRequest(readbuf)
 		if terminate {
 			return nil
 		}
-		rb.send(conn, err)
+
+		if err := rb.send(conn, err); err != nil {
+			return fmt.Errorf("writing to socket: %w", err)
+		}
 	}
 }

cmd/skopeo/sync.go

@@ -500,7 +500,7 @@ func imagesToCopy(source string, transport string, sourceCtx *types.SystemContex
 	return descriptors, nil
 }
 
-func (opts *syncOptions) run(args []string, stdout io.Writer) error {
+func (opts *syncOptions) run(args []string, stdout io.Writer) (retErr error) {
 	if len(args) != 2 {
 		return errorShouldDisplayUsage{errors.New("Exactly two arguments expected")}
 	}
@@ -510,7 +510,11 @@ func (opts *syncOptions) run(args []string, stdout io.Writer) error {
 	if err != nil {
 		return errors.Wrapf(err, "Error loading trust policy")
 	}
-	defer policyContext.Destroy()
+	defer func() {
+		if err := policyContext.Destroy(); err != nil {
+			retErr = fmt.Errorf("(error tearing down policy context: %v): %w", err, retErr)
+		}
+	}()
 
 	// validate source and destination options
 	contains := func(val string, list []string) (_ bool) {

cmd/skopeo/utils.go

@@ -34,7 +34,7 @@ func commandAction(handler func(args []string, stdout io.Writer) error) func(cmd
 	return func(c *cobra.Command, args []string) error {
 		err := handler(args, c.OutOrStdout())
 		if _, ok := err.(errorShouldDisplayUsage); ok {
-			c.Help()
+			return c.Help()
 		}
 		return err
 	}

integration/check_test.go

@@ -36,12 +36,12 @@ func (s *SkopeoSuite) SetUpSuite(c *check.C) {
 
 func (s *SkopeoSuite) TearDownSuite(c *check.C) {
 	if s.regV2 != nil {
-		s.regV2.Close()
+		s.regV2.tearDown(c)
 	}
 	if s.regV2WithAuth != nil {
 		//cmd := exec.Command("docker", "logout", s.regV2WithAuth)
 		//c.Assert(cmd.Run(), check.IsNil)
-		s.regV2WithAuth.Close()
+		s.regV2WithAuth.tearDown(c)
 	}
 }
 

integration/copy_test.go

@@ -86,10 +86,10 @@ func (s *CopySuite) TearDownSuite(c *check.C) {
 		os.RemoveAll(s.gpgHome)
 	}
 	if s.registry != nil {
-		s.registry.Close()
+		s.registry.tearDown(c)
 	}
 	if s.s1Registry != nil {
-		s.s1Registry.Close()
+		s.s1Registry.tearDown(c)
 	}
 	if s.cluster != nil {
 		s.cluster.tearDown(c)

integration/openshift.go

@@ -258,12 +258,16 @@ func (cluster *openshiftCluster) relaxImageSignerPermissions(c *check.C) {
 // tearDown stops the cluster services and deletes (only some!) of the state.
 func (cluster *openshiftCluster) tearDown(c *check.C) {
 	for i := len(cluster.processes) - 1; i >= 0; i-- {
-		cluster.processes[i].Process.Kill()
+		// It’s undocumented what Kill() returns if the process has terminated,
+		// so we couldn’t check just for that. This is running in a container anyway…
+		_ = cluster.processes[i].Process.Kill()
 	}
 	if cluster.workingDir != "" {
-		os.RemoveAll(cluster.workingDir)
+		err := os.RemoveAll(cluster.workingDir)
+		c.Assert(err, check.IsNil)
 	}
 	if cluster.dockerDir != "" {
-		os.RemoveAll(cluster.dockerDir)
+		err := os.RemoveAll(cluster.dockerDir)
+		c.Assert(err, check.IsNil)
 	}
 }

integration/proxy_test.go

@@ -57,7 +57,7 @@ type pipefd struct {
 	fd *os.File
 }
 
-func (self *proxy) call(method string, args []interface{}) (rval interface{}, fd *pipefd, err error) {
+func (p *proxy) call(method string, args []interface{}) (rval interface{}, fd *pipefd, err error) {
 	req := request{
 		Method: method,
 		Args:   args,
@@ -66,7 +66,7 @@ func (self *proxy) call(method string, args []interface{}) (rval interface{}, fd
 	if err != nil {
 		return
 	}
-	n, err := self.c.Write(reqbuf)
+	n, err := p.c.Write(reqbuf)
 	if err != nil {
 		return
 	}
@@ -76,7 +76,7 @@ func (self *proxy) call(method string, args []interface{}) (rval interface{}, fd
 	}
 	oob := make([]byte, syscall.CmsgSpace(1))
 	replybuf := make([]byte, maxMsgSize)
-	n, oobn, _, _, err := self.c.ReadMsgUnix(replybuf, oob)
+	n, oobn, _, _, err := p.c.ReadMsgUnix(replybuf, oob)
 	if err != nil {
 		err = fmt.Errorf("reading reply: %v", err)
 		return
@@ -119,9 +119,9 @@ func (self *proxy) call(method string, args []interface{}) (rval interface{}, fd
 	return
 }
 
-func (self *proxy) callNoFd(method string, args []interface{}) (rval interface{}, err error) {
+func (p *proxy) callNoFd(method string, args []interface{}) (rval interface{}, err error) {
 	var fd *pipefd
-	rval, fd, err = self.call(method, args)
+	rval, fd, err = p.call(method, args)
 	if err != nil {
 		return
 	}
@@ -132,9 +132,9 @@ func (self *proxy) callNoFd(method string, args []interface{}) (rval interface{}
 	return rval, nil
 }
 
-func (self *proxy) callReadAllBytes(method string, args []interface{}) (rval interface{}, buf []byte, err error) {
+func (p *proxy) callReadAllBytes(method string, args []interface{}) (rval interface{}, buf []byte, err error) {
 	var fd *pipefd
-	rval, fd, err = self.call(method, args)
+	rval, fd, err = p.call(method, args)
 	if err != nil {
 		return
 	}
@@ -150,7 +150,7 @@ func (self *proxy) callReadAllBytes(method string, args []interface{}) (rval int
 			err:     err,
 		}
 	}()
-	_, err = self.callNoFd("FinishPipe", []interface{}{fd.id})
+	_, err = p.callNoFd("FinishPipe", []interface{}{fd.id})
 	if err != nil {
 		return
 	}
@@ -241,7 +241,7 @@ func runTestGetManifestAndConfig(p *proxy, img string) error {
 	}
 	imgid := uint32(imgidv)
 
-	v, manifestBytes, err := p.callReadAllBytes("GetManifest", []interface{}{imgid})
+	_, manifestBytes, err := p.callReadAllBytes("GetManifest", []interface{}{imgid})
 	if err != nil {
 		return err
 	}
@@ -250,7 +250,7 @@ func runTestGetManifestAndConfig(p *proxy, img string) error {
 		return err
 	}
 
-	v, configBytes, err := p.callReadAllBytes("GetFullConfig", []interface{}{imgid})
+	_, configBytes, err := p.callReadAllBytes("GetFullConfig", []interface{}{imgid})
 	if err != nil {
 		return err
 	}
@@ -269,7 +269,7 @@ func runTestGetManifestAndConfig(p *proxy, img string) error {
 	}
 
 	// Also test this legacy interface
-	v, ctrconfigBytes, err := p.callReadAllBytes("GetConfig", []interface{}{imgid})
+	_, ctrconfigBytes, err := p.callReadAllBytes("GetConfig", []interface{}{imgid})
 	if err != nil {
 		return err
 	}
@@ -285,6 +285,9 @@ func runTestGetManifestAndConfig(p *proxy, img string) error {
 	}
 
 	_, err = p.callNoFd("CloseImage", []interface{}{imgid})
+	if err != nil {
+		return err
+	}
 
 	return nil
 }

integration/registry.go

@@ -126,7 +126,10 @@ func (t *testRegistryV2) Ping() error {
 	return nil
 }
 
-func (t *testRegistryV2) Close() {
+func (t *testRegistryV2) tearDown(c *check.C) {
-	t.cmd.Process.Kill()
+	// It’s undocumented what Kill() returns if the process has terminated,
-	os.RemoveAll(t.dir)
+	// so we couldn’t check just for that. This is running in a container anyway…
+	_ = t.cmd.Process.Kill()
+	err := os.RemoveAll(t.dir)
+	c.Assert(err, check.IsNil)
 }

integration/sync_test.go

@@ -100,7 +100,7 @@ func (s *SyncSuite) TearDownSuite(c *check.C) {
 		os.RemoveAll(s.gpgHome)
 	}
 	if s.registry != nil {
-		s.registry.Close()
+		s.registry.tearDown(c)
 	}
 	if s.cluster != nil {
 		s.cluster.tearDown(c)
@@ -278,7 +278,8 @@ func (s *SyncSuite) TestYamlUntagged(c *check.C) {
 
 	// sync to the local registry
 	yamlFile := path.Join(tmpDir, "registries.yaml")
-	ioutil.WriteFile(yamlFile, []byte(yamlConfig), 0644)
+	err = ioutil.WriteFile(yamlFile, []byte(yamlConfig), 0644)
+	c.Assert(err, check.IsNil)
 	assertSkopeoSucceeds(c, "", "sync", "--scoped", "--src", "yaml", "--dest", "docker", "--dest-tls-verify=false", yamlFile, v2DockerRegistryURL)
 	// sync back from local registry to a folder
 	os.Remove(yamlFile)
@@ -289,7 +290,8 @@ func (s *SyncSuite) TestYamlUntagged(c *check.C) {
     %s: []
 `, v2DockerRegistryURL, imagePath)
 
-	ioutil.WriteFile(yamlFile, []byte(yamlConfig), 0644)
+	err = ioutil.WriteFile(yamlFile, []byte(yamlConfig), 0644)
+	c.Assert(err, check.IsNil)
 	assertSkopeoSucceeds(c, "", "sync", "--scoped", "--src", "yaml", "--dest", "dir", yamlFile, dir1)
 
 	sysCtx = types.SystemContext{
@@ -334,7 +336,8 @@ k8s.gcr.io:
 	c.Assert(nTags, check.Not(check.Equals), 0)
 
 	yamlFile := path.Join(tmpDir, "registries.yaml")
-	ioutil.WriteFile(yamlFile, []byte(yamlConfig), 0644)
+	err = ioutil.WriteFile(yamlFile, []byte(yamlConfig), 0644)
+	c.Assert(err, check.IsNil)
 	assertSkopeoSucceeds(c, "", "sync", "--scoped", "--src", "yaml", "--dest", "dir", yamlFile, dir1)
 
 	nManifests := 0
@@ -365,7 +368,8 @@ k8s.gcr.io:
     - sha256:59eec8837a4d942cc19a52b8c09ea75121acc38114a2c68b98983ce9356b8610
 `
 	yamlFile := path.Join(tmpDir, "registries.yaml")
-	ioutil.WriteFile(yamlFile, []byte(yamlConfig), 0644)
+	err = ioutil.WriteFile(yamlFile, []byte(yamlConfig), 0644)
+	c.Assert(err, check.IsNil)
 	assertSkopeoSucceeds(c, "", "sync", "--scoped", "--src", "yaml", "--dest", "dir", yamlFile, dir1)
 
 	nManifests := 0
@@ -417,7 +421,8 @@ quay.io:
 	c.Assert(nTags, check.Not(check.Equals), 0)
 
 	yamlFile := path.Join(tmpDir, "registries.yaml")
-	ioutil.WriteFile(yamlFile, []byte(yamlConfig), 0644)
+	err = ioutil.WriteFile(yamlFile, []byte(yamlConfig), 0644)
+	c.Assert(err, check.IsNil)
 	assertSkopeoSucceeds(c, "", "sync", "--scoped", "--src", "yaml", "--dest", "dir", yamlFile, dir1)
 
 	nManifests := 0
@@ -481,7 +486,8 @@ func (s *SyncSuite) TestYamlTLSVerify(c *check.C) {
 	for _, cfg := range testCfg {
 		yamlConfig := fmt.Sprintf(yamlTemplate, v2DockerRegistryURL, cfg.tlsVerify, image, tag)
 		yamlFile := path.Join(tmpDir, "registries.yaml")
-		ioutil.WriteFile(yamlFile, []byte(yamlConfig), 0644)
+		err = ioutil.WriteFile(yamlFile, []byte(yamlConfig), 0644)
+		c.Assert(err, check.IsNil)
 
 		cfg.checker(c, cfg.msg, "sync", "--scoped", "--src", "yaml", "--dest", "dir", yamlFile, dir1)
 		os.Remove(yamlFile)