From 6bf29e17aa6f57dafaa4aa80e2b42b11b7f848d9 Mon Sep 17 00:00:00 2001 From: Steve Kriss Date: Sun, 31 Mar 2019 17:09:17 -0600 Subject: [PATCH] objectstores/volumesnapshotters: check for invalid keys in config Signed-off-by: Steve Kriss --- changelogs/unreleased/1338-skriss | 1 + pkg/cloudprovider/aws/object_store.go | 15 ++++++- pkg/cloudprovider/aws/volume_snapshotter.go | 8 +++- pkg/cloudprovider/azure/object_store.go | 8 +++- pkg/cloudprovider/azure/volume_snapshotter.go | 8 +++- pkg/cloudprovider/config.go | 39 +++++++++++++++++++ pkg/cloudprovider/config_test.go | 34 ++++++++++++++++ pkg/cloudprovider/gcp/object_store.go | 8 +++- pkg/cloudprovider/gcp/volume_snapshotter.go | 8 +++- 9 files changed, 123 insertions(+), 6 deletions(-) create mode 100644 changelogs/unreleased/1338-skriss create mode 100644 pkg/cloudprovider/config.go create mode 100644 pkg/cloudprovider/config_test.go diff --git a/changelogs/unreleased/1338-skriss b/changelogs/unreleased/1338-skriss new file mode 100644 index 000000000..16ab9e015 --- /dev/null +++ b/changelogs/unreleased/1338-skriss @@ -0,0 +1 @@ +aws/azure/gcp: fail fast if unsupported keys are provided in BackupStorageLocation/VolumeSnapshotLocation config diff --git a/pkg/cloudprovider/aws/object_store.go b/pkg/cloudprovider/aws/object_store.go index 3490482d6..0b1d09e7e 100644 --- a/pkg/cloudprovider/aws/object_store.go +++ b/pkg/cloudprovider/aws/object_store.go @@ -1,5 +1,5 @@ /* -Copyright 2017 the Velero contributors. +Copyright 2017, 2019 the Velero contributors. Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with the License. @@ -29,6 +29,8 @@ import ( "github.com/aws/aws-sdk-go/service/s3/s3manager" "github.com/pkg/errors" "github.com/sirupsen/logrus" + + "github.com/heptio/velero/pkg/cloudprovider" ) const ( @@ -62,6 +64,17 @@ func isValidSignatureVersion(signatureVersion string) bool { } func (o *ObjectStore) Init(config map[string]string) error { + if err := cloudprovider.ValidateConfigKeys(config, + regionKey, + s3URLKey, + publicURLKey, + kmsKeyIDKey, + s3ForcePathStyleKey, + signatureVersionKey, + ); err != nil { + return err + } + var ( region = config[regionKey] s3URL = config[s3URLKey] diff --git a/pkg/cloudprovider/aws/volume_snapshotter.go b/pkg/cloudprovider/aws/volume_snapshotter.go index 2405cc77b..3d32fc8c8 100644 --- a/pkg/cloudprovider/aws/volume_snapshotter.go +++ b/pkg/cloudprovider/aws/volume_snapshotter.go @@ -1,5 +1,5 @@ /* -Copyright 2017 the Velero contributors. +Copyright 2017, 2019 the Velero contributors. Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with the License. @@ -32,6 +32,8 @@ import ( "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/util/sets" + + "github.com/heptio/velero/pkg/cloudprovider" ) const regionKey = "region" @@ -64,6 +66,10 @@ func NewVolumeSnapshotter(logger logrus.FieldLogger) *VolumeSnapshotter { } func (b *VolumeSnapshotter) Init(config map[string]string) error { + if err := cloudprovider.ValidateConfigKeys(config, regionKey); err != nil { + return err + } + region := config[regionKey] if region == "" { return errors.Errorf("missing %s in aws configuration", regionKey) diff --git a/pkg/cloudprovider/azure/object_store.go b/pkg/cloudprovider/azure/object_store.go index 30ca3779c..441924376 100644 --- a/pkg/cloudprovider/azure/object_store.go +++ b/pkg/cloudprovider/azure/object_store.go @@ -1,5 +1,5 @@ /* -Copyright 2017 the Velero contributors. +Copyright 2017, 2019 the Velero contributors. Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with the License. @@ -29,6 +29,8 @@ import ( "github.com/Azure/go-autorest/autorest/azure" "github.com/pkg/errors" "github.com/sirupsen/logrus" + + "github.com/heptio/velero/pkg/cloudprovider" ) const ( @@ -99,6 +101,10 @@ func mapLookup(data map[string]string) func(string) string { } func (o *ObjectStore) Init(config map[string]string) error { + if err := cloudprovider.ValidateConfigKeys(config, resourceGroupConfigKey, storageAccountConfigKey); err != nil { + return err + } + storageAccountKey, err := getStorageAccountKey(config) if err != nil { return err diff --git a/pkg/cloudprovider/azure/volume_snapshotter.go b/pkg/cloudprovider/azure/volume_snapshotter.go index ab938f0a8..e88eb17e7 100644 --- a/pkg/cloudprovider/azure/volume_snapshotter.go +++ b/pkg/cloudprovider/azure/volume_snapshotter.go @@ -1,5 +1,5 @@ /* -Copyright 2017 the Velero contributors. +Copyright 2017, 2019 the Velero contributors. Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with the License. @@ -34,6 +34,8 @@ import ( v1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" "k8s.io/apimachinery/pkg/runtime" + + "github.com/heptio/velero/pkg/cloudprovider" ) const ( @@ -70,6 +72,10 @@ func NewVolumeSnapshotter(logger logrus.FieldLogger) *VolumeSnapshotter { } func (b *VolumeSnapshotter) Init(config map[string]string) error { + if err := cloudprovider.ValidateConfigKeys(config, resourceGroupConfigKey, apiTimeoutConfigKey); err != nil { + return err + } + // 1. we need AZURE_TENANT_ID, AZURE_CLIENT_ID, AZURE_CLIENT_SECRET, AZURE_SUBSCRIPTION_ID, AZURE_RESOURCE_GROUP envVars, err := getRequiredValues(os.Getenv, tenantIDEnvVar, clientIDEnvVar, clientSecretEnvVar, subscriptionIDEnvVar, resourceGroupEnvVar) if err != nil { diff --git a/pkg/cloudprovider/config.go b/pkg/cloudprovider/config.go new file mode 100644 index 000000000..90dfb6321 --- /dev/null +++ b/pkg/cloudprovider/config.go @@ -0,0 +1,39 @@ +/* +Copyright 2019 the Velero contributors. + +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 cloudprovider + +import ( + "github.com/pkg/errors" + "k8s.io/apimachinery/pkg/util/sets" +) + +func ValidateConfigKeys(config map[string]string, validKeys ...string) error { + validKeysSet := sets.NewString(validKeys...) + + var invalidKeys []string + for k := range config { + if !validKeysSet.Has(k) { + invalidKeys = append(invalidKeys, k) + } + } + + if len(invalidKeys) > 0 { + return errors.Errorf("config has invalid keys %v; valid keys are %v", invalidKeys, validKeys) + } + + return nil +} diff --git a/pkg/cloudprovider/config_test.go b/pkg/cloudprovider/config_test.go new file mode 100644 index 000000000..b8eca7570 --- /dev/null +++ b/pkg/cloudprovider/config_test.go @@ -0,0 +1,34 @@ +/* +Copyright 2019 the Velero contributors. + +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 cloudprovider + +import ( + "testing" + + "github.com/stretchr/testify/assert" +) + +func TestValidateConfigKeys(t *testing.T) { + assert.NoError(t, ValidateConfigKeys(nil)) + assert.NoError(t, ValidateConfigKeys(map[string]string{})) + assert.NoError(t, ValidateConfigKeys(map[string]string{"foo": "bar"}, "foo")) + assert.NoError(t, ValidateConfigKeys(map[string]string{"foo": "bar", "bar": "baz"}, "foo", "bar")) + + assert.Error(t, ValidateConfigKeys(map[string]string{"foo": "bar"})) + assert.Error(t, ValidateConfigKeys(map[string]string{"foo": "bar"}, "Foo")) + assert.Error(t, ValidateConfigKeys(map[string]string{"foo": "bar", "boo": ""}, "foo")) +} diff --git a/pkg/cloudprovider/gcp/object_store.go b/pkg/cloudprovider/gcp/object_store.go index 7a7079525..7eedcfe81 100644 --- a/pkg/cloudprovider/gcp/object_store.go +++ b/pkg/cloudprovider/gcp/object_store.go @@ -1,5 +1,5 @@ /* -Copyright 2017 the Velero contributors. +Copyright 2017, 2019 the Velero contributors. Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with the License. @@ -29,6 +29,8 @@ import ( "golang.org/x/oauth2/google" "google.golang.org/api/iterator" "google.golang.org/api/option" + + "github.com/heptio/velero/pkg/cloudprovider" ) const credentialsEnvVar = "GOOGLE_APPLICATION_CREDENTIALS" @@ -60,6 +62,10 @@ func NewObjectStore(logger logrus.FieldLogger) *ObjectStore { } func (o *ObjectStore) Init(config map[string]string) error { + if err := cloudprovider.ValidateConfigKeys(config); err != nil { + return err + } + credentialsFile := os.Getenv(credentialsEnvVar) if credentialsFile == "" { return errors.Errorf("%s is undefined", credentialsEnvVar) diff --git a/pkg/cloudprovider/gcp/volume_snapshotter.go b/pkg/cloudprovider/gcp/volume_snapshotter.go index 84bb57022..6e9ecd445 100644 --- a/pkg/cloudprovider/gcp/volume_snapshotter.go +++ b/pkg/cloudprovider/gcp/volume_snapshotter.go @@ -1,5 +1,5 @@ /* -Copyright 2017 the Velero contributors. +Copyright 2017, 2019 the Velero contributors. Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with the License. @@ -33,6 +33,8 @@ import ( v1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" "k8s.io/apimachinery/pkg/runtime" + + "github.com/heptio/velero/pkg/cloudprovider" ) const ( @@ -51,6 +53,10 @@ func NewVolumeSnapshotter(logger logrus.FieldLogger) *VolumeSnapshotter { } func (b *VolumeSnapshotter) Init(config map[string]string) error { + if err := cloudprovider.ValidateConfigKeys(config); err != nil { + return err + } + project, err := extractProjectFromCreds() if err != nil { return err