Merge pull request #192 from staebler/azure_other_cloud_env

support other Azure cloud environments

go.mod

@@ -16,7 +16,7 @@ require (
 	github.com/go-bindata/go-bindata v3.1.2+incompatible
 	github.com/golang/glog v0.0.0-20160126235308-23def4e6c14b
 	github.com/golang/mock v1.4.3
-	github.com/openshift/api v0.0.0-20200521162313-4090b8d67ad8
+	github.com/openshift/api v0.0.0-20200609191024-dca637550e8c
 	github.com/openshift/build-machinery-go v0.0.0-20200424080330-082bf86082cc
 	github.com/openshift/client-go v0.0.0-20200521150516-05eb9880269c
 	github.com/openshift/library-go v0.0.0-20200521170207-eeebfaa62843

go.sum

@@ -369,8 +369,8 @@ github.com/opencontainers/go-digest v1.0.0-rc1/go.mod h1:cMLVZDEM3+U2I4VmLI6N8jQ
 github.com/opencontainers/image-spec v1.0.1/go.mod h1:BtxoFyWECRxE4U/7sNtV5W15zMzWCbyJoFRP3s7yZA0=
 github.com/opencontainers/runc v0.0.0-20191031171055-b133feaeeb2e/go.mod h1:qT5XzbpPznkRYVz/mWwUaVBUv2rmF59PVA73FjuZG0U=
 github.com/openshift/api v0.0.0-20200521101457-60c476765272/go.mod h1:TkhafijfTiRi1Q3120/ZSE4oIWKQ4DGRh3byPywv4Mw=
-github.com/openshift/api v0.0.0-20200521162313-4090b8d67ad8 h1:hmeRDYagWib4YM2xFGv7AQ4r/+1wCl27pOii9kHmPT4=
-github.com/openshift/api v0.0.0-20200521162313-4090b8d67ad8/go.mod h1:TkhafijfTiRi1Q3120/ZSE4oIWKQ4DGRh3byPywv4Mw=
+github.com/openshift/api v0.0.0-20200609191024-dca637550e8c h1:bKFV0gBWD+jg/xbR4jrf4jBFQE/c/jXj3bzDw8nZghU=
+github.com/openshift/api v0.0.0-20200609191024-dca637550e8c/go.mod h1:l6TGeqJ92DrZBuWMNKcot1iZUHfbYSJyBWHGgg6Dn6s=
 github.com/openshift/build-machinery-go v0.0.0-20200424080330-082bf86082cc h1:Bu1p7+ItPqhJhmMve7sVluKCYV+o+x1Ede0WY2/BI8Q=
 github.com/openshift/build-machinery-go v0.0.0-20200424080330-082bf86082cc/go.mod h1:1CkcsT3aVebzRBzVTSbiKSkJMsC/CASqxesfqEMfJEc=
 github.com/openshift/client-go v0.0.0-20200521150516-05eb9880269c h1:l7CmbzzkyWl4Y6qHmy6m4FvbH4iLnIXGrXqOfE5IFNA=

pkg/azure/actuator.go

@@ -57,7 +57,7 @@ type Actuator struct {
 	generateServicePrincipalName servicePrincipalNameBuilder
 }
 
-func NewActuator(c client.Client) (*Actuator, error) {
+func NewActuator(c client.Client, cloudName configv1.AzureCloudEnvironment) (*Actuator, error) {
 	codec, err := minterv1.NewCodec()
 	if err != nil {
 		log.WithError(err).Error("error creating Azure codec")
@@ -66,9 +66,11 @@ func NewActuator(c client.Client) (*Actuator, error) {
 
 	client := newClientWrapper(c)
 	return &Actuator{
-		client:                       client,
-		codec:                        codec,
-		credentialMinterBuilder:      NewAzureCredentialsMinter,
+		client: client,
+		codec:  codec,
+		credentialMinterBuilder: func(logger log.FieldLogger, clientID, clientSecret, tenantID, subscriptionID string) (*AzureCredentialsMinter, error) {
+			return NewAzureCredentialsMinter(logger, clientID, clientSecret, cloudName, tenantID, subscriptionID)
+		},
 		generateServicePrincipalName: generateServicePrincipalName,
 	}, nil
 }

pkg/azure/actuator_test.go

@@ -113,7 +113,7 @@ func TestAnnotations(t *testing.T) {
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
 			f := fake.NewFakeClient(&tt.in, &validSecret)
-			actuator, err := azure.NewActuator(f)
+			actuator, err := azure.NewActuator(f, openshiftapiv1.AzurePublicCloud)
 			if err != nil {
 				assert.Regexp(t, tt.errRegexp, err)
 				assert.Nil(t, actuator)

pkg/azure/clients.go

@@ -6,6 +6,7 @@ import (
 	"github.com/Azure/azure-sdk-for-go/services/authorization/mgmt/2015-07-01/authorization"
 	"github.com/Azure/azure-sdk-for-go/services/graphrbac/1.6/graphrbac"
 	"github.com/Azure/go-autorest/autorest"
+	"github.com/Azure/go-autorest/autorest/azure"
 )
 
 //go:generate mockgen -source=./clients.go -destination=./mock/client_generated.go -package=mock
@@ -47,8 +48,8 @@ func (appClient *appClient) Delete(ctx context.Context, applicationObjectID stri
 
 var _ AppClient = &appClient{}
 
-func NewAppClient(tenantID string, authorizer autorest.Authorizer) *appClient {
-	client := graphrbac.NewApplicationsClient(tenantID)
+func NewAppClient(env azure.Environment, tenantID string, authorizer autorest.Authorizer) *appClient {
+	client := graphrbac.NewApplicationsClientWithBaseURI(env.GraphEndpoint, tenantID)
 	client.Authorizer = authorizer
 	return &appClient{
 		client: client,
@@ -80,8 +81,8 @@ func (spClient *servicePrincipalClient) Create(ctx context.Context, parameters g
 
 var _ ServicePrincipalClient = &servicePrincipalClient{}
 
-func NewServicePrincipalClient(tenantID string, authorizer autorest.Authorizer) *servicePrincipalClient {
-	client := graphrbac.NewServicePrincipalsClient(tenantID)
+func NewServicePrincipalClient(env azure.Environment, tenantID string, authorizer autorest.Authorizer) *servicePrincipalClient {
+	client := graphrbac.NewServicePrincipalsClientWithBaseURI(env.GraphEndpoint, tenantID)
 	client.Authorizer = authorizer
 	return &servicePrincipalClient{
 		client: client,
@@ -119,8 +120,8 @@ func (raClient *roleAssignmentsClient) DeleteByID(ctx context.Context, roleAssig
 
 var _ RoleAssignmentsClient = &roleAssignmentsClient{}
 
-func NewRoleAssignmentsClient(subscriptionID string, authorizer autorest.Authorizer) *roleAssignmentsClient {
-	client := authorization.NewRoleAssignmentsClient(subscriptionID)
+func NewRoleAssignmentsClient(env azure.Environment, subscriptionID string, authorizer autorest.Authorizer) *roleAssignmentsClient {
+	client := authorization.NewRoleAssignmentsClientWithBaseURI(env.ResourceManagerEndpoint, subscriptionID)
 	client.Authorizer = authorizer
 	return &roleAssignmentsClient{
 		client: client,
@@ -147,8 +148,8 @@ func (rdClient *roleDefinitionClient) List(ctx context.Context, scope string, fi
 
 var _ RoleDefinitionClient = &roleDefinitionClient{}
 
-func NewRoleDefinitionClient(subscriptionID string, authorizer autorest.Authorizer) *roleDefinitionClient {
-	client := authorization.NewRoleDefinitionsClient(subscriptionID)
+func NewRoleDefinitionClient(env azure.Environment, subscriptionID string, authorizer autorest.Authorizer) *roleDefinitionClient {
+	client := authorization.NewRoleDefinitionsClientWithBaseURI(env.ResourceManagerEndpoint, subscriptionID)
 	client.Authorizer = authorizer
 	return &roleDefinitionClient{
 		client: client,

pkg/azure/minter.go

@@ -18,6 +18,8 @@ import (
 	"github.com/Azure/go-autorest/autorest/date"
 	"github.com/Azure/go-autorest/autorest/to"
 	uuid "github.com/satori/go.uuid"
+
+	configv1 "github.com/openshift/api/config/v1"
 )
 
 func getAuthorizer(clientID, clientSecret, tenantID, resourceEndpoint string) (autorest.Authorizer, error) {
@@ -50,22 +52,26 @@ func NewFakeAzureCredentialsMinter(logger log.FieldLogger, clientID, clientSecre
 	}, nil
 }
 
-func NewAzureCredentialsMinter(logger log.FieldLogger, clientID, clientSecret, tenantID, subscriptionID string) (*AzureCredentialsMinter, error) {
-	graphAuthorizer, err := getAuthorizer(clientID, clientSecret, tenantID, azure.PublicCloud.GraphEndpoint)
+func NewAzureCredentialsMinter(logger log.FieldLogger, clientID, clientSecret string, cloudName configv1.AzureCloudEnvironment, tenantID, subscriptionID string) (*AzureCredentialsMinter, error) {
+	env, err := azure.EnvironmentFromName(string(cloudName))
+	if err != nil {
+		return nil, fmt.Errorf("Unable to determine Azure environment: %w", err)
+	}
+	graphAuthorizer, err := getAuthorizer(clientID, clientSecret, tenantID, env.GraphEndpoint)
 	if err != nil {
 		return nil, fmt.Errorf("Unable to construct GraphEndpoint authorizer: %v", err)
 	}
 
-	rmAuthorizer, err := getAuthorizer(clientID, clientSecret, tenantID, azure.PublicCloud.ResourceManagerEndpoint)
+	rmAuthorizer, err := getAuthorizer(clientID, clientSecret, tenantID, env.ResourceManagerEndpoint)
 	if err != nil {
 		return nil, fmt.Errorf("Unable to construct ResourceManagerEndpoint authorizer: %v", err)
 	}
 
 	return &AzureCredentialsMinter{
-		appClient:             NewAppClient(tenantID, graphAuthorizer),
-		spClient:              NewServicePrincipalClient(tenantID, graphAuthorizer),
-		roleAssignmentsClient: NewRoleAssignmentsClient(subscriptionID, rmAuthorizer),
-		roleDefinitionClient:  NewRoleDefinitionClient(subscriptionID, rmAuthorizer),
+		appClient:             NewAppClient(env, tenantID, graphAuthorizer),
+		spClient:              NewServicePrincipalClient(env, tenantID, graphAuthorizer),
+		roleAssignmentsClient: NewRoleAssignmentsClient(env, subscriptionID, rmAuthorizer),
+		roleDefinitionClient:  NewRoleDefinitionClient(env, subscriptionID, rmAuthorizer),
 		tenantID:              tenantID,
 		subscriptionID:        subscriptionID,
 		logger:                logger,

pkg/azure/passthrough_test.go

@@ -167,7 +167,7 @@ func TestPassthroughExists(t *testing.T) {
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
 			f := fake.NewFakeClient(&validRootSecret, &validSecret)
-			actuator, err := azure.NewActuator(f)
+			actuator, err := azure.NewActuator(f, openshiftapiv1.AzurePublicCloud)
 			assert.Nil(t, err)
 
 			cr, err := newCredentialsRequest(tt.in)
@@ -198,7 +198,7 @@ func TestPassthroughCreate(t *testing.T) {
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
 			f := fake.NewFakeClient(&validRootSecret, &validSecret, &clusterInfra, &clusterDNS)
-			actuator, err := azure.NewActuator(f)
+			actuator, err := azure.NewActuator(f, openshiftapiv1.AzurePublicCloud)
 			assert.Nil(t, err)
 
 			cr, err := newCredentialsRequest(tt.in)
@@ -239,7 +239,7 @@ func TestPassthroughUpdate(t *testing.T) {
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
 			f := fake.NewFakeClient(&validRootSecret, &validSecret, &clusterInfra, &clusterDNS)
-			actuator, err := azure.NewActuator(f)
+			actuator, err := azure.NewActuator(f, openshiftapiv1.AzurePublicCloud)
 			assert.Nil(t, err)
 
 			cr, err := newCredentialsRequest(tt.in)
@@ -271,7 +271,7 @@ func TestPassthroughDelete(t *testing.T) {
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
 			f := fake.NewFakeClient(&validRootSecret, &validSecret)
-			actuator, err := azure.NewActuator(f)
+			actuator, err := azure.NewActuator(f, openshiftapiv1.AzurePublicCloud)
 			assert.Nil(t, err)
 
 			cr, err := newCredentialsRequest(tt.in)

pkg/operator/controller.go

@@ -30,6 +30,7 @@ import (
 	"github.com/openshift/cloud-credential-operator/pkg/operator/platform"
 	"github.com/openshift/cloud-credential-operator/pkg/operator/secretannotator"
 	"github.com/openshift/cloud-credential-operator/pkg/ovirt"
+	"github.com/openshift/cloud-credential-operator/pkg/util"
 	vsphereactuator "github.com/openshift/cloud-credential-operator/pkg/vsphere/actuator"
 
 	configv1 "github.com/openshift/api/config/v1"
@@ -87,7 +88,7 @@ func AddToManager(m manager.Manager, explicitKubeconfig string) error {
 			}
 		case configv1.AzurePlatformType:
 			log.Info("initializing Azure actuator")
-			a, err = azure.NewActuator(m.GetClient())
+			a, err = azure.NewActuator(m.GetClient(), util.GetAzureCloudName(infraStatus))
 			if err != nil {
 				return err
 			}

pkg/operator/secretannotator/azure/adal.go

@@ -4,7 +4,6 @@ import (
 	"fmt"
 
 	"github.com/Azure/go-autorest/autorest/adal"
-	"github.com/Azure/go-autorest/autorest/azure"
 )
 
 //go:generate mockgen -source=./adal.go -destination=./mock/adal_generated.go -package=mock
@@ -18,7 +17,7 @@ type AdalService interface {
 type adalService struct{}
 
 func (a *adalService) NewOAuthConfig(activeDirectoryEndpoint, tenantID string) (*adal.OAuthConfig, error) {
-	return adal.NewOAuthConfig(azure.PublicCloud.ActiveDirectoryEndpoint, tenantID)
+	return adal.NewOAuthConfig(activeDirectoryEndpoint, tenantID)
 }
 
 func (a *adalService) NewServicePrincipalToken(oauthConfig adal.OAuthConfig, clientID string, secret string, resource string, callbacks ...adal.TokenRefreshCallback) (*adal.ServicePrincipalToken, error) {

pkg/operator/secretannotator/azure/reconciler.go

@@ -5,8 +5,10 @@ import (
 	"fmt"
 	"time"
 
+	configv1 "github.com/openshift/api/config/v1"
 	corev1 "k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/apimachinery/pkg/types"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 	"sigs.k8s.io/controller-runtime/pkg/controller"
 	"sigs.k8s.io/controller-runtime/pkg/event"
@@ -22,6 +24,8 @@ import (
 	"github.com/openshift/cloud-credential-operator/pkg/operator/metrics"
 	secretconstants "github.com/openshift/cloud-credential-operator/pkg/operator/secretannotator/constants"
 	"github.com/openshift/cloud-credential-operator/pkg/operator/utils"
+	"github.com/openshift/cloud-credential-operator/pkg/util"
+
 	log "github.com/sirupsen/logrus"
 )
 
@@ -167,13 +171,24 @@ func (r *ReconcileCloudCredSecret) updateSecretAnnotations(secret *corev1.Secret
 }
 
 func (r *ReconcileCloudCredSecret) checkCloudCredCreation(tenantID, clientID, secret string) (bool, error) {
-	oauthConfig, err := r.Adal.NewOAuthConfig(azure.PublicCloud.ActiveDirectoryEndpoint, tenantID)
+	infra := &configv1.Infrastructure{}
+	if err := r.Get(context.Background(), types.NamespacedName{Name: "cluster"}, infra); err != nil {
+		return false, fmt.Errorf("could not get infrastructure resource: %w", err)
+	}
+
+	cloudName := util.GetAzureCloudName(&infra.Status)
+	env, err := azure.EnvironmentFromName(string(cloudName))
+	if err != nil {
+		return false, fmt.Errorf("unable to determine Azure environment: %w", err)
+	}
+
+	oauthConfig, err := r.Adal.NewOAuthConfig(env.ActiveDirectoryEndpoint, tenantID)
 	if err != nil {
 		r.Logger.WithError(err).Error("error while creating oAuthConfig")
 		return false, err
 	}
 
-	token, err := r.Adal.NewServicePrincipalToken(*oauthConfig, clientID, secret, azure.PublicCloud.GraphEndpoint)
+	token, err := r.Adal.NewServicePrincipalToken(*oauthConfig, clientID, secret, env.GraphEndpoint)
 	if err != nil {
 		r.Logger.WithError(err).Error("error while creating service principal")
 		return false, err

pkg/operator/secretannotator/azure/reconciler_test.go

@@ -85,7 +85,8 @@ func TestAzureSecretAnnotatorReconcile(t *testing.T) {
 	for _, test := range tests {
 		t.Run(test.name, func(t *testing.T) {
 			base := getInputSecret()
-			fakeClient := fake.NewFakeClient(base)
+			infra := &configv1.Infrastructure{ObjectMeta: metav1.ObjectMeta{Name: "cluster"}}
+			fakeClient := fake.NewFakeClient(base, infra)
 			mockCtrl := gomock.NewController(t)
 			defer mockCtrl.Finish()
 			mockAdalClient := mock.NewMockAdalService(mockCtrl)

pkg/util/infra.go

@@ -0,0 +1,17 @@
+package util
+
+import (
+	configv1 "github.com/openshift/api/config/v1"
+)
+
+// GetAzureCloudName gets the Azure cloud name to use given the specified infrastructure status.
+func GetAzureCloudName(infraStatus *configv1.InfrastructureStatus) configv1.AzureCloudEnvironment {
+	if s := infraStatus.PlatformStatus; s != nil {
+		if a := s.Azure; a != nil {
+			if c := a.CloudName; c != "" {
+				return c
+			}
+		}
+	}
+	return configv1.AzurePublicCloud
+}

pkg/util/infra_test.go

@@ -0,0 +1,67 @@
+package util
+
+import (
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+
+	configv1 "github.com/openshift/api/config/v1"
+)
+
+func TestGetAzureCloudName(t *testing.T) {
+	cases := []struct {
+		name              string
+		infraStatus       *configv1.InfrastructureStatus
+		expectedCloudName configv1.AzureCloudEnvironment
+	}{
+		{
+			name:              "no platform status",
+			infraStatus:       &configv1.InfrastructureStatus{},
+			expectedCloudName: configv1.AzurePublicCloud,
+		},
+		{
+			name: "no azure",
+			infraStatus: &configv1.InfrastructureStatus{
+				PlatformStatus: &configv1.PlatformStatus{},
+			},
+			expectedCloudName: configv1.AzurePublicCloud,
+		},
+		{
+			name: "no cloud name",
+			infraStatus: &configv1.InfrastructureStatus{
+				PlatformStatus: &configv1.PlatformStatus{
+					Azure: &configv1.AzurePlatformStatus{},
+				},
+			},
+			expectedCloudName: configv1.AzurePublicCloud,
+		},
+		{
+			name: "default cloud name",
+			infraStatus: &configv1.InfrastructureStatus{
+				PlatformStatus: &configv1.PlatformStatus{
+					Azure: &configv1.AzurePlatformStatus{
+						CloudName: configv1.AzurePublicCloud,
+					},
+				},
+			},
+			expectedCloudName: configv1.AzurePublicCloud,
+		},
+		{
+			name: "non-default cloud name",
+			infraStatus: &configv1.InfrastructureStatus{
+				PlatformStatus: &configv1.PlatformStatus{
+					Azure: &configv1.AzurePlatformStatus{
+						CloudName: configv1.AzureUSGovernmentCloud,
+					},
+				},
+			},
+			expectedCloudName: configv1.AzureUSGovernmentCloud,
+		},
+	}
+	for _, tc := range cases {
+		t.Run(tc.name, func(t *testing.T) {
+			actualCloudName := GetAzureCloudName(tc.infraStatus)
+			assert.Equal(t, tc.expectedCloudName, actualCloudName)
+		})
+	}
+}