From cb3d62483487f3d807c53e6a8c789f3ae6a6cc25 Mon Sep 17 00:00:00 2001
From: Tom Bamford <tom@bamford.io>
Date: Mon, 27 Sep 2021 12:33:42 +0100
Subject: [PATCH] Bugfix: allow service principal with no features / all
 features disabled

---
 .github/ISSUE_TEMPLATE/Bug_Report.md          | 46 +++++++++++------
 .../service_principal_resource_test.go        | 50 +++++++++++++++++++
 .../serviceprincipals/serviceprincipals.go    |  8 +--
 3 files changed, 87 insertions(+), 17 deletions(-)

diff --git a/.github/ISSUE_TEMPLATE/Bug_Report.md b/.github/ISSUE_TEMPLATE/Bug_Report.md
index 6a8e9e9eda..ea3fdda776 100644
--- a/.github/ISSUE_TEMPLATE/Bug_Report.md
+++ b/.github/ISSUE_TEMPLATE/Bug_Report.md
@@ -1,22 +1,28 @@
 ---
 name: 🐛 Bug Report
-about: If something isn't working as expected 🤔.
+about: If something isn't working as expected 🤔
 
 ---
 
-<!---
+<!--
+
+Thank you for reporting a bug to the Terraform AzureAD Provider! We're grateful for your effort and any information you can provide that will help us improve the provider.
+
 Please note the following potential times when an issue might be in Terraform core:
 
-* [Configuration Language](https://www.terraform.io/docs/configuration/index.html) or resource ordering issues
-* [State](https://www.terraform.io/docs/state/index.html) and [State Backend](https://www.terraform.io/docs/backends/index.html) issues
-* [Provisioner](https://www.terraform.io/docs/provisioners/index.html) issues
-* [Registry](https://registry.terraform.io/) issues
-* Spans resources across multiple providers
+* Configuration Language or ordering issues: https://www.terraform.io/docs/configuration/index.html
+* State issues: https://www.terraform.io/docs/state/index.html
+* State Backend issues: https://www.terraform.io/docs/backends/index.html
+* Provisioner issues: https://www.terraform.io/docs/provisioners/index.html
+* Registry issues: https://registry.terraform.io/
+* Any resources which do not belong to this provider, e.g. should begin with `azuread_`
 
-If you are running into one of these scenarios, we recommend opening an issue in the [Terraform core repository](https://github.com/hashicorp/terraform/) instead.
---->
+If you are running into one of these scenarios, we recommend instead opening an issue in the Terraform core repository (https://github.com/hashicorp/terraform/),
+or if your issue is about a resource from another provider, open an issue in that provider's GitHub repository.
+
+-->
 
-<!--- Please keep this note for the community --->
+<!-- Please keep the following note for the community -->
 
 ### Community Note
 
@@ -24,21 +30,33 @@ If you are running into one of these scenarios, we recommend opening an issue in
 * Please do not leave "+1" or "me too" comments, they generate extra noise for issue followers and do not help prioritise the request
 * If you are interested in working on this issue or have submitted a pull request, please leave a comment
 
-<!--- Thank you for keeping this note for the community --->
+<!-- Thank you for keeping this note for the community! -->
 
 ### Terraform (and AzureAD Provider) Version
 
-<!--- Please run `terraform -v` to show the Terraform core version and provider version(s). If you are not running the latest version of Terraform or the provider, please upgrade because your issue may have already been fixed. [Terraform documentation on provider versioning](https://www.terraform.io/docs/configuration/providers.html#provider-versions). --->
+<!--
+
+Please run `terraform -v` to show the Terraform core version and provider version(s). If you are not running the latest version of Terraform or the provider, please upgrade because your issue may have already been fixed.
+
+Terraform documentation on provider versioning: https://www.terraform.io/docs/configuration/providers.html#provider-versions
+
+-->
 
 ### Affected Resource(s)
 
-<!--- Please list the affected resources and data sources. --->
+<!-- Please list the affected resources and data sources -->
 
 * `azuread_XXXXX`
 
 ### Terraform Configuration Files
 
-<!--- Information about code formatting: https://help.github.com/articles/basic-writing-and-formatting-syntax/#quoting-code --->
+<!--
+
+Please format your code using markdown syntax. Code that is not formatted does not display correctly and is difficult to read.
+
+Information about code formatting: https://help.github.com/articles/basic-writing-and-formatting-syntax/#quoting-code
+
+-->
 
 ```hcl
 # Copy-paste your Terraform configurations here - for large Terraform configs,
diff --git a/internal/services/serviceprincipals/service_principal_resource_test.go b/internal/services/serviceprincipals/service_principal_resource_test.go
index ba843f8423..bddb7e16e5 100644
--- a/internal/services/serviceprincipals/service_principal_resource_test.go
+++ b/internal/services/serviceprincipals/service_principal_resource_test.go
@@ -136,6 +136,13 @@ func TestAccServicePrincipal_featuresUpdate(t *testing.T) {
 	r := ServicePrincipalResource{}
 
 	data.ResourceTest(t, r, []resource.TestStep{
+		{
+			Config: r.noFeatures(data),
+			Check: resource.ComposeTestCheckFunc(
+				check.That(data.ResourceName).ExistsInAzure(r),
+			),
+		},
+		data.ImportStep("use_existing"),
 		{
 			Config: r.basic(data),
 			Check: resource.ComposeTestCheckFunc(
@@ -178,6 +185,20 @@ func TestAccServicePrincipal_featuresUpdate(t *testing.T) {
 			),
 		},
 		data.ImportStep("use_existing"),
+		{
+			Config: r.noFeatures(data),
+			Check: resource.ComposeTestCheckFunc(
+				check.That(data.ResourceName).ExistsInAzure(r),
+			),
+		},
+		data.ImportStep("use_existing"),
+		{
+			Config: r.features(data),
+			Check: resource.ComposeTestCheckFunc(
+				check.That(data.ResourceName).ExistsInAzure(r),
+			),
+		},
+		data.ImportStep("use_existing"),
 	})
 }
 
@@ -464,6 +485,35 @@ resource "azuread_service_principal" "test" {
 `, r.templateComplete(data), data.RandomInteger)
 }
 
+func (r ServicePrincipalResource) noFeatures(data acceptance.TestData) string {
+	return fmt.Sprintf(`
+%[1]s
+
+resource "azuread_service_principal" "test" {
+  application_id = azuread_application.test.application_id
+
+  account_enabled               = false
+  alternative_names             = ["foo", "bar"]
+  app_role_assignment_required  = true
+  description                   = "An internal app for testing"
+  login_url                     = "https://test-%[2]d.internal/login"
+  notes                         = "Just testing something"
+  preferred_single_sign_on_mode = "saml"
+
+  features {
+    custom_single_sign_on_app = false
+    enterprise_application    = false
+    gallery_application       = false
+  }
+
+  notification_email_addresses = [
+    "alerts.internal@hashitown.net",
+    "cto@hashitown.net",
+  ]
+}
+`, r.templateComplete(data), data.RandomInteger)
+}
+
 func (ServicePrincipalResource) templateThreeUsers(data acceptance.TestData) string {
 	return fmt.Sprintf(`
 provider "azuread" {}
diff --git a/internal/services/serviceprincipals/serviceprincipals.go b/internal/services/serviceprincipals/serviceprincipals.go
index 71fe5bb198..761142e4e5 100644
--- a/internal/services/serviceprincipals/serviceprincipals.go
+++ b/internal/services/serviceprincipals/serviceprincipals.go
@@ -7,9 +7,11 @@ import (
 	"github.com/manicminer/hamilton/msgraph"
 )
 
-func expandFeatures(in []interface{}) (out []string) {
+func expandFeatures(in []interface{}) []string {
+	out := make([]string, 0)
+
 	if len(in) == 0 || in[0] == nil {
-		return
+		return out
 	}
 
 	features := in[0].(map[string]interface{})
@@ -30,7 +32,7 @@ func expandFeatures(in []interface{}) (out []string) {
 		out = append(out, "HideApp")
 	}
 
-	return
+	return out
 }
 
 func expandSamlSingleSignOn(in []interface{}) *msgraph.SamlSingleSignOnSettings {