Merge pull request #3643 from paulmey/tenant_id_optional

builder/azure-arm: Make tenant_id optional

builder/azure/TODO.md

@@ -7,7 +7,7 @@ Here's a list of things we like to get done in no particular order:
 - [ ] support cross-storage account image source (ie. pre-build blob copy)
 - [ ] look up object id when using device code (graph api /me ?)
 - [ ] device flow support for Windows
-- [ ] look up tenant id in all cases (see device flow code)
+- [x] look up tenant id in all cases (see device flow code)
 - [ ] look up resource group of storage account
 - [ ] include all _data_ disks in artifact too
 - [ ] windows sysprep provisioner (since it seems to generate a certain issue volume)

builder/azure/arm/builder.go

@@ -52,7 +52,11 @@ func (b *Builder) Prepare(raws ...interface{}) ([]string, error) {
 }
 
 func (b *Builder) Run(ui packer.Ui, hook packer.Hook, cache packer.Cache) (packer.Artifact, error) {
-	ui.Say("Preparing builder ...")
+	ui.Say("Running builder ...")
+
+	if err := newConfigRetriever().FillParameters(b.config); err != nil {
+		return nil, err
+	}
 
 	log.Print(":: Configuration")
 	packerAzureCommon.DumpConfig(b.config, func(s string) { log.Print(s) })
@@ -224,7 +228,7 @@ func (b *Builder) getServicePrincipalTokens(say func(string)) (*azure.ServicePri
 	var err error
 
 	if b.config.useDeviceLogin {
-		servicePrincipalToken, err = packerAzureCommon.Authenticate(*b.config.cloudEnvironment, b.config.SubscriptionID, say)
+		servicePrincipalToken, err = packerAzureCommon.Authenticate(*b.config.cloudEnvironment, b.config.SubscriptionID, b.config.TenantID, say)
 		if err != nil {
 			return nil, nil, err
 		}

builder/azure/arm/config.go

@@ -362,10 +362,6 @@ func assertRequiredParametersSet(c *Config, errs *packer.MultiError) {
 			errs = packer.MultiErrorAppend(errs, fmt.Errorf("A client_secret must be specified"))
 		}
 
-		if c.TenantID == "" {
-			errs = packer.MultiErrorAppend(errs, fmt.Errorf("A tenant_id must be specified"))
-		}
-
 		if c.SubscriptionID == "" {
 			errs = packer.MultiErrorAppend(errs, fmt.Errorf("A subscription_id must be specified"))
 		}

builder/azure/arm/config_retriever.go

@@ -0,0 +1,26 @@
+package arm
+
+import (
+	"github.com/Azure/go-autorest/autorest/azure"
+	"github.com/mitchellh/packer/builder/azure/common"
+)
+
+type configRetriever struct {
+	// test seams
+	findTenantID func(azure.Environment, string) (string, error)
+}
+
+func newConfigRetriever() configRetriever {
+	return configRetriever{common.FindTenantID}
+}
+
+func (cr configRetriever) FillParameters(c *Config) error {
+	if c.TenantID == "" {
+		tenantID, err := cr.findTenantID(*c.cloudEnvironment, c.SubscriptionID)
+		if err != nil {
+			return err
+		}
+		c.TenantID = tenantID
+	}
+	return nil
+}

builder/azure/arm/config_retriever_test.go

@@ -0,0 +1,62 @@
+package arm
+
+import (
+	"errors"
+	"testing"
+
+	"github.com/Azure/go-autorest/autorest/azure"
+)
+
+func TestConfigRetrieverFillsTenantIDWhenEmpty(t *testing.T) {
+	c, _, _ := newConfig(getArmBuilderConfiguration(), getPackerConfiguration())
+	if expected := ""; c.TenantID != expected {
+		t.Errorf("Expected TenantID to be %q but got %q", expected, c.TenantID)
+	}
+
+	sut := newTestConfigRetriever()
+	retrievedTid := "my-tenant-id"
+	sut.findTenantID = func(azure.Environment, string) (string, error) { return retrievedTid, nil }
+	if err := sut.FillParameters(c); err != nil {
+		t.Errorf("Unexpected error when calling sut.FillParameters: %v", err)
+	}
+
+	if expected := retrievedTid; c.TenantID != expected {
+		t.Errorf("Expected TenantID to be %q but got %q", expected, c.TenantID)
+	}
+}
+
+func TestConfigRetrieverLeavesTenantIDWhenNotEmpty(t *testing.T) {
+	c, _, _ := newConfig(getArmBuilderConfiguration(), getPackerConfiguration())
+	userSpecifiedTid := "not-empty"
+	c.TenantID = userSpecifiedTid
+
+	sut := newTestConfigRetriever()
+	sut.findTenantID = nil // assert that this not even called
+	if err := sut.FillParameters(c); err != nil {
+		t.Errorf("Unexpected error when calling sut.FillParameters: %v", err)
+	}
+
+	if expected := userSpecifiedTid; c.TenantID != expected {
+		t.Errorf("Expected TenantID to be %q but got %q", expected, c.TenantID)
+	}
+}
+
+func TestConfigRetrieverReturnsErrorWhenTenantIDEmptyAndRetrievalFails(t *testing.T) {
+	c, _, _ := newConfig(getArmBuilderConfiguration(), getPackerConfiguration())
+	if expected := ""; c.TenantID != expected {
+		t.Errorf("Expected TenantID to be %q but got %q", expected, c.TenantID)
+	}
+
+	sut := newTestConfigRetriever()
+	errorString := "sorry, I failed"
+	sut.findTenantID = func(azure.Environment, string) (string, error) { return "", errors.New(errorString) }
+	if err := sut.FillParameters(c); err != nil && err.Error() != errorString {
+		t.Errorf("Unexpected error when calling sut.FillParameters: %v", err)
+	}
+}
+
+func newTestConfigRetriever() configRetriever {
+	return configRetriever{
+		findTenantID: func(azure.Environment, string) (string, error) { return "findTenantID is mocked", nil },
+	}
+}

builder/azure/arm/config_test.go

@@ -27,7 +27,6 @@ var requiredConfigValues = []string{
 	"storage_account",
 	"resource_group_name",
 	"subscription_id",
-	"tenant_id",
 }
 
 func TestConfigShouldProvideReasonableDefaultValues(t *testing.T) {
@@ -350,8 +349,8 @@ func TestUseDeviceLoginIsDisabledForWindows(t *testing.T) {
 	}
 
 	multiError, _ := err.(*packer.MultiError)
-	if len(multiError.Errors) != 3 {
-		t.Errorf("Expected to find 3 errors, but found %d errors", len(multiError.Errors))
+	if len(multiError.Errors) != 2 {
+		t.Errorf("Expected to find 2 errors, but found %d errors", len(multiError.Errors))
 	}
 
 	if !strings.Contains(err.Error(), "client_id must be specified") {
@@ -360,9 +359,6 @@ func TestUseDeviceLoginIsDisabledForWindows(t *testing.T) {
 	if !strings.Contains(err.Error(), "client_secret must be specified") {
 		t.Errorf("Expected to find error for 'client_secret must be specified")
 	}
-	if !strings.Contains(err.Error(), "tenant_id must be specified") {
-		t.Errorf("Expected to find error for 'tenant_id must be specified")
-	}
 }
 
 func TestConfigShouldRejectMalformedCaptureNamePrefix(t *testing.T) {

builder/azure/common/devicelogin.go

@@ -39,21 +39,12 @@ var (
 
 // Authenticate fetches a token from the local file cache or initiates a consent
 // flow and waits for token to be obtained.
-func Authenticate(env azure.Environment, subscriptionID string, say func(string)) (*azure.ServicePrincipalToken, error) {
+func Authenticate(env azure.Environment, subscriptionID, tenantID string, say func(string)) (*azure.ServicePrincipalToken, error) {
 	clientID, ok := clientIDs[env.Name]
 	if !ok {
 		return nil, fmt.Errorf("packer-azure application not set up for Azure environment %q", env.Name)
 	}
 
-	// First we locate the tenant ID of the subscription as we store tokens per
-	// tenant (which could have multiple subscriptions)
-	say(fmt.Sprintf("Looking up AAD Tenant ID: subscriptionID=%s.", subscriptionID))
-	tenantID, err := findTenantID(env, subscriptionID)
-	if err != nil {
-		return nil, err
-	}
-	say(fmt.Sprintf("Found AAD Tenant ID: tenantID=%s", tenantID))
-
 	oauthCfg, err := env.OAuthConfigForTenant(tenantID)
 	if err != nil {
 		return nil, fmt.Errorf("Failed to obtain oauth config for azure environment: %v", err)
@@ -205,10 +196,10 @@ func validateToken(env azure.Environment, token *azure.ServicePrincipalToken) er
 	return nil
 }
 
-// findTenantID figures out the AAD tenant ID of the subscription by making an
+// FindTenantID figures out the AAD tenant ID of the subscription by making an
 // unauthenticated request to the Get Subscription Details endpoint and parses
 // the value from WWW-Authenticate header.
-func findTenantID(env azure.Environment, subscriptionID string) (string, error) {
+func FindTenantID(env azure.Environment, subscriptionID string) (string, error) {
 	const hdrKey = "WWW-Authenticate"
 	c := subscriptionsClient(env.ResourceManagerEndpoint)
 

contrib/azure-setup.sh

@@ -174,7 +174,6 @@ showConfigs() {
 	echo "      \"resource_group_name\": \"$azure_group_name\","
 	echo "      \"storage_account\": \"$azure_storage_name\","
 	echo "      \"subscription_id\": \"$azure_subscription_id\","
-	echo "      \"tenant_id\": \"$azure_tenant_id\","
 	echo ""
 }
 

examples/azure/debian.json

@@ -5,7 +5,6 @@
     "resource_group": "{{env `ARM_RESOURCE_GROUP`}}",
     "storage_account": "{{env `ARM_STORAGE_ACCOUNT`}}",
     "subscription_id": "{{env `ARM_SUBSCRIPTION_ID`}}",
-    "tenant_id": "{{env `ARM_TENANT_ID`}}",
     "ssh_user": "packer",
     "ssh_pass": null
   },
@@ -17,7 +16,6 @@
     "resource_group_name": "{{user `resource_group`}}",
     "storage_account": "{{user `storage_account`}}",
     "subscription_id": "{{user `subscription_id`}}",
-    "tenant_id": "{{user `tenant_id`}}",
 
     "capture_container_name": "images",
     "capture_name_prefix": "packer",

examples/azure/opensuse.json

@@ -5,7 +5,6 @@
     "resource_group": "{{env `ARM_RESOURCE_GROUP`}}",
     "storage_account": "{{env `ARM_STORAGE_ACCOUNT`}}",
     "subscription_id": "{{env `ARM_SUBSCRIPTION_ID`}}",
-    "tenant_id": "{{env `ARM_TENANT_ID`}}",
     "ssh_user": "packer",
     "ssh_pass": null
   },
@@ -17,7 +16,6 @@
     "resource_group_name": "{{user `resource_group`}}",
     "storage_account": "{{user `storage_account`}}",
     "subscription_id": "{{user `subscription_id`}}",
-    "tenant_id": "{{user `tenant_id`}}",
 
     "capture_container_name": "images",
     "capture_name_prefix": "packer",

examples/azure/ubuntu.json

@@ -5,7 +5,6 @@
     "resource_group": "{{env `ARM_RESOURCE_GROUP`}}",
     "storage_account": "{{env `ARM_STORAGE_ACCOUNT`}}",
     "subscription_id": "{{env `ARM_SUBSCRIPTION_ID`}}",
-    "tenant_id": "{{env `ARM_TENANT_ID`}}"
   },
   "builders": [{
     "type": "azure-arm",
@@ -15,7 +14,6 @@
     "resource_group_name": "{{user `resource_group`}}",
     "storage_account": "{{user `storage_account`}}",
     "subscription_id": "{{user `subscription_id`}}",
-    "tenant_id": "{{user `tenant_id`}}",
 
     "capture_container_name": "images",
     "capture_name_prefix": "packer",

examples/azure/windows.json

@@ -5,7 +5,6 @@
     "resource_group": "{{env `ARM_RESOURCE_GROUP`}}",
     "storage_account": "{{env `ARM_STORAGE_ACCOUNT`}}",
     "subscription_id": "{{env `ARM_SUBSCRIPTION_ID`}}",
-    "tenant_id": "{{env `ARM_TENANT_ID`}}",
     "object_id": "{{env `ARM_OBJECT_ID`}}"
   },
   "builders": [{
@@ -17,7 +16,6 @@
     "storage_account": "{{user `storage_account`}}",
     "subscription_id": "{{user `subscription_id`}}",
     "object_id": "{{user `object_id`}}",
-    "tenant_id": "{{user `tenant_id`}}",
 
 
             "capture_container_name": "images",

website/source/docs/builders/azure-setup.html.md

@@ -9,7 +9,6 @@ page_title: Authorizing Packer Builds in Azure
 
 In order to build VMs in Azure Packer needs 6 configuration options to be specified:
 
-- `tenant_id` - UUID identifying your Azure account (where you login)
 - `subscription_id` - UUID identifying your Azure subscription (where billing is handled)
 - `client_id` - UUID identifying the Active Directory service principal that will run your Packer builds
 - `client_secret` - service principal secret / password
@@ -35,7 +34,7 @@ There are three pieces of information you must provide to enable device login mo
  1. Resource Group - parent resource group that Packer uses to build an image.
  1. Storage Account - storage account where the image will be placed.
 
-> Device login mode is enabled by not setting client_id, client_secret, and tenant_id.
+> Device login mode is enabled by not setting client_id and client_secret.
 
 The device login flow asks that you open a web browser, navigate to http://aka.ms/devicelogin, and input the supplied
 code. This authorizes the Packer for Azure application to act on your behalf. An OAuth token will be created, and stored
@@ -71,16 +70,15 @@ Get your account information
 
     azure account list --json | jq .[].name
     azure account set ACCOUNTNAME
-    azure account show --json | jq ".[] | .tenantId, .id"
+    azure account show --json | jq ".[] | .id"
 
--> Throughout this document when you see a command pipe to `jq` you may instead omit `--json` and everything after it, but the output will be more verbose. For example you can simply run `azure account list` instead._
+-> Throughout this document when you see a command pipe to `jq` you may instead omit `--json` and everything after it, but the output will be more verbose. For example you can simply run `azure account list` instead.
 
-This will print out two lines that look like this:
+This will print out one line that look like this:
 
     "4f562e88-8caf-421a-b4da-e3f6786c52ec"
-    "b68319b-2180-4c3e-ac1f-d44f5af2c6907"
 
-The first one is your `tenant_id`. The second is your `subscription_id`. Note these for later.
+This is your `subscription_id`. Note it for later.
 
 ### Create a Resource Group
 
@@ -138,9 +136,9 @@ There are a lot of pre-defined roles and you can define your own with more granu
 
 Now (finally) everything has been setup in Azure. Let's get our configuration keys together:
 
-Get `tenant_id` and `subscription_id`:
+Get `subscription_id`:
 
-    azure account show --json | jq ".[] | .tenantId, .id"
+    azure account show --json | jq ".[] | .id"
 
 Get `client_id`
 

website/source/docs/builders/azure.html.md

@@ -35,8 +35,6 @@ builder.
 
 -   `subscription_id` (string) Subscription under which the build will be performed. **The service principal specified in `client_id` must have full access to this subscription.**
 
--   `tenant_id` (string) The account identifier with which your `client_id` and `subscription_id` are associated.
-
 -   `capture_container_name` (string) Destination container name. Essentially the "folder" where your VHD will be organized in Azure.
 
 -   `capture_name_prefix` (string) VHD prefix. The final artifacts will be named `PREFIX-osDisk.UUID` and `PREFIX-vmTemplate.UUID`.
@@ -72,6 +70,8 @@ builder.
 -   `image_url` (string) Specify a custom VHD to use.  If this value is set, do not set image_publisher, image_offer,
      image_sku, or image_version.
 
+-   `tenant_id` (string) The account identifier with which your `client_id` and `subscription_id` are associated. If not specified, `tenant_id` will be looked up using `subscription_id`.
+
 -   `object_id` (string) Specify an OAuth Object ID to protect WinRM certificates
     created at runtime.  This variable is required when creating images based on
     Windows; this variable is not used by non-Windows builds.  See `Windows`