From caa9b2632218e3ca491a90ad5779287805f23674 Mon Sep 17 00:00:00 2001 From: Steve Kuznetsov Date: Tue, 14 Jan 2025 07:45:12 -0700 Subject: [PATCH] *: update to use the new msi-dataplane library Signed-off-by: Steve Kuznetsov --- go.mod | 2 +- pkg/cluster/cluster.go | 45 ++- pkg/cluster/clustermsi.go | 98 ++++--- pkg/cluster/clustermsi_test.go | 270 +++++++----------- pkg/cluster/delete.go | 7 +- pkg/cluster/delete_test.go | 19 +- pkg/env/env.go | 4 +- pkg/env/prod.go | 114 +++++--- pkg/frontend/middleware/msi.go | 10 +- pkg/frontend/middleware/msi_test.go | 10 +- .../openshiftcluster_putorpatch_test.go | 5 +- pkg/util/azureclient/environments.go | 17 +- pkg/util/azureclient/keyvault/base.go | 1 + pkg/util/keyvault/keyvault.go | 5 + .../mocks/azureclient/keyvault/keyvault.go | 15 + pkg/util/mocks/env/env.go | 23 +- pkg/util/mocks/keyvault/keyvault.go | 15 + pkg/util/mocks/msidataplane/client.go | 100 +++++++ pkg/util/mocks/msidataplane/client_factory.go | 55 ++++ pkg/util/msidataplane/generate.go | 14 + 20 files changed, 511 insertions(+), 318 deletions(-) create mode 100644 pkg/util/mocks/msidataplane/client.go create mode 100644 pkg/util/mocks/msidataplane/client_factory.go create mode 100644 pkg/util/msidataplane/generate.go diff --git a/go.mod b/go.mod index 6e7071ccedc..bb7c84ffd20 100644 --- a/go.mod +++ b/go.mod @@ -1,6 +1,6 @@ module github.com/Azure/ARO-RP -go 1.23.2 +go 1.21 require ( github.com/Azure/azure-sdk-for-go v63.1.0+incompatible diff --git a/pkg/cluster/cluster.go b/pkg/cluster/cluster.go index a9f4931a698..487204e614d 100644 --- a/pkg/cluster/cluster.go +++ b/pkg/cluster/cluster.go @@ -12,7 +12,6 @@ import ( "github.com/Azure/go-autorest/autorest" "github.com/Azure/go-autorest/autorest/azure" "github.com/Azure/msi-dataplane/pkg/dataplane" - "github.com/Azure/msi-dataplane/pkg/store" configclient "github.com/openshift/client-go/config/clientset/versioned" imageregistryclient "github.com/openshift/client-go/imageregistry/clientset/versioned" machineclient "github.com/openshift/client-go/machine/clientset/versioned" @@ -36,7 +35,6 @@ import ( "github.com/Azure/ARO-RP/pkg/util/azureclient/azuresdk/armauthorization" "github.com/Azure/ARO-RP/pkg/util/azureclient/azuresdk/armmsi" "github.com/Azure/ARO-RP/pkg/util/azureclient/azuresdk/armnetwork" - "github.com/Azure/ARO-RP/pkg/util/azureclient/azuresdk/azsecrets" "github.com/Azure/ARO-RP/pkg/util/azureclient/mgmt/authorization" "github.com/Azure/ARO-RP/pkg/util/azureclient/mgmt/compute" "github.com/Azure/ARO-RP/pkg/util/azureclient/mgmt/features" @@ -48,6 +46,7 @@ import ( "github.com/Azure/ARO-RP/pkg/util/dns" "github.com/Azure/ARO-RP/pkg/util/encryption" utilgraph "github.com/Azure/ARO-RP/pkg/util/graph" + "github.com/Azure/ARO-RP/pkg/util/keyvault" "github.com/Azure/ARO-RP/pkg/util/platformworkloadidentity" "github.com/Azure/ARO-RP/pkg/util/refreshable" "github.com/Azure/ARO-RP/pkg/util/storage" @@ -128,8 +127,8 @@ type manager struct { aroOperatorDeployer deploy.Operator - msiDataplane *dataplane.ManagedIdentityClient - clusterMsiKeyVaultStore *store.MsiKeyVaultStore + msiDataplane dataplane.ClientFactory + clusterMsiKeyVaultStore keyvault.Manager clusterMsiFederatedIdentityCredentials armmsi.FederatedIdentityCredentialsClient now func() time.Time @@ -316,36 +315,36 @@ func New(ctx context.Context, log *logrus.Entry, _env env.Interface, db database return nil, err } - msiDataplaneClientOptions, err := _env.MsiDataplaneClientOptions(msiResourceId) - if err != nil { - return nil, err - } + var msiDataplane dataplane.ClientFactory + if _env.FeatureIsSet(env.FeatureUseMockMsiRp) { + msiDataplane = _env.MockMSIResponses(msiResourceId) + } else { + msiDataplaneClientOptions, err := _env.MsiDataplaneClientOptions() + if err != nil { + return nil, err + } - cloud, err := _env.Environment().CloudNameForMsiDataplane() - if err != nil { - return nil, err - } + // MSI dataplane client receives tenant from the bearer challenge, so we can't limit the allowed tenants in the credential + fpMSICred, err := _env.FPNewClientCertificateCredential(_env.TenantID(), []string{"*"}) + if err != nil { + return nil, err + } - // MSI dataplane client receives tenant from the bearer challenge, so we can't limit the allowed tenants in the credential - fpMSICred, err := _env.FPNewClientCertificateCredential(_env.TenantID(), []string{"*"}) - if err != nil { - return nil, err - } - authenticatorPolicy := dataplane.NewAuthenticatorPolicy(fpMSICred, _env.MsiRpEndpoint()) - msiDataplane, err := dataplane.NewClient(cloud, authenticatorPolicy, msiDataplaneClientOptions) - if err != nil { - return nil, err + msiDataplane, err = dataplane.NewClientFactory(fpMSICred, _env.MsiRpEndpoint(), msiDataplaneClientOptions) + if err != nil { + return nil, err + } } clusterMsiKeyVaultName := _env.ClusterMsiKeyVaultName() clusterMsiKeyVaultURL := fmt.Sprintf("https://%s.%s", clusterMsiKeyVaultName, _env.Environment().KeyVaultDNSSuffix) - clusterMsiSecretsClient, err := azsecrets.NewClient(clusterMsiKeyVaultURL, msiCredential, clientOptions) + authorizer, err := _env.NewMSIAuthorizer(_env.Environment().KeyVaultScope) if err != nil { return nil, err } m.msiDataplane = msiDataplane - m.clusterMsiKeyVaultStore = store.NewMsiKeyVaultStore(clusterMsiSecretsClient) + m.clusterMsiKeyVaultStore = keyvault.NewManager(authorizer, clusterMsiKeyVaultURL) } return m, nil diff --git a/pkg/cluster/clustermsi.go b/pkg/cluster/clustermsi.go index 2ae9b03a70c..e6e9019642f 100644 --- a/pkg/cluster/clustermsi.go +++ b/pkg/cluster/clustermsi.go @@ -5,6 +5,7 @@ package cluster import ( "context" + "encoding/json" "errors" "fmt" "net/http" @@ -12,9 +13,10 @@ import ( "time" "github.com/Azure/azure-sdk-for-go/sdk/azcore" + "github.com/Azure/azure-sdk-for-go/services/keyvault/v7.0/keyvault" + "github.com/Azure/go-autorest/autorest/date" "github.com/Azure/msi-dataplane/pkg/dataplane" - "github.com/Azure/msi-dataplane/pkg/dataplane/swagger" - "github.com/Azure/msi-dataplane/pkg/store" + "k8s.io/utils/ptr" "github.com/Azure/ARO-RP/pkg/api" "github.com/Azure/ARO-RP/pkg/env" @@ -39,7 +41,7 @@ func (m *manager) ensureClusterMsiCertificate(ctx context.Context) error { return err } - _, err = m.clusterMsiKeyVaultStore.GetCredentialsObject(ctx, secretName) + _, err = m.clusterMsiKeyVaultStore.GetSecret(ctx, secretName) if err == nil { return nil } else if azcoreErr, ok := err.(*azcore.ResponseError); !ok || azcoreErr.StatusCode != http.StatusNotFound { @@ -51,13 +53,16 @@ func (m *manager) ensureClusterMsiCertificate(ctx context.Context) error { return err } - uaMsiRequest := dataplane.UserAssignedMSIRequest{ - IdentityURL: m.doc.OpenShiftCluster.Identity.IdentityURL, - ResourceIDs: []string{clusterMsiResourceId.String()}, - TenantID: m.doc.OpenShiftCluster.Identity.TenantID, + uaMsiRequest := dataplane.UserAssignedIdentitiesRequest{ + DelegatedResources: &[]string{clusterMsiResourceId.String()}, } - msiCredObj, err := m.msiDataplane.GetUserAssignedIdentities(ctx, uaMsiRequest) + client, err := m.msiDataplane.NewClient(m.doc.OpenShiftCluster.Identity.IdentityURL) + if err != nil { + return err + } + + msiCredObj, err := client.GetUserAssignedIdentitiesCredentials(ctx, uaMsiRequest) if err != nil { return err } @@ -76,21 +81,27 @@ func (m *manager) ensureClusterMsiCertificate(ctx context.Context) error { return errors.New("unable to pull NotAfter from the MSI CredentialsObject") } - // The swagger API spec for the MI RP specifies that NotAfter will be "in the format 2017-03-01T14:11:00Z". + // The OpenAPI spec for the MI RP specifies that NotAfter will be "in the format 2017-03-01T14:11:00Z". expirationDate, err = time.Parse(time.RFC3339, *identity.NotAfter) if err != nil { return err } } - secretProperties := store.SecretProperties{ - Enabled: true, - Expires: expirationDate, - Name: secretName, - NotBefore: now, + raw, err := json.Marshal(msiCredObj) + if err != nil { + return err } - return m.clusterMsiKeyVaultStore.SetCredentialsObject(ctx, secretProperties, msiCredObj.CredentialsObject) + return m.clusterMsiKeyVaultStore.SetSecret(ctx, secretName, keyvault.SecretSetParameters{ + Value: ptr.To(string(raw)), + SecretAttributes: &keyvault.SecretAttributes{ + Enabled: ptr.To(true), + Expires: ptr.To(date.UnixTime(expirationDate)), + NotBefore: ptr.To(date.UnixTime(expirationDate)), + }, + Tags: nil, + }) } // initializeClusterMsiClients intializes any Azure clients that use the cluster @@ -101,17 +112,21 @@ func (m *manager) initializeClusterMsiClients(ctx context.Context) error { return err } - kvSecret, err := m.clusterMsiKeyVaultStore.GetCredentialsObject(ctx, secretName) + kvSecretResponse, err := m.clusterMsiKeyVaultStore.GetSecret(ctx, secretName) if err != nil { return err } - cloud, err := m.env.Environment().CloudNameForMsiDataplane() - if err != nil { + if kvSecretResponse.Value == nil { + return fmt.Errorf("secret %q in keyvault missing value", secretName) + } + + var kvSecret dataplane.ManagedIdentityCredentials + if err := json.Unmarshal([]byte(*kvSecretResponse.Value), &kvSecret); err != nil { return err } - uaIdentities, err := dataplane.NewUserAssignedIdentities(kvSecret.CredentialsObject, cloud) + cloud, err := m.env.Environment().CloudNameForMsiDataplane() if err != nil { return err } @@ -121,9 +136,20 @@ func (m *manager) initializeClusterMsiClients(ctx context.Context) error { return err } - azureCred, err := uaIdentities.GetCredential(msiResourceId.String()) - if err != nil { - return err + var azureCred azcore.TokenCredential + if kvSecret.ExplicitIdentities != nil { + for _, identity := range *kvSecret.ExplicitIdentities { + if identity.ResourceId != nil && *identity.ResourceId == msiResourceId.String() { + var err error + azureCred, err = dataplane.GetCredential(cloud, identity) + if err != nil { + return fmt.Errorf("failed to get credential for msi identity %q: %v", msiResourceId, err) + } + } + } + } + if azureCred == nil { + return fmt.Errorf("managed identity credential missing user-assigned identity %q", msiResourceId) } // Note that we are assuming that all of the platform MIs are in the same subscription as the ARO resource. @@ -165,13 +191,16 @@ func (m *manager) clusterIdentityIDs(ctx context.Context) error { return err } - uaMsiRequest := dataplane.UserAssignedMSIRequest{ - IdentityURL: m.doc.OpenShiftCluster.Identity.IdentityURL, - ResourceIDs: []string{clusterMsiResourceId.String()}, - TenantID: m.doc.OpenShiftCluster.Identity.TenantID, + uaMsiRequest := dataplane.UserAssignedIdentitiesRequest{ + DelegatedResources: &[]string{clusterMsiResourceId.String()}, + } + + client, err := m.msiDataplane.NewClient(m.doc.OpenShiftCluster.Identity.IdentityURL) + if err != nil { + return err } - msiCredObj, err := m.msiDataplane.GetUserAssignedIdentities(ctx, uaMsiRequest) + msiCredObj, err := client.GetUserAssignedIdentitiesCredentials(ctx, uaMsiRequest) if err != nil { return err } @@ -180,7 +209,7 @@ func (m *manager) clusterIdentityIDs(ctx context.Context) error { if err != nil { return err } - if identity.ClientID == nil || identity.ObjectID == nil { + if identity.ClientId == nil || identity.ObjectId == nil { return fmt.Errorf("unable to pull clientID and objectID from the MSI CredentialsObject") } @@ -190,8 +219,8 @@ func (m *manager) clusterIdentityIDs(ctx context.Context) error { // passed-in casing on IDs even if it may be incorrect for k, v := range doc.OpenShiftCluster.Identity.UserAssignedIdentities { if strings.EqualFold(k, clusterMsiResourceId.String()) { - v.ClientID = *identity.ClientID - v.PrincipalID = *identity.ObjectID + v.ClientID = *identity.ClientId + v.PrincipalID = *identity.ObjectId doc.OpenShiftCluster.Identity.UserAssignedIdentities[k] = v return nil @@ -207,14 +236,13 @@ func (m *manager) clusterIdentityIDs(ctx context.Context) error { // We expect the GetUserAssignedIdentities request to only ever be made for one identity // at a time (the cluster MSI) and thus we expect the response to only contain a single // identity's details. -func getSingleExplicitIdentity(msiCredObj *dataplane.UserAssignedIdentities) (*swagger.NestedCredentialsObject, error) { +func getSingleExplicitIdentity(msiCredObj *dataplane.ManagedIdentityCredentials) (dataplane.UserAssignedIdentityCredentials, error) { if msiCredObj.ExplicitIdentities == nil || - len(msiCredObj.ExplicitIdentities) == 0 || - msiCredObj.ExplicitIdentities[0] == nil { - return nil, errClusterMsiNotPresentInResponse + len(*msiCredObj.ExplicitIdentities) == 0 { + return dataplane.UserAssignedIdentityCredentials{}, errClusterMsiNotPresentInResponse } - return msiCredObj.ExplicitIdentities[0], nil + return (*msiCredObj.ExplicitIdentities)[0], nil } // fixupClusterMsiTenantID repopulates the cluster MSI's tenant ID in the cluster doc by diff --git a/pkg/cluster/clustermsi_test.go b/pkg/cluster/clustermsi_test.go index 94ed16eca05..b0573f474a7 100644 --- a/pkg/cluster/clustermsi_test.go +++ b/pkg/cluster/clustermsi_test.go @@ -5,17 +5,15 @@ package cluster import ( "context" + "encoding/json" + "errors" "fmt" "strings" "testing" "github.com/Azure/azure-sdk-for-go/sdk/azcore" - "github.com/Azure/azure-sdk-for-go/sdk/azcore/policy" - "github.com/Azure/azure-sdk-for-go/sdk/security/keyvault/azsecrets" + azkeyvault "github.com/Azure/azure-sdk-for-go/services/keyvault/v7.0/keyvault" "github.com/Azure/msi-dataplane/pkg/dataplane" - "github.com/Azure/msi-dataplane/pkg/dataplane/swagger" - "github.com/Azure/msi-dataplane/pkg/store" - mockkvclient "github.com/Azure/msi-dataplane/pkg/store/mock_kvclient" "github.com/sirupsen/logrus" "github.com/stretchr/testify/assert" "go.uber.org/mock/gomock" @@ -24,6 +22,8 @@ import ( "github.com/Azure/ARO-RP/pkg/env" "github.com/Azure/ARO-RP/pkg/frontend/middleware" mock_env "github.com/Azure/ARO-RP/pkg/util/mocks/env" + mock_keyvault "github.com/Azure/ARO-RP/pkg/util/mocks/keyvault" + mock_msidataplane "github.com/Azure/ARO-RP/pkg/util/mocks/msidataplane" testdatabase "github.com/Azure/ARO-RP/test/database" utilerror "github.com/Azure/ARO-RP/test/util/error" ) @@ -40,35 +40,26 @@ func TestEnsureClusterMsiCertificate(t *testing.T) { StatusCode: 404, } - msiDataPlaneNotFoundError := `failed to get credentials: Request information not available --------------------------------------------------------------------------------- -RESPONSE 404: -ERROR CODE UNAVAILABLE --------------------------------------------------------------------------------- -Response contained no body --------------------------------------------------------------------------------- -` - placeholderString := "placeholder" - placeholderCredentialsObject := swagger.CredentialsObject{ - ExplicitIdentities: []*swagger.NestedCredentialsObject{ + placeholderCredentialsObject := &dataplane.ManagedIdentityCredentials{ + ExplicitIdentities: &[]dataplane.UserAssignedIdentityCredentials{ { - ClientID: &placeholderString, + ClientId: &placeholderString, ClientSecret: &placeholderString, - TenantID: &placeholderString, - ResourceID: &miResourceId, + TenantId: &placeholderString, + ResourceId: &miResourceId, AuthenticationEndpoint: &placeholderString, CannotRenewAfter: &placeholderString, - ClientSecretURL: &placeholderString, + ClientSecretUrl: &placeholderString, MtlsAuthenticationEndpoint: &placeholderString, NotAfter: &placeholderString, NotBefore: &placeholderString, RenewAfter: &placeholderString, - CustomClaims: &swagger.CustomClaims{ - XMSAzNwperimid: []*string{&placeholderString}, - XMSAzTm: &placeholderString, + CustomClaims: &dataplane.CustomClaims{ + XmsAzNwperimid: &[]string{placeholderString}, + XmsAzTm: &placeholderString, }, - ObjectID: &placeholderString, + ObjectId: &placeholderString, }, }, } @@ -76,24 +67,11 @@ Response contained no body tests := []struct { name string doc *api.OpenShiftClusterDocument - msiDataplaneStub policy.ClientOptions + msiDataplaneStub func(*mock_msidataplane.MockClient) envMocks func(*mock_env.MockInterface) - kvClientMocks func(*mockkvclient.MockKeyVaultClient) + kvClientMocks func(*mock_keyvault.MockManager) wantErr string }{ - { - name: "error - invalid resource ID (theoretically not possible, but still)", - doc: &api.OpenShiftClusterDocument{ - OpenShiftCluster: &api.OpenShiftCluster{ - Identity: &api.ManagedServiceIdentity{ - UserAssignedIdentities: map[string]api.UserAssignedIdentity{ - "Hi hello I'm not a valid resource ID": {}, - }, - }, - }, - }, - wantErr: "invalid resource ID: resource id 'Hi hello I'm not a valid resource ID' must start with '/'", - }, { name: "error - encounter error checking for an existing certificate in the key vault", doc: &api.OpenShiftClusterDocument{ @@ -111,8 +89,8 @@ Response contained no body }, }, }, - kvClientMocks: func(kvclient *mockkvclient.MockKeyVaultClient) { - kvclient.EXPECT().GetSecret(gomock.Any(), secretName, gomock.Any(), gomock.Any()).Times(1).Return(azsecrets.GetSecretResponse{}, fmt.Errorf("error in GetSecret")) + kvClientMocks: func(kvclient *mock_keyvault.MockManager) { + kvclient.EXPECT().GetSecret(gomock.Any(), secretName).Times(1).Return(azkeyvault.SecretBundle{}, fmt.Errorf("error in GetSecret")) }, wantErr: "error in GetSecret", }, @@ -133,17 +111,13 @@ Response contained no body }, }, }, - msiDataplaneStub: policy.ClientOptions{ - Transport: dataplane.NewStub([]*dataplane.CredentialsObject{ - { - CredentialsObject: swagger.CredentialsObject{}, - }, - }), + msiDataplaneStub: func(client *mock_msidataplane.MockClient) { + client.EXPECT().GetUserAssignedIdentitiesCredentials(gomock.Any(), gomock.Any()).Return(&dataplane.ManagedIdentityCredentials{}, errors.New("error in msi")) }, - kvClientMocks: func(kvclient *mockkvclient.MockKeyVaultClient) { - kvclient.EXPECT().GetSecret(gomock.Any(), secretName, gomock.Any(), gomock.Any()).Times(1).Return(azsecrets.GetSecretResponse{}, secretNotFoundError) + kvClientMocks: func(kvclient *mock_keyvault.MockManager) { + kvclient.EXPECT().GetSecret(gomock.Any(), secretName).Times(1).Return(azkeyvault.SecretBundle{}, secretNotFoundError) }, - wantErr: msiDataPlaneNotFoundError, + wantErr: "error in msi", }, { name: "success - exit early because there is already a certificate in the key vault", @@ -162,19 +136,17 @@ Response contained no body }, }, }, - kvClientMocks: func(kvclient *mockkvclient.MockKeyVaultClient) { - credentialsObjectBuffer, err := placeholderCredentialsObject.MarshalJSON() + kvClientMocks: func(kvclient *mock_keyvault.MockManager) { + credentialsObjectBuffer, err := json.Marshal(placeholderCredentialsObject) if err != nil { panic(err) } credentialsObjectString := string(credentialsObjectBuffer) - getSecretResponse := azsecrets.GetSecretResponse{ - Secret: azsecrets.Secret{ - Value: &credentialsObjectString, - }, + getSecretResponse := azkeyvault.SecretBundle{ + Value: &credentialsObjectString, } - kvclient.EXPECT().GetSecret(gomock.Any(), secretName, gomock.Any(), gomock.Any()).Times(1).Return(getSecretResponse, nil) + kvclient.EXPECT().GetSecret(gomock.Any(), secretName).Times(1).Return(getSecretResponse, nil) }, }, { @@ -194,19 +166,15 @@ Response contained no body }, }, }, - msiDataplaneStub: policy.ClientOptions{ - Transport: dataplane.NewStub([]*dataplane.CredentialsObject{ - { - CredentialsObject: placeholderCredentialsObject, - }, - }), + msiDataplaneStub: func(client *mock_msidataplane.MockClient) { + client.EXPECT().GetUserAssignedIdentitiesCredentials(gomock.Any(), gomock.Any()).Return(placeholderCredentialsObject, nil) }, envMocks: func(mockEnv *mock_env.MockInterface) { mockEnv.EXPECT().FeatureIsSet(env.FeatureUseMockMsiRp).Return(true) }, - kvClientMocks: func(kvclient *mockkvclient.MockKeyVaultClient) { - kvclient.EXPECT().GetSecret(gomock.Any(), secretName, gomock.Any(), gomock.Any()).Times(1).Return(azsecrets.GetSecretResponse{}, secretNotFoundError) - kvclient.EXPECT().SetSecret(gomock.Any(), secretName, gomock.Any(), gomock.Any()).Times(1).Return(azsecrets.SetSecretResponse{}, nil) + kvClientMocks: func(kvclient *mock_keyvault.MockManager) { + kvclient.EXPECT().GetSecret(gomock.Any(), secretName).Times(1).Return(azkeyvault.SecretBundle{}, secretNotFoundError) + kvclient.EXPECT().SetSecret(gomock.Any(), secretName, gomock.Any()).Times(1).Return(nil) }, }, } @@ -220,6 +188,7 @@ Response contained no body if tt.envMocks != nil { tt.envMocks(mockEnv) } + mockEnv.EXPECT().FeatureIsSet(env.FeatureUseMockMsiRp).Return(false).AnyTimes() m := manager{ log: logrus.NewEntry(logrus.StandardLogger()), @@ -227,21 +196,23 @@ Response contained no body env: mockEnv, } - msiDataplane, err := dataplane.NewClient(dataplane.AzurePublicCloud, nil, &tt.msiDataplaneStub) - if err != nil { - panic(err) + factory := mock_msidataplane.NewMockClientFactory(controller) + client := mock_msidataplane.NewMockClient(controller) + if tt.msiDataplaneStub != nil { + tt.msiDataplaneStub(client) } + factory.EXPECT().NewClient(gomock.Any()).Return(client, nil).AnyTimes() - m.msiDataplane = msiDataplane + m.msiDataplane = factory - mockKvClient := mockkvclient.NewMockKeyVaultClient(controller) + mockKvClient := mock_keyvault.NewMockManager(controller) if tt.kvClientMocks != nil { tt.kvClientMocks(mockKvClient) } - m.clusterMsiKeyVaultStore = store.NewMsiKeyVaultStore(mockKvClient) + m.clusterMsiKeyVaultStore = mockKvClient - err = m.ensureClusterMsiCertificate(ctx) + err := m.ensureClusterMsiCertificate(ctx) utilerror.AssertErrorMessage(t, err, tt.wantErr) }) } @@ -318,52 +289,40 @@ func TestClusterIdentityIDs(t *testing.T) { miResourceId := fmt.Sprintf("/subscriptions/%s/resourceGroups/%s/providers/Microsoft.ManagedIdentity/userAssignedIdentities/%s", mockGuid, clusterRGName, miName) miResourceIdIncorrectCasing := fmt.Sprintf("/subscriptions/%s/resourcegroups/%s/providers/Microsoft.ManagedIdentity/userAssignedIdentities/%s", mockGuid, clusterRGName, miName) - msiDataPlaneNotFoundError := `failed to get credentials: Request information not available --------------------------------------------------------------------------------- -RESPONSE 404: -ERROR CODE UNAVAILABLE --------------------------------------------------------------------------------- -Response contained no body --------------------------------------------------------------------------------- -` miClientId := "ffffffff-ffff-ffff-ffff-ffffffffffff" miObjectId := "99999999-9999-9999-9999-999999999999" placeholderString := "placeholder" - msiDataPlaneValidStub := policy.ClientOptions{ - Transport: dataplane.NewStub([]*dataplane.CredentialsObject{ - { - CredentialsObject: swagger.CredentialsObject{ - ExplicitIdentities: []*swagger.NestedCredentialsObject{ - { - ClientID: &miClientId, - ObjectID: &miObjectId, - ResourceID: &miResourceId, - - ClientSecret: &placeholderString, - TenantID: &placeholderString, - AuthenticationEndpoint: &placeholderString, - CannotRenewAfter: &placeholderString, - ClientSecretURL: &placeholderString, - MtlsAuthenticationEndpoint: &placeholderString, - NotAfter: &placeholderString, - NotBefore: &placeholderString, - RenewAfter: &placeholderString, - CustomClaims: &swagger.CustomClaims{ - XMSAzNwperimid: []*string{&placeholderString}, - XMSAzTm: &placeholderString, - }, - }, + msiDataPlaneValidStub := func(client *mock_msidataplane.MockClient) { + client.EXPECT().GetUserAssignedIdentitiesCredentials(gomock.Any(), gomock.Any()).Return(&dataplane.ManagedIdentityCredentials{ + ExplicitIdentities: &[]dataplane.UserAssignedIdentityCredentials{ + { + ClientId: &miClientId, + ObjectId: &miObjectId, + ResourceId: &miResourceId, + + ClientSecret: &placeholderString, + TenantId: &placeholderString, + AuthenticationEndpoint: &placeholderString, + CannotRenewAfter: &placeholderString, + ClientSecretUrl: &placeholderString, + MtlsAuthenticationEndpoint: &placeholderString, + NotAfter: &placeholderString, + NotBefore: &placeholderString, + RenewAfter: &placeholderString, + CustomClaims: &dataplane.CustomClaims{ + XmsAzNwperimid: &[]string{placeholderString}, + XmsAzTm: &placeholderString, }, }, }, - }), + }, nil) } for _, tt := range []struct { name string doc *api.OpenShiftClusterDocument - msiDataplaneStub policy.ClientOptions + msiDataplaneStub func(*mock_msidataplane.MockClient) wantDoc *api.OpenShiftClusterDocument wantErr string }{ @@ -419,14 +378,10 @@ Response contained no body }, }, }, - msiDataplaneStub: policy.ClientOptions{ - Transport: dataplane.NewStub([]*dataplane.CredentialsObject{ - { - CredentialsObject: swagger.CredentialsObject{}, - }, - }), + msiDataplaneStub: func(client *mock_msidataplane.MockClient) { + client.EXPECT().GetUserAssignedIdentitiesCredentials(gomock.Any(), gomock.Any()).Return(&dataplane.ManagedIdentityCredentials{}, errors.New("error in msi")) }, - wantErr: msiDataPlaneNotFoundError, + wantErr: "error in msi", }, { name: "success - ClientID and PrincipalID are updated", @@ -508,26 +463,31 @@ Response contained no body }, } { t.Run(tt.name, func(t *testing.T) { - msiDataplane, err := dataplane.NewClient(dataplane.AzurePublicCloud, nil, &tt.msiDataplaneStub) - if err != nil { - t.Fatal(err) - } + controller := gomock.NewController(t) + defer controller.Finish() openShiftClustersDatabase, _ := testdatabase.NewFakeOpenShiftClusters() fixture := testdatabase.NewFixture().WithOpenShiftClusters(openShiftClustersDatabase) fixture.AddOpenShiftClusterDocuments(tt.doc) - if err = fixture.Create(); err != nil { + if err := fixture.Create(); err != nil { t.Fatal(err) } + factory := mock_msidataplane.NewMockClientFactory(controller) + client := mock_msidataplane.NewMockClient(controller) + if tt.msiDataplaneStub != nil { + tt.msiDataplaneStub(client) + } + factory.EXPECT().NewClient(gomock.Any()).Return(client, nil).AnyTimes() + m := manager{ log: logrus.NewEntry(logrus.StandardLogger()), doc: tt.doc, db: openShiftClustersDatabase, - msiDataplane: msiDataplane, + msiDataplane: factory, } - err = m.clusterIdentityIDs(ctx) + err := m.clusterIdentityIDs(ctx) utilerror.AssertErrorMessage(t, err, tt.wantErr) if tt.wantDoc != nil { @@ -539,82 +499,48 @@ Response contained no body func TestGetSingleExplicitIdentity(t *testing.T) { placeholderString := "placeholder" - validIdentity := &swagger.NestedCredentialsObject{ - ClientID: &placeholderString, + validIdentity := dataplane.UserAssignedIdentityCredentials{ + ClientId: &placeholderString, ClientSecret: &placeholderString, - TenantID: &placeholderString, - ResourceID: &placeholderString, + TenantId: &placeholderString, + ResourceId: &placeholderString, AuthenticationEndpoint: &placeholderString, CannotRenewAfter: &placeholderString, - ClientSecretURL: &placeholderString, + ClientSecretUrl: &placeholderString, MtlsAuthenticationEndpoint: &placeholderString, NotAfter: &placeholderString, NotBefore: &placeholderString, RenewAfter: &placeholderString, - CustomClaims: &swagger.CustomClaims{ - XMSAzNwperimid: []*string{&placeholderString}, - XMSAzTm: &placeholderString, + CustomClaims: &dataplane.CustomClaims{ + XmsAzNwperimid: &[]string{placeholderString}, + XmsAzTm: &placeholderString, }, - ObjectID: &placeholderString, + ObjectId: &placeholderString, } for _, tt := range []struct { name string - msiCredObj *dataplane.UserAssignedIdentities - want *swagger.NestedCredentialsObject + msiCredObj *dataplane.ManagedIdentityCredentials + want dataplane.UserAssignedIdentityCredentials wantErr string }{ { name: "ExplicitIdentities nil, returns error", - msiCredObj: &dataplane.UserAssignedIdentities{}, + msiCredObj: &dataplane.ManagedIdentityCredentials{}, wantErr: errClusterMsiNotPresentInResponse.Error(), }, { name: "ExplicitIdentities empty, returns error", - msiCredObj: &dataplane.UserAssignedIdentities{ - CredentialsObject: dataplane.CredentialsObject{ - CredentialsObject: swagger.CredentialsObject{ - ExplicitIdentities: []*swagger.NestedCredentialsObject{}, - }, - }, - }, - wantErr: errClusterMsiNotPresentInResponse.Error(), - }, - { - name: "ExplicitIdentities first element is nil, returns error", - msiCredObj: &dataplane.UserAssignedIdentities{ - CredentialsObject: dataplane.CredentialsObject{ - CredentialsObject: swagger.CredentialsObject{ - ExplicitIdentities: []*swagger.NestedCredentialsObject{ - nil, - }, - }, - }, - }, - wantErr: errClusterMsiNotPresentInResponse.Error(), - }, - { - name: "ExplicitIdentities first element is nil, returns error", - msiCredObj: &dataplane.UserAssignedIdentities{ - CredentialsObject: dataplane.CredentialsObject{ - CredentialsObject: swagger.CredentialsObject{ - ExplicitIdentities: []*swagger.NestedCredentialsObject{ - nil, - }, - }, - }, + msiCredObj: &dataplane.ManagedIdentityCredentials{ + ExplicitIdentities: &[]dataplane.UserAssignedIdentityCredentials{}, }, wantErr: errClusterMsiNotPresentInResponse.Error(), }, { name: "ExplicitIdentities first element is valid, returns it", - msiCredObj: &dataplane.UserAssignedIdentities{ - CredentialsObject: dataplane.CredentialsObject{ - CredentialsObject: swagger.CredentialsObject{ - ExplicitIdentities: []*swagger.NestedCredentialsObject{ - validIdentity, - }, - }, + msiCredObj: &dataplane.ManagedIdentityCredentials{ + ExplicitIdentities: &[]dataplane.UserAssignedIdentityCredentials{ + validIdentity, }, }, want: validIdentity, diff --git a/pkg/cluster/delete.go b/pkg/cluster/delete.go index 0dbe8e11af4..c6db95d2c7d 100644 --- a/pkg/cluster/delete.go +++ b/pkg/cluster/delete.go @@ -369,12 +369,11 @@ func (m *manager) deleteClusterMsiCertificate(ctx context.Context) error { return err } - err = m.clusterMsiKeyVaultStore.DeleteCredentialsObject(ctx, secretName) - if err == nil || azuresdkerrors.IsNotFoundError(err) { - return nil + if _, err := m.clusterMsiKeyVaultStore.DeleteSecret(ctx, secretName); err != nil && !azuresdkerrors.IsNotFoundError(err) { + return err } - return err + return nil } func (m *manager) deleteFederatedCredentials(ctx context.Context) error { diff --git a/pkg/cluster/delete_test.go b/pkg/cluster/delete_test.go index 84c84b467ad..39b87f4cefc 100644 --- a/pkg/cluster/delete_test.go +++ b/pkg/cluster/delete_test.go @@ -12,14 +12,12 @@ import ( sdkmsi "github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/msi/armmsi" "github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/network/armnetwork/v2" - "github.com/Azure/azure-sdk-for-go/sdk/security/keyvault/azsecrets" + "github.com/Azure/azure-sdk-for-go/services/keyvault/v7.0/keyvault" mgmtnetwork "github.com/Azure/azure-sdk-for-go/services/network/mgmt/2020-08-01/network" mgmtfeatures "github.com/Azure/azure-sdk-for-go/services/resources/mgmt/2019-07-01/features" "github.com/Azure/go-autorest/autorest" "github.com/Azure/go-autorest/autorest/azure" "github.com/Azure/go-autorest/autorest/to" - "github.com/Azure/msi-dataplane/pkg/store" - mockkvclient "github.com/Azure/msi-dataplane/pkg/store/mock_kvclient" "github.com/sirupsen/logrus" "github.com/stretchr/testify/assert" "go.uber.org/mock/gomock" @@ -31,6 +29,7 @@ import ( mock_features "github.com/Azure/ARO-RP/pkg/util/mocks/azureclient/mgmt/features" mock_network "github.com/Azure/ARO-RP/pkg/util/mocks/azureclient/mgmt/network" mock_env "github.com/Azure/ARO-RP/pkg/util/mocks/env" + mock_keyvault "github.com/Azure/ARO-RP/pkg/util/mocks/keyvault" mock_subnet "github.com/Azure/ARO-RP/pkg/util/mocks/subnet" "github.com/Azure/ARO-RP/pkg/util/platformworkloadidentity" "github.com/Azure/ARO-RP/pkg/util/pointerutils" @@ -380,7 +379,7 @@ func TestDeleteClusterMsiCertificate(t *testing.T) { tests := []struct { name string doc *api.OpenShiftClusterDocument - mocks func(*mockkvclient.MockKeyVaultClient) + mocks func(mockManager *mock_keyvault.MockManager) wantErr string }{ { @@ -442,8 +441,8 @@ func TestDeleteClusterMsiCertificate(t *testing.T) { }, }, }, - mocks: func(kvclient *mockkvclient.MockKeyVaultClient) { - kvclient.EXPECT().DeleteSecret(gomock.Any(), fmt.Sprintf("%s-%s", mockGuid, miName), gomock.Any()).Times(1).Return(azsecrets.DeleteSecretResponse{}, fmt.Errorf("error in DeleteSecret")) + mocks: func(kvclient *mock_keyvault.MockManager) { + kvclient.EXPECT().DeleteSecret(gomock.Any(), fmt.Sprintf("%s-%s", mockGuid, miName)).Times(1).Return(keyvault.DeletedSecretBundle{}, fmt.Errorf("error in DeleteSecret")) }, wantErr: "error in DeleteSecret", }, @@ -462,8 +461,8 @@ func TestDeleteClusterMsiCertificate(t *testing.T) { }, }, }, - mocks: func(kvclient *mockkvclient.MockKeyVaultClient) { - kvclient.EXPECT().DeleteSecret(gomock.Any(), fmt.Sprintf("%s-%s", mockGuid, miName), gomock.Any()).Times(1).Return(azsecrets.DeleteSecretResponse{}, nil) + mocks: func(kvclient *mock_keyvault.MockManager) { + kvclient.EXPECT().DeleteSecret(gomock.Any(), fmt.Sprintf("%s-%s", mockGuid, miName)).Times(1).Return(keyvault.DeletedSecretBundle{}, nil) }, }, } @@ -478,12 +477,12 @@ func TestDeleteClusterMsiCertificate(t *testing.T) { doc: tt.doc, } - mockKvClient := mockkvclient.NewMockKeyVaultClient(controller) + mockKvClient := mock_keyvault.NewMockManager(controller) if tt.mocks != nil { tt.mocks(mockKvClient) } - m.clusterMsiKeyVaultStore = store.NewMsiKeyVaultStore(mockKvClient) + m.clusterMsiKeyVaultStore = mockKvClient err := m.deleteClusterMsiCertificate(ctx) utilerror.AssertErrorMessage(t, err, tt.wantErr) diff --git a/pkg/env/env.go b/pkg/env/env.go index 46601fa2cc8..815ce3dfbe6 100644 --- a/pkg/env/env.go +++ b/pkg/env/env.go @@ -17,6 +17,7 @@ import ( "github.com/Azure/azure-sdk-for-go/sdk/azidentity" mgmtcompute "github.com/Azure/azure-sdk-for-go/services/compute/mgmt/2020-06-01/compute" "github.com/Azure/go-autorest/autorest" + "github.com/Azure/msi-dataplane/pkg/dataplane" "github.com/hashicorp/go-multierror" "github.com/sirupsen/logrus" @@ -102,7 +103,8 @@ type Interface interface { OIDCEndpoint() string OIDCKeyBitSize() int MsiRpEndpoint() string - MsiDataplaneClientOptions(msiResourceId *arm.ResourceID) (*policy.ClientOptions, error) + MsiDataplaneClientOptions() (*policy.ClientOptions, error) + MockMSIResponses(msiResourceId *arm.ResourceID) dataplane.ClientFactory AROOperatorImage() string LiveConfig() liveconfig.Manager diff --git a/pkg/env/prod.go b/pkg/env/prod.go index 4fd76f60e51..70ae54b9f6f 100644 --- a/pkg/env/prod.go +++ b/pkg/env/prod.go @@ -21,7 +21,6 @@ import ( "github.com/Azure/go-autorest/autorest" "github.com/Azure/go-autorest/autorest/azure" "github.com/Azure/msi-dataplane/pkg/dataplane" - "github.com/Azure/msi-dataplane/pkg/dataplane/swagger" "github.com/jongio/azidext/go/azidext" "github.com/sirupsen/logrus" "k8s.io/utils/ptr" @@ -399,52 +398,87 @@ func (p *prod) MsiRpEndpoint() string { return fmt.Sprintf("https://%s", os.Getenv("MSI_RP_ENDPOINT")) } -func (p *prod) MsiDataplaneClientOptions(msiResourceId *arm.ResourceID) (*policy.ClientOptions, error) { +func (p *prod) MsiDataplaneClientOptions() (*policy.ClientOptions, error) { armClientOptions := p.Environment().ArmClientOptions() clientOptions := armClientOptions.ClientOptions - if p.FeatureIsSet(FeatureUseMockMsiRp) { - keysToValidate := []string{ - "MOCK_MSI_CLIENT_ID", - "MOCK_MSI_OBJECT_ID", - "MOCK_MSI_CERT", - "MOCK_MSI_TENANT_ID", - } + return &clientOptions, nil +} - if err := ValidateVars(keysToValidate...); err != nil { - return nil, err - } +func (p *prod) MockMSIResponses(msiResourceId *arm.ResourceID) dataplane.ClientFactory { + return &mockFactory{aadHost: p.Environment().Cloud.ActiveDirectoryAuthorityHost, msiResourceId: msiResourceId.String()} +} + +func MockMSIResponses(aadHost string, msiResourceId *arm.ResourceID) dataplane.ClientFactory { + return &mockFactory{aadHost: aadHost, msiResourceId: msiResourceId.String()} +} + +type mockFactory struct { + aadHost string + msiResourceId string +} + +var _ dataplane.ClientFactory = (*mockFactory)(nil) + +func (m *mockFactory) NewClient(identityURL string) (dataplane.Client, error) { + return &mockClient{ + aadHost: m.aadHost, + msiResourceId: m.msiResourceId, + }, nil +} + +type mockClient struct { + aadHost string + msiResourceId string +} + +var _ dataplane.Client = (*mockClient)(nil) + +func (m *mockClient) DeleteSystemAssignedIdentity(ctx context.Context) error { + panic("not yet implemented") +} + +func (m *mockClient) GetSystemAssignedIdentityCredentials(ctx context.Context) (*dataplane.ManagedIdentityCredentials, error) { + panic("not yet implemented") +} + +func (m *mockClient) GetUserAssignedIdentitiesCredentials(ctx context.Context, request dataplane.UserAssignedIdentitiesRequest) (*dataplane.ManagedIdentityCredentials, error) { + keysToValidate := []string{ + "MOCK_MSI_CLIENT_ID", + "MOCK_MSI_OBJECT_ID", + "MOCK_MSI_CERT", + "MOCK_MSI_TENANT_ID", + } + + if err := ValidateVars(keysToValidate...); err != nil { + return nil, err + } - // Every attribute on the NestedCredentialsObject needs to be set for the mock MSI - // dataplane to be happy. - placeholder := "placeholder" - clientOptions.Transport = dataplane.NewStub([]*dataplane.CredentialsObject{ + placeholder := "placeholder" + return &dataplane.ManagedIdentityCredentials{ + ExplicitIdentities: &[]dataplane.UserAssignedIdentityCredentials{ { - CredentialsObject: swagger.CredentialsObject{ - ExplicitIdentities: []*swagger.NestedCredentialsObject{ - { - ClientID: ptr.To(os.Getenv("MOCK_MSI_CLIENT_ID")), - ClientSecret: ptr.To(os.Getenv("MOCK_MSI_CERT")), - TenantID: ptr.To(os.Getenv("MOCK_MSI_TENANT_ID")), - ObjectID: ptr.To(os.Getenv("MOCK_MSI_OBJECT_ID")), - ResourceID: ptr.To(msiResourceId.String()), - AuthenticationEndpoint: ptr.To(p.Environment().Cloud.ActiveDirectoryAuthorityHost), - CannotRenewAfter: &placeholder, - ClientSecretURL: &placeholder, - MtlsAuthenticationEndpoint: &placeholder, - NotAfter: &placeholder, - NotBefore: &placeholder, - RenewAfter: &placeholder, - CustomClaims: &swagger.CustomClaims{ - XMSAzNwperimid: []*string{&placeholder}, - XMSAzTm: &placeholder, - }, - }, - }, + ClientId: ptr.To(os.Getenv("MOCK_MSI_CLIENT_ID")), + ClientSecret: ptr.To(os.Getenv("MOCK_MSI_CERT")), + TenantId: ptr.To(os.Getenv("MOCK_MSI_TENANT_ID")), + ObjectId: ptr.To(os.Getenv("MOCK_MSI_OBJECT_ID")), + ResourceId: ptr.To(m.msiResourceId), + AuthenticationEndpoint: ptr.To(m.aadHost), + CannotRenewAfter: &placeholder, + ClientSecretUrl: &placeholder, + MtlsAuthenticationEndpoint: &placeholder, + NotAfter: &placeholder, + NotBefore: &placeholder, + RenewAfter: &placeholder, + CustomClaims: &dataplane.CustomClaims{ + XmsAzNwperimid: &[]string{placeholder}, + XmsAzTm: &placeholder, }, }, - }) - } + }, + }, nil +} - return &clientOptions, nil +func (m *mockClient) MoveIdentity(ctx context.Context, request dataplane.MoveIdentityRequest) (*dataplane.MoveIdentityResponse, error) { + panic("not yet implemented") } diff --git a/pkg/frontend/middleware/msi.go b/pkg/frontend/middleware/msi.go index c4462231d0c..84996ea359d 100644 --- a/pkg/frontend/middleware/msi.go +++ b/pkg/frontend/middleware/msi.go @@ -6,19 +6,21 @@ package middleware import ( "net/http" "os" - - "github.com/Azure/msi-dataplane/pkg/dataplane" ) const ( + MsiIdentityURLHeader = "x-ms-identity-url" + MsiPrincipalIDHeader = "x-ms-identity-principal-id" + MsiTenantHeader = "x-ms-home-tenant-id" + MockIdentityURL = "https://bogus.identity.azure.net/subscriptions/00000000-0000-0000-0000-000000000000/resourcegroups/rg/providers/Microsoft.ApiManagement/service/test/credentials?tid=00000000-0000-0000-0000-000000000000&oid=00000000-0000-0000-0000-000000000000&aid=00000000-0000-0000-0000-000000000000" mockTenantIDEnvVar = "MOCK_MSI_TENANT_ID" ) func MockMSIMiddleware(h http.Handler) http.Handler { return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - r.Header.Set(dataplane.MsiIdentityURLHeader, MockIdentityURL) - r.Header.Set(dataplane.MsiTenantHeader, os.Getenv(mockTenantIDEnvVar)) + r.Header.Set(MsiIdentityURLHeader, MockIdentityURL) + r.Header.Set(MsiTenantHeader, os.Getenv(mockTenantIDEnvVar)) h.ServeHTTP(w, r) }) } diff --git a/pkg/frontend/middleware/msi_test.go b/pkg/frontend/middleware/msi_test.go index 0fcde2aec63..52904f61945 100644 --- a/pkg/frontend/middleware/msi_test.go +++ b/pkg/frontend/middleware/msi_test.go @@ -7,8 +7,6 @@ import ( "net/http" "net/http/httptest" "testing" - - "github.com/Azure/msi-dataplane/pkg/dataplane" ) func TestMockMSIMiddleware(t *testing.T) { @@ -23,10 +21,10 @@ func TestMockMSIMiddleware(t *testing.T) { handler.ServeHTTP(rr, req) - if req.Header.Get(dataplane.MsiIdentityURLHeader) != MockIdentityURL { - t.Errorf("Expected %s, got %s", MockIdentityURL, req.Header.Get(dataplane.MsiIdentityURLHeader)) + if req.Header.Get(MsiIdentityURLHeader) != MockIdentityURL { + t.Errorf("Expected %s, got %s", MockIdentityURL, req.Header.Get(MsiIdentityURLHeader)) } - if req.Header.Get(dataplane.MsiTenantHeader) != mockTenantID { - t.Errorf("Expected %s, got %s", mockTenantID, req.Header.Get(dataplane.MsiTenantHeader)) + if req.Header.Get(MsiTenantHeader) != mockTenantID { + t.Errorf("Expected %s, got %s", mockTenantID, req.Header.Get(MsiTenantHeader)) } } diff --git a/pkg/frontend/openshiftcluster_putorpatch_test.go b/pkg/frontend/openshiftcluster_putorpatch_test.go index b111dd100ed..8b1ca1974f3 100644 --- a/pkg/frontend/openshiftcluster_putorpatch_test.go +++ b/pkg/frontend/openshiftcluster_putorpatch_test.go @@ -14,7 +14,6 @@ import ( "testing" "time" - "github.com/Azure/msi-dataplane/pkg/dataplane" "go.uber.org/mock/gomock" "github.com/Azure/ARO-RP/pkg/api" @@ -4106,8 +4105,8 @@ func TestPutOrPatchOpenShiftCluster(t *testing.T) { var internal api.OpenShiftCluster f.apis["2024-08-12-preview"].OpenShiftClusterConverter.ToInternal(oc, &internal) if internal.UsesWorkloadIdentity() { - requestHeaders.Add(dataplane.MsiIdentityURLHeader, middleware.MockIdentityURL) - requestHeaders.Add(dataplane.MsiTenantHeader, mockGuid) + requestHeaders.Add(middleware.MsiIdentityURLHeader, middleware.MockIdentityURL) + requestHeaders.Add(middleware.MsiTenantHeader, mockGuid) } resp, b, err := ti.request(method, diff --git a/pkg/util/azureclient/environments.go b/pkg/util/azureclient/environments.go index c85a1f3426d..7c2ce4b9a99 100644 --- a/pkg/util/azureclient/environments.go +++ b/pkg/util/azureclient/environments.go @@ -4,7 +4,6 @@ package azureclient // Licensed under the Apache License 2.0. import ( - "errors" "fmt" "net/http" "strings" @@ -183,18 +182,6 @@ func (e *AROEnvironment) NewGraphServiceClient(tokenCredential azcore.TokenCrede // cloud name. This function might seem a little strange, but it's necessary because // the cloud names stored in the AROEnvironments are in all-caps, whereas the ones // defined as constants in the dataplane module are in camel case. -func (e *AROEnvironment) CloudNameForMsiDataplane() (string, error) { - cloud := "" - switch strings.ToUpper(e.Name) { - case dataplane.AzurePublicCloud: - cloud = dataplane.AzurePublicCloud - case dataplane.AzureUSGovCloud: - cloud = dataplane.AzureUSGovCloud - } - - if cloud == "" { - return "", errors.New("could not determine which Azure Cloud to use to instantiate MSI dataplane client") - } - - return cloud, nil +func (e *AROEnvironment) CloudNameForMsiDataplane() (dataplane.AzureCloud, error) { + return dataplane.AzureCloud(e.Name), nil } diff --git a/pkg/util/azureclient/keyvault/base.go b/pkg/util/azureclient/keyvault/base.go index 8ceadc3e9df..8c5467e2846 100644 --- a/pkg/util/azureclient/keyvault/base.go +++ b/pkg/util/azureclient/keyvault/base.go @@ -19,6 +19,7 @@ type BaseClient interface { GetCertificateOperation(ctx context.Context, vaultBaseURL string, certificateName string) (result azkeyvault.CertificateOperation, err error) GetCertificatePolicy(ctx context.Context, vaultBaseURL string, certificateName string) (result azkeyvault.CertificatePolicy, err error) GetSecret(ctx context.Context, vaultBaseURL string, secretName string, secretVersion string) (result azkeyvault.SecretBundle, err error) + DeleteSecret(ctx context.Context, vaultBaseURL string, secretName string) (result azkeyvault.DeletedSecretBundle, err error) GetCertificate(ctx context.Context, vaultBaseURL string, certificateName string, certificateVersion string) (result azkeyvault.CertificateBundle, err error) GetCertificates(ctx context.Context, vaultBaseURL string, maxresults *int32, includePending *bool) (result azkeyvault.CertificateListResultPage, err error) SetSecret(ctx context.Context, vaultBaseURL string, secretName string, parameters azkeyvault.SecretSetParameters) (result azkeyvault.SecretBundle, err error) diff --git a/pkg/util/keyvault/keyvault.go b/pkg/util/keyvault/keyvault.go index dc4c8c3102e..4503a233095 100644 --- a/pkg/util/keyvault/keyvault.go +++ b/pkg/util/keyvault/keyvault.go @@ -41,6 +41,7 @@ type Manager interface { GetCertificatePolicy(ctx context.Context, certificateName string) (azkeyvault.CertificatePolicy, error) GetCertificateSecret(context.Context, string) (*rsa.PrivateKey, []*x509.Certificate, error) GetSecret(context.Context, string) (azkeyvault.SecretBundle, error) + DeleteSecret(ctx context.Context, secretName string) (azkeyvault.DeletedSecretBundle, error) GetSecrets(context.Context) ([]azkeyvault.SecretItem, error) SetCertificateIssuer(ctx context.Context, issuerName string, parameter azkeyvault.CertificateIssuerSetParameters) (result azkeyvault.IssuerBundle, err error) SetSecret(context.Context, string, azkeyvault.SecretSetParameters) error @@ -212,6 +213,10 @@ func (m *manager) GetSecret(ctx context.Context, secretName string) (azkeyvault. return m.kv.GetSecret(ctx, m.keyvaultURI, secretName, "") } +func (m *manager) DeleteSecret(ctx context.Context, secretName string) (azkeyvault.DeletedSecretBundle, error) { + return m.kv.DeleteSecret(ctx, m.keyvaultURI, secretName) +} + func (m *manager) GetSecrets(ctx context.Context) ([]azkeyvault.SecretItem, error) { return m.kv.GetSecrets(ctx, m.keyvaultURI, nil) } diff --git a/pkg/util/mocks/azureclient/keyvault/keyvault.go b/pkg/util/mocks/azureclient/keyvault/keyvault.go index 1c25d07fe76..9014b14aff8 100644 --- a/pkg/util/mocks/azureclient/keyvault/keyvault.go +++ b/pkg/util/mocks/azureclient/keyvault/keyvault.go @@ -70,6 +70,21 @@ func (mr *MockBaseClientMockRecorder) DeleteCertificate(arg0, arg1, arg2 any) *g return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "DeleteCertificate", reflect.TypeOf((*MockBaseClient)(nil).DeleteCertificate), arg0, arg1, arg2) } +// DeleteSecret mocks base method. +func (m *MockBaseClient) DeleteSecret(arg0 context.Context, arg1, arg2 string) (keyvault.DeletedSecretBundle, error) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "DeleteSecret", arg0, arg1, arg2) + ret0, _ := ret[0].(keyvault.DeletedSecretBundle) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// DeleteSecret indicates an expected call of DeleteSecret. +func (mr *MockBaseClientMockRecorder) DeleteSecret(arg0, arg1, arg2 any) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "DeleteSecret", reflect.TypeOf((*MockBaseClient)(nil).DeleteSecret), arg0, arg1, arg2) +} + // GetCertificate mocks base method. func (m *MockBaseClient) GetCertificate(arg0 context.Context, arg1, arg2, arg3 string) (keyvault.CertificateBundle, error) { m.ctrl.T.Helper() diff --git a/pkg/util/mocks/env/env.go b/pkg/util/mocks/env/env.go index 787e5935143..fcf45e2771a 100644 --- a/pkg/util/mocks/env/env.go +++ b/pkg/util/mocks/env/env.go @@ -22,6 +22,7 @@ import ( azidentity "github.com/Azure/azure-sdk-for-go/sdk/azidentity" compute "github.com/Azure/azure-sdk-for-go/services/compute/mgmt/2020-06-01/compute" autorest "github.com/Azure/go-autorest/autorest" + dataplane "github.com/Azure/msi-dataplane/pkg/dataplane" logrus "github.com/sirupsen/logrus" gomock "go.uber.org/mock/gomock" @@ -499,19 +500,33 @@ func (mr *MockInterfaceMockRecorder) Logger() *gomock.Call { return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Logger", reflect.TypeOf((*MockInterface)(nil).Logger)) } +// MockMSIResponses mocks base method. +func (m *MockInterface) MockMSIResponses(msiResourceId *arm.ResourceID) dataplane.ClientFactory { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "MockMSIResponses", msiResourceId) + ret0, _ := ret[0].(dataplane.ClientFactory) + return ret0 +} + +// MockMSIResponses indicates an expected call of MockMSIResponses. +func (mr *MockInterfaceMockRecorder) MockMSIResponses(msiResourceId any) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "MockMSIResponses", reflect.TypeOf((*MockInterface)(nil).MockMSIResponses), msiResourceId) +} + // MsiDataplaneClientOptions mocks base method. -func (m *MockInterface) MsiDataplaneClientOptions(msiResourceId *arm.ResourceID) (*policy.ClientOptions, error) { +func (m *MockInterface) MsiDataplaneClientOptions() (*policy.ClientOptions, error) { m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "MsiDataplaneClientOptions", msiResourceId) + ret := m.ctrl.Call(m, "MsiDataplaneClientOptions") ret0, _ := ret[0].(*policy.ClientOptions) ret1, _ := ret[1].(error) return ret0, ret1 } // MsiDataplaneClientOptions indicates an expected call of MsiDataplaneClientOptions. -func (mr *MockInterfaceMockRecorder) MsiDataplaneClientOptions(msiResourceId any) *gomock.Call { +func (mr *MockInterfaceMockRecorder) MsiDataplaneClientOptions() *gomock.Call { mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "MsiDataplaneClientOptions", reflect.TypeOf((*MockInterface)(nil).MsiDataplaneClientOptions), msiResourceId) + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "MsiDataplaneClientOptions", reflect.TypeOf((*MockInterface)(nil).MsiDataplaneClientOptions)) } // MsiRpEndpoint mocks base method. diff --git a/pkg/util/mocks/keyvault/keyvault.go b/pkg/util/mocks/keyvault/keyvault.go index cb7836c1e01..c513bcb35a9 100644 --- a/pkg/util/mocks/keyvault/keyvault.go +++ b/pkg/util/mocks/keyvault/keyvault.go @@ -58,6 +58,21 @@ func (mr *MockManagerMockRecorder) CreateSignedCertificate(arg0, arg1, arg2, arg return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "CreateSignedCertificate", reflect.TypeOf((*MockManager)(nil).CreateSignedCertificate), arg0, arg1, arg2, arg3, arg4) } +// DeleteSecret mocks base method. +func (m *MockManager) DeleteSecret(arg0 context.Context, arg1 string) (keyvault0.DeletedSecretBundle, error) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "DeleteSecret", arg0, arg1) + ret0, _ := ret[0].(keyvault0.DeletedSecretBundle) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// DeleteSecret indicates an expected call of DeleteSecret. +func (mr *MockManagerMockRecorder) DeleteSecret(arg0, arg1 any) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "DeleteSecret", reflect.TypeOf((*MockManager)(nil).DeleteSecret), arg0, arg1) +} + // EnsureCertificateDeleted mocks base method. func (m *MockManager) EnsureCertificateDeleted(arg0 context.Context, arg1 string) error { m.ctrl.T.Helper() diff --git a/pkg/util/mocks/msidataplane/client.go b/pkg/util/mocks/msidataplane/client.go new file mode 100644 index 00000000000..b16159ee9bd --- /dev/null +++ b/pkg/util/mocks/msidataplane/client.go @@ -0,0 +1,100 @@ +// Code generated by MockGen. DO NOT EDIT. +// Source: github.com/Azure/msi-dataplane/pkg/dataplane (interfaces: Client) +// +// Generated by this command: +// +// mockgen -imports=internal=github.com/Azure/msi-dataplane/pkg/dataplane -destination=../mocks/msidataplane/client.go github.com/Azure/msi-dataplane/pkg/dataplane Client +// + +// Package mock_dataplane is a generated GoMock package. +package mock_dataplane + +import ( + context "context" + reflect "reflect" + + "github.com/Azure/msi-dataplane/pkg/dataplane" + gomock "go.uber.org/mock/gomock" +) + +// MockClient is a mock of Client interface. +type MockClient struct { + ctrl *gomock.Controller + recorder *MockClientMockRecorder +} + +// MockClientMockRecorder is the mock recorder for MockClient. +type MockClientMockRecorder struct { + mock *MockClient +} + +// NewMockClient creates a new mock instance. +func NewMockClient(ctrl *gomock.Controller) *MockClient { + mock := &MockClient{ctrl: ctrl} + mock.recorder = &MockClientMockRecorder{mock} + return mock +} + +// EXPECT returns an object that allows the caller to indicate expected use. +func (m *MockClient) EXPECT() *MockClientMockRecorder { + return m.recorder +} + +// DeleteSystemAssignedIdentity mocks base method. +func (m *MockClient) DeleteSystemAssignedIdentity(arg0 context.Context) error { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "DeleteSystemAssignedIdentity", arg0) + ret0, _ := ret[0].(error) + return ret0 +} + +// DeleteSystemAssignedIdentity indicates an expected call of DeleteSystemAssignedIdentity. +func (mr *MockClientMockRecorder) DeleteSystemAssignedIdentity(arg0 any) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "DeleteSystemAssignedIdentity", reflect.TypeOf((*MockClient)(nil).DeleteSystemAssignedIdentity), arg0) +} + +// GetSystemAssignedIdentityCredentials mocks base method. +func (m *MockClient) GetSystemAssignedIdentityCredentials(arg0 context.Context) (*dataplane.ManagedIdentityCredentials, error) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "GetSystemAssignedIdentityCredentials", arg0) + ret0, _ := ret[0].(*dataplane.ManagedIdentityCredentials) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// GetSystemAssignedIdentityCredentials indicates an expected call of GetSystemAssignedIdentityCredentials. +func (mr *MockClientMockRecorder) GetSystemAssignedIdentityCredentials(arg0 any) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetSystemAssignedIdentityCredentials", reflect.TypeOf((*MockClient)(nil).GetSystemAssignedIdentityCredentials), arg0) +} + +// GetUserAssignedIdentitiesCredentials mocks base method. +func (m *MockClient) GetUserAssignedIdentitiesCredentials(arg0 context.Context, arg1 dataplane.UserAssignedIdentitiesRequest) (*dataplane.ManagedIdentityCredentials, error) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "GetUserAssignedIdentitiesCredentials", arg0, arg1) + ret0, _ := ret[0].(*dataplane.ManagedIdentityCredentials) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// GetUserAssignedIdentitiesCredentials indicates an expected call of GetUserAssignedIdentitiesCredentials. +func (mr *MockClientMockRecorder) GetUserAssignedIdentitiesCredentials(arg0, arg1 any) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetUserAssignedIdentitiesCredentials", reflect.TypeOf((*MockClient)(nil).GetUserAssignedIdentitiesCredentials), arg0, arg1) +} + +// MoveIdentity mocks base method. +func (m *MockClient) MoveIdentity(arg0 context.Context, arg1 dataplane.MoveIdentityRequest) (*dataplane.MoveIdentityResponse, error) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "MoveIdentity", arg0, arg1) + ret0, _ := ret[0].(*dataplane.MoveIdentityResponse) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// MoveIdentity indicates an expected call of MoveIdentity. +func (mr *MockClientMockRecorder) MoveIdentity(arg0, arg1 any) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "MoveIdentity", reflect.TypeOf((*MockClient)(nil).MoveIdentity), arg0, arg1) +} diff --git a/pkg/util/mocks/msidataplane/client_factory.go b/pkg/util/mocks/msidataplane/client_factory.go new file mode 100644 index 00000000000..b41cb0f654b --- /dev/null +++ b/pkg/util/mocks/msidataplane/client_factory.go @@ -0,0 +1,55 @@ +// Code generated by MockGen. DO NOT EDIT. +// Source: github.com/Azure/msi-dataplane/pkg/dataplane (interfaces: ClientFactory) +// +// Generated by this command: +// +// mockgen -destination=../mocks/msidataplane/client_factory.go github.com/Azure/msi-dataplane/pkg/dataplane ClientFactory +// + +// Package mock_dataplane is a generated GoMock package. +package mock_dataplane + +import ( + reflect "reflect" + + dataplane "github.com/Azure/msi-dataplane/pkg/dataplane" + gomock "go.uber.org/mock/gomock" +) + +// MockClientFactory is a mock of ClientFactory interface. +type MockClientFactory struct { + ctrl *gomock.Controller + recorder *MockClientFactoryMockRecorder +} + +// MockClientFactoryMockRecorder is the mock recorder for MockClientFactory. +type MockClientFactoryMockRecorder struct { + mock *MockClientFactory +} + +// NewMockClientFactory creates a new mock instance. +func NewMockClientFactory(ctrl *gomock.Controller) *MockClientFactory { + mock := &MockClientFactory{ctrl: ctrl} + mock.recorder = &MockClientFactoryMockRecorder{mock} + return mock +} + +// EXPECT returns an object that allows the caller to indicate expected use. +func (m *MockClientFactory) EXPECT() *MockClientFactoryMockRecorder { + return m.recorder +} + +// NewClient mocks base method. +func (m *MockClientFactory) NewClient(arg0 string) (dataplane.Client, error) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "NewClient", arg0) + ret0, _ := ret[0].(dataplane.Client) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// NewClient indicates an expected call of NewClient. +func (mr *MockClientFactoryMockRecorder) NewClient(arg0 any) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "NewClient", reflect.TypeOf((*MockClientFactory)(nil).NewClient), arg0) +} diff --git a/pkg/util/msidataplane/generate.go b/pkg/util/msidataplane/generate.go new file mode 100644 index 00000000000..83cb0cde2f9 --- /dev/null +++ b/pkg/util/msidataplane/generate.go @@ -0,0 +1,14 @@ +package msidataplane + +// Copyright (c) Microsoft Corporation. +// Licensed under the Apache License 2.0. + +//go:generate rm -rf ../mocks/$GOPACKAGE/client_factory.go +//go:generate mockgen -destination=../mocks/$GOPACKAGE/client_factory.go github.com/Azure/msi-dataplane/pkg/dataplane ClientFactory +//go:generate goimports -local=github.com/Azure/ARO-RP -e -w ../mocks/$GOPACKAGE/client_factory.go + +// mockgen unfortunately walks the type aliases in the dataplane package, and tries to import their +// underlying sources from an internal package, so the generated mock is useful as a first step but +// is not functional as-is +// go:generate mockgen -destination=../mocks/$GOPACKAGE/client.go github.com/Azure/msi-dataplane/pkg/dataplane Client +// go:generate goimports -local=github.com/Azure/ARO-RP -e -w ../mocks/$GOPACKAGE/client.go