From 2a5c18d88b196029f512f13a3c6006fef384539d Mon Sep 17 00:00:00 2001 From: Clint Shryock Date: Mon, 3 Aug 2015 15:12:30 -0500 Subject: [PATCH 1/2] provider/azure: Allow settings_file to accept XML string --- builtin/providers/azure/config.go | 13 +--- builtin/providers/azure/provider.go | 76 ++++++++++++++++--- builtin/providers/azure/provider_test.go | 71 +++++++++++++++++ .../azure/resource_azure_instance_test.go | 15 ++-- .../docs/providers/azure/index.html.markdown | 4 +- 5 files changed, 151 insertions(+), 28 deletions(-) diff --git a/builtin/providers/azure/config.go b/builtin/providers/azure/config.go index fff31853d..cbb23d58b 100644 --- a/builtin/providers/azure/config.go +++ b/builtin/providers/azure/config.go @@ -2,7 +2,6 @@ package azure import ( "fmt" - "os" "sync" "github.com/Azure/azure-sdk-for-go/management" @@ -22,7 +21,7 @@ import ( // Config is the configuration structure used to instantiate a // new Azure management client. type Config struct { - SettingsFile string + Settings []byte SubscriptionID string Certificate []byte ManagementURL string @@ -96,14 +95,8 @@ func (c Client) getStorageServiceQueueClient(serviceName string) (storage.QueueS return storageClient.GetQueueService(), err } -// NewClientFromSettingsFile returns a new Azure management -// client created using a publish settings file. -func (c *Config) NewClientFromSettingsFile() (*Client, error) { - if _, err := os.Stat(c.SettingsFile); os.IsNotExist(err) { - return nil, fmt.Errorf("Publish Settings file %q does not exist!", c.SettingsFile) - } - - mc, err := management.ClientFromPublishSettingsFile(c.SettingsFile, c.SubscriptionID) +func (c *Config) NewClientFromSettingsData() (*Client, error) { + mc, err := management.ClientFromPublishSettingsData(c.Settings, c.SubscriptionID) if err != nil { return nil, nil } diff --git a/builtin/providers/azure/provider.go b/builtin/providers/azure/provider.go index 2c44b54f6..46b3028d5 100644 --- a/builtin/providers/azure/provider.go +++ b/builtin/providers/azure/provider.go @@ -1,7 +1,10 @@ package azure import ( + "encoding/xml" "fmt" + "io/ioutil" + "os" "github.com/hashicorp/terraform/helper/schema" "github.com/hashicorp/terraform/terraform" @@ -13,9 +16,10 @@ func Provider() terraform.ResourceProvider { return &schema.Provider{ Schema: map[string]*schema.Schema{ "settings_file": &schema.Schema{ - Type: schema.TypeString, - Optional: true, - DefaultFunc: schema.EnvDefaultFunc("AZURE_SETTINGS_FILE", nil), + Type: schema.TypeString, + Required: true, + DefaultFunc: schema.EnvDefaultFunc("AZURE_SETTINGS_FILE", nil), + ValidateFunc: validateSettingsFile, }, "subscription_id": &schema.Schema{ @@ -55,19 +59,28 @@ func Provider() terraform.ResourceProvider { } func providerConfigure(d *schema.ResourceData) (interface{}, error) { - settingsFile, err := homedir.Expand(d.Get("settings_file").(string)) - if err != nil { - return nil, fmt.Errorf("Error expanding the settings file path: %s", err) - } - config := Config{ - SettingsFile: settingsFile, SubscriptionID: d.Get("subscription_id").(string), Certificate: []byte(d.Get("certificate").(string)), } - if config.SettingsFile != "" { - return config.NewClientFromSettingsFile() + 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) + } + return config.NewClientFromSettingsData() } if config.SubscriptionID != "" && len(config.Certificate) > 0 { @@ -78,3 +91,44 @@ func providerConfigure(d *schema.ResourceData) (interface{}, error) { "Insufficient configuration data. Please specify either a 'settings_file'\n" + "or both a 'subscription_id' and 'certificate'.") } + +func validateSettingsFile(v interface{}, k string) (warnings []string, errors []error) { + value := v.(string) + + if value == "" { + return + } + + var settings settingsData + if err := xml.Unmarshal([]byte(value), &settings); err != nil { + warnings = append(warnings, ` +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 { + return + } + + if ok, err := isFile(value); !ok { + errors = append(errors, + fmt.Errorf( + "account_file path could not be read from '%s': %s", + value, + err)) + } + + return +} + +func isFile(v string) (bool, error) { + if _, err := os.Stat(v); err != nil { + return false, err + } + return true, nil +} + +// settingsData is a private struct used to test the unmarshalling of the +// settingsFile contents, to determine if the contents are valid XML +type settingsData struct { + XMLName xml.Name `xml:"PublishData"` +} diff --git a/builtin/providers/azure/provider_test.go b/builtin/providers/azure/provider_test.go index 3d5ec342c..5c720640f 100644 --- a/builtin/providers/azure/provider_test.go +++ b/builtin/providers/azure/provider_test.go @@ -1,6 +1,9 @@ package azure import ( + "io" + "io/ioutil" + "log" "os" "testing" @@ -58,3 +61,71 @@ func testAccPreCheck(t *testing.T) { t.Fatal("AZURE_STORAGE must be set for acceptance tests") } } + +func TestAzure_validateSettingsFile(t *testing.T) { + f, err := ioutil.TempFile("", "tf-test") + if err != nil { + t.Fatalf("Error creating temporary file in TestAzure_validateSettingsFile: %s", err) + } + + fx, err := ioutil.TempFile("", "tf-test-xml") + if err != nil { + t.Fatalf("Error creating temporary file with XML in TestAzure_validateSettingsFile: %s", err) + } + + _, err = io.WriteString(fx, "") + if err != nil { + t.Fatalf("Error writing XML File: %s", err) + } + + log.Printf("fx name: %s", fx.Name()) + fx.Close() + + cases := []struct { + Input string // String of XML or a path to an XML file + W int // expected count of warnings + E int // expected count of errors + }{ + {"test", 1, 1}, + {f.Name(), 1, 0}, + {fx.Name(), 1, 0}, + {"", 0, 0}, + } + + for _, tc := range cases { + 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) + } + if len(e) != tc.E { + t.Errorf("Error in TestAzureValidateSettingsFile: input: %s , warnings: %#v, errors: %#v", tc.Input, w, e) + } + } +} + +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) + } + } +} diff --git a/builtin/providers/azure/resource_azure_instance_test.go b/builtin/providers/azure/resource_azure_instance_test.go index 679aa015b..3bc09b230 100644 --- a/builtin/providers/azure/resource_azure_instance_test.go +++ b/builtin/providers/azure/resource_azure_instance_test.go @@ -2,7 +2,9 @@ package azure import ( "fmt" + "math/rand" "testing" + "time" "github.com/Azure/azure-sdk-for-go/management" "github.com/Azure/azure-sdk-for-go/management/virtualmachine" @@ -10,6 +12,9 @@ import ( "github.com/hashicorp/terraform/terraform" ) +var randInt = rand.New(rand.NewSource(time.Now().UnixNano())).Int() +var instanceName = fmt.Sprintf("terraform-test-%d", randInt) + func TestAccAzureInstance_basic(t *testing.T) { var dpmt virtualmachine.DeploymentResponse @@ -25,9 +30,9 @@ func TestAccAzureInstance_basic(t *testing.T) { "azure_instance.foo", "", &dpmt), testAccCheckAzureInstanceBasicAttributes(&dpmt), resource.TestCheckResourceAttr( - "azure_instance.foo", "name", "terraform-test"), + "azure_instance.foo", "name", instanceName), resource.TestCheckResourceAttr( - "azure_instance.foo", "hosted_service_name", "terraform-test"), + "azure_instance.foo", "hosted_service_name", instanceName), resource.TestCheckResourceAttr( "azure_instance.foo", "location", "West US"), resource.TestCheckResourceAttr( @@ -194,7 +199,7 @@ func testAccCheckAzureInstanceBasicAttributes( dpmt *virtualmachine.DeploymentResponse) resource.TestCheckFunc { return func(s *terraform.State) error { - if dpmt.Name != "terraform-test" { + if dpmt.Name != instanceName { return fmt.Errorf("Bad name: %s", dpmt.Name) } @@ -363,7 +368,7 @@ func testAccCheckAzureInstanceDestroyed(hostedServiceName string) resource.TestC var testAccAzureInstance_basic = fmt.Sprintf(` resource "azure_instance" "foo" { - name = "terraform-test" + name = "%s" image = "Ubuntu Server 14.04 LTS" size = "Basic_A1" storage_service_name = "%s" @@ -377,7 +382,7 @@ resource "azure_instance" "foo" { public_port = 22 private_port = 22 } -}`, testAccStorageServiceName) +}`, instanceName, testAccStorageServiceName) var testAccAzureInstance_seperateHostedService = fmt.Sprintf(` resource "azure_hosted_service" "foo" { diff --git a/website/source/docs/providers/azure/index.html.markdown b/website/source/docs/providers/azure/index.html.markdown index eec8494a1..0e88cf7c7 100644 --- a/website/source/docs/providers/azure/index.html.markdown +++ b/website/source/docs/providers/azure/index.html.markdown @@ -20,7 +20,7 @@ Use the navigation to the left to read about the available resources. ``` # Configure the Azure Provider provider "azure" { - settings_file = "${var.azure_settings_file}" + settings_file = "${file("credentials.publishsettings")}" } # Create a web server @@ -33,7 +33,7 @@ resource "azure_instance" "web" { The following arguments are supported: -* `settings_file` - (Optional) The path to a publish settings file used to +* `settings_file` - (Required) 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 From a7543de39377db72977d9182e3764392409689e7 Mon Sep 17 00:00:00 2001 From: Clint Shryock Date: Mon, 3 Aug 2015 15:34:34 -0500 Subject: [PATCH 2/2] settings file is not required --- builtin/providers/azure/provider.go | 2 +- website/source/docs/providers/azure/index.html.markdown | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/builtin/providers/azure/provider.go b/builtin/providers/azure/provider.go index 46b3028d5..fe100be35 100644 --- a/builtin/providers/azure/provider.go +++ b/builtin/providers/azure/provider.go @@ -17,7 +17,7 @@ func Provider() terraform.ResourceProvider { Schema: map[string]*schema.Schema{ "settings_file": &schema.Schema{ Type: schema.TypeString, - Required: true, + Optional: true, DefaultFunc: schema.EnvDefaultFunc("AZURE_SETTINGS_FILE", nil), ValidateFunc: validateSettingsFile, }, diff --git a/website/source/docs/providers/azure/index.html.markdown b/website/source/docs/providers/azure/index.html.markdown index 0e88cf7c7..5d7afdd20 100644 --- a/website/source/docs/providers/azure/index.html.markdown +++ b/website/source/docs/providers/azure/index.html.markdown @@ -33,7 +33,7 @@ resource "azure_instance" "web" { The following arguments are supported: -* `settings_file` - (Required) Contents of a valid `publishsettings` file, used to +* `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