Skip to content

Commit

Permalink
Remove all whitelisted classes and locations
Browse files Browse the repository at this point in the history
Instead we will let the Google API return the appropriate error message

[#170136311](https://www.pivotaltracker.com/story/show/170136311)
  • Loading branch information
Shaan Sapra committed Feb 13, 2020
1 parent 5868f48 commit 8230367
Show file tree
Hide file tree
Showing 6 changed files with 3 additions and 263 deletions.
9 changes: 3 additions & 6 deletions client/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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
Expand Down
40 changes: 0 additions & 40 deletions config/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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.
//
Expand Down Expand Up @@ -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)
}
78 changes: 0 additions & 78 deletions config/config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
78 changes: 0 additions & 78 deletions config/regions.go

This file was deleted.

13 changes: 0 additions & 13 deletions integration/configurations.go
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
48 changes: 0 additions & 48 deletions integration/gcs_storagecompat_test.go

This file was deleted.

0 comments on commit 8230367

Please sign in to comment.