operator: Provide Azure region for managed credentials using environm…

…ent variable (grafana#11964)

operator/CHANGELOG.md

@@ -1,5 +1,6 @@
 ## Main
 
+- [11964](https://github.com/grafana/loki/pull/11964) **xperimental**: Provide Azure region for managed credentials using environment variable
 - [11920](https://github.com/grafana/loki/pull/11920) **xperimental**: Refactor handling of credentials in managed-auth mode
 - [11869](https://github.com/grafana/loki/pull/11869) **periklis**: Add support for running with Google Workload Identity
 - [11868](https://github.com/grafana/loki/pull/11868) **xperimental**: Integrate support for OpenShift-managed credentials in Azure

operator/internal/config/managed_auth.go

@@ -26,6 +26,7 @@ func discoverManagedAuthConfig() *ManagedAuthConfig {
 	clientID := os.Getenv("CLIENTID")
 	tenantID := os.Getenv("TENANTID")
 	subscriptionID := os.Getenv("SUBSCRIPTIONID")
+	region := os.Getenv("REGION")
 
 	switch {
 	case roleARN != "":
@@ -40,6 +41,7 @@ func discoverManagedAuthConfig() *ManagedAuthConfig {
 				ClientID:       clientID,
 				SubscriptionID: subscriptionID,
 				TenantID:       tenantID,
+				Region:         region,
 			},
 		}
 	}

operator/internal/handlers/credentialsrequest_create.go → operator/internal/handlers/credentialsrequest.go

@@ -2,12 +2,10 @@ package handlers
 
 import (
 	"context"
-	"errors"
 	"fmt"
 
 	"github.com/ViaQ/logerr/v2/kverrors"
 	"github.com/go-logr/logr"
-	corev1 "k8s.io/api/core/v1"
 	apierrors "k8s.io/apimachinery/pkg/api/errors"
 	"k8s.io/apimachinery/pkg/runtime"
 	ctrl "sigs.k8s.io/controller-runtime"
@@ -19,11 +17,8 @@ import (
 	"github.com/grafana/loki/operator/internal/external/k8s"
 	"github.com/grafana/loki/operator/internal/manifests"
 	"github.com/grafana/loki/operator/internal/manifests/openshift"
-	"github.com/grafana/loki/operator/internal/manifests/storage"
 )
 
-var errAzureNoRegion = errors.New("can not create CredentialsRequest: missing secret field: region")
-
 // CreateCredentialsRequest creates a new CredentialsRequest resource for a Lokistack
 // to request a cloud credentials Secret resource from the OpenShift cloud-credentials-operator.
 func CreateCredentialsRequest(ctx context.Context, log logr.Logger, scheme *runtime.Scheme, managedAuth *config.ManagedAuthConfig, k k8s.Client, req ctrl.Request) error {
@@ -39,32 +34,6 @@ func CreateCredentialsRequest(ctx context.Context, log logr.Logger, scheme *runt
 		return kverrors.Wrap(err, "failed to lookup LokiStack", "name", req.String())
 	}
 
-	if managedAuth.Azure != nil && managedAuth.Azure.Region == "" {
-		// Managed environment for Azure does not provide Region, but we need this for the CredentialsRequest.
-		// This looks like an oversight when creating the UI in OpenShift, but for now we need to pull this data
-		// from somewhere else -> the Azure Storage Secret
-		storageSecretName := client.ObjectKey{
-			Namespace: stack.Namespace,
-			Name:      stack.Spec.Storage.Secret.Name,
-		}
-		storageSecret := &corev1.Secret{}
-		if err := k.Get(ctx, storageSecretName, storageSecret); err != nil {
-			if apierrors.IsNotFound(err) {
-				// Skip this error here as it will be picked up by the LokiStack handler instead
-				ll.Error(err, "could not find secret for LokiStack", "name", req.String())
-				return nil
-			}
-			return err
-		}
-
-		region := storageSecret.Data[storage.KeyAzureRegion]
-		if len(region) == 0 {
-			return errAzureNoRegion
-		}
-
-		managedAuth.Azure.Region = string(region)
-	}
-
 	opts := openshift.Options{
 		BuildOpts: openshift.BuildOptions{
 			LokiStackName:      stack.Name,

operator/internal/handlers/credentialsrequest_create_test.go → operator/internal/handlers/credentialsrequest_test.go

@@ -6,7 +6,6 @@ import (
 
 	cloudcredentialv1 "github.com/openshift/cloud-credential-operator/pkg/apis/cloudcredential/v1"
 	"github.com/stretchr/testify/require"
-	corev1 "k8s.io/api/core/v1"
 	"k8s.io/apimachinery/pkg/api/errors"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/runtime/schema"
@@ -19,7 +18,7 @@ import (
 	"github.com/grafana/loki/operator/internal/external/k8s/k8sfakes"
 )
 
-func credentialsRequestFakeClient(cr *cloudcredentialv1.CredentialsRequest, lokistack *lokiv1.LokiStack, secret *corev1.Secret) *k8sfakes.FakeClient {
+func credentialsRequestFakeClient(cr *cloudcredentialv1.CredentialsRequest, lokistack *lokiv1.LokiStack) *k8sfakes.FakeClient {
 	k := &k8sfakes.FakeClient{}
 	k.GetStub = func(_ context.Context, name types.NamespacedName, object client.Object, _ ...client.GetOption) error {
 		switch object.(type) {
@@ -33,11 +32,6 @@ func credentialsRequestFakeClient(cr *cloudcredentialv1.CredentialsRequest, loki
 				return errors.NewNotFound(schema.GroupResource{}, name.Name)
 			}
 			k.SetClientObject(object, lokistack)
-		case *corev1.Secret:
-			if secret == nil {
-				return errors.NewNotFound(schema.GroupResource{}, name.Name)
-			}
-			k.SetClientObject(object, secret)
 		}
 		return nil
 	}
@@ -58,7 +52,7 @@ func TestCreateCredentialsRequest_CreateNewResource(t *testing.T) {
 		},
 	}
 
-	k := credentialsRequestFakeClient(nil, lokistack, nil)
+	k := credentialsRequestFakeClient(nil, lokistack)
 	req := ctrl.Request{
 		NamespacedName: client.ObjectKey{Name: "my-stack", Namespace: "ns"},
 	}
@@ -89,13 +83,8 @@ func TestCreateCredentialsRequest_CreateNewResourceAzure(t *testing.T) {
 			Namespace: "ns",
 		},
 	}
-	secret := &corev1.Secret{
-		Data: map[string][]byte{
-			"region": []byte(wantRegion),
-		},
-	}
 
-	k := credentialsRequestFakeClient(nil, lokistack, secret)
+	k := credentialsRequestFakeClient(nil, lokistack)
 	req := ctrl.Request{
 		NamespacedName: client.ObjectKey{Name: "my-stack", Namespace: "ns"},
 	}
@@ -105,6 +94,7 @@ func TestCreateCredentialsRequest_CreateNewResourceAzure(t *testing.T) {
 			ClientID:       "test-client-id",
 			SubscriptionID: "test-tenant-id",
 			TenantID:       "test-subscription-id",
+			Region:         "test-region",
 		},
 	}
 
@@ -122,47 +112,6 @@ func TestCreateCredentialsRequest_CreateNewResourceAzure(t *testing.T) {
 	require.Equal(t, wantRegion, providerSpec.AzureRegion)
 }
 
-func TestCreateCredentialsRequest_CreateNewResourceAzure_Errors(t *testing.T) {
-	lokistack := &lokiv1.LokiStack{
-		ObjectMeta: metav1.ObjectMeta{
-			Name:      "my-stack",
-			Namespace: "ns",
-		},
-	}
-	req := ctrl.Request{
-		NamespacedName: client.ObjectKey{Name: "my-stack", Namespace: "ns"},
-	}
-
-	tt := []struct {
-		secret    *corev1.Secret
-		wantError string
-	}{
-		{
-			secret:    &corev1.Secret{},
-			wantError: errAzureNoRegion.Error(),
-		},
-	}
-
-	for _, tc := range tt {
-		tc := tc
-		t.Run(tc.wantError, func(t *testing.T) {
-			t.Parallel()
-
-			managedAuth := &config.ManagedAuthConfig{
-				Azure: &config.AzureEnvironment{
-					ClientID:       "test-client-id",
-					SubscriptionID: "test-tenant-id",
-					TenantID:       "test-subscription-id",
-				},
-			}
-			k := credentialsRequestFakeClient(nil, lokistack, tc.secret)
-
-			err := CreateCredentialsRequest(context.Background(), logger, scheme, managedAuth, k, req)
-			require.EqualError(t, err, tc.wantError)
-		})
-	}
-}
-
 func TestCreateCredentialsRequest_DoNothing_WhenCredentialsRequestExist(t *testing.T) {
 	req := ctrl.Request{
 		NamespacedName: client.ObjectKey{Name: "my-stack", Namespace: "ns"},
@@ -187,7 +136,7 @@ func TestCreateCredentialsRequest_DoNothing_WhenCredentialsRequestExist(t *testi
 		},
 	}
 
-	k := credentialsRequestFakeClient(cr, lokistack, nil)
+	k := credentialsRequestFakeClient(cr, lokistack)
 
 	err := CreateCredentialsRequest(context.Background(), logger, scheme, managedAuth, k, req)
 	require.NoError(t, err)

operator/internal/handlers/internal/storage/secrets.go

@@ -182,18 +182,11 @@ func extractAzureConfigSecret(s *corev1.Secret, fg configv1.FeatureGates) (*stor
 	// Extract and validate optional fields
 	endpointSuffix := s.Data[storage.KeyAzureStorageEndpointSuffix]
 	audience := s.Data[storage.KeyAzureAudience]
-	region := s.Data[storage.KeyAzureRegion]
 
 	if !workloadIdentity && len(audience) > 0 {
 		return nil, fmt.Errorf("%w: %s", errSecretFieldNotAllowed, storage.KeyAzureAudience)
 	}
 
-	if fg.OpenShift.ManagedAuthEnv {
-		if len(region) == 0 {
-			return nil, fmt.Errorf("%w: %s", errSecretMissingField, storage.KeyAzureRegion)
-		}
-	}
-
 	return &storage.AzureStorageConfig{
 		Env:              string(env),
 		Container:        string(container),

operator/internal/handlers/internal/storage/secrets_test.go

@@ -156,27 +156,6 @@ func TestAzureExtract(t *testing.T) {
 			},
 			wantError: "missing secret field: subscription_id",
 		},
-		{
-			name: "managed auth - no region",
-			secret: &corev1.Secret{
-				ObjectMeta: metav1.ObjectMeta{Name: "test"},
-				Data: map[string][]byte{
-					"environment":  []byte("here"),
-					"account_name": []byte("test-account-name"),
-					"container":    []byte("this,that"),
-				},
-			},
-			managedSecret: &corev1.Secret{
-				Data: map[string][]byte{},
-			},
-			featureGates: configv1.FeatureGates{
-				OpenShift: configv1.OpenShiftFeatureGates{
-					Enabled:        true,
-					ManagedAuthEnv: true,
-				},
-			},
-			wantError: "missing secret field: region",
-		},
 		{
 			name: "managed auth - no auth override",
 			secret: &corev1.Secret{

operator/internal/manifests/openshift/credentialsrequest.go

@@ -12,6 +12,8 @@ import (
 	"github.com/grafana/loki/operator/internal/manifests/storage"
 )
 
+const azureFallbackRegion = "centralus"
+
 func BuildCredentialsRequest(opts Options) (*cloudcredentialv1.CredentialsRequest, error) {
 	stack := client.ObjectKey{Name: opts.BuildOpts.LokiStackName, Namespace: opts.BuildOpts.LokiStackNamespace}
 
@@ -62,6 +64,15 @@ func encodeProviderSpec(env *config.ManagedAuthConfig) (*runtime.RawExtension, e
 		}
 	case env.Azure != nil:
 		azure := env.Azure
+		if azure.Region == "" {
+			// The OpenShift Console currently does not provide a UI to configure the Azure Region
+			// for an operator using managed credentials. Because the CredentialsRequest is currently
+			// not used to create a Managed Identity, the region is actually never used.
+			// We default to the US region if nothing is set, so that the CredentialsRequest can be
+			// created. This should have no effect on the generated credential secret.
+			// The region can be configured by setting an environment variable on the operator Subscription.
+			azure.Region = azureFallbackRegion
+		}
 
 		spec = &cloudcredentialv1.AzureProviderSpec{
 			Permissions: []string{

operator/internal/manifests/storage/options.go

@@ -63,7 +63,6 @@ type AzureStorageConfig struct {
 	Container        string
 	EndpointSuffix   string
 	Audience         string
-	Region           string
 	WorkloadIdentity bool
 }
 

operator/internal/manifests/storage/var.go

@@ -88,8 +88,6 @@ const (
 	KeyAzureStorageEndpointSuffix = "endpoint_suffix"
 	// KeyAzureEnvironmentName is the secret data key for the Azure cloud environment name.
 	KeyAzureEnvironmentName = "environment"
-	// KeyAzureRegion is the secret data key for storing the Azure cloud region.
-	KeyAzureRegion = "region"
 	// KeyAzureAudience is the secret data key for customizing the audience used for the ServiceAccount token.
 	KeyAzureAudience = "audience"