azure: refactor to not use helpers/ pkg, validate all env/config inputs

Signed-off-by: Steve Kriss <steve@heptio.com>

Gopkg.lock

@@ -18,7 +18,6 @@
   name = "github.com/Azure/azure-sdk-for-go"
   packages = [
     "arm/disk",
-    "arm/examples/helpers",
     "services/storage/mgmt/2017-10-01/storage",
     "storage"
   ]
@@ -807,6 +806,6 @@
 [solve-meta]
   analyzer-name = "dep"
   analyzer-version = 1
-  inputs-digest = "84d160fa2e769b80040762566acadbe7c23ee774124dfdf7a498c0e65cd8011a"
+  inputs-digest = "4706135745ec21274791f454998d264dd167c78b472674ba813dca08cc962d7d"
   solver-name = "gps-cdcl"
   solver-version = 1

pkg/cloudprovider/azure/block_store.go

@@ -26,7 +26,6 @@ import (
 	"time"
 
 	"github.com/Azure/azure-sdk-for-go/arm/disk"
-	"github.com/Azure/azure-sdk-for-go/arm/examples/helpers"
 	"github.com/Azure/go-autorest/autorest"
 	"github.com/Azure/go-autorest/autorest/azure"
 	"github.com/pkg/errors"
@@ -40,14 +39,10 @@ import (
 )
 
 const (
-	azureTenantIDKey       = "AZURE_TENANT_ID"
-	azureSubscriptionIDKey = "AZURE_SUBSCRIPTION_ID"
-	azureClientIDKey       = "AZURE_CLIENT_ID"
-	azureClientSecretKey   = "AZURE_CLIENT_SECRET"
-	azureResourceGroupKey  = "AZURE_RESOURCE_GROUP"
-	apiTimeoutKey          = "apiTimeout"
-	snapshotsResource      = "snapshots"
-	disksResource          = "disks"
+	resourceGroupEnvVar = "AZURE_RESOURCE_GROUP"
+	apiTimeoutConfigKey = "apiTimeout"
+	snapshotsResource   = "snapshots"
+	disksResource       = "disks"
 )
 
 type blockStore struct {
@@ -69,50 +64,37 @@ func (si *snapshotIdentifier) String() string {
 	return getComputeResourceName(si.subscription, si.resourceGroup, snapshotsResource, si.name)
 }
 
-func getAzureEnvVars() map[string]string {
-	cfg := map[string]string{
-		azureTenantIDKey:       "",
-		azureSubscriptionIDKey: "",
-		azureClientIDKey:       "",
-		azureClientSecretKey:   "",
-		azureResourceGroupKey:  "",
-	}
-
-	for key := range cfg {
-		cfg[key] = os.Getenv(key)
-	}
-
-	return cfg
-}
-
 func NewBlockStore(logger logrus.FieldLogger) cloudprovider.BlockStore {
 	return &blockStore{log: logger}
 }
 
 func (b *blockStore) Init(config map[string]string) error {
-	var (
-		apiTimeoutVal = config[apiTimeoutKey]
-		apiTimeout    time.Duration
-		err           error
-	)
-
-	if apiTimeout, err = time.ParseDuration(apiTimeoutVal); err != nil {
-		return errors.Wrapf(err, "could not parse %s (expected time.Duration)", apiTimeoutKey)
-	}
-
-	if apiTimeout == 0 {
-		apiTimeout = 2 * time.Minute
-	}
-
-	cfg := getAzureEnvVars()
-
-	spt, err := helpers.NewServicePrincipalTokenFromCredentials(cfg, azure.PublicCloud.ResourceManagerEndpoint)
+	// 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 {
-		return errors.Wrap(err, "error creating new service principal token")
+		return errors.Wrap(err, "unable to get all required environment variables")
 	}
 
-	disksClient := disk.NewDisksClient(cfg[azureSubscriptionIDKey])
-	snapsClient := disk.NewSnapshotsClient(cfg[azureSubscriptionIDKey])
+	// 2. if config["apiTimeout"] is empty, default to 2m; otherwise, parse it
+	var apiTimeout time.Duration
+	if val := config[apiTimeoutConfigKey]; val == "" {
+		apiTimeout = 2 * time.Minute
+	} else {
+		apiTimeout, err = time.ParseDuration(val)
+		if err != nil {
+			return errors.Wrapf(err, "unable to parse value %q for config key %q (expected a duration string)", val, apiTimeoutConfigKey)
+		}
+	}
+
+	// 3. get SPT
+	spt, err := newServicePrincipalToken(envVars[tenantIDEnvVar], envVars[clientIDEnvVar], envVars[clientSecretEnvVar], azure.PublicCloud.ResourceManagerEndpoint)
+	if err != nil {
+		return errors.Wrap(err, "error getting service principal token")
+	}
+
+	// 4. set up clients
+	disksClient := disk.NewDisksClient(envVars[subscriptionIDEnvVar])
+	snapsClient := disk.NewSnapshotsClient(envVars[subscriptionIDEnvVar])
 
 	disksClient.PollingDelay = 5 * time.Second
 	snapsClient.PollingDelay = 5 * time.Second
@@ -123,8 +105,8 @@ func (b *blockStore) Init(config map[string]string) error {
 
 	b.disks = &disksClient
 	b.snaps = &snapsClient
-	b.subscription = cfg[azureSubscriptionIDKey]
-	b.resourceGroup = cfg[azureResourceGroupKey]
+	b.subscription = envVars[subscriptionIDEnvVar]
+	b.resourceGroup = envVars[resourceGroupEnvVar]
 	b.apiTimeout = apiTimeout
 
 	return nil

pkg/cloudprovider/azure/common.go

@@ -0,0 +1,60 @@
+/*
+Copyright 2018 the Heptio Ark 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 azure
+
+import (
+	"strings"
+
+	"github.com/Azure/go-autorest/autorest/adal"
+	"github.com/Azure/go-autorest/autorest/azure"
+	"github.com/pkg/errors"
+)
+
+const (
+	tenantIDEnvVar       = "AZURE_TENANT_ID"
+	subscriptionIDEnvVar = "AZURE_SUBSCRIPTION_ID"
+	clientIDEnvVar       = "AZURE_CLIENT_ID"
+	clientSecretEnvVar   = "AZURE_CLIENT_SECRET"
+)
+
+func newServicePrincipalToken(tenantID, clientID, clientSecret, scope string) (*adal.ServicePrincipalToken, error) {
+	oauthConfig, err := adal.NewOAuthConfig(azure.PublicCloud.ActiveDirectoryEndpoint, tenantID)
+	if err != nil {
+		return nil, errors.Wrap(err, "error getting OAuthConfig")
+	}
+
+	return adal.NewServicePrincipalToken(*oauthConfig, clientID, clientSecret, scope)
+}
+
+func getRequiredValues(getValue func(string) string, keys ...string) (map[string]string, error) {
+	missing := []string{}
+	results := map[string]string{}
+
+	for _, key := range keys {
+		if val := getValue(key); val == "" {
+			missing = append(missing, key)
+		} else {
+			results[key] = val
+		}
+	}
+
+	if len(missing) > 0 {
+		return nil, errors.Errorf("the following keys do not have values: %s", strings.Join(missing, ", "))
+	}
+
+	return results, nil
+}

pkg/cloudprovider/azure/object_store.go

@@ -18,10 +18,10 @@ package azure
 
 import (
 	"io"
+	"os"
 	"strings"
 	"time"
 
-	"github.com/Azure/azure-sdk-for-go/arm/examples/helpers"
 	storagemgmt "github.com/Azure/azure-sdk-for-go/services/storage/mgmt/2017-10-01/storage"
 	"github.com/Azure/azure-sdk-for-go/storage"
 	"github.com/Azure/go-autorest/autorest"
@@ -32,6 +32,11 @@ import (
 	"github.com/heptio/ark/pkg/cloudprovider"
 )
 
+const (
+	resourceGroupConfigKey  = "resourceGroup"
+	storageAccountConfigKey = "storageAccount"
+)
+
 type objectStore struct {
 	blobClient *storage.BlobStorageClient
 	log        logrus.FieldLogger
@@ -41,19 +46,7 @@ func NewObjectStore(logger logrus.FieldLogger) cloudprovider.ObjectStore {
 	return &objectStore{log: logger}
 }
 
-func getStorageAccountsClient(envVars map[string]string) (*storagemgmt.AccountsClient, error) {
-	spt, err := helpers.NewServicePrincipalTokenFromCredentials(envVars, azure.PublicCloud.ResourceManagerEndpoint)
-	if err != nil {
-		return nil, errors.Wrap(err, "error creating new service principal token")
-	}
-
-	accountsClient := storagemgmt.NewAccountsClient(envVars[azureSubscriptionIDKey])
-	accountsClient.Authorizer = autorest.NewBearerAuthorizer(spt)
-
-	return &accountsClient, nil
-}
-
-func getStorageAccountKey(client *storagemgmt.AccountsClient, resourceGroup, storageAccount string) (string, error) {
+func getStorageAccountKey(client storagemgmt.AccountsClient, resourceGroup, storageAccount string) (string, error) {
 	res, err := client.ListKeys(resourceGroup, storageAccount)
 	if err != nil {
 		return "", errors.WithStack(err)
@@ -80,24 +73,47 @@ func getStorageAccountKey(client *storagemgmt.AccountsClient, resourceGroup, sto
 	return storageKey, nil
 }
 
+func mapLookup(data map[string]string) func(string) string {
+	return func(key string) string {
+		return data[key]
+	}
+}
+
 func (o *objectStore) Init(config map[string]string) error {
-	storageAccountsClient, err := getStorageAccountsClient(getAzureEnvVars())
+	// 1. we need AZURE_TENANT_ID, AZURE_CLIENT_ID, AZURE_CLIENT_SECRET, AZURE_SUBSCRIPTION_ID
+	envVars, err := getRequiredValues(os.Getenv, tenantIDEnvVar, clientIDEnvVar, clientSecretEnvVar, subscriptionIDEnvVar)
 	if err != nil {
-		return err
+		return errors.Wrap(err, "unable to get all required environment variables")
 	}
 
-	storageAccountKey, err := getStorageAccountKey(storageAccountsClient, config["resourceGroup"], config["storageAccount"])
-	if err != nil {
-		return err
+	// 2. we need config["resourceGroup"], config["storageAccount"]
+	if _, err := getRequiredValues(mapLookup(config), resourceGroupConfigKey, storageAccountConfigKey); err != nil {
+		return errors.Wrap(err, "unable to get all required config values")
 	}
 
-	storageClient, err := storage.NewBasicClient(config["storageAccount"], storageAccountKey)
+	// 3. get SPT
+	spt, err := newServicePrincipalToken(envVars[tenantIDEnvVar], envVars[clientIDEnvVar], envVars[clientSecretEnvVar], azure.PublicCloud.ResourceManagerEndpoint)
 	if err != nil {
-		return errors.WithStack(err)
+		return errors.Wrap(err, "error getting service principal token")
+	}
+
+	// 4. get storageAccountsClient
+	storageAccountsClient := storagemgmt.NewAccountsClient(envVars[subscriptionIDEnvVar])
+	storageAccountsClient.Authorizer = autorest.NewBearerAuthorizer(spt)
+
+	// 5. get storage key
+	storageAccountKey, err := getStorageAccountKey(storageAccountsClient, config[resourceGroupConfigKey], config[storageAccountConfigKey])
+	if err != nil {
+		return errors.Wrap(err, "error getting storage account key")
+	}
+
+	// 6. get storageClient and blobClient
+	storageClient, err := storage.NewBasicClient(config[storageAccountConfigKey], storageAccountKey)
+	if err != nil {
+		return errors.Wrap(err, "error getting storage client")
 	}
 
 	blobClient := storageClient.GetBlobService()
-
 	o.blobClient = &blobClient
 
 	return nil

vendor/github.com/Azure/azure-sdk-for-go/arm/examples/helpers/helpers.go

@@ -1,64 +0,0 @@
-package helpers
-
-// Copyright 2017 Microsoft Corporation
-//
-//  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.
-
-import (
-	"encoding/json"
-	"fmt"
-
-	"github.com/Azure/go-autorest/autorest/adal"
-	"github.com/Azure/go-autorest/autorest/azure"
-)
-
-const (
-	credentialsPath = "/.azure/credentials.json"
-)
-
-// ToJSON returns the passed item as a pretty-printed JSON string. If any JSON error occurs,
-// it returns the empty string.
-func ToJSON(v interface{}) (string, error) {
-	j, err := json.MarshalIndent(v, "", "  ")
-	return string(j), err
-}
-
-// NewServicePrincipalTokenFromCredentials creates a new ServicePrincipalToken using values of the
-// passed credentials map.
-func NewServicePrincipalTokenFromCredentials(c map[string]string, scope string) (*adal.ServicePrincipalToken, error) {
-	oauthConfig, err := adal.NewOAuthConfig(azure.PublicCloud.ActiveDirectoryEndpoint, c["AZURE_TENANT_ID"])
-	if err != nil {
-		panic(err)
-	}
-	return adal.NewServicePrincipalToken(*oauthConfig, c["AZURE_CLIENT_ID"], c["AZURE_CLIENT_SECRET"], scope)
-}
-
-func ensureValueStrings(mapOfInterface map[string]interface{}) map[string]string {
-	mapOfStrings := make(map[string]string)
-	for key, value := range mapOfInterface {
-		mapOfStrings[key] = ensureValueString(value)
-	}
-	return mapOfStrings
-}
-
-func ensureValueString(value interface{}) string {
-	if value == nil {
-		return ""
-	}
-	switch v := value.(type) {
-	case string:
-		return v
-	default:
-		return fmt.Sprintf("%v", v)
-	}
-}