blobs: enforce disabled external-io-dir

The `nodelocal` cloud storage implementation previously did its own
file IO, and checked for and enforced the ExternalIODirectory, including
checking when it was disabled completely ("").

However after the switch to backing `nodelocal` with the local-or-remote
blob service, this enforcement now needs to be done in that service,
when it actually goes to do local IO,  as it is a per-node decision if
and where to allow IO.

Release note: none.

pkg/blobs/local_storage.go

@@ -31,6 +31,14 @@ type LocalStorage struct {
 // NewLocalStorage creates a new LocalStorage object and returns
 // an error when we cannot take the absolute path of `externalIODir`.
 func NewLocalStorage(externalIODir string) (*LocalStorage, error) {
+	// An empty externalIODir indicates external IO is completely disabled.
+	// Returning a nil *LocalStorage in this case and then hanldling `nil` in the
+	// prependExternalIODir helper ensures that that is respected throughout the
+	// implementation (as a failure to do so would likely fail loudly with a
+	// nil-pointer dereference).
+	if externalIODir == "" {
+		return nil, nil
+	}
 	absPath, err := filepath.Abs(externalIODir)
 	if err != nil {
 		return nil, errors.Wrap(err, "creating LocalStorage object")
@@ -46,6 +54,9 @@ func NewLocalStorage(externalIODir string) (*LocalStorage, error) {
 // operators to "open up" their I/O directory via symlinks. Therefore,
 // a full check via filepath.Abs() would be inadequate.
 func (l *LocalStorage) prependExternalIODir(path string) (string, error) {
+	if l == nil {
+		return "", errors.Errorf("local file access is disabled")
+	}
 	localBase := filepath.Join(l.externalIODir, path)
 	if !strings.HasPrefix(localBase, l.externalIODir) {
 		return "", errors.Errorf("local file access to paths outside of external-io-dir is not allowed: %s", path)
@@ -55,8 +66,7 @@ func (l *LocalStorage) prependExternalIODir(path string) (string, error) {
 
 // WriteFile prepends IO dir to filename and writes the content to that local file.
 func (l *LocalStorage) WriteFile(filename string, content io.Reader) (err error) {
-	var fullPath string
-	fullPath, err = l.prependExternalIODir(filename)
+	fullPath, err := l.prependExternalIODir(filename)
 	if err != nil {
 		return err
 	}

pkg/cli/cli_test.go

@@ -62,11 +62,12 @@ type cliTest struct {
 }
 
 type cliTestParams struct {
-	t          *testing.T
-	insecure   bool
-	noServer   bool
-	storeSpecs []base.StoreSpec
-	locality   roachpb.Locality
+	t           *testing.T
+	insecure    bool
+	noServer    bool
+	storeSpecs  []base.StoreSpec
+	locality    roachpb.Locality
+	noNodelocal bool
 }
 
 func (c *cliTest) fail(err interface{}) {
@@ -125,13 +126,17 @@ func newCLITest(params cliTestParams) cliTest {
 			baseCfg.SSLCertsDir = certsDir
 		}
 
-		s, err := serverutils.StartServerRaw(base.TestServerArgs{
+		args := base.TestServerArgs{
 			Insecure:      params.insecure,
 			SSLCertsDir:   c.certsDir,
 			StoreSpecs:    params.storeSpecs,
 			Locality:      params.locality,
 			ExternalIODir: filepath.Join(certsDir, "extern"),
-		})
+		}
+		if params.noNodelocal {
+			args.ExternalIODir = ""
+		}
+		s, err := serverutils.StartServerRaw(args)
 		if err != nil {
 			c.fail(err)
 		}

pkg/cli/nodelocal_test.go

@@ -26,7 +26,7 @@ func Example_nodelocal() {
 	c := newCLITest(cliTestParams{})
 	defer c.cleanup()
 
-	file, cleanUp := createTestFile()
+	file, cleanUp := createTestFile("test.csv", "content")
 	defer cleanUp()
 
 	c.Run(fmt.Sprintf("nodelocal upload %s /test/file1.csv", file))
@@ -49,6 +49,28 @@ func Example_nodelocal() {
 	// ERROR: open notexist.csv: no such file or directory
 }
 
+func Example_nodelocal_disabled() {
+	c := newCLITest(cliTestParams{noNodelocal: true})
+	defer c.cleanup()
+
+	file, cleanUp := createTestFile("test.csv", "non-empty-file")
+	defer cleanUp()
+
+	empty, cleanUpEmpty := createTestFile("empty.csv", "")
+	defer cleanUpEmpty()
+
+	c.Run(fmt.Sprintf("nodelocal upload %s /test/file1.csv", empty))
+	c.Run(fmt.Sprintf("nodelocal upload %s /test/file1.csv", file))
+
+	// Output:
+	// nodelocal upload empty.csv /test/file1.csv
+	// ERROR: current transaction is aborted, commands ignored until end of transaction block
+	// SQLSTATE: 25P02
+	// nodelocal upload test.csv /test/file1.csv
+	// ERROR: current transaction is aborted, commands ignored until end of transaction block
+	// SQLSTATE: 25P02
+}
+
 func TestNodeLocalFileUpload(t *testing.T) {
 	defer leaktest.AfterTest(t)()
 
@@ -62,6 +84,10 @@ func TestNodeLocalFileUpload(t *testing.T) {
 		name        string
 		fileContent []byte
 	}{
+		{
+			"empty",
+			[]byte{},
+		},
 		{
 			"exactly-one-chunk",
 			make([]byte, chunkSize),
@@ -102,13 +128,12 @@ func TestNodeLocalFileUpload(t *testing.T) {
 	}
 }
 
-func createTestFile() (string, func()) {
-	filePath := "test.csv"
-	err := ioutil.WriteFile(filePath, []byte("file content"), 0666)
+func createTestFile(name, content string) (string, func()) {
+	err := ioutil.WriteFile(name, []byte(content), 0666)
 	if err != nil {
 		return "", func() {}
 	}
-	return filePath, func() {
-		_ = os.Remove(filePath)
+	return name, func() {
+		_ = os.Remove(name)
 	}
 }

pkg/kv/kvserver/cloud/nodelocal_storage.go

@@ -55,13 +55,6 @@ func makeLocalStorage(
 	if err != nil {
 		return nil, errors.Wrap(err, "failed to create blob client")
 	}
-
-	// In non-server execution we have no settings and no restriction on local IO.
-	if settings != nil {
-		if settings.ExternalIODir == "" {
-			return nil, errors.Errorf("local file access is disabled")
-		}
-	}
 	return &localFileStorage{base: cfg.Path, cfg: cfg, blobClient: client}, nil
 }