Merge pull request #1805 from RomanBednar/gcp-wif

STOR-1988: support GCP WIF in credentials request controller

Author: openshift-merge-bot[bot]

pkg/operator/csi/credentialsrequestcontroller/credentials_request_controller.go

@@ -3,6 +3,7 @@ package credentialsrequestcontroller
 import (
 	"context"
 	"fmt"
+	"k8s.io/klog/v2"
 	"os"
 	"strings"
 	"time"
@@ -89,13 +90,11 @@ func (c CredentialsRequestController) sync(ctx context.Context, syncContext fact
 		return err
 	}
 
-	clusterCloudCredential, err := c.operatorLister.Get(clusterCloudCredentialName)
+	sync, err := shouldSync(c.operatorLister)
 	if err != nil {
 		return err
 	}
-
-	// if clusterCloudCredential is in manual mode without STS, do not sync cloud credentials
-	if clusterCloudCredential.Spec.CredentialsMode == opv1.CloudCredentialsModeManual && os.Getenv("ROLEARN") == "" {
+	if !sync {
 		return nil
 	}
 
@@ -193,3 +192,29 @@ func isProvisioned(cr *unstructured.Unstructured) (bool, error) {
 
 	return provisionedValBool, nil
 }
+
+func shouldSync(cloudCredentialLister operatorv1lister.CloudCredentialLister) (bool, error) {
+	clusterCloudCredential, err := cloudCredentialLister.Get(clusterCloudCredentialName)
+	if err != nil {
+		klog.Errorf("Failed to get cluster cloud credential: %v", err)
+		return false, err
+	}
+
+	isManualMode := clusterCloudCredential.Spec.CredentialsMode == opv1.CloudCredentialsModeManual
+
+	isAWSSTSEnabled := os.Getenv("ROLEARN") != ""
+
+	poolID := os.Getenv("POOL_ID")
+	providerID := os.Getenv("PROVIDER_ID")
+	serviceAccountEmail := os.Getenv("SERVICE_ACCOUNT_EMAIL")
+	projectNumber := os.Getenv("PROJECT_NUMBER")
+
+	isGCPWIFEnabled := poolID != "" && providerID != "" && serviceAccountEmail != "" && projectNumber != ""
+
+	// If cluster is in manual mode without short-term credentials enabled, do not sync cloud credentials.
+	if isManualMode && !isAWSSTSEnabled && !isGCPWIFEnabled {
+		return false, nil
+	}
+
+	return true, nil
+}

pkg/operator/csi/credentialsrequestcontroller/credentials_request_controller_test.go

@@ -4,6 +4,7 @@ import (
 	"context"
 	"errors"
 	"fmt"
+	"os"
 	"reflect"
 	"strconv"
 	"testing"
@@ -319,3 +320,138 @@ func (fake *fakeDynamicClient) Apply(ctx context.Context, name string, obj *unst
 func (fake *fakeDynamicClient) ApplyStatus(ctx context.Context, name string, obj *unstructured.Unstructured, options metav1.ApplyOptions) (*unstructured.Unstructured, error) {
 	return nil, errors.New("not implemented")
 }
+
+func TestShouldSync(t *testing.T) {
+	tests := []struct {
+		name               string
+		cloudCredential    *opv1.CloudCredential
+		envVars            map[string]string
+		expectedShouldSync bool
+		expectedError      bool
+	}{
+		{
+			name: "Default mode",
+			cloudCredential: &opv1.CloudCredential{
+				ObjectMeta: metav1.ObjectMeta{
+					Name: clusterCloudCredentialName,
+				},
+				Spec: opv1.CloudCredentialSpec{
+					CredentialsMode: opv1.CloudCredentialsModeDefault,
+				},
+			},
+			expectedShouldSync: true,
+			expectedError:      false,
+		},
+		{
+			name: "Manual mode without short-term credentials",
+			cloudCredential: &opv1.CloudCredential{
+				ObjectMeta: metav1.ObjectMeta{
+					Name: clusterCloudCredentialName,
+				},
+				Spec: opv1.CloudCredentialSpec{
+					CredentialsMode: opv1.CloudCredentialsModeManual,
+				},
+			},
+			expectedShouldSync: false,
+			expectedError:      false,
+		},
+		{
+			name: "Manual mode with AWS STS enabled",
+			cloudCredential: &opv1.CloudCredential{
+				ObjectMeta: metav1.ObjectMeta{
+					Name: clusterCloudCredentialName,
+				},
+				Spec: opv1.CloudCredentialSpec{
+					CredentialsMode: opv1.CloudCredentialsModeManual,
+				},
+			},
+			envVars: map[string]string{
+				"ROLEARN": "arn:aws:iam::123456789012:role/test-role",
+			},
+			expectedShouldSync: true,
+			expectedError:      false,
+		},
+		{
+			name: "Manual mode with GCP WIF enabled",
+			cloudCredential: &opv1.CloudCredential{
+				ObjectMeta: metav1.ObjectMeta{
+					Name: clusterCloudCredentialName,
+				},
+				Spec: opv1.CloudCredentialSpec{
+					CredentialsMode: opv1.CloudCredentialsModeManual,
+				},
+			},
+			envVars: map[string]string{
+				"POOL_ID":               "test-pool",
+				"PROVIDER_ID":           "test-provider",
+				"SERVICE_ACCOUNT_EMAIL": "[email protected]",
+				"PROJECT_NUMBER":        "123456789",
+			},
+			expectedShouldSync: true,
+			expectedError:      false,
+		},
+		{
+			name: "Manual mode with partial GCP WIF configuration",
+			cloudCredential: &opv1.CloudCredential{
+				ObjectMeta: metav1.ObjectMeta{
+					Name: clusterCloudCredentialName,
+				},
+				Spec: opv1.CloudCredentialSpec{
+					CredentialsMode: opv1.CloudCredentialsModeManual,
+				},
+			},
+			envVars: map[string]string{
+				"POOL_ID":               "test-pool",
+				"PROVIDER_ID":           "test-provider",
+				"SERVICE_ACCOUNT_EMAIL": "[email protected]",
+			},
+			expectedShouldSync: false,
+			expectedError:      false,
+		},
+		{
+			name:               "Error getting cloud credential",
+			cloudCredential:    nil,
+			expectedShouldSync: false,
+			expectedError:      true,
+		},
+	}
+
+	for _, tc := range tests {
+		t.Run(tc.name, func(t *testing.T) {
+			// Setup
+			typedVersionedOperatorClient := fakeoperatorv1client.NewSimpleClientset()
+			cloudCredentialInformer := operatorinformer.NewSharedInformerFactory(typedVersionedOperatorClient, 1*time.Minute)
+
+			if tc.cloudCredential != nil {
+				err := cloudCredentialInformer.Operator().V1().CloudCredentials().Informer().GetStore().Add(tc.cloudCredential)
+				if err != nil {
+					t.Fatalf("Failed to add cloud credential to store: %v", err)
+				}
+			}
+
+			// Set environment variables
+			for k, v := range tc.envVars {
+				os.Setenv(k, v)
+			}
+			defer func() {
+				for k := range tc.envVars {
+					os.Unsetenv(k)
+				}
+			}()
+
+			// Act
+			shouldSync, err := shouldSync(cloudCredentialInformer.Operator().V1().CloudCredentials().Lister())
+
+			// Assert
+			if tc.expectedError && err == nil {
+				t.Error("Expected an error, but got none")
+			}
+			if !tc.expectedError && err != nil {
+				t.Errorf("Unexpected error: %v", err)
+			}
+			if shouldSync != tc.expectedShouldSync {
+				t.Errorf("Expected shouldSync to be %v, but got %v", tc.expectedShouldSync, shouldSync)
+			}
+		})
+	}
+}