Merge pull request #3496 from hashicorp/b-azure-settings-tilde

provider/azure: fix issues loading config from homedir

builtin/providers/azure/config.go

@@ -98,7 +98,7 @@ func (c Client) getStorageServiceQueueClient(serviceName string) (storage.QueueS
 func (c *Config) NewClientFromSettingsData() (*Client, error) {
 	mc, err := management.ClientFromPublishSettingsData(c.Settings, c.SubscriptionID)
 	if err != nil {
-		return nil, nil
+		return nil, err
 	}
 
 	return &Client{

builtin/providers/azure/provider.go

@@ -64,22 +64,12 @@ func providerConfigure(d *schema.ResourceData) (interface{}, error) {
 		Certificate:    []byte(d.Get("certificate").(string)),
 	}
 
-	settings := d.Get("settings_file").(string)
-
-	if settings != "" {
-		if ok, _ := isFile(settings); ok {
-			settingsFile, err := homedir.Expand(settings)
-			if err != nil {
-				return nil, fmt.Errorf("Error expanding the settings file path: %s", err)
-			}
-			publishSettingsContent, err := ioutil.ReadFile(settingsFile)
-			if err != nil {
-				return nil, fmt.Errorf("Error reading settings file: %s", err)
-			}
-			config.Settings = publishSettingsContent
-		} else {
-			config.Settings = []byte(settings)
-		}
+	settingsFile := d.Get("settings_file").(string)
+	if settingsFile != "" {
+		// any errors from readSettings would have been caught at the validate
+		// step, so we can avoid handling them now
+		settings, _, _ := readSettings(settingsFile)
+		config.Settings = settings
 		return config.NewClientFromSettingsData()
 	}
 
@@ -92,31 +82,39 @@ func providerConfigure(d *schema.ResourceData) (interface{}, error) {
 			"or both a 'subscription_id' and 'certificate'.")
 }
 
-func validateSettingsFile(v interface{}, k string) (warnings []string, errors []error) {
+func validateSettingsFile(v interface{}, k string) ([]string, []error) {
 	value := v.(string)
-
 	if value == "" {
-		return
+		return nil, nil
 	}
 
-	var settings settingsData
-	if err := xml.Unmarshal([]byte(value), &settings); err != nil {
-		warnings = append(warnings, `
+	_, warnings, errors := readSettings(value)
+	return warnings, errors
+}
+
+const settingsPathWarnMsg = `
 settings_file is not valid XML, so we are assuming it is a file path. This
 support will be removed in the future. Please update your configuration to use
-${file("filename.publishsettings")} instead.`)
-	} else {
+${file("filename.publishsettings")} instead.`
+
+func readSettings(pathOrContents string) (s []byte, ws []string, es []error) {
+	var settings settingsData
+	if err := xml.Unmarshal([]byte(pathOrContents), &settings); err == nil {
+		s = []byte(pathOrContents)
 		return
 	}
 
-	if ok, err := isFile(value); !ok {
-		errors = append(errors,
-			fmt.Errorf(
-				"account_file path could not be read from '%s': %s",
-				value,
-				err))
+	ws = append(ws, settingsPathWarnMsg)
+	path, err := homedir.Expand(pathOrContents)
+	if err != nil {
+		es = append(es, fmt.Errorf("Error expanding path: %s", err))
+		return
 	}
 
+	s, err = ioutil.ReadFile(path)
+	if err != nil {
+		es = append(es, fmt.Errorf("Could not read file '%s': %s", path, err))
+	}
 	return
 }
 

builtin/providers/azure/provider_test.go

@@ -3,12 +3,14 @@ package azure
 import (
 	"io"
 	"io/ioutil"
-	"log"
 	"os"
+	"strings"
 	"testing"
 
+	"github.com/hashicorp/terraform/config"
 	"github.com/hashicorp/terraform/helper/schema"
 	"github.com/hashicorp/terraform/terraform"
+	"github.com/mitchellh/go-homedir"
 )
 
 var testAccProviders map[string]terraform.ResourceProvider
@@ -67,20 +69,33 @@ func TestAzure_validateSettingsFile(t *testing.T) {
 	if err != nil {
 		t.Fatalf("Error creating temporary file in TestAzure_validateSettingsFile: %s", err)
 	}
+	defer os.Remove(f.Name())
 
 	fx, err := ioutil.TempFile("", "tf-test-xml")
 	if err != nil {
 		t.Fatalf("Error creating temporary file with XML in TestAzure_validateSettingsFile: %s", err)
 	}
+	defer os.Remove(fx.Name())
+
+	home, err := homedir.Dir()
+	if err != nil {
+		t.Fatalf("Error fetching homedir: %s", err)
+	}
+	fh, err := ioutil.TempFile(home, "tf-test-home")
+	if err != nil {
+		t.Fatalf("Error creating homedir-based temporary file: %s", err)
+	}
+	defer os.Remove(fh.Name())
 
 	_, err = io.WriteString(fx, "<PublishData></PublishData>")
 	if err != nil {
 		t.Fatalf("Error writing XML File: %s", err)
 	}
-
-	log.Printf("fx name: %s", fx.Name())
 	fx.Close()
 
+	r := strings.NewReplacer(home, "~")
+	homePath := r.Replace(fh.Name())
+
 	cases := []struct {
 		Input string // String of XML or a path to an XML file
 		W     int    // expected count of warnings
@@ -89,6 +104,7 @@ func TestAzure_validateSettingsFile(t *testing.T) {
 		{"test", 1, 1},
 		{f.Name(), 1, 0},
 		{fx.Name(), 1, 0},
+		{homePath, 1, 0},
 		{"<PublishData></PublishData>", 0, 0},
 	}
 
@@ -104,6 +120,53 @@ func TestAzure_validateSettingsFile(t *testing.T) {
 	}
 }
 
+func TestAzure_providerConfigure(t *testing.T) {
+	home, err := homedir.Dir()
+	if err != nil {
+		t.Fatalf("Error fetching homedir: %s", err)
+	}
+	fh, err := ioutil.TempFile(home, "tf-test-home")
+	if err != nil {
+		t.Fatalf("Error creating homedir-based temporary file: %s", err)
+	}
+	defer os.Remove(fh.Name())
+
+	_, err = io.WriteString(fh, testAzurePublishSettingsStr)
+	if err != nil {
+		t.Fatalf("err: %s", err)
+	}
+	fh.Close()
+
+	r := strings.NewReplacer(home, "~")
+	homePath := r.Replace(fh.Name())
+
+	cases := []struct {
+		SettingsFile string // String of XML or a path to an XML file
+		NilMeta      bool   // whether meta is expected to be nil
+	}{
+		{testAzurePublishSettingsStr, false},
+		{homePath, false},
+	}
+
+	for _, tc := range cases {
+		rp := Provider()
+		raw := map[string]interface{}{
+			"settings_file": tc.SettingsFile,
+		}
+
+		rawConfig, err := config.NewRawConfig(raw)
+		if err != nil {
+			t.Fatalf("err: %s", err)
+		}
+
+		err = rp.Configure(terraform.NewResourceConfig(rawConfig))
+		meta := rp.(*schema.Provider).Meta()
+		if (meta == nil) != tc.NilMeta {
+			t.Fatalf("expected NilMeta: %t, got meta: %#v", tc.NilMeta, meta)
+		}
+	}
+}
+
 func TestAzure_isFile(t *testing.T) {
 	f, err := ioutil.TempFile("", "tf-test-file")
 	if err != nil {
@@ -129,3 +192,19 @@ func TestAzure_isFile(t *testing.T) {
 		}
 	}
 }
+
+// testAzurePublishSettingsStr is a revoked publishsettings file
+const testAzurePublishSettingsStr = `
+<?xml version="1.0" encoding="utf-8"?>
+<PublishData>
+  <PublishProfile
+    SchemaVersion="2.0"
+    PublishMethod="AzureServiceManagementAPI">
+    <Subscription
+      ServiceManagementUrl="https://management.core.windows.net"
+      Id="a65bf94f-26b3-4fb1-9d50-6e27c6096df1"
+      Name="terraform-testing"
+      ManagementCertificate="MIIJ/AIBAzCCCbwGCSqGSIb3DQEHAaCCCa0EggmpMIIJpTCCBe4GCSqGSIb3DQEHAaCCBd8EggXbMIIF1zCCBdMGCyqGSIb3DQEMCgECoIIE7jCCBOowHAYKKoZIhvcNAQwBAzAOBAhqcsGLGr+LsQICB9AEggTImIsD3qDkT8IkH4qOlRanUFVQIWCUfXBf5U0QnXS/7N2a5fOeSou0dFuxXg81emaxecr8Myge9rBMHvLi2m4h9JIah6K/33hJhGu3nwiu+n7MzjwpjOfkc4tFMUqD1m/TAF32feq3hYqDjc3FLHrAXNIsrvaucmPipsfT/sq29xC6cWN1sUw6X43F18rqqDKyyGUuEMOJwK9s2Vir/oXlzl6bspVRJHCf0Yyo5+2GWhgcEWjzAOjIZCF7iciYj75aG3mUZjcJYT5DqUQyiyKD/LjWhiYkmHRioaCo4amyrCX92uFuZMIlHOk4LhU+UCyTn/dsvavdj8IH146u/5tUxOIsjP5hN3CcZS/TlMvX9W74uGr5BBs7EWvccUCrYyhmhFOl0YY2+99wob3VOUDSEF73VerYpFEM5POxFzjBj8K7NleB8lEuSjJXn9FbYVUpcZ/u1qhAYewFgf7KBWUTKPjGuf1b8nRVndSIaLyxSZOVbCfUtlAindZoBWjGzCa0opie1axZgouObFxHeo7ZJGjiO2q73YrZOqpPB0zOi/sycadHRKBp4O2Svz4WXBKqa2RV9oM4PYrRnH51cdFmCFqQ4eKGJCnc/Kzdwlt6ldMiCV6gsHTm44NcfPwZW5ivKZPG5aM2mad4rPpQiX+6dQz/ForKZj3WpwI+UIchc7fhwvKykCRpH/GLDBKVrjgWioKHcTDRiqOimCjLkJA+u4Qg/qHKkMOIyr75zfTEw7S9MTiYPSEnFJO60pt3rRrMU99N1Jw07Ld24SsmK4iZExLGFxYKh6jkBWV6CgqWg7qHHH3j1MZTarZSa4W1QdLjwxCQxIPU1O4L5xEa2Ki1prJyDp2E49mo6r2LDkwJrTP9GSvyGBoEpkpKVzgHsRtotikcNetsdlfDCnJiYs2Z6IvcQ2sCZaQXlofVoHZxI3OJUNvulLtuX0L8XedZtbgoFKX1u1KcgRBpae6/S+4VAjB03R+kELoC9BzicBJMifHhpOZuWqhD4zfWq1WQvBqiHI1M0GB5RDtDxxQ0IhdDJavDU6NrgNBQGxfAv1TFd/y/Mvyaq94n1+LrN0joSrxWL6QyxZF4fECGHCf0FDOHSJovkrpc6Fbc4a5mfnzIzsVeLa+m40/3rwkqs+vISCGiVwKd5xmLCmkRrae97Qh/tVRVgpFtRiUOgYVfYulhqURW4fV76giLEZydWvJUkpBxn0LNgpSHO6NJGNHtC1PoSkLEFVae1OVZaAIcshdfssCuVkuZWA3ujxkcnvzQ5uKUyRb6jF3+ID+lqzTh8hY+R+h7iLf7WRICuVedxbNa+TS+bO0/mG90eZo7An1naWy4ty9Hpn+uhKdJ3NpY8LWFZbWkHBF7IHbvlzG59GRmwJWts69y95BiqMWn4wW+1QdAdRL3WvOoMV9McVi/RQVxNskpZ65HiIq4L4VgIgx1G7Yd37zQqDEoBIxLXEq84tyXl1UVmYSt68MFBOPklUtqSiLaDgues2+l+iRjqhsDgsfZXTttxMig6W5WDsOl+xlYt+XaSiLIomjCmCy52cVlhhRjDV92Wl+RTRfi6YlHFeKtnPL1MjuIrz4c+f4PQ4JIn5TRselc6LNTbopr+DinUlz/odjM492AMYHRMBMGCSqGSIb3DQEJFTEGBAQBAAAAMFsGCSqGSIb3DQEJFDFOHkwAewA0AEQAMQBBADYANABFADQALQAzADcAQgBGAC0ANAA5AEYAMgAtADkAOQBEADEALQA1ADkANgBGAEEAMgAzADUARgBBAEQAMQB9MF0GCSsGAQQBgjcRATFQHk4ATQBpAGMAcgBvAHMAbwBmAHQAIABTAG8AZgB0AHcAYQByAGUAIABLAGUAeQAgAFMAdABvAHIAYQBnAGUAIABQAHIAbwB2AGkAZABlAHIwggOvBgkqhkiG9w0BBwagggOgMIIDnAIBADCCA5UGCSqGSIb3DQEHATAcBgoqhkiG9w0BDAEGMA4ECPLFyVJDDpzhAgIH0ICCA2hEt7WDTT87EBsNidgZgcuPvMH41IqmN3dd7Vk6RKwwO8dnLGzD6sCA9sLaQ3uKeX9WrxnBbrIzqk4yq6RRPBhW9Gegs85oldLfBsDFpyD4Wi4tQ6LBkH20/Ziy+vPZbZXDlCrrF75ruhtBQrLgtEJ6b/fj9MBw336917A9ALXKa8qcIykq5lBKTz1gRITUkIl35Ylb3kl6wB8L1hSq7jf0tuqMTREI33T3WCn8oBEPdVlgR5L4D6yVGlp62ogUnfFJ8C1V6vLiE45Z9w3ttxi3WCsG/rqz/pWkY2ctGE4Mv5ORuqwZDSChK60DbkfANpdUzqgb+Lw39CLAnmkfQMuZVJyAs/PV65yuVFmdfy5n+m2YzQNLztbsYhdyYHVrgTNrAEsy+3N0OhT3OKschHMoN4YPyu09gxHQWXuSo3X8HvoBHD6NeJ6FIdu1NJx3qCrVJPREMX30Zf6AmmWe3aIFjDz351bIc0Rc4YDAc1RRf1A/JDzeYRZrPDwdbJAj/g4oBEeZEdSmcNFxc42Tz5igTaJWyxHOkAU2iRGU17xb2diVUSCfbVsUwfiSQNcOArMl+JvLfvZp9Ye5lhZKrgTQbWdrDm9jvtCyzAxBILjjBdmQJEoJth9WlgS3ASVxarO194cqjlRvTmmNZ8kdOLt1Ybr2ZlAG2g3gOn7NQeEzyd8WBcxVCGiEyeJBvqpVSMnDGJ4VLHXsiknstr42USzAQN+t7cLOJ+J2Y0phgZ87oAixJnpEoz8z/Z65VV5syREeubiHK5uQmz3pc5qL/5LbYNT1ZqXWbDO+HXpTFJwbZ2DubNjSG1zrGNctzoRuhQidTOepyMvnlJN1PfKZoIQcA+G6PHkrNnBqo13tE9faQA8x2gvOoQYGSFi95UGlc4sTXER0+EbOCYwXkUGatQSlMLpfVXrMkRwlO6g9rC63LZC7ubqqzPPlQwdwbHTMEDxZ5ZsO21RT1JIiXfQEu/gp+HAL+Xqbsiq3Q4CCKTh04mV0Dj4a+kg6XU6BETgdwSjBbxxsbhK7yc0jlgGrNXvC72Ua7IN19zcwsrvwqtkVSc850/i1qQf066h1g/5i5Co7eIgAdRT1/S4nw5CBYGsgr5bl1ZAB2OmmkEiZqYYi3LdeYgr2yK5XcwrcPcOCWv/iN5AHhpgPqzA3MB8wBwYFKw4DAhoEFCcvtRx98fW7DF3rONM5nagH2ffMBBQi0PdBdLzm4i8p2Dhdjj4Vi0whig==" />
+  </PublishProfile>
+</PublishData>
+`