Skip to content

Commit

Permalink
provider/azure: read publish_settings as contents instead of path
Browse files Browse the repository at this point in the history
Building on the work in #3846, shifting the Azure provider's
configuration option from `settings_file` to `publish_settings`.
  • Loading branch information
phinze committed Nov 12, 2015
1 parent b6aed3f commit 4588241
Show file tree
Hide file tree
Showing 3 changed files with 68 additions and 68 deletions.
68 changes: 40 additions & 28 deletions builtin/providers/azure/provider.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,12 +3,10 @@ package azure
import (
"encoding/xml"
"fmt"
"io/ioutil"
"os"

"github.com/hashicorp/terraform/helper/pathorcontents"
"github.com/hashicorp/terraform/helper/schema"
"github.com/hashicorp/terraform/terraform"
"github.com/mitchellh/go-homedir"
)

// Provider returns a terraform.ResourceProvider.
Expand All @@ -20,6 +18,14 @@ func Provider() terraform.ResourceProvider {
Optional: true,
DefaultFunc: schema.EnvDefaultFunc("AZURE_SETTINGS_FILE", nil),
ValidateFunc: validateSettingsFile,
Deprecated: "Use the publish_settings field instead",
},

"publish_settings": &schema.Schema{
Type: schema.TypeString,
Optional: true,
DefaultFunc: schema.EnvDefaultFunc("AZURE_PUBLISH_SETTINGS", nil),
ValidateFunc: validatePublishSettings,
},

"subscription_id": &schema.Schema{
Expand Down Expand Up @@ -64,11 +70,14 @@ func providerConfigure(d *schema.ResourceData) (interface{}, error) {
Certificate: []byte(d.Get("certificate").(string)),
}

settingsFile := d.Get("settings_file").(string)
if settingsFile != "" {
publishSettings := d.Get("publish_settings").(string)
if publishSettings == "" {
publishSettings = d.Get("settings_file").(string)
}
if publishSettings != "" {
// any errors from readSettings would have been caught at the validate
// step, so we can avoid handling them now
settings, _, _ := readSettings(settingsFile)
settings, _, _ := readSettings(publishSettings)
config.Settings = settings
return config.NewClientFromSettingsData()
}
Expand All @@ -92,37 +101,40 @@ func validateSettingsFile(v interface{}, k string) ([]string, []error) {
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.`

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)
func validatePublishSettings(v interface{}, k string) (ws []string, es []error) {
value := v.(string)
if value == "" {
return
}

ws = append(ws, settingsPathWarnMsg)
path, err := homedir.Expand(pathOrContents)
if err != nil {
es = append(es, fmt.Errorf("Error expanding path: %s", err))
return
var settings settingsData
if err := xml.Unmarshal([]byte(value), &settings); err != nil {
es = append(es, fmt.Errorf("error parsing publish_settings as XML: %s", err))
}

s, err = ioutil.ReadFile(path)
if err != nil {
es = append(es, fmt.Errorf("Could not read file '%s': %s", path, err))
}
return
}

func isFile(v string) (bool, error) {
if _, err := os.Stat(v); err != nil {
return false, err
const settingsPathWarnMsg = `
settings_file was provided as a file path. This support
will be removed in the future. Please update your configuration
to use ${file("filename.publishsettings")} instead.`

func readSettings(pathOrContents string) (s []byte, ws []string, es []error) {
contents, wasPath, err := pathorcontents.Read(pathOrContents)
if err != nil {
es = append(es, fmt.Errorf("error reading settings_file: %s", err))
}
return true, nil
if wasPath {
ws = append(ws, settingsPathWarnMsg)
}

var settings settingsData
if err := xml.Unmarshal([]byte(contents), &settings); err != nil {
es = append(es, fmt.Errorf("error parsing settings_file as XML: %s", err))
}

return
}

// settingsData is a private struct used to test the unmarshalling of the
Expand Down
48 changes: 13 additions & 35 deletions builtin/providers/azure/provider_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -51,12 +51,12 @@ func TestProvider_impl(t *testing.T) {
}

func testAccPreCheck(t *testing.T) {
if v := os.Getenv("AZURE_SETTINGS_FILE"); v == "" {
if v := os.Getenv("AZURE_PUBLISH_SETTINGS"); v == "" {
subscriptionID := os.Getenv("AZURE_SUBSCRIPTION_ID")
certificate := os.Getenv("AZURE_CERTIFICATE")

if subscriptionID == "" || certificate == "" {
t.Fatal("either AZURE_SETTINGS_FILE, or AZURE_SUBSCRIPTION_ID " +
t.Fatal("either AZURE_PUBLISH_SETTINGS, or AZURE_SUBSCRIPTION_ID " +
"and AZURE_CERTIFICATE must be set for acceptance tests")
}
}
Expand All @@ -78,6 +78,11 @@ func TestAzure_validateSettingsFile(t *testing.T) {
t.Fatalf("Error creating temporary file with XML in TestAzure_validateSettingsFile: %s", err)
}
defer os.Remove(fx.Name())
_, err = io.WriteString(fx, "<PublishData></PublishData>")
if err != nil {
t.Fatalf("Error writing XML File: %s", err)
}
fx.Close()

home, err := homedir.Dir()
if err != nil {
Expand All @@ -88,12 +93,11 @@ func TestAzure_validateSettingsFile(t *testing.T) {
t.Fatalf("Error creating homedir-based temporary file: %s", err)
}
defer os.Remove(fh.Name())

_, err = io.WriteString(fx, "<PublishData></PublishData>")
_, err = io.WriteString(fh, "<PublishData></PublishData>")
if err != nil {
t.Fatalf("Error writing XML File: %s", err)
}
fx.Close()
fh.Close()

r := strings.NewReplacer(home, "~")
homePath := r.Replace(fh.Name())
Expand All @@ -103,8 +107,8 @@ func TestAzure_validateSettingsFile(t *testing.T) {
W int // expected count of warnings
E int // expected count of errors
}{
{"test", 1, 1},
{f.Name(), 1, 0},
{"test", 0, 1},
{f.Name(), 1, 1},
{fx.Name(), 1, 0},
{homePath, 1, 0},
{"<PublishData></PublishData>", 0, 0},
Expand All @@ -114,10 +118,10 @@ func TestAzure_validateSettingsFile(t *testing.T) {
w, e := validateSettingsFile(tc.Input, "")

if len(w) != tc.W {
t.Errorf("Error in TestAzureValidateSettingsFile: input: %s , warnings: %#v, errors: %#v", tc.Input, w, e)
t.Errorf("Error in TestAzureValidateSettingsFile: input: %s , warnings: %v, errors: %v", tc.Input, w, e)
}
if len(e) != tc.E {
t.Errorf("Error in TestAzureValidateSettingsFile: input: %s , warnings: %#v, errors: %#v", tc.Input, w, e)
t.Errorf("Error in TestAzureValidateSettingsFile: input: %s , warnings: %v, errors: %v", tc.Input, w, e)
}
}
}
Expand Down Expand Up @@ -169,32 +173,6 @@ func TestAzure_providerConfigure(t *testing.T) {
}
}

func TestAzure_isFile(t *testing.T) {
f, err := ioutil.TempFile("", "tf-test-file")
if err != nil {
t.Fatalf("Error creating temporary file with XML in TestAzure_isFile: %s", err)
}
cases := []struct {
Input string // String path to file
B bool // expected true/false
E bool // expect error
}{
{"test", false, true},
{f.Name(), true, false},
}

for _, tc := range cases {
x, y := isFile(tc.Input)
if tc.B != x {
t.Errorf("Error in TestAzure_isFile: input: %s , returned: %#v, expected: %#v", tc.Input, x, tc.B)
}

if tc.E != (y != nil) {
t.Errorf("Error in TestAzure_isFile: input: %s , returned: %#v, expected: %#v", tc.Input, y, tc.E)
}
}
}

func genRandInt() int {
return rand.New(rand.NewSource(time.Now().UnixNano())).Int() % 100000
}
Expand Down
20 changes: 15 additions & 5 deletions website/source/docs/providers/azure/index.html.markdown
Original file line number Diff line number Diff line change
Expand Up @@ -33,11 +33,11 @@ resource "azure_instance" "web" {

The following arguments are supported:

* `settings_file` - (Optional) Contents of a valid `publishsettings` file, used to
authenticate with the Azure API. You can download the settings file here:
https://manage.windowsazure.com/publishsettings. You must either provide
(or source from the `AZURE_SETTINGS_FILE` environment variable) a settings
file or both a `subscription_id` and `certificate`.
* `publish_settings` - (Optional) Contents of a valid `publishsettings` file,
used to authenticate with the Azure API. You can download the settings file
here: https://manage.windowsazure.com/publishsettings. You must either
provide publish settings or both a `subscription_id` and `certificate`. It
can also be sourced from the `AZURE_PUBLISH_SETTINGS` environment variable.

* `subscription_id` - (Optional) The subscription ID to use. If a
`settings_file` is not provided `subscription_id` is required. It can also
Expand All @@ -47,6 +47,16 @@ The following arguments are supported:
Azure API. If a `settings_file` is not provided `certificate` is required.
It can also be sourced from the `AZURE_CERTIFICATE` environment variable.

These arguments are supported for backwards compatibility, and may be removed
in a future version:

* `settings_file` - __Deprecated: please use `publish_settings` instead.__
Path to or contents of a valid `publishsettings` file, used to
authenticate with the Azure API. You can download the settings file here:
https://manage.windowsazure.com/publishsettings. You must either provide
(or source from the `AZURE_SETTINGS_FILE` environment variable) a settings
file or both a `subscription_id` and `certificate`.

## Testing:

The following environment variables must be set for the running of the
Expand Down

0 comments on commit 4588241

Please sign in to comment.