feat(operator): Extend Azure secret validation (grafana#12007)

Co-authored-by: Periklis Tsirakidis <periklis@redhat.com>
inloco · Apr 12, 2024 · b08de45 · b08de45
1 parent 883bbc6
commit b08de45
Show file tree

Hide file tree

Showing 3 changed files with 55 additions and 18 deletions.
diff --git a/operator/CHANGELOG.md b/operator/CHANGELOG.md
@@ -1,5 +1,6 @@
 ## Main
 
+- [12007](https://github.com/grafana/loki/pull/12007) **xperimental**: Extend Azure secret validation
 - [12008](https://github.com/grafana/loki/pull/12008) **xperimental**: Support using multiple buckets with AWS STS
 - [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

diff --git a/operator/internal/handlers/internal/storage/secrets.go b/operator/internal/handlers/internal/storage/secrets.go
@@ -1,11 +1,14 @@
 package storage
 
 import (
+	"bytes"
 	"context"
 	"crypto/sha1"
+	"encoding/base64"
 	"encoding/json"
 	"errors"
 	"fmt"
+	"io"
 	"sort"
 
 	corev1 "k8s.io/api/core/v1"
@@ -33,9 +36,18 @@ var (
 	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")
+	errAzureInvalidEnvironment        = errors.New("azure environment invalid (valid values: AzureGlobal, AzureChinaCloud, AzureGermanCloud, AzureUSGovernment)")
+	errAzureInvalidAccountKey         = errors.New("azure account key is not valid base64")
 
 	errGCPParseCredentialsFile      = errors.New("gcp storage secret cannot be parsed from JSON content")
 	errGCPWrongCredentialSourceFile = errors.New("credential source in secret needs to point to token file")
+
+	azureValidEnvironments = map[string]bool{
+		"AzureGlobal":       true,
+		"AzureChinaCloud":   true,
+		"AzureGermanCloud":  true,
+		"AzureUSGovernment": true,
+	}
 )
 
 const gcpAccountTypeExternal = "external_account"
@@ -159,11 +171,15 @@ func hashSecretData(s *corev1.Secret) (string, 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 {
+	env := string(s.Data[storage.KeyAzureEnvironmentName])
+	if env == "" {
 		return nil, fmt.Errorf("%w: %s", errSecretMissingField, storage.KeyAzureEnvironmentName)
 	}
 
+	if !azureValidEnvironments[env] {
+		return nil, fmt.Errorf("%w: %s", errAzureInvalidEnvironment, env)
+	}
+
 	accountName := s.Data[storage.KeyAzureStorageAccountName]
 	if len(accountName) == 0 {
 		return nil, fmt.Errorf("%w: %s", errSecretMissingField, storage.KeyAzureStorageAccountName)
@@ -188,7 +204,7 @@ func extractAzureConfigSecret(s *corev1.Secret, fg configv1.FeatureGates) (*stor
 	}
 
 	return &storage.AzureStorageConfig{
-		Env:              string(env),
+		Env:              env,
 		Container:        string(container),
 		EndpointSuffix:   string(endpointSuffix),
 		Audience:         string(audience),
@@ -219,6 +235,10 @@ func validateAzureCredentials(s *corev1.Secret, fg configv1.FeatureGates) (workl
 	}
 
 	if len(accountKey) > 0 {
+		if err := validateBase64(accountKey); err != nil {
+			return false, errAzureInvalidAccountKey
+		}
+
 		// have both account_name and account_key -> no workload identity federation
 		return false, nil
 	}
@@ -235,6 +255,13 @@ func validateAzureCredentials(s *corev1.Secret, fg configv1.FeatureGates) (workl
 	return true, nil
 }
 
+func validateBase64(data []byte) error {
+	buf := bytes.NewBuffer(data)
+	reader := base64.NewDecoder(base64.StdEncoding, buf)
+	_, err := io.ReadAll(reader)
+	return err
+}
+
 func extractGCSConfigSecret(s *corev1.Secret) (*storage.GCSStorageConfig, error) {
 	// Extract and validate mandatory fields
 	bucket := s.Data[storage.KeyGCPStorageBucketName]

diff --git a/operator/internal/handlers/internal/storage/secrets_test.go b/operator/internal/handlers/internal/storage/secrets_test.go
@@ -84,11 +84,20 @@ func TestAzureExtract(t *testing.T) {
 			secret:    &corev1.Secret{},
 			wantError: "missing secret field: environment",
 		},
+		{
+			name: "invalid environment",
+			secret: &corev1.Secret{
+				Data: map[string][]byte{
+					"environment": []byte("invalid-environment"),
+				},
+			},
+			wantError: "azure environment invalid (valid values: AzureGlobal, AzureChinaCloud, AzureGermanCloud, AzureUSGovernment): invalid-environment",
+		},
 		{
 			name: "missing account_name",
 			secret: &corev1.Secret{
 				Data: map[string][]byte{
-					"environment": []byte("here"),
+					"environment": []byte("AzureGlobal"),
 				},
 			},
 			wantError: "missing secret field: account_name",
@@ -97,7 +106,7 @@ func TestAzureExtract(t *testing.T) {
 			name: "missing container",
 			secret: &corev1.Secret{
 				Data: map[string][]byte{
-					"environment":  []byte("here"),
+					"environment":  []byte("AzureGlobal"),
 					"account_name": []byte("id"),
 				},
 			},
@@ -108,7 +117,7 @@ func TestAzureExtract(t *testing.T) {
 			secret: &corev1.Secret{
 				ObjectMeta: metav1.ObjectMeta{Name: "test"},
 				Data: map[string][]byte{
-					"environment":  []byte("here"),
+					"environment":  []byte("AzureGlobal"),
 					"container":    []byte("this,that"),
 					"account_name": []byte("id"),
 				},
@@ -120,7 +129,7 @@ func TestAzureExtract(t *testing.T) {
 			secret: &corev1.Secret{
 				ObjectMeta: metav1.ObjectMeta{Name: "test"},
 				Data: map[string][]byte{
-					"environment":  []byte("here"),
+					"environment":  []byte("AzureGlobal"),
 					"container":    []byte("this,that"),
 					"account_name": []byte("test-account-name"),
 					"account_key":  []byte("test-account-key"),
@@ -134,7 +143,7 @@ func TestAzureExtract(t *testing.T) {
 			secret: &corev1.Secret{
 				ObjectMeta: metav1.ObjectMeta{Name: "test"},
 				Data: map[string][]byte{
-					"environment":  []byte("here"),
+					"environment":  []byte("AzureGlobal"),
 					"container":    []byte("this,that"),
 					"account_name": []byte("test-account-name"),
 					"client_id":    []byte("test-client-id"),
@@ -147,7 +156,7 @@ func TestAzureExtract(t *testing.T) {
 			secret: &corev1.Secret{
 				ObjectMeta: metav1.ObjectMeta{Name: "test"},
 				Data: map[string][]byte{
-					"environment":  []byte("here"),
+					"environment":  []byte("AzureGlobal"),
 					"container":    []byte("this,that"),
 					"account_name": []byte("test-account-name"),
 					"client_id":    []byte("test-client-id"),
@@ -161,7 +170,7 @@ func TestAzureExtract(t *testing.T) {
 			secret: &corev1.Secret{
 				ObjectMeta: metav1.ObjectMeta{Name: "test"},
 				Data: map[string][]byte{
-					"environment":  []byte("here"),
+					"environment":  []byte("AzureGlobal"),
 					"account_name": []byte("test-account-name"),
 					"container":    []byte("this,that"),
 					"region":       []byte("test-region"),
@@ -184,10 +193,10 @@ func TestAzureExtract(t *testing.T) {
 			secret: &corev1.Secret{
 				ObjectMeta: metav1.ObjectMeta{Name: "test"},
 				Data: map[string][]byte{
-					"environment":  []byte("here"),
+					"environment":  []byte("AzureGlobal"),
 					"container":    []byte("this,that"),
 					"account_name": []byte("id"),
-					"account_key":  []byte("secret"),
+					"account_key":  []byte("dGVzdC1hY2NvdW50LWtleQ=="), // test-account-key
 					"audience":     []byte("test-audience"),
 				},
 			},
@@ -198,10 +207,10 @@ func TestAzureExtract(t *testing.T) {
 			secret: &corev1.Secret{
 				ObjectMeta: metav1.ObjectMeta{Name: "test"},
 				Data: map[string][]byte{
-					"environment":  []byte("here"),
+					"environment":  []byte("AzureGlobal"),
 					"container":    []byte("this,that"),
 					"account_name": []byte("id"),
-					"account_key":  []byte("secret"),
+					"account_key":  []byte("dGVzdC1hY2NvdW50LWtleQ=="), // test-account-key
 				},
 			},
 			wantCredentialMode: lokiv1.CredentialModeStatic,
@@ -211,7 +220,7 @@ func TestAzureExtract(t *testing.T) {
 			secret: &corev1.Secret{
 				ObjectMeta: metav1.ObjectMeta{Name: "test"},
 				Data: map[string][]byte{
-					"environment":     []byte("here"),
+					"environment":     []byte("AzureGlobal"),
 					"container":       []byte("this,that"),
 					"account_name":    []byte("test-account-name"),
 					"client_id":       []byte("test-client-id"),
@@ -227,7 +236,7 @@ func TestAzureExtract(t *testing.T) {
 			secret: &corev1.Secret{
 				ObjectMeta: metav1.ObjectMeta{Name: "test"},
 				Data: map[string][]byte{
-					"environment":  []byte("here"),
+					"environment":  []byte("AzureGlobal"),
 					"account_name": []byte("test-account-name"),
 					"container":    []byte("this,that"),
 					"region":       []byte("test-region"),
@@ -256,10 +265,10 @@ func TestAzureExtract(t *testing.T) {
 			secret: &corev1.Secret{
 				ObjectMeta: metav1.ObjectMeta{Name: "test"},
 				Data: map[string][]byte{
-					"environment":     []byte("here"),
+					"environment":     []byte("AzureGlobal"),
 					"container":       []byte("this,that"),
 					"account_name":    []byte("id"),
-					"account_key":     []byte("secret"),
+					"account_key":     []byte("dGVzdC1hY2NvdW50LWtleQ=="), // test-account-key
 					"endpoint_suffix": []byte("suffix"),
 				},
 			},