Repository: Azure/ARO-RP
Pull request #1877 (2021-12-13 22:41:34)

Update ARM authentication metadata endpoint for public cloud
### Which issue this PR addresses:

This should fix our deploy issues in westcentralus.

### What this PR does / why we need it:

Currently, our deployment in westcentralus fails intermittently because the ARM endpoint to authentication between the RP and ARM is not correct for public cloud.  We need to update it to use the correct endpoint so we don't hit redirects and issues when deploying.  

The correct endpoints and structure can be found in the ARM Wiki [Authenticate between ARM and RP](https://armwiki.azurewebsites.net/authorization/AuthenticateBetweenARMandRP.html?q=client%20certificate)

### Test plan for issue:

Updated the tests to allow for a different endpoint if it's public cloud.  Will have to test in INT/Canary on deploy though.  

### Is there any documentation that needs to be updated for this PR?

Nope, just gotta know about the arm wiki


bennerv: @m1kola @hawkowl I think this should fix our deployment issues in westcentralus.  Check the ARM wiki to confirm the endpoints.  It doesn't look like it's consistent when using the ResourceManagerEndpoint in the `AzureEnvironment` struct.  

Patch:
diff --git a/pkg/util/clientauthorizer/arm.go b/pkg/util/clientauthorizer/arm.go
index ab09cda47..8db940b35 100644
--- a/pkg/util/clientauthorizer/arm.go
+++ b/pkg/util/clientauthorizer/arm.go
@@ -9,10 +9,12 @@ import (
 	"encoding/json"
 	"fmt"
 	"net/http"
+	"reflect"
 	"strings"
 	"sync"
 	"time"
 
+	"github.com/Azure/go-autorest/autorest/azure"
 	"github.com/sirupsen/logrus"
 
 	"github.com/Azure/ARO-RP/pkg/util/instancemetadata"
@@ -101,7 +103,13 @@ func (a *arm) refresh() {
 func (a *arm) refreshOnce() error {
 	now := a.now()
 
-	req, err := http.NewRequest(http.MethodGet, strings.TrimSuffix(a.im.Environment().ResourceManagerEndpoint, "/")+":24582/metadata/authentication?api-version=2015-01-01", nil)
+	// ARM <-> RP Authentication endpoint is not consistent.  Check ARM wiki for up-to-date metadata endpoints
+	endpoint := strings.TrimSuffix(a.im.Environment().ResourceManagerEndpoint, "/") + ":24582"
+	if reflect.DeepEqual(a.im.Environment().Environment, azure.PublicCloud) {
+		endpoint = "https://admin.management.azure.com"
+	}
+
+	req, err := http.NewRequest(http.MethodGet, endpoint+"/metadata/authentication?api-version=2015-01-01", nil)
 	if err != nil {
 		return err
 	}


Test patch:
diff --git a/pkg/util/clientauthorizer/arm_test.go b/pkg/util/clientauthorizer/arm_test.go
index 7045cd9db..0dbd43ecc 100644
--- a/pkg/util/clientauthorizer/arm_test.go
+++ b/pkg/util/clientauthorizer/arm_test.go
@@ -10,10 +10,12 @@ import (
 	"fmt"
 	"io/ioutil"
 	"net/http"
+	"reflect"
 	"strings"
 	"testing"
 	"time"
 
+	"github.com/Azure/go-autorest/autorest/azure"
 	"github.com/golang/mock/gomock"
 
 	"github.com/Azure/ARO-RP/pkg/util/azureclient"
@@ -22,12 +24,14 @@ import (
 
 func TestARMRefreshOnce(t *testing.T) {
 	for _, tt := range []struct {
-		name    string
-		do      func(*http.Request) (*http.Response, error)
-		wantErr string
+		name     string
+		azureEnv azureclient.AROEnvironment
+		do       func(*http.Request) (*http.Response, error)
+		wantErr  string
 	}{
 		{
-			name: "valid",
+			name:     "valid public cloud",
+			azureEnv: azureclient.PublicCloud,
 			do: func(*http.Request) (*http.Response, error) {
 				return &http.Response{
 					StatusCode: http.StatusOK,
@@ -48,7 +52,30 @@ func TestARMRefreshOnce(t *testing.T) {
 			},
 		},
 		{
-			name: "invalid - no certificate for now",
+			name:     "valid gov cloud",
+			azureEnv: azureclient.USGovernmentCloud,
+			do: func(*http.Request) (*http.Response, error) {
+				return &http.Response{
+					StatusCode: http.StatusOK,
+					Header: http.Header{
+						"Content-Type": []string{"application/json; charset=utf-8"},
+					},
+					Body: ioutil.NopCloser(strings.NewReader(
+						`{
+							"clientCertificates": [
+								{
+									"notBefore": "2020-01-19T23:00:00Z",
+									"notAfter": "2020-01-20T01:00:00Z"
+								}
+							]
+						}`,
+					)),
+				}, nil
+			},
+		},
+		{
+			name:     "invalid - no certificate for now",
+			azureEnv: azureclient.PublicCloud,
 			do: func(*http.Request) (*http.Response, error) {
 				return &http.Response{
 					StatusCode: http.StatusOK,
@@ -70,7 +97,8 @@ func TestARMRefreshOnce(t *testing.T) {
 			wantErr: "did not receive current certificate",
 		},
 		{
-			name: "invalid JSON",
+			name:     "invalid JSON",
+			azureEnv: azureclient.PublicCloud,
 			do: func(*http.Request) (*http.Response, error) {
 				return &http.Response{
 					StatusCode: http.StatusOK,
@@ -83,14 +111,16 @@ func TestARMRefreshOnce(t *testing.T) {
 			wantErr: "invalid character 'o' in literal null (expecting 'u')",
 		},
 		{
-			name: "invalid - error",
+			name:     "invalid - error",
+			azureEnv: azureclient.PublicCloud,
 			do: func(*http.Request) (*http.Response, error) {
 				return nil, errors.New("fake error")
 			},
 			wantErr: "fake error",
 		},
 		{
-			name: "invalid - status code",
+			name:     "invalid - status code",
+			azureEnv: azureclient.PublicCloud,
 			do: func(*http.Request) (*http.Response, error) {
 				return &http.Response{
 					StatusCode: http.StatusBadGateway,
@@ -100,7 +130,8 @@ func TestARMRefreshOnce(t *testing.T) {
 			wantErr: "unexpected status code 502",
 		},
 		{
-			name: "invalid - content type",
+			name:     "invalid - content type",
+			azureEnv: azureclient.PublicCloud,
 			do: func(*http.Request) (*http.Response, error) {
 				return &http.Response{
 					StatusCode: http.StatusOK,
@@ -118,7 +149,7 @@ func TestARMRefreshOnce(t *testing.T) {
 			defer controller.Finish()
 
 			im := mock_instancemetadata.NewMockInstanceMetadata(controller)
-			im.EXPECT().Environment().AnyTimes().Return(&azureclient.PublicCloud)
+			im.EXPECT().Environment().AnyTimes().Return(&tt.azureEnv)
 
 			a := &arm{
 				now: func() time.Time { return time.Date(2020, 1, 20, 0, 0, 0, 0, time.UTC) },
@@ -126,7 +157,11 @@ func TestARMRefreshOnce(t *testing.T) {
 					if req.Method != http.MethodGet {
 						return nil, fmt.Errorf("unexpected method %q", req.Method)
 					}
-					if req.URL.String() != strings.TrimSuffix(im.Environment().ResourceManagerEndpoint, "/")+":24582/metadata/authentication?api-version=2015-01-01" {
+					endpoint := strings.TrimSuffix(im.Environment().ResourceManagerEndpoint, "/") + ":24582"
+					if reflect.DeepEqual(im.Environment().Environment, azure.PublicCloud) {
+						endpoint = "https://admin.management.azure.com"
+					}
+					if req.URL.String() != endpoint+"/metadata/authentication?api-version=2015-01-01" {
 						return nil, fmt.Errorf("unexpected URL %q", req.URL.String())
 					}
 					return tt.do(req)