Repository: Azure/ARO-RP
Pull request #1544 (2021-06-10 22:17:26)

Support short CN in any cloud
### Which issue this PR addresses:

In non-public cloud we cannot use "reserved.aroapp.io" CN.

### What this PR does / why we need it:

Use long CN to fabricate a shorter CN.

### Test plan for issue:

Unit test and E2E.

### Is there any documentation that needs to be updated for this PR?

No.


jewzaam: I am going to add some unit tests tomorrow.  Prelim review welcome though.

Patch:
diff --git a/README.md b/README.md
index d642c89ab..17768f5a0 100644
--- a/README.md
+++ b/README.md
@@ -107,7 +107,7 @@ questions or comments.
       immutable
 
     * dnsmasq: Ensures that a dnsmasq systemd service is defined as a machineconfig for all
-      nodes. The dnsmasq config contains records for azure load balancers such as api, api-int and *.apps domains so they will resolve even if custom DNS on the VNET is set.
+      nodes to allow for api-int and *.apps domains resolve even if custom DNS on the VNET is set.
 
     * genevalogging: Ensures all the Geneva logging resources in the
       `openshift-azure-logging` namespace matches the pre-defined specification
diff --git a/pkg/util/keyvault/keyvault.go b/pkg/util/keyvault/keyvault.go
index 8dbd2f6bd..f43c51bab 100644
--- a/pkg/util/keyvault/keyvault.go
+++ b/pkg/util/keyvault/keyvault.go
@@ -57,7 +57,7 @@ func NewManager(kvAuthorizer autorest.Authorizer, keyvaultURI string) Manager {
 	}
 }
 
-func (m *manager) CreateSignedCertificate(ctx context.Context, issuer string, certificateName, commonName string, eku Eku) error {
+func getShortCommonName(commonName string) string {
 	shortCommonName := commonName
 	if len(shortCommonName) > 64 {
 		// RFC 5280 requires that the common name be <= 64 characters.  Also see
@@ -67,8 +67,17 @@ func (m *manager) CreateSignedCertificate(ctx context.Context, issuer string, ce
 		// subject with an empty common name.  So, in the case where the domain
 		// name is too long, we use a reserved domain name which cannot be
 		// allocated by an end user as the common name.
-		shortCommonName = "reserved.aroapp.io"
+
+		// each cloud has different base domain and we have int and prod.
+		// the common string is 'aroapp' so we use that to build the base domain for a shorter CN
+		baseDomain := shortCommonName[strings.LastIndex(shortCommonName, "aroapp"):]
+		shortCommonName = "reserved." + baseDomain
 	}
+	return shortCommonName
+}
+
+func (m *manager) CreateSignedCertificate(ctx context.Context, issuer string, certificateName, commonName string, eku Eku) error {
+	shortCommonName := getShortCommonName(commonName)
 
 	op, err := m.kv.CreateCertificate(ctx, m.keyvaultURI, certificateName, azkeyvault.CertificateCreateParameters{
 		CertificatePolicy: &azkeyvault.CertificatePolicy{


Test patch:
diff --git a/pkg/util/keyvault/keyvault_test.go b/pkg/util/keyvault/keyvault_test.go
new file mode 100644
index 000000000..dd0c26e19
--- /dev/null
+++ b/pkg/util/keyvault/keyvault_test.go
@@ -0,0 +1,38 @@
+package keyvault
+
+// Copyright (c) Microsoft Corporation.
+// Licensed under the Apache License 2.0.
+
+import (
+	"fmt"
+	"testing"
+)
+
+func TestShortCommonName(t *testing.T) {
+	tests := []struct {
+		name                string
+		commonName          string
+		wantShortCommonName string
+	}{
+		{
+			name:                "commonName not shortened",
+			commonName:          "someShort.common.name.aroapp-dev.something",
+			wantShortCommonName: "someShort.common.name.aroapp-dev.something",
+		},
+		{
+			name:                "commonName shortened",
+			commonName:          "my-very-long-common-name-needs-shortened.else-things-blow-up.aroapp-dev.something",
+			wantShortCommonName: "reserved.aroapp-dev.something",
+		},
+	}
+
+	for _, test := range tests {
+		t.Run(test.name, func(t *testing.T) {
+			got := getShortCommonName(test.commonName)
+
+			if got != test.wantShortCommonName {
+				t.Error(fmt.Errorf("got != want: %s != %s", got, test.wantShortCommonName))
+			}
+		})
+	}
+}