Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
4 changes: 2 additions & 2 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -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=
Expand Down
10 changes: 6 additions & 4 deletions pkg/azure/actuator.go
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,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")
Expand All @@ -64,9 +64,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
}
Expand Down
2 changes: 1 addition & 1 deletion pkg/azure/actuator_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -112,7 +112,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)
Expand Down
17 changes: 9 additions & 8 deletions pkg/azure/clients.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand All @@ -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,
Expand Down
20 changes: 13 additions & 7 deletions pkg/azure/minter.go
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down Expand Up @@ -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,
Expand Down
8 changes: 4 additions & 4 deletions pkg/azure/passthrough_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -166,7 +166,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)
Expand Down Expand Up @@ -197,7 +197,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)
Expand Down Expand Up @@ -238,7 +238,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)
Expand Down Expand Up @@ -270,7 +270,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)
Expand Down
3 changes: 2 additions & 1 deletion pkg/operator/controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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
}
Expand Down
3 changes: 1 addition & 2 deletions pkg/operator/secretannotator/azure/adal.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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) {
Expand Down
19 changes: 17 additions & 2 deletions pkg/operator/secretannotator/azure/reconciler.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -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"
)

Expand Down Expand Up @@ -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
Expand Down
3 changes: 2 additions & 1 deletion pkg/operator/secretannotator/azure/reconciler_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
17 changes: 17 additions & 0 deletions pkg/util/infra.go
Original file line number Diff line number Diff line change
@@ -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
}
67 changes: 67 additions & 0 deletions pkg/util/infra_test.go
Original file line number Diff line number Diff line change
@@ -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)
})
}
}

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

11 changes: 11 additions & 0 deletions vendor/github.com/openshift/api/OWNERS

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading