diff --git a/pkg/cloud/external_storage.go b/pkg/cloud/external_storage.go index f726d80fe063..bae20674f731 100644 --- a/pkg/cloud/external_storage.go +++ b/pkg/cloud/external_storage.go @@ -128,6 +128,10 @@ type SQLConnI interface { // ErrFileDoesNotExist is a sentinel error for indicating that a specified // bucket/object/key/file (depending on storage terminology) does not exist. // This error is raised by the ReadFile method. +// +// On GCS, a non-existent bucket returns a 403 forbidden, so a not found is +// only returned if the bucket exists and the user has the permissions needed +// to interact with the bucket. var ErrFileDoesNotExist = errors.New("external_storage: file doesn't exist") // ErrListingUnsupported is a marker for indicating listing is unsupported. diff --git a/pkg/cloud/gcp/gcs_storage_test.go b/pkg/cloud/gcp/gcs_storage_test.go index e1454f9ee816..3cec94c77d5b 100644 --- a/pkg/cloud/gcp/gcs_storage_test.go +++ b/pkg/cloud/gcp/gcs_storage_test.go @@ -327,8 +327,8 @@ func TestAntagonisticGCSRead(t *testing.T) { cloudtestutils.CheckAntagonisticRead(t, conf, testSettings) } -// TestFileDoesNotExist ensures that the ReadFile method of google cloud storage -// returns a sentinel error when the `Bucket` or `Object` being read do not +// TestFileDoesNotExist ensures that the ReadFile method of google cloud +// storage returns a sentinel error when the `Object` being read does not // exist. func TestFileDoesNotExist(t *testing.T) { defer leaktest.AfterTest(t)() @@ -341,41 +341,20 @@ func TestFileDoesNotExist(t *testing.T) { testSettings := cluster.MakeTestingClusterSettings() - { - // Invalid gsFile. - gsFile := "gs://cockroach-fixtures-us-east1/tpch-csv/sf-1/invalid_region.tbl?AUTH=implicit" - conf, err := cloud.ExternalStorageConfFromURI(gsFile, user) - require.NoError(t, err) - - s, err := cloud.MakeExternalStorage(context.Background(), conf, base.ExternalIODirConfig{}, testSettings, - nil, /* blobClientFactory */ - nil, /* db */ - nil, /* limiters */ - cloud.NilMetrics, - ) - require.NoError(t, err) - _, _, err = s.ReadFile(context.Background(), "", cloud.ReadOptions{NoFileSize: true}) - require.Error(t, err, "") - require.True(t, errors.Is(err, cloud.ErrFileDoesNotExist)) - } - - { - // Invalid gsBucket. - gsFile := "gs://cockroach-fixtures-us-east1-invalid/tpch-csv/sf-1/region.tbl?AUTH=implicit" - conf, err := cloud.ExternalStorageConfFromURI(gsFile, user) - require.NoError(t, err) + // Invalid gsFile. + gsFile := "gs://cockroach-fixtures-us-east1/tpch-csv/sf-1/invalid_region.tbl?AUTH=implicit" + conf, err := cloud.ExternalStorageConfFromURI(gsFile, user) + require.NoError(t, err) - s, err := cloud.MakeExternalStorage(context.Background(), conf, base.ExternalIODirConfig{}, testSettings, - nil, /* blobClientFactory */ - nil, /* db */ - nil, /* limiters */ - cloud.NilMetrics, - ) - require.NoError(t, err) - _, _, err = s.ReadFile(context.Background(), "", cloud.ReadOptions{NoFileSize: true}) - require.Error(t, err, "") - require.True(t, errors.Is(err, cloud.ErrFileDoesNotExist)) - } + s, err := cloud.MakeExternalStorage(context.Background(), conf, base.ExternalIODirConfig{}, testSettings, + nil, /* blobClientFactory */ + nil, /* db */ + nil, /* limiters */ + cloud.NilMetrics, + ) + require.NoError(t, err) + _, _, err = s.ReadFile(context.Background(), "", cloud.ReadOptions{NoFileSize: true}) + require.ErrorIs(t, err, cloud.ErrFileDoesNotExist) } func TestCompressedGCS(t *testing.T) {