From 8230367b36beadb0a19850738d1e2256a364a672 Mon Sep 17 00:00:00 2001 From: Shaan Sapra Date: Thu, 13 Feb 2020 13:55:48 -0800 Subject: [PATCH] Remove all whitelisted classes and locations Instead we will let the Google API return the appropriate error message [#170136311](https://www.pivotaltracker.com/story/show/170136311) --- client/client.go | 9 ++-- config/config.go | 40 -------------- config/config_test.go | 78 --------------------------- config/regions.go | 78 --------------------------- integration/configurations.go | 13 ----- integration/gcs_storagecompat_test.go | 48 ----------------- 6 files changed, 3 insertions(+), 263 deletions(-) delete mode 100644 config/regions.go delete mode 100644 integration/gcs_storagecompat_test.go diff --git a/client/client.go b/client/client.go index b7b71949..83643ae0 100644 --- a/client/client.go +++ b/client/client.go @@ -43,7 +43,7 @@ type GCSBlobstore struct { } // validateRemoteConfig determines if the configuration of the client matches -// against the remote configuration and the StorageClass is valid for the location. +// against the remote configuration // // If operating in read-only mode, no mutations can be performed // so the remote bucket location is always compatible. @@ -53,11 +53,8 @@ func (client *GCSBlobstore) validateRemoteConfig() error { } bucket := client.authenticatedGCS.Bucket(client.config.BucketName) - attrs, err := bucket.Attrs(context.Background()) - if err != nil { - return err - } - return client.config.FitCompatibleLocation(attrs.Location) + _, err := bucket.Attrs(context.Background()) + return err } // getObjectHandle returns a handle to an object named src diff --git a/config/config.go b/config/config.go index a6d451b1..ca109f6a 100644 --- a/config/config.go +++ b/config/config.go @@ -45,13 +45,6 @@ type GCSCli struct { EncryptionKey []byte `json:"encryption_key"` } -const ( - defaultRegionalLocation = "US-EAST1" - defaultMultiRegionalLocation = "US" - defaultRegionalStorageClass = "REGIONAL" - defaultMultiRegionalStorageClass = "MULTI_REGIONAL" -) - // DefaultCredentialsSource specifies that credentials should be detected. // Application Default Credentials will be used if avaliable. // A read-only client will be used otherwise. @@ -76,17 +69,6 @@ var ErrEmptyServiceAccountFile = errors.New("json_key must be set") // in the config is not exactly 32 bytes. var ErrWrongLengthEncryptionKey = errors.New("encryption_key not 32 bytes") -// getDefaultStorageClass returns the default StorageClass for a given location. -// This takes into account regional/multi-regional incompatibility. -// -// Empty string is returned if the location cannot be matched. -func getDefaultStorageClass(location string) string { - if _, ok := GCSMultiRegionalLocations[location]; ok { - return defaultMultiRegionalStorageClass - } - return "" -} - // NewFromReader returns the new gcscli configuration struct from the // contents of the reader. // @@ -114,25 +96,3 @@ func NewFromReader(reader io.Reader) (GCSCli, error) { return c, nil } - -// FitCompatibleLocation returns whether a provided Location -// can have c.StorageClass objects written to it. -// -// When c.StorageClass is empty, a compatible default is filled in. -// -// nil return value when compatible, otherwise a non-nil explanation. -func (c *GCSCli) FitCompatibleLocation(loc string) error { - if c.StorageClass == "" { - c.StorageClass = getDefaultStorageClass(loc) - } - - if c.StorageClass == "" { - return nil - } - - if _, ok := GCSStorageClass[c.StorageClass]; !ok { - return ErrUnknownStorageClass(c.StorageClass) - } - - return validLocationStorageClass(loc, c.StorageClass) -} diff --git a/config/config_test.go b/config/config_test.go index edeb8805..4b6e5ca1 100644 --- a/config/config_test.go +++ b/config/config_test.go @@ -22,88 +22,10 @@ import ( . "github.com/cloudfoundry/bosh-gcscli/config" . "github.com/onsi/ginkgo" - . "github.com/onsi/ginkgo/extensions/table" . "github.com/onsi/gomega" ) var _ = Describe("BlobstoreClient configuration", func() { - Describe("checking that location or storage_class has been set", func() { - Context("when storage_class has been set to MULTI_REGIONAL", func() { - dummyJSONBytes := []byte(`{"storage_class":"MULTI_REGIONAL","bucket_name": "some-bucket"}`) - dummyJSONReader := bytes.NewReader(dummyJSONBytes) - - It("US is compatible location", func() { - c, err := NewFromReader(dummyJSONReader) - Expect(err).ToNot(HaveOccurred()) - err = c.FitCompatibleLocation("US") - Expect(err).ToNot(HaveOccurred()) - - }) - }) - - Context("when storage_class has been set to REGIONAL", func() { - dummyJSONBytes := []byte(`{"storage_class":"REGIONAL","bucket_name": "some-bucket"}`) - dummyJSONReader := bytes.NewReader(dummyJSONBytes) - - It("us-east1 is compatible location", func() { - c, err := NewFromReader(dummyJSONReader) - Expect(err).ToNot(HaveOccurred()) - err = c.FitCompatibleLocation("US-EAST1") - Expect(err).ToNot(HaveOccurred()) - }) - }) - - Context("when location has been set to US", func() { - dummyJSONBytes := []byte(`{"bucket_name": "some-bucket"}`) - dummyJSONReader := bytes.NewReader(dummyJSONBytes) - - It("defaults to MULTI_REGIONAL", func() { - c, err := NewFromReader(dummyJSONReader) - Expect(err).ToNot(HaveOccurred()) - err = c.FitCompatibleLocation("US") - Expect(err).ToNot(HaveOccurred()) - Expect(c.StorageClass).To(Equal("MULTI_REGIONAL")) - }) - }) - - Context("when location has been set to US-WEST1", func() { - dummyJSONBytes := []byte(`{"bucket_name": "some-bucket"}`) - dummyJSONReader := bytes.NewReader(dummyJSONBytes) - - It("does not look up the storage class", func() { - c, err := NewFromReader(dummyJSONReader) - Expect(err).ToNot(HaveOccurred()) - err = c.FitCompatibleLocation("US-WEST1") - Expect(err).ToNot(HaveOccurred()) - Expect(c.StorageClass).To(Equal("")) - }) - }) - - DescribeTable("invalid storage_class and location combinations", - func(dummyJSON, loc string, expected error) { - dummyJSONBytes := []byte(dummyJSON) - dummyJSONReader := bytes.NewReader(dummyJSONBytes) - - c, err := NewFromReader(dummyJSONReader) - Expect(err).ToNot(HaveOccurred()) - err = c.FitCompatibleLocation(loc) - Expect(err).To(MatchError(expected)) - }, - Entry("storage_class is MULTI_REGIONAL and location is regional", - `{"storage_class": "MULTI_REGIONAL","bucket_name": "some-bucket"}`, - "US-WEST1", - ErrBadLocationStorageClass("US-WEST1", "MULTI_REGIONAL")), - Entry("storage_class is REGIONAL and location is multi-regional", - `{"storage_class": "REGIONAL","bucket_name": "some-bucket"}`, - "US", - ErrBadLocationStorageClass("US", "REGIONAL")), - Entry("storage_class is unknown", - `{"storage_class": "asdasdasd","bucket_name": "some-bucket"}`, - "US", - ErrUnknownStorageClass("asdasdasd")), - ) - }) - Describe("when bucket is not specified", func() { dummyJSONBytes := []byte(`{}`) dummyJSONReader := bytes.NewReader(dummyJSONBytes) diff --git a/config/regions.go b/config/regions.go deleted file mode 100644 index 38afe173..00000000 --- a/config/regions.go +++ /dev/null @@ -1,78 +0,0 @@ -/* - * Copyright 2017 Google Inc. - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -package config - -import ( - "fmt" -) - -// GCSMultiRegionalLocations are the valid locations for -// a multi-regional bucket -var GCSMultiRegionalLocations = map[string]struct{}{ - "ASIA": struct{}{}, - "EU": struct{}{}, - "US": struct{}{}, -} - -const ( - multiRegional = "MULTI_REGIONAL" - regional = "REGIONAL" - nearline = "NEARLINE" - coldline = "COLDLINE" -) - -// GCSStorageClass are the valid storage classes for a bucket. -var GCSStorageClass = map[string]struct{}{ - multiRegional: struct{}{}, - regional: struct{}{}, - nearline: struct{}{}, - coldline: struct{}{}, -} - -// ErrBadLocationStorageClass is returned when location and storage_class -// cannot be combined -func ErrBadLocationStorageClass(location, storageClass string) error { - return fmt.Errorf("incompatible location %s and storage_class %s", location, storageClass) -} - -// ErrUnknownStorageClass is returned when a stroage_class is chosen that -// this package has no knowledge of. -func ErrUnknownStorageClass(storageClass string) error { - return fmt.Errorf("unknown storage_class: %s", storageClass) -} - -// validDurability returns nil error on valid location-durability combination -// and non-nil explanation on all else. -func validLocationStorageClass(location, storageClass string) error { - if _, ok := GCSStorageClass[storageClass]; !ok { - return ErrUnknownStorageClass(storageClass) - } - - if storageClass == regional { - if _, ok := GCSMultiRegionalLocations[location]; ok { - return ErrBadLocationStorageClass(location, storageClass) - } - return nil - } else if _, ok := GCSStorageClass[storageClass]; ok { - if _, ok := GCSMultiRegionalLocations[location]; !ok { - return ErrBadLocationStorageClass(location, storageClass) - } - return nil - } else { - return nil - } -} diff --git a/integration/configurations.go b/integration/configurations.go index b573ee60..f253c8d2 100644 --- a/integration/configurations.go +++ b/integration/configurations.go @@ -95,19 +95,6 @@ func getPublicConfig() *config.GCSCli { } } -func getInvalidStorageClassConfigs() []TableEntry { - regional := getRegionalConfig() - multiRegion := getMultiRegionConfig() - - multiRegion.StorageClass = "REGIONAL" - regional.StorageClass = "MULTI_REGIONAL" - - return []TableEntry{ - Entry("Multi-Region bucket, regional StorageClass", regional), - Entry("Regional bucket, Multi-Region StorageClass", multiRegion), - } -} - // newSDK builds the GCS SDK Client from a valid config.GCSCli // TODO: Simplify and remove this. Tests should expect a single config and use it. func newSDK(ctx context.Context, c config.GCSCli) (*storage.Client, error) { diff --git a/integration/gcs_storagecompat_test.go b/integration/gcs_storagecompat_test.go deleted file mode 100644 index 61d4f08a..00000000 --- a/integration/gcs_storagecompat_test.go +++ /dev/null @@ -1,48 +0,0 @@ -/* - * Copyright 2017 Google Inc. - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -package integration - -import ( - "github.com/cloudfoundry/bosh-gcscli/config" - . "github.com/onsi/ginkgo" - . "github.com/onsi/ginkgo/extensions/table" - . "github.com/onsi/gomega" -) - -var _ = Describe("StorageCompat Integration", func() { - Context("invalid storage_class for bucket (Default Applicaton Credentials) configuration", func() { - var ctx AssertContext - BeforeEach(func() { - ctx = NewAssertContext(AsDefaultCredentials) - }) - AfterEach(func() { - ctx.Cleanup() - }) - - configurations := getInvalidStorageClassConfigs() - - DescribeTable("Invalid Put should fail", - func(config *config.GCSCli) { - ctx.AddConfig(config) - - session, err := RunGCSCLI(gcsCLIPath, ctx.ConfigPath, "put", ctx.ContentFile, ctx.GCSFileName) - Expect(err).ToNot(HaveOccurred()) - Expect(session.ExitCode()).ToNot(BeZero()) - }, - configurations...) - }) -})