Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

operator: Provide Azure region for managed credentials using environment variable #11964

Merged
merged 3 commits into from Feb 15, 2024
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.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions 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
Expand Down
2 changes: 2 additions & 0 deletions operator/internal/config/managed_auth.go
Expand Up @@ -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 != "":
Expand All @@ -40,6 +41,7 @@ func discoverManagedAuthConfig() *ManagedAuthConfig {
ClientID: clientID,
SubscriptionID: subscriptionID,
TenantID: tenantID,
Region: region,
},
}
}
Expand Down
Expand Up @@ -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"
Expand All @@ -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 {
Expand All @@ -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,
Expand Down
Expand Up @@ -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"
Expand All @@ -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) {
Expand All @@ -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
}
Expand All @@ -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"},
}
Expand Down Expand Up @@ -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"},
}
Expand All @@ -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",
},
}

Expand All @@ -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"},
Expand All @@ -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)
Expand Down
7 changes: 0 additions & 7 deletions operator/internal/handlers/internal/storage/secrets.go
Expand Up @@ -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),
Expand Down
21 changes: 0 additions & 21 deletions operator/internal/handlers/internal/storage/secrets_test.go
Expand Up @@ -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{
Expand Down
11 changes: 11 additions & 0 deletions operator/internal/manifests/openshift/credentialsrequest.go
Expand Up @@ -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}

Expand Down Expand Up @@ -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{
Expand Down
1 change: 0 additions & 1 deletion operator/internal/manifests/storage/options.go
Expand Up @@ -63,7 +63,6 @@ type AzureStorageConfig struct {
Container string
EndpointSuffix string
Audience string
Region string
WorkloadIdentity bool
}

Expand Down
2 changes: 0 additions & 2 deletions operator/internal/manifests/storage/var.go
Expand Up @@ -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"

Expand Down