Sort ACM cert subject alternative names and domain validation opts

aws/resource_aws_acm_certificate.go

@@ -210,15 +210,16 @@ func resourceAwsAcmCertificateRead(d *schema.ResourceData, meta interface{}) err
 		d.Set("domain_name", resp.Certificate.DomainName)
 		d.Set("arn", resp.Certificate.CertificateArn)
 
-		if err := d.Set("subject_alternative_names", cleanUpSubjectAlternativeNames(resp.Certificate)); err != nil {
+		sans := readSubjectAlternativeNames(resp.Certificate, d)
+		if err := d.Set("subject_alternative_names", sans); err != nil {
 			return resource.NonRetryableError(err)
 		}
 
 		domainValidationOptions, emailValidationOptions, err := convertValidationOptions(resp.Certificate)
-
 		if err != nil {
 			return resource.RetryableError(err)
 		}
+		sortDomainValidationOptions(d.Get("domain_name").(*string), sans, domainValidationOptions)
 
 		if err := d.Set("domain_validation_options", domainValidationOptions); err != nil {
 			return resource.NonRetryableError(err)
@@ -244,6 +245,7 @@ func resourceAwsAcmCertificateRead(d *schema.ResourceData, meta interface{}) err
 		return nil
 	})
 }
+
 func resourceAwsAcmCertificateGuessValidationMethod(domainValidationOptions []map[string]interface{}, emailValidationOptions []string) string {
 	// The DescribeCertificate Response doesn't have information on what validation method was used
 	// so we need to guess from the validation options we see...
@@ -275,16 +277,43 @@ func resourceAwsAcmCertificateUpdate(d *schema.ResourceData, meta interface{}) e
 	return resourceAwsAcmCertificateRead(d, meta)
 }
 
-func cleanUpSubjectAlternativeNames(cert *acm.CertificateDetail) []string {
-	sans := cert.SubjectAlternativeNames
-	vs := make([]string, 0)
-	for _, v := range sans {
-		if aws.StringValue(v) != aws.StringValue(cert.DomainName) {
-			vs = append(vs, aws.StringValue(v))
+// readSubjectAlternativeNames returns all subject alternative names (SANs)
+// fetched from remote in cert but ordered according to user-defined state.
+// Also, the domain_name is excluded from the result.
+//
+// Sorting keeps the state stable and is necessary because the API may return
+// SANS ordred differetnly from request to request.
+func readSubjectAlternativeNames(cert *acm.CertificateDetail, d *schema.ResourceData) []string {
+	remotes := cert.SubjectAlternativeNames
+	sans := make([]string, 0)
+
+	// intersection of all SANs, user defined (ud) and remote. ordered as user defined (ud) in state.
+	for _, ud := range d.Get("subject_alternative_names").([]interface{}) {
+		san := ud.(string)
+		for _, r := range remotes {
+			if san == aws.StringValue(r) {
+				sans = append(sans, san)
+				break
+			}
+		}
+	}
+
+	// append SANs exsting only in slice fetched from remote. exclude domainName.
+	for _, r := range remotes {
+		san := aws.StringValue(r)
+		found := false
+		for _, ud := range sans {
+			if san == ud {
+				found = true
+				break
+			}
+		}
+		if !found && san != aws.StringValue(cert.DomainName) {
+			sans = append(sans, san)
 		}
 	}
-	return vs
 
+	return sans
 }
 
 func convertValidationOptions(certificate *acm.CertificateDetail) ([]map[string]interface{}, []string, error) {
@@ -315,6 +344,29 @@ func convertValidationOptions(certificate *acm.CertificateDetail) ([]map[string]
 	return domainValidationResult, emailValidationResult, nil
 }
 
+// sortDomainValidationOptions sorts a slice of domain validation options
+// according to the order of domain names in the subject alternatnive names slice
+func sortDomainValidationOptions(domainName *string, subjectAlterntaiveNames []string, domainValidationOptions []map[string]interface{}) {
+	sans := append([]string{*domainName}, subjectAlterntaiveNames...)
+
+	// validation method is email and domainValidationOptions is empty
+	if len(sans) != len(domainValidationOptions) {
+		return
+	}
+
+	// This works because the requesting a certificate is either by email or by dns,
+	// but not mixed and furthermore the method cannot be changed after creation.
+	for i, san := range sans {
+		for j, opt := range domainValidationOptions {
+			if san == opt["domain_name"].(string) {
+				domainValidationOptions[i], domainValidationOptions[j] =
+					domainValidationOptions[j], domainValidationOptions[i]
+				break
+			}
+		}
+	}
+}
+
 func resourceAwsAcmCertificateDelete(d *schema.ResourceData, meta interface{}) error {
 	acmconn := meta.(*AWSClient).acmconn
 

aws/resource_aws_acm_certificate_test.go

@@ -13,6 +13,7 @@ import (
 	"github.com/aws/aws-sdk-go/service/acm"
 	"github.com/hashicorp/terraform/helper/acctest"
 	"github.com/hashicorp/terraform/helper/resource"
+	terraformSchema "github.com/hashicorp/terraform/helper/schema"
 	"github.com/hashicorp/terraform/terraform"
 )
 
@@ -650,3 +651,197 @@ func testAccCheckAcmCertificateDestroy(s *terraform.State) error {
 
 	return nil
 }
+
+func TestSortDomainValidationOptions(t *testing.T) {
+	domainName := "www.example.com"
+
+	data := []map[string][]string{
+		{
+			"domainValidationOptions": {},
+			"subjectAlternativeNames": {},
+			"expectedResult":          {},
+		},
+		{
+			"domainValidationOptions": {"www.example.com"},
+			"subjectAlternativeNames": {},
+			"expectedResult":          {"www.example.com"},
+		},
+		{
+			"domainValidationOptions": {"www.example.com"},
+			"subjectAlternativeNames": {"a.example.com"},
+			"expectedResult":          {"www.example.com"},
+		},
+		{
+			"domainValidationOptions": {"www.example.com", "a.example.com", "b.example.com"},
+			"subjectAlternativeNames": {"a.example.com", "b.example.com"},
+			"expectedResult":          {"www.example.com", "a.example.com", "b.example.com"},
+		},
+		{
+			"domainValidationOptions": {"a.example.com", "b.example.com", "www.example.com", "c.example.com"},
+			"subjectAlternativeNames": {"a.example.com", "b.example.com", "c.example.com"},
+			"expectedResult":          {"www.example.com", "a.example.com", "b.example.com", "c.example.com"},
+		},
+		{
+			"domainValidationOptions": {"a.example.com", "b.example.com", "www.example.com", "c.example.com"},
+			"subjectAlternativeNames": {"c.example.com", "b.example.com", "a.example.com"},
+			"expectedResult":          {"www.example.com", "c.example.com", "b.example.com", "a.example.com"},
+		},
+		{
+			"domainValidationOptions": {"y.example.com", "b.example.com", "www.example.com", "a.example.com"},
+			"subjectAlternativeNames": {"a.example.com", "b.example.com", "y.example.com"},
+			"expectedResult":          {"www.example.com", "a.example.com", "b.example.com", "y.example.com"},
+		},
+	}
+
+	for _, testCase := range data {
+		var options []map[string]interface{}
+
+		for _, domainName := range testCase["domainValidationOptions"] {
+			options = append(options,
+				map[string]interface{}{
+					"domain_name": domainName,
+				},
+			)
+		}
+
+		subjectAlternativeNames := testCase["subjectAlternativeNames"]
+		sortDomainValidationOptions(&domainName, subjectAlternativeNames, options)
+
+		expected := testCase["expectedResult"]
+		if len(options) != len(expected) {
+			t.Errorf("expected same length for input %v and expected output %v\n",
+				options, expected)
+			break
+		}
+		for i := range options {
+			actual := options[i]["domain_name"].(string)
+			if actual != expected[i] {
+				t.Errorf("input was: %v and %v\ngot: %v\nwant: %v\n",
+					testCase["domainValidationOptions"], subjectAlternativeNames, options, testCase["exptectedResult"])
+				break
+			}
+		}
+	}
+}
+
+func TestReadSubjectAlternativeNames(t *testing.T) {
+	domainName := "www.example.com"
+
+	data := []map[string][]string{
+		{
+			"local":    {},
+			"remote":   {"www.example.com"},
+			"expected": {},
+		},
+		{
+			"local":    {},
+			"remote":   {},
+			"expected": {},
+		},
+		{
+			"local":    {"a.example.com"},
+			"remote":   {"www.example.com"},
+			"expected": {},
+		},
+		{
+			"local":    {"a.example.com"},
+			"remote":   {"www.example.com", "a.example.com"},
+			"expected": {"a.example.com"},
+		},
+		{
+			"local":    {"z.example.com"},
+			"remote":   {"www.example.com", "z.example.com"},
+			"expected": {"z.example.com"},
+		},
+		{
+			"local":    {"a.example.com"},
+			"remote":   {"www.example.com", "a.example.com", "b.example.com"},
+			"expected": {"a.example.com", "b.example.com"},
+		},
+		{
+			"local":    {"a.example.com"},
+			"remote":   {"www.example.com", "b.example.com", "a.example.com"},
+			"expected": {"a.example.com", "b.example.com"},
+		},
+		{
+			"local":    {"a.example.com"},
+			"remote":   {"a.example.com", "b.example.com", "www.example.com"},
+			"expected": {"a.example.com", "b.example.com"},
+		},
+		{
+			"local":    {"a.example.com"},
+			"remote":   {"b.example.com", "a.example.com", "www.example.com"},
+			"expected": {"a.example.com", "b.example.com"},
+		},
+		{
+			"local":    {"z.example.com"},
+			"remote":   {"a.example.com", "z.example.com", "www.example.com"},
+			"expected": {"z.example.com", "a.example.com"},
+		},
+		{
+			"local":    {"a.example.com", "b.example.com", "z.example.com"},
+			"remote":   {"www.example.com"},
+			"expected": {},
+		},
+		{
+			"local":    {"a.example.com", "b.example.com", "z.example.com"},
+			"remote":   {"b.example.com", "www.example.com"},
+			"expected": {"b.example.com"},
+		},
+		{
+			"local":    {"a.example.com", "b.example.com", "z.example.com"},
+			"remote":   {"z.example.com", "a.example.com", "www.example.com", "b.example.com"},
+			"expected": {"a.example.com", "b.example.com", "z.example.com"},
+		},
+		{
+			"local":    {"a.example.com", "b.example.com", "x.example.com", "y.example.com", "z.example.com"},
+			"remote":   {"z.example.com", "a.example.com", "www.example.com", "b.example.com", "o.example.com"},
+			"expected": {"a.example.com", "b.example.com", "z.example.com", "o.example.com"},
+		},
+	}
+
+	for _, testCase := range data {
+		resourceDataMock, err := terraformSchema.InternalMap(
+			map[string]*terraformSchema.Schema{
+				"domain_name": {
+					Type:     terraformSchema.TypeString,
+					Optional: true,
+					Computed: true,
+				},
+				"subject_alternative_names": {
+					Type:     terraformSchema.TypeList,
+					Optional: true,
+					Computed: true,
+					Elem:     &terraformSchema.Schema{Type: terraformSchema.TypeString},
+				},
+			},
+		).Data(nil, nil)
+
+		if err != nil {
+			t.Errorf("cannot build ResourceData\n")
+		}
+
+		resourceDataMock.Set("domain_name", domainName)
+		resourceDataMock.Set("subject_alternative_names", testCase["local"])
+
+		responseMock := acm.CertificateDetail{
+			DomainName:              &domainName,
+			SubjectAlternativeNames: aws.StringSlice(testCase["remote"]),
+		}
+
+		actual := readSubjectAlternativeNames(&responseMock, resourceDataMock)
+		expected := testCase["expected"]
+
+		if len(actual) != len(expected) {
+			t.Errorf("expected same length for input %v and expected output %v\n", actual, expected)
+			break
+		}
+		for i := range actual {
+			if actual[i] != expected[i] {
+				t.Errorf("input was: %v and %v\ngot: %v\nwant: %v\n",
+					testCase["local"], testCase["remote"], actual, testCase["expected"])
+				break
+			}
+		}
+	}
+}