*: use a separate client for root credentials

If we're using a filtered LIST + WATCH for Secrets to do our work on CredentialRequests, we also need to open a second LIST + WATCH for the root credential, as this won't have the labels we add to our own secrets. Signed-off-by: Steve Kuznetsov <skuznets@redhat.com>
openshift · Jul 5, 2023 · 3cc1acf · 3cc1acf
1 parent 8d56106
commit 3cc1acf
Show file tree

Hide file tree

Showing 22 changed files with 438 additions and 258 deletions.
diff --git a/pkg/aws/actuator/actuator.go b/pkg/aws/actuator/actuator.go
@@ -69,14 +69,15 @@ var _ actuatoriface.Actuator = (*AWSActuator)(nil)
 // AWSActuator implements the CredentialsRequest Actuator interface to create credentials in AWS.
 type AWSActuator struct {
 	Client                             client.Client
+	RootCredClient                     client.Client
 	Codec                              *minterv1.ProviderCodec
 	AWSClientBuilder                   func(accessKeyID, secretAccessKey []byte, c client.Client) (ccaws.Client, error)
 	Scheme                             *runtime.Scheme
 	AWSSecurityTokenServiceGateEnabled bool
 }
 
 // NewAWSActuator creates a new AWSActuator.
-func NewAWSActuator(client client.Client, scheme *runtime.Scheme, awsSecurityTokenServiceGateEnabled bool) (*AWSActuator, error) {
+func NewAWSActuator(client, rootCredClient client.Client, scheme *runtime.Scheme, awsSecurityTokenServiceGateEnabled bool) (*AWSActuator, error) {
 	codec, err := minterv1.NewCodec()
 	if err != nil {
 		log.WithError(err).Error("error creating AWS codec")
@@ -86,6 +87,7 @@ func NewAWSActuator(client client.Client, scheme *runtime.Scheme, awsSecurityTok
 	return &AWSActuator{
 		Codec:                              codec,
 		Client:                             client,
+		RootCredClient:                     rootCredClient,
 		AWSClientBuilder:                   awsutils.ClientBuilder,
 		Scheme:                             scheme,
 		AWSSecurityTokenServiceGateEnabled: awsSecurityTokenServiceGateEnabled,
@@ -943,13 +945,13 @@ func (a *AWSActuator) buildRootAWSClient(cr *minterv1.CredentialsRequest) (minte
 	logger.Debug("loading AWS credentials from secret")
 	// TODO: Running in a 4.0 cluster we expect this secret to exist. When we run in a Hive
 	// cluster, we need to load different secrets for each cluster.
-	accessKeyID, secretAccessKey, err := utils.LoadCredsFromSecret(a.Client, constants.CloudCredSecretNamespace, constants.AWSCloudCredSecretName)
+	accessKeyID, secretAccessKey, err := utils.LoadCredsFromSecret(a.RootCredClient, constants.CloudCredSecretNamespace, constants.AWSCloudCredSecretName)
 	if err != nil {
 		return nil, err
 	}
 
 	logger.Debug("creating root AWS client")
-	return a.AWSClientBuilder(accessKeyID, secretAccessKey, a.Client)
+	return a.AWSClientBuilder(accessKeyID, secretAccessKey, a.RootCredClient)
 }
 
 // buildReadAWSClient will return an AWS client using the the scaled down read only AWS creds
@@ -1109,7 +1111,7 @@ func (a *AWSActuator) GetCredentialsRootSecretLocation() types.NamespacedName {
 func (a *AWSActuator) GetCredentialsRootSecret(ctx context.Context, cr *minterv1.CredentialsRequest) (*corev1.Secret, error) {
 	logger := a.getLogger(cr)
 	cloudCredSecret := &corev1.Secret{}
-	if err := a.Client.Get(ctx, a.GetCredentialsRootSecretLocation(), cloudCredSecret); err != nil {
+	if err := a.RootCredClient.Get(ctx, a.GetCredentialsRootSecretLocation(), cloudCredSecret); err != nil {
 		msg := "unable to fetch root cloud cred secret"
 		logger.WithError(err).Error(msg)
 		return nil, &actuatoriface.ActuatorError{
@@ -1397,7 +1399,7 @@ func awsSTSIAMRoleARN(codec *minterv1.ProviderCodec, credentialsRequest *minterv
 // if the system is considered not upgradeable. Otherwise, return nil as the default
 // value is for things to be upgradeable.
 func (a *AWSActuator) Upgradeable(mode operatorv1.CloudCredentialsMode) *configv1.ClusterOperatorStatusCondition {
-	return utils.UpgradeableCheck(a.Client, mode, a.GetCredentialsRootSecretLocation())
+	return utils.UpgradeableCheck(a.RootCredClient, mode, a.GetCredentialsRootSecretLocation())
 }
 
 func generateAWSCredentialsConfig(accessKeyID, secretAccessKey string) []byte {

diff --git a/pkg/aws/actuator/actuator_test.go b/pkg/aws/actuator/actuator_test.go
@@ -84,6 +84,7 @@ func TestCredentialsFetching(t *testing.T) {
 	tests := []struct {
 		name                  string
 		existing              []runtime.Object
+		existingAdmin         []runtime.Object
 		credentialsRequest    *minterv1.CredentialsRequest
 		expectedError         bool
 		validate              func(*testing.T, *awsClientBuilderRecorder)
@@ -110,8 +111,9 @@ func TestCredentialsFetching(t *testing.T) {
 			},
 		},
 		{
-			name: "no read only secret",
-			existing: []runtime.Object{
+			name:     "no read only secret",
+			existing: []runtime.Object{},
+			existingAdmin: []runtime.Object{
 				testRootSecret(),
 			},
 			clientBuilderRecorder: func(mockCtrl *gomock.Controller) *awsClientBuilderRecorder {
@@ -131,6 +133,8 @@ func TestCredentialsFetching(t *testing.T) {
 			name: "read only creds not ready",
 			existing: []runtime.Object{
 				testReadOnlySecret(),
+			},
+			existingAdmin: []runtime.Object{
 				testRootSecret(),
 			},
 			clientBuilderRecorder: func(mockCtrl *gomock.Controller) *awsClientBuilderRecorder {
@@ -196,6 +200,7 @@ func TestCredentialsFetching(t *testing.T) {
 
 			test.existing = append(test.existing, test.credentialsRequest)
 			fakeClient := fake.NewClientBuilder().WithRuntimeObjects(test.existing...).Build()
+			fakeAdminClient := fake.NewClientBuilder().WithRuntimeObjects(test.existingAdmin...).Build()
 
 			mockCtrl := gomock.NewController(t)
 			defer mockCtrl.Finish()
@@ -204,6 +209,7 @@ func TestCredentialsFetching(t *testing.T) {
 
 			a := &AWSActuator{
 				Client:           fakeClient,
+				RootCredClient:   fakeAdminClient,
 				Codec:            codec,
 				AWSClientBuilder: clientRecorder.ClientBuilder,
 			}
@@ -363,8 +369,8 @@ func TestUpgradeable(t *testing.T) {
 			defer mockCtrl.Finish()
 
 			a := &AWSActuator{
-				Client: fakeClient,
-				Codec:  codec,
+				RootCredClient: fakeClient,
+				Codec:          codec,
 			}
 
 			cond := a.Upgradeable(test.mode)
@@ -653,12 +659,14 @@ func TestDetectSTS(t *testing.T) {
 				WithScheme(scheme.Scheme).
 				WithStatusSubresource(&minterv1.CredentialsRequest{}).
 				WithRuntimeObjects(test.existing...).Build()
+			fakeAdminClient := fake.NewClientBuilder().Build()
 			err := fakeClient.Create(context.TODO(), testAuthentication(test.issuer))
 			if err != nil {
 				panic(err)
 			}
 			a := &AWSActuator{
 				Client:                             fakeClient,
+				RootCredClient:                     fakeAdminClient,
 				Codec:                              codec,
 				AWSSecurityTokenServiceGateEnabled: test.stsEnabled,
 			}

diff --git a/pkg/azure/actuator.go b/pkg/azure/actuator.go
@@ -54,14 +54,14 @@ func (a *Actuator) STSFeatureGateEnabled() bool {
 	return false
 }
 
-func NewActuator(c client.Client, cloudName configv1.AzureCloudEnvironment) (*Actuator, error) {
+func NewActuator(c, rootCredClient client.Client, cloudName configv1.AzureCloudEnvironment) (*Actuator, error) {
 	codec, err := minterv1.NewCodec()
 	if err != nil {
 		log.WithError(err).Error("error creating Azure codec")
 		return nil, fmt.Errorf("error creating Azure codec: %v", err)
 	}
 
-	client := newClientWrapper(c)
+	client := newClientWrapper(c, rootCredClient)
 	return &Actuator{
 		client: client,
 		codec:  codec,
@@ -71,11 +71,11 @@ func NewActuator(c client.Client, cloudName configv1.AzureCloudEnvironment) (*Ac
 	}, nil
 }
 
-func NewFakeActuator(c client.Client, codec *minterv1.ProviderCodec,
+func NewFakeActuator(c, rootCredClient client.Client, codec *minterv1.ProviderCodec,
 	credentialMinterBuilder credentialMinterBuilder,
 ) *Actuator {
 	return &Actuator{
-		client:                  newClientWrapper(c),
+		client:                  newClientWrapper(c, rootCredClient),
 		codec:                   codec,
 		credentialMinterBuilder: credentialMinterBuilder,
 	}
@@ -437,7 +437,7 @@ func (a *Actuator) GetCredentialsRootSecretLocation() types.NamespacedName {
 func (a *Actuator) GetCredentialsRootSecret(ctx context.Context, cr *minterv1.CredentialsRequest) (*corev1.Secret, error) {
 	logger := a.getLogger(cr)
 	cloudCredSecret := &corev1.Secret{}
-	if err := a.client.Client.Get(ctx, a.GetCredentialsRootSecretLocation(), cloudCredSecret); err != nil {
+	if err := a.client.RootCredClient.Get(ctx, a.GetCredentialsRootSecretLocation(), cloudCredSecret); err != nil {
 		msg := "unable to fetch root cloud cred secret"
 		logger.WithError(err).Error(msg)
 		return nil, &actuatoriface.ActuatorError{
@@ -506,5 +506,5 @@ func (a *Actuator) getLogger(cr *minterv1.CredentialsRequest) log.FieldLogger {
 // if the system is considered not upgradeable. Otherwise, return nil as the default
 // value is for things to be upgradeable.
 func (a *Actuator) Upgradeable(mode operatorv1.CloudCredentialsMode) *configv1.ClusterOperatorStatusCondition {
-	return utils.UpgradeableCheck(a.client.Client, mode, a.GetCredentialsRootSecretLocation())
+	return utils.UpgradeableCheck(a.client.RootCredClient, mode, a.GetCredentialsRootSecretLocation())
 }