Merge cockroachdb#56048

56048: *: use `oserror.Is` instead of `os.Is` predicates r=jbowens a=knz

(Meant to also include cockroachdb/errors#54 after it merges.) 

Go 1.13 has standardized the idea of error wrappers, unfortunately
the 4 special predicates in the `os` package have not been updated
accordingly: `os.IsPermission`, `os.IsExist`, `os.IsNotExist` and
`os.IsTimeout` do not properly recognize wrapped errors.

Additionally, the CockroachDB `errors` library did not properly
preserve these special errors over the network across encode/decode
cycles.

This patch corrects the problem by updating the `errors` dependency
to include the new encode/decode logic for `os` errors, as well
as a new `oserror` package with replacement predicates.

The patch also adds a linter to prevent future uses of the `os`
predicates and suggest using `oserror` instead.

Release note: None

Co-authored-by: Raphael 'kena' Poss <[email protected]>

DEPS.bzl

@@ -412,8 +412,8 @@ def go_deps():
         name = "com_github_cockroachdb_errors",
         build_file_proto_mode = "disable_global",
         importpath = "github.com/cockroachdb/errors",
-        sum = "h1:ptyO1BLW+sBxwBTSKJfS6kGzYCVKhI7MyBhoXAnPIKM=",
-        version = "v1.7.5",
+        sum = "h1:A5+txlVZfOqFBDa4mGz2bUWSp0aHElvHX2bKkdbQu+Y=",
+        version = "v1.8.1",
     )
     go_repository(
         name = "com_github_cockroachdb_go_test_teamcity",

go.mod

@@ -36,7 +36,7 @@ require (
 	github.com/cockroachdb/cockroach-go v0.0.0-20200504194139-73ffeee90b62
 	github.com/cockroachdb/crlfmt v0.0.0-20200116191136-a78e1c207bc0
 	github.com/cockroachdb/datadriven v1.0.1-0.20201022032720-3e27adf87325
-	github.com/cockroachdb/errors v1.7.5
+	github.com/cockroachdb/errors v1.8.1
 	github.com/cockroachdb/go-test-teamcity v0.0.0-20191211140407-cff980ad0a55
 	github.com/cockroachdb/gostdlib v1.13.0
 	github.com/cockroachdb/logtags v0.0.0-20190617123548-eb05cc24525f

pkg/acceptance/cluster/BUILD.bazel

@@ -24,6 +24,7 @@ go_library(
         "//pkg/util/timeutil",
         "//pkg/util/uuid",
         "//vendor/github.com/cockroachdb/errors",
+        "//vendor/github.com/cockroachdb/errors/oserror",
         "//vendor/github.com/docker/distribution/reference",
         "//vendor/github.com/docker/docker/api/types",
         "//vendor/github.com/docker/docker/api/types/container",

pkg/acceptance/cluster/docker.go

@@ -28,6 +28,7 @@ import (
 	"github.com/cockroachdb/cockroach/pkg/util/contextutil"
 	"github.com/cockroachdb/cockroach/pkg/util/log"
 	"github.com/cockroachdb/errors"
+	"github.com/cockroachdb/errors/oserror"
 	"github.com/docker/distribution/reference"
 	"github.com/docker/docker/api/types"
 	"github.com/docker/docker/api/types/container"
@@ -218,7 +219,7 @@ func createContainer(
 	// container.
 	for _, bind := range hostConfig.Binds {
 		hostPath, _ := splitBindSpec(bind)
-		if _, err := os.Stat(hostPath); os.IsNotExist(err) {
+		if _, err := os.Stat(hostPath); oserror.IsNotExist(err) {
 			maybePanic(os.MkdirAll(hostPath, 0755))
 		} else {
 			maybePanic(err)

pkg/acceptance/cluster/dockercluster.go

@@ -39,6 +39,7 @@ import (
 	"github.com/cockroachdb/cockroach/pkg/util/timeutil"
 	"github.com/cockroachdb/cockroach/pkg/util/uuid"
 	"github.com/cockroachdb/errors"
+	"github.com/cockroachdb/errors/oserror"
 	"github.com/docker/docker/api/types"
 	"github.com/docker/docker/api/types/container"
 	"github.com/docker/docker/api/types/events"
@@ -78,7 +79,7 @@ var CockroachBinary = flag.String("b", func() string {
 }(), "the host-side binary to run")
 
 func exists(path string) bool {
-	if _, err := os.Stat(path); os.IsNotExist(err) {
+	if _, err := os.Stat(path); oserror.IsNotExist(err) {
 		return false
 	}
 	return true

pkg/acceptance/localcluster/BUILD.bazel

@@ -27,6 +27,7 @@ go_library(
         "//pkg/util/timeutil",
         "//pkg/util/tracing",
         "//vendor/github.com/cockroachdb/errors",
+        "//vendor/github.com/cockroachdb/errors/oserror",
         "//vendor/github.com/gogo/protobuf/proto",
         "//vendor/github.com/lib/pq",
     ],

pkg/acceptance/localcluster/cluster.go

@@ -47,6 +47,7 @@ import (
 	"github.com/cockroachdb/cockroach/pkg/util/timeutil"
 	"github.com/cockroachdb/cockroach/pkg/util/tracing"
 	"github.com/cockroachdb/errors"
+	"github.com/cockroachdb/errors/oserror"
 	"github.com/gogo/protobuf/proto"
 	// Import postgres driver.
 	_ "github.com/lib/pq"
@@ -667,7 +668,7 @@ func (n *Node) httpAddrFile() string {
 func readFileOrEmpty(f string) string {
 	c, err := ioutil.ReadFile(f)
 	if err != nil {
-		if !os.IsNotExist(err) {
+		if !oserror.IsNotExist(err) {
 			panic(err)
 		}
 		return ""

pkg/base/BUILD.bazel

@@ -34,6 +34,7 @@ go_library(
         "//pkg/util/syncutil",
         "//pkg/util/uuid",
         "//vendor/github.com/cockroachdb/errors",
+        "//vendor/github.com/cockroachdb/errors/oserror",
         "//vendor/github.com/cockroachdb/pebble",
         "//vendor/github.com/cockroachdb/redact",
         "//vendor/github.com/dustin/go-humanize",

pkg/base/store_spec.go

@@ -15,7 +15,6 @@ import (
 	"fmt"
 	"io/ioutil"
 	"net"
-	"os"
 	"path/filepath"
 	"regexp"
 	"sort"
@@ -28,6 +27,7 @@ import (
 	"github.com/cockroachdb/cockroach/pkg/util/humanizeutil"
 	"github.com/cockroachdb/cockroach/pkg/util/netutil"
 	"github.com/cockroachdb/errors"
+	"github.com/cockroachdb/errors/oserror"
 	"github.com/cockroachdb/pebble"
 	humanize "github.com/dustin/go-humanize"
 	"github.com/spf13/pflag"
@@ -441,7 +441,7 @@ func (ssl StoreSpecList) PriorCriticalAlertError() (err error) {
 		}
 		b, err := ioutil.ReadFile(path)
 		if err != nil {
-			if !os.IsNotExist(err) {
+			if !oserror.IsNotExist(err) {
 				addError(errors.Wrapf(err, "%s", path))
 			}
 			continue

pkg/blobs/BUILD.bazel

@@ -18,6 +18,7 @@ go_library(
         "//pkg/rpc/nodedialer",
         "//pkg/util/fileutil",
         "//vendor/github.com/cockroachdb/errors",
+        "//vendor/github.com/cockroachdb/errors/oserror",
         "//vendor/google.golang.org/grpc/codes",
         "//vendor/google.golang.org/grpc/metadata",
         "//vendor/google.golang.org/grpc/status",
@@ -45,6 +46,7 @@ go_test(
         "//pkg/util/netutil",
         "//pkg/util/stop",
         "//vendor/github.com/cockroachdb/errors",
+        "//vendor/github.com/cockroachdb/errors/oserror",
         "//vendor/github.com/stretchr/testify/assert",
     ],
 )

pkg/blobs/service.go

@@ -27,10 +27,10 @@ package blobs
 
 import (
 	"context"
-	"os"
 
 	"github.com/cockroachdb/cockroach/pkg/blobs/blobspb"
 	"github.com/cockroachdb/errors"
+	"github.com/cockroachdb/errors/oserror"
 	"google.golang.org/grpc/codes"
 	"google.golang.org/grpc/metadata"
 	"google.golang.org/grpc/status"
@@ -93,7 +93,7 @@ func (s *Service) Delete(
 // Stat implements the gRPC service.
 func (s *Service) Stat(ctx context.Context, req *blobspb.StatRequest) (*blobspb.BlobStat, error) {
 	resp, err := s.localStorage.Stat(req.Filename)
-	if os.IsNotExist(err) {
+	if oserror.IsNotExist(err) {
 		// gRPC hides the underlying golang ErrNotExist error, so we send back an
 		// equivalent gRPC error which can be handled gracefully on the client side.
 		return nil, status.Error(codes.NotFound, err.Error())

pkg/blobs/service_test.go

@@ -18,6 +18,7 @@ import (
 
 	"github.com/cockroachdb/cockroach/pkg/blobs/blobspb"
 	"github.com/cockroachdb/cockroach/pkg/testutils"
+	"github.com/cockroachdb/errors/oserror"
 )
 
 func TestBlobServiceList(t *testing.T) {
@@ -87,7 +88,7 @@ func TestBlobServiceDelete(t *testing.T) {
 		if err != nil {
 			t.Fatal(err)
 		}
-		if _, err := os.Stat(filepath.Join(tmpDir, filename)); !os.IsNotExist(err) {
+		if _, err := os.Stat(filepath.Join(tmpDir, filename)); !oserror.IsNotExist(err) {
 			t.Fatalf("expected not exists err, got: %s", err)
 		}
 	})

pkg/ccl/backupccl/BUILD.bazel

@@ -186,6 +186,7 @@ go_test(
         "//vendor/github.com/cockroachdb/cockroach-go/crdb",
         "//vendor/github.com/cockroachdb/datadriven",
         "//vendor/github.com/cockroachdb/errors",
+        "//vendor/github.com/cockroachdb/errors/oserror",
         "//vendor/github.com/gogo/protobuf/proto",
         "//vendor/github.com/gogo/protobuf/types",
         "//vendor/github.com/gorhill/cronexpr",

pkg/ccl/backupccl/backup_test.go

@@ -73,6 +73,7 @@ import (
 	"github.com/cockroachdb/cockroach/pkg/util/timeutil"
 	"github.com/cockroachdb/datadriven"
 	"github.com/cockroachdb/errors"
+	"github.com/cockroachdb/errors/oserror"
 	"github.com/gogo/protobuf/proto"
 	"github.com/jackc/pgx"
 	"github.com/kr/pretty"
@@ -1405,7 +1406,7 @@ func TestBackupRestoreCheckpointing(t *testing.T) {
 
 	if _, err := os.Stat(checkpointPath); err == nil {
 		t.Fatalf("backup checkpoint descriptor at %s not cleaned up", checkpointPath)
-	} else if !os.IsNotExist(err) {
+	} else if !oserror.IsNotExist(err) {
 		t.Fatal(err)
 	}
 }

pkg/ccl/cliccl/BUILD.bazel

@@ -39,6 +39,7 @@ go_library(
         "//pkg/util/uuid",
         "//vendor/github.com/cockroachdb/cmux",
         "//vendor/github.com/cockroachdb/errors",
+        "//vendor/github.com/cockroachdb/errors/oserror",
         "//vendor/github.com/spf13/cobra",
         "//vendor/golang.org/x/sync/errgroup",
     ],

pkg/ccl/cliccl/debug.go

@@ -28,6 +28,7 @@ import (
 	"github.com/cockroachdb/cockroach/pkg/util/stop"
 	"github.com/cockroachdb/cockroach/pkg/util/timeutil"
 	"github.com/cockroachdb/errors"
+	"github.com/cockroachdb/errors/oserror"
 	"github.com/spf13/cobra"
 )
 
@@ -299,7 +300,7 @@ func getActiveEncryptionkey(dir string) (string, string, error) {
 	// Open the file registry. Return plaintext if it does not exist.
 	contents, err := ioutil.ReadFile(registryFile)
 	if err != nil {
-		if os.IsNotExist(err) {
+		if oserror.IsNotExist(err) {
 			return enginepbccl.EncryptionType_Plaintext.String(), "", nil
 		}
 		return "", "", errors.Wrapf(err, "could not open registry file %s", registryFile)

pkg/ccl/storageccl/engineccl/BUILD.bazel

@@ -21,6 +21,7 @@ go_library(
         "//pkg/util/protoutil",
         "//pkg/util/syncutil",
         "//vendor/github.com/cockroachdb/errors",
+        "//vendor/github.com/cockroachdb/errors/oserror",
         "//vendor/github.com/cockroachdb/pebble/vfs",
         "//vendor/github.com/gogo/protobuf/proto",
     ],
@@ -55,6 +56,7 @@ go_test(
         "//pkg/util/randutil",
         "//pkg/util/timeutil",
         "//vendor/github.com/cockroachdb/datadriven",
+        "//vendor/github.com/cockroachdb/errors/oserror",
         "//vendor/github.com/cockroachdb/pebble",
         "//vendor/github.com/cockroachdb/pebble/vfs",
         "//vendor/github.com/gogo/protobuf/proto",

pkg/ccl/storageccl/engineccl/bench_test.go

@@ -25,6 +25,7 @@ import (
 	"github.com/cockroachdb/cockroach/pkg/util/hlc"
 	"github.com/cockroachdb/cockroach/pkg/util/log"
 	"github.com/cockroachdb/cockroach/pkg/util/randutil"
+	"github.com/cockroachdb/errors/oserror"
 )
 
 // loadTestData writes numKeys keys in numBatches separate batches. Keys are
@@ -47,7 +48,7 @@ func loadTestData(
 	ctx := context.Background()
 
 	exists := true
-	if _, err := os.Stat(dir); os.IsNotExist(err) {
+	if _, err := os.Stat(dir); oserror.IsNotExist(err) {
 		exists = false
 	}
 

pkg/ccl/storageccl/engineccl/pebble_key_manager.go

@@ -14,14 +14,14 @@ import (
 	"encoding/hex"
 	"fmt"
 	"io/ioutil"
-	"os"
 	"time"
 
 	"github.com/cockroachdb/cockroach/pkg/ccl/storageccl/engineccl/enginepbccl"
 	"github.com/cockroachdb/cockroach/pkg/storage"
 	"github.com/cockroachdb/cockroach/pkg/util/log"
 	"github.com/cockroachdb/cockroach/pkg/util/protoutil"
 	"github.com/cockroachdb/cockroach/pkg/util/syncutil"
+	"github.com/cockroachdb/errors/oserror"
 	"github.com/cockroachdb/pebble/vfs"
 	"github.com/gogo/protobuf/proto"
 )
@@ -178,7 +178,7 @@ func (m *DataKeyManager) Load(ctx context.Context) error {
 	_, err := m.fs.Stat(m.registryFilename)
 	m.mu.Lock()
 	defer m.mu.Unlock()
-	if os.IsNotExist(err) {
+	if oserror.IsNotExist(err) {
 		// First run.
 		m.mu.keyRegistry = makeRegistryProto()
 		return nil

pkg/cli/BUILD.bazel

@@ -180,6 +180,7 @@ go_library(
         "//vendor/github.com/cockroachdb/apd/v2:apd",
         "//vendor/github.com/cockroachdb/cockroach-go/crdb",
         "//vendor/github.com/cockroachdb/errors",
+        "//vendor/github.com/cockroachdb/errors/oserror",
         "//vendor/github.com/cockroachdb/pebble",
         "//vendor/github.com/cockroachdb/pebble/tool",
         "//vendor/github.com/cockroachdb/redact",

pkg/cli/gen.go

@@ -22,6 +22,7 @@ import (
 	"github.com/cockroachdb/cockroach/pkg/settings"
 	"github.com/cockroachdb/cockroach/pkg/settings/cluster"
 	"github.com/cockroachdb/cockroach/pkg/sqlmigrations"
+	"github.com/cockroachdb/errors/oserror"
 	"github.com/spf13/cobra"
 	"github.com/spf13/cobra/doc"
 )
@@ -55,7 +56,7 @@ func runGenManCmd(cmd *cobra.Command, args []string) error {
 	}
 
 	if _, err := os.Stat(manPath); err != nil {
-		if os.IsNotExist(err) {
+		if oserror.IsNotExist(err) {
 			if err := os.MkdirAll(manPath, 0755); err != nil {
 				return err
 			}

pkg/cmd/roachprod/cloud/BUILD.bazel

@@ -13,6 +13,7 @@ go_library(
         "//pkg/cmd/roachprod/vm",
         "//pkg/util/timeutil",
         "//vendor/github.com/cockroachdb/errors",
+        "//vendor/github.com/cockroachdb/errors/oserror",
         "//vendor/github.com/nlopes/slack",
     ],
 )

pkg/cmd/roachprod/cloud/gc.go

@@ -27,6 +27,7 @@ import (
 	"github.com/cockroachdb/cockroach/pkg/cmd/roachprod/vm"
 	"github.com/cockroachdb/cockroach/pkg/util/timeutil"
 	"github.com/cockroachdb/errors"
+	"github.com/cockroachdb/errors/oserror"
 	"github.com/nlopes/slack"
 )
 
@@ -254,7 +255,7 @@ func shouldSend(channel string, status *status) (bool, error) {
 	}
 	hashPath := os.ExpandEnv(filepath.Join(hashDir, "notification-"+channel))
 	fileBytes, err := ioutil.ReadFile(hashPath)
-	if err != nil && !os.IsNotExist(err) {
+	if err != nil && !oserror.IsNotExist(err) {
 		return true, err
 	}
 	oldHash := string(fileBytes)

pkg/cmd/roachprod/ssh/BUILD.bazel

@@ -12,6 +12,7 @@ go_library(
         "//pkg/cmd/roachprod/config",
         "//pkg/util/syncutil",
         "//vendor/github.com/cockroachdb/errors",
+        "//vendor/github.com/cockroachdb/errors/oserror",
         "//vendor/golang.org/x/crypto/ssh",
         "//vendor/golang.org/x/crypto/ssh/agent",
         "//vendor/golang.org/x/crypto/ssh/knownhosts",

pkg/cmd/roachprod/ssh/ssh.go

@@ -25,6 +25,7 @@ import (
 	"github.com/cockroachdb/cockroach/pkg/cmd/roachprod/config"
 	"github.com/cockroachdb/cockroach/pkg/util/syncutil"
 	"github.com/cockroachdb/errors"
+	"github.com/cockroachdb/errors/oserror"
 	"golang.org/x/crypto/ssh"
 	"golang.org/x/crypto/ssh/agent"
 	"golang.org/x/crypto/ssh/knownhosts"
@@ -74,7 +75,7 @@ func getSSHAgentSigners() []ssh.Signer {
 func getSSHKeySigner(path string, haveAgent bool) ssh.Signer {
 	key, err := ioutil.ReadFile(path)
 	if err != nil {
-		if !os.IsNotExist(err) {
+		if !oserror.IsNotExist(err) {
 			log.Printf("unable to read SSH key %q: %s", path, err)
 		}
 		return nil

pkg/cmd/roachprod/vm/aws/BUILD.bazel

@@ -17,6 +17,7 @@ go_library(
         "//pkg/util/retry",
         "//pkg/util/syncutil",
         "//vendor/github.com/cockroachdb/errors",
+        "//vendor/github.com/cockroachdb/errors/oserror",
         "//vendor/github.com/spf13/pflag",
         "//vendor/golang.org/x/sync/errgroup",
         "//vendor/golang.org/x/time/rate",

pkg/cmd/roachprod/vm/aws/keys.go

@@ -19,6 +19,7 @@ import (
 	"strings"
 
 	"github.com/cockroachdb/errors"
+	"github.com/cockroachdb/errors/oserror"
 )
 
 const sshPublicKeyFile = "${HOME}/.ssh/id_rsa.pub"
@@ -51,7 +52,7 @@ func (p *Provider) sshKeyExists(keyName string, region string) (bool, error) {
 func (p *Provider) sshKeyImport(keyName string, region string) error {
 	keyBytes, err := ioutil.ReadFile(os.ExpandEnv(sshPublicKeyFile))
 	if err != nil {
-		if os.IsNotExist(err) {
+		if oserror.IsNotExist(err) {
 			return errors.Wrapf(err, "please run ssh-keygen externally to create your %s file", sshPublicKeyFile)
 		}
 		return err
@@ -85,7 +86,7 @@ func (p *Provider) sshKeyName() (string, error) {
 
 	keyBytes, err := ioutil.ReadFile(os.ExpandEnv(sshPublicKeyFile))
 	if err != nil {
-		if os.IsNotExist(err) {
+		if oserror.IsNotExist(err) {
 			return "", errors.Wrapf(err, "please run ssh-keygen externally to create your %s file", sshPublicKeyFile)
 		}
 		return "", err

pkg/cmd/uptodate/BUILD.bazel

@@ -7,6 +7,7 @@ go_library(
     visibility = ["//visibility:private"],
     deps = [
         "//vendor/github.com/MichaelTJones/walk",
+        "//vendor/github.com/cockroachdb/errors/oserror",
         "//vendor/github.com/spf13/pflag",
     ],
 )