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: Integrate support for OpenShift-managed credentials in Azure #11868

Merged
merged 8 commits into from
Feb 6, 2024
1 change: 1 addition & 0 deletions operator/CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
## Main

- [11868](https://github.com/grafana/loki/pull/11868) **xperimental**: Integrate support for OpenShift-managed credentials in Azure
- [11854](https://github.com/grafana/loki/pull/11854) **periklis**: Allow custom audience for managed-auth on STS
- [11802](https://github.com/grafana/loki/pull/11802) **xperimental**: Add support for running with Azure Workload Identity
- [11824](https://github.com/grafana/loki/pull/11824) **xperimental**: Improve messages for errors in storage secret
Expand Down
4 changes: 3 additions & 1 deletion operator/apis/config/v1/projectconfig_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,9 @@ type OpenShiftFeatureGates struct {
ManagedAuthEnv bool
}

func (o OpenShiftFeatureGates) ManagedAuthEnabled() bool {
// ManagedAuthEnabled returns true when OpenShift-functions are enabled and the operator has detected that it is
// running with some kind of "workload identity" (AWS STS, Azure WIF) enabled.
func (o *OpenShiftFeatureGates) ManagedAuthEnabled() bool {
return o.Enabled && o.ManagedAuthEnv
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -150,7 +150,7 @@ metadata:
categories: OpenShift Optional, Logging & Tracing
certified: "false"
containerImage: docker.io/grafana/loki-operator:0.5.0
createdAt: "2024-01-25T11:08:43Z"
createdAt: "2024-01-31T16:48:07Z"
description: The Community Loki Operator provides Kubernetes native deployment
and management of Loki and related logging components.
features.operators.openshift.io/disconnected: "true"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -150,7 +150,7 @@ metadata:
categories: OpenShift Optional, Logging & Tracing
certified: "false"
containerImage: docker.io/grafana/loki-operator:0.5.0
createdAt: "2024-01-25T11:08:41Z"
createdAt: "2024-01-31T16:48:04Z"
description: The Community Loki Operator provides Kubernetes native deployment
and management of Loki and related logging components.
operators.operatorframework.io/builder: operator-sdk-unknown
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -150,7 +150,7 @@ metadata:
categories: OpenShift Optional, Logging & Tracing
certified: "false"
containerImage: quay.io/openshift-logging/loki-operator:0.1.0
createdAt: "2024-01-25T11:08:45Z"
createdAt: "2024-01-31T16:48:10Z"
description: |
The Loki Operator for OCP provides a means for configuring and managing a Loki stack for cluster logging.
## Prerequisites and Requirements
Expand All @@ -165,7 +165,7 @@ metadata:
features.operators.openshift.io/proxy-aware: "true"
features.operators.openshift.io/tls-profiles: "true"
features.operators.openshift.io/token-auth-aws: "true"
features.operators.openshift.io/token-auth-azure: "false"
features.operators.openshift.io/token-auth-azure: "true"
features.operators.openshift.io/token-auth-gcp: "false"
olm.skipRange: '>=5.7.0-0 <5.9.0'
operatorframework.io/cluster-monitoring: "true"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ metadata:
features.operators.openshift.io/proxy-aware: "true"
features.operators.openshift.io/tls-profiles: "true"
features.operators.openshift.io/token-auth-aws: "true"
features.operators.openshift.io/token-auth-azure: "false"
features.operators.openshift.io/token-auth-azure: "true"
features.operators.openshift.io/token-auth-gcp: "false"
olm.skipRange: '>=5.7.0-0 <5.9.0'
operatorframework.io/cluster-monitoring: "true"
Expand Down
13 changes: 12 additions & 1 deletion operator/controllers/loki/credentialsrequests_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import (
"context"

"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 Down Expand Up @@ -46,7 +47,17 @@ func (r *CredentialsRequestsReconciler) Reconcile(ctx context.Context, req ctrl.
return ctrl.Result{}, nil
}

secretRef, err := handlers.CreateCredentialsRequest(ctx, r.Client, req.NamespacedName)
storageSecretName := client.ObjectKey{
Namespace: req.Namespace,
Name: stack.Spec.Storage.Secret.Name,
}
storageSecret := &corev1.Secret{}
err = r.Client.Get(ctx, storageSecretName, storageSecret)
if err != nil {
return ctrl.Result{}, err
}

secretRef, err := handlers.CreateCredentialsRequest(ctx, r.Client, req.NamespacedName, storageSecret)
if err != nil {
return ctrl.Result{}, err
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import (

cloudcredentialsv1 "github.com/openshift/cloud-credential-operator/pkg/apis/cloudcredential/v1"
"github.com/stretchr/testify/require"
corev1 "k8s.io/api/core/v1"
apierrors "k8s.io/apimachinery/pkg/api/errors"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/runtime/schema"
Expand Down Expand Up @@ -81,16 +82,24 @@ func TestCredentialsRequestController_CreateCredentialsRequest_WhenLokiStackNotA
ManagementState: lokiv1.ManagementStateManaged,
},
}
secret := &corev1.Secret{}

// Set managed auth environment
t.Setenv("ROLEARN", "a-role-arn")

k.GetStub = func(_ context.Context, key types.NamespacedName, out client.Object, _ ...client.GetOption) error {
if key.Name == r.Name && key.Namespace == r.Namespace {
k.SetClientObject(out, &s)
switch out.(type) {
case *lokiv1.LokiStack:
if key.Name == r.Name && key.Namespace == r.Namespace {
k.SetClientObject(out, &s)
return nil
}
return apierrors.NewNotFound(schema.GroupResource{}, "lokistack not found")
case *corev1.Secret:
k.SetClientObject(out, secret)
return nil
}
return apierrors.NewNotFound(schema.GroupResource{}, "lokistack not found")
return nil
}

k.CreateStub = func(_ context.Context, o client.Object, _ ...client.CreateOption) error {
Expand Down
26 changes: 25 additions & 1 deletion operator/internal/handlers/credentialsrequest_create.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,23 +2,47 @@ package handlers

import (
"context"
"errors"

"github.com/ViaQ/logerr/v2/kverrors"
corev1 "k8s.io/api/core/v1"
apierrors "k8s.io/apimachinery/pkg/api/errors"
"sigs.k8s.io/controller-runtime/pkg/client"

"github.com/grafana/loki/operator/internal/external/k8s"
"github.com/grafana/loki/operator/internal/manifests/openshift"
"github.com/grafana/loki/operator/internal/manifests/storage"
)

var (
errAzureNoSecretFound = errors.New("can not create CredentialsRequest: no azure secret found")
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, k k8s.Client, stack client.ObjectKey) (string, error) {
func CreateCredentialsRequest(ctx context.Context, k k8s.Client, stack client.ObjectKey, secret *corev1.Secret) (string, error) {
managedAuthEnv := openshift.DiscoverManagedAuthEnv()
if managedAuthEnv == nil {
return "", nil
}

if managedAuthEnv.Azure != nil && managedAuthEnv.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
if secret == nil {
return "", errAzureNoSecretFound
}

region := secret.Data[storage.KeyAzureRegion]
if len(region) == 0 {
return "", errAzureNoRegion
}

managedAuthEnv.Azure.Region = string(region)
}

opts := openshift.Options{
BuildOpts: openshift.BuildOptions{
LokiStackName: stack.Name,
Expand Down
70 changes: 67 additions & 3 deletions operator/internal/handlers/credentialsrequest_create_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,9 @@ import (
"context"
"testing"

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"
"k8s.io/apimachinery/pkg/runtime/schema"
"sigs.k8s.io/controller-runtime/pkg/client"
Expand All @@ -16,7 +18,7 @@ func TestCreateCredentialsRequest_DoNothing_WhenManagedAuthEnvMissing(t *testing
k := &k8sfakes.FakeClient{}
key := client.ObjectKey{Name: "my-stack", Namespace: "ns"}

secretRef, err := CreateCredentialsRequest(context.Background(), k, key)
secretRef, err := CreateCredentialsRequest(context.Background(), k, key, nil)
require.NoError(t, err)
require.Empty(t, secretRef)
}
Expand All @@ -27,12 +29,74 @@ func TestCreateCredentialsRequest_CreateNewResource(t *testing.T) {

t.Setenv("ROLEARN", "a-role-arn")

secretRef, err := CreateCredentialsRequest(context.Background(), k, key)
secretRef, err := CreateCredentialsRequest(context.Background(), k, key, nil)
require.NoError(t, err)
require.NotEmpty(t, secretRef)
require.Equal(t, 1, k.CreateCallCount())
}

func TestCreateCredentialsRequest_CreateNewResourceAzure(t *testing.T) {
wantRegion := "test-region"

k := &k8sfakes.FakeClient{}
key := client.ObjectKey{Name: "my-stack", Namespace: "ns"}
secret := &corev1.Secret{
Data: map[string][]byte{
"region": []byte(wantRegion),
},
}

t.Setenv("CLIENTID", "test-client-id")
t.Setenv("TENANTID", "test-tenant-id")
t.Setenv("SUBSCRIPTIONID", "test-subscription-id")

secretRef, err := CreateCredentialsRequest(context.Background(), k, key, secret)
require.NoError(t, err)
require.NotEmpty(t, secretRef)

require.Equal(t, 1, k.CreateCallCount())
_, obj, _ := k.CreateArgsForCall(0)
credReq, ok := obj.(*cloudcredentialv1.CredentialsRequest)
require.True(t, ok)

providerSpec := &cloudcredentialv1.AzureProviderSpec{}
require.NoError(t, cloudcredentialv1.Codec.DecodeProviderSpec(credReq.Spec.ProviderSpec, providerSpec))

require.Equal(t, wantRegion, providerSpec.AzureRegion)
}

func TestCreateCredentialsRequest_CreateNewResourceAzure_Errors(t *testing.T) {
k := &k8sfakes.FakeClient{}
key := client.ObjectKey{Name: "my-stack", Namespace: "ns"}

tt := []struct {
secret *corev1.Secret
wantError string
}{
{
secret: nil,
wantError: errAzureNoSecretFound.Error(),
},
{
secret: &corev1.Secret{},
wantError: errAzureNoRegion.Error(),
},
}

for _, tc := range tt {
tc := tc
t.Run(tc.wantError, func(t *testing.T) {
// Not parallel (environment variables)
t.Setenv("CLIENTID", "test-client-id")
t.Setenv("TENANTID", "test-tenant-id")
t.Setenv("SUBSCRIPTIONID", "test-subscription-id")

_, err := CreateCredentialsRequest(context.Background(), k, key, tc.secret)
require.EqualError(t, err, tc.wantError)
})
}
}

func TestCreateCredentialsRequest_DoNothing_WhenCredentialsRequestExist(t *testing.T) {
k := &k8sfakes.FakeClient{}
key := client.ObjectKey{Name: "my-stack", Namespace: "ns"}
Expand All @@ -43,7 +107,7 @@ func TestCreateCredentialsRequest_DoNothing_WhenCredentialsRequestExist(t *testi
return errors.NewAlreadyExists(schema.GroupResource{}, "credentialsrequest exists")
}

secretRef, err := CreateCredentialsRequest(context.Background(), k, key)
secretRef, err := CreateCredentialsRequest(context.Background(), k, key, nil)
require.NoError(t, err)
require.NotEmpty(t, secretRef)
require.Equal(t, 1, k.CreateCallCount())
Expand Down
40 changes: 31 additions & 9 deletions operator/internal/handlers/internal/storage/secrets.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,8 +29,9 @@ var (

errS3NoAuth = errors.New("missing secret fields for static or sts authentication")

errAzureNoCredentials = errors.New("azure storage secret does contain neither account_key or client_id")
errAzureMixedCredentials = errors.New("azure storage secret can not contain both account_key and client_id")
errAzureNoCredentials = errors.New("azure storage secret does contain neither account_key or client_id")
errAzureMixedCredentials = errors.New("azure storage secret can not contain both account_key and client_id")
errAzureManagedIdentityNoOverride = errors.New("when in managed mode, storage secret can not contain credentials")
)

func getSecrets(ctx context.Context, k k8s.Client, stack *lokiv1.LokiStack, fg configv1.FeatureGates) (*corev1.Secret, *corev1.Secret, error) {
Expand Down Expand Up @@ -110,7 +111,7 @@ func extractSecrets(secretType lokiv1.ObjectStorageSecretType, objStore, managed

switch secretType {
case lokiv1.ObjectStorageSecretAzure:
storageOpts.Azure, err = extractAzureConfigSecret(objStore)
storageOpts.Azure, err = extractAzureConfigSecret(objStore, fg)
case lokiv1.ObjectStorageSecretGCS:
storageOpts.GCS, err = extractGCSConfigSecret(objStore)
case lokiv1.ObjectStorageSecretS3:
Expand Down Expand Up @@ -158,41 +159,62 @@ func hashSecretData(s *corev1.Secret) (string, error) {
return fmt.Sprintf("%x", h.Sum(nil)), nil
}

func extractAzureConfigSecret(s *corev1.Secret) (*storage.AzureStorageConfig, error) {
func extractAzureConfigSecret(s *corev1.Secret, fg configv1.FeatureGates) (*storage.AzureStorageConfig, error) {
// Extract and validate mandatory fields
env := s.Data[storage.KeyAzureEnvironmentName]
if len(env) == 0 {
return nil, fmt.Errorf("%w: %s", errSecretMissingField, storage.KeyAzureEnvironmentName)
}

accountName := s.Data[storage.KeyAzureStorageAccountName]
if len(accountName) == 0 {
return nil, fmt.Errorf("%w: %s", errSecretMissingField, storage.KeyAzureStorageAccountName)
}

container := s.Data[storage.KeyAzureStorageContainerName]
if len(container) == 0 {
return nil, fmt.Errorf("%w: %s", errSecretMissingField, storage.KeyAzureStorageContainerName)
}
workloadIdentity, err := validateAzureCredentials(s)

workloadIdentity, err := validateAzureCredentials(s, fg)
if err != nil {
return nil, err
}

// Extract and validate optional fields
endpointSuffix := s.Data[storage.KeyAzureStorageEndpointSuffix]
audience := s.Data[storage.KeyAzureAudience]

if !workloadIdentity && len(audience) > 0 {
return nil, fmt.Errorf("%w: %s", errSecretFieldNotAllowed, storage.KeyAzureAudience)
}

return &storage.AzureStorageConfig{
Env: string(env),
Container: string(container),
EndpointSuffix: string(endpointSuffix),
Audience: string(audience),
WorkloadIdentity: workloadIdentity,
}, nil
}

func validateAzureCredentials(s *corev1.Secret) (workloadIdentity bool, err error) {
accountName := s.Data[storage.KeyAzureStorageAccountName]
func validateAzureCredentials(s *corev1.Secret, fg configv1.FeatureGates) (workloadIdentity bool, err error) {
accountKey := s.Data[storage.KeyAzureStorageAccountKey]
clientID := s.Data[storage.KeyAzureStorageClientID]
tenantID := s.Data[storage.KeyAzureStorageTenantID]
subscriptionID := s.Data[storage.KeyAzureStorageSubscriptionID]

if len(accountName) == 0 {
return false, fmt.Errorf("%w: %s", errSecretMissingField, storage.KeyAzureStorageAccountName)
if fg.OpenShift.ManagedAuthEnabled() {
region := s.Data[storage.KeyAzureRegion]
if len(region) == 0 {
return false, fmt.Errorf("%w: %s", errSecretMissingField, storage.KeyAzureRegion)
}

if len(accountKey) > 0 || len(clientID) > 0 || len(tenantID) > 0 || len(subscriptionID) > 0 {
return false, errAzureManagedIdentityNoOverride
}

return true, nil
}

if len(accountKey) == 0 && len(clientID) == 0 {
Expand Down
Loading
Loading