diff --git a/operator/CHANGELOG.md b/operator/CHANGELOG.md index b51e5c7e6a611..9570477e735ab 100644 --- a/operator/CHANGELOG.md +++ b/operator/CHANGELOG.md @@ -1,5 +1,9 @@ ## Main +## Release 5.6.17 + +- [11824](https://github.com/grafana/loki/pull/11824) **xperimental**: Improve messages for errors in storage secret + ## Release 5.6.16 - [11778](https://github.com/grafana/loki/pull/11778) **periklis**: Update Loki operand to v2.9.4 diff --git a/operator/internal/handlers/internal/storage/secrets.go b/operator/internal/handlers/internal/storage/secrets.go index 4c466b9d83bef..1da87fbdefcee 100644 --- a/operator/internal/handlers/internal/storage/secrets.go +++ b/operator/internal/handlers/internal/storage/secrets.go @@ -3,10 +3,10 @@ package storage import ( "context" "crypto/sha1" + "errors" "fmt" "sort" - "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" @@ -17,7 +17,13 @@ import ( "github.com/grafana/loki/operator/internal/status" ) -var hashSeparator = []byte(",") +var ( + hashSeparator = []byte(",") + + errSecretUnknownType = errors.New("unknown secret type") + errSecretMissingField = errors.New("missing secret field") + errSecretHashError = errors.New("error calculating hash for secret") +) func getSecret(ctx context.Context, k k8s.Client, stack *lokiv1.LokiStack) (*corev1.Secret, error) { var storageSecret corev1.Secret @@ -30,7 +36,7 @@ func getSecret(ctx context.Context, k k8s.Client, stack *lokiv1.LokiStack) (*cor Requeue: false, } } - return nil, kverrors.Wrap(err, "failed to lookup lokistack storage secret", "name", key) + return nil, fmt.Errorf("failed to lookup lokistack storage secret: %w", err) } return &storageSecret, nil @@ -40,7 +46,7 @@ func getSecret(ctx context.Context, k k8s.Client, stack *lokiv1.LokiStack) (*cor func extractSecret(s *corev1.Secret, secretType lokiv1.ObjectStorageSecretType) (storage.Options, error) { hash, err := hashSecretData(s) if err != nil { - return storage.Options{}, kverrors.Wrap(err, "error calculating hash for secret", "type", secretType) + return storage.Options{}, errSecretHashError } storageOpts := storage.Options{ @@ -59,7 +65,7 @@ func extractSecret(s *corev1.Secret, secretType lokiv1.ObjectStorageSecretType) case lokiv1.ObjectStorageSecretSwift: storageOpts.Swift, err = extractSwiftConfigSecret(s) default: - return storage.Options{}, kverrors.New("unknown secret type", "type", secretType) + return storage.Options{}, fmt.Errorf("%w: %s", errSecretUnknownType, secretType) } if err != nil { @@ -101,19 +107,19 @@ func extractAzureConfigSecret(s *corev1.Secret) (*storage.AzureStorageConfig, er // Extract and validate mandatory fields env := s.Data[storage.KeyAzureEnvironmentName] if len(env) == 0 { - return nil, kverrors.New("missing secret field", "field", storage.KeyAzureEnvironmentName) + return nil, fmt.Errorf("%w: %s", errSecretMissingField, storage.KeyAzureEnvironmentName) } container := s.Data[storage.KeyAzureStorageContainerName] if len(container) == 0 { - return nil, kverrors.New("missing secret field", "field", storage.KeyAzureStorageContainerName) + return nil, fmt.Errorf("%w: %s", errSecretMissingField, storage.KeyAzureStorageContainerName) } name := s.Data[storage.KeyAzureStorageAccountName] if len(name) == 0 { - return nil, kverrors.New("missing secret field", "field", storage.KeyAzureStorageAccountName) + return nil, fmt.Errorf("%w: %s", errSecretMissingField, storage.KeyAzureStorageAccountName) } key := s.Data[storage.KeyAzureStorageAccountKey] if len(key) == 0 { - return nil, kverrors.New("missing secret field", "field", storage.KeyAzureStorageAccountKey) + return nil, fmt.Errorf("%w: %s", errSecretMissingField, storage.KeyAzureStorageAccountKey) } return &storage.AzureStorageConfig{ @@ -126,13 +132,13 @@ func extractGCSConfigSecret(s *corev1.Secret) (*storage.GCSStorageConfig, error) // Extract and validate mandatory fields bucket := s.Data[storage.KeyGCPStorageBucketName] if len(bucket) == 0 { - return nil, kverrors.New("missing secret field", "field", storage.KeyGCPStorageBucketName) + return nil, fmt.Errorf("%w: %s", errSecretMissingField, storage.KeyGCPStorageBucketName) } // Check if google authentication credentials is provided keyJSON := s.Data[storage.KeyGCPServiceAccountKeyFilename] if len(keyJSON) == 0 { - return nil, kverrors.New("missing google authentication credentials", "field", storage.KeyGCPServiceAccountKeyFilename) + return nil, fmt.Errorf("%w: %s", errSecretMissingField, storage.KeyGCPServiceAccountKeyFilename) } return &storage.GCSStorageConfig{ @@ -142,21 +148,21 @@ func extractGCSConfigSecret(s *corev1.Secret) (*storage.GCSStorageConfig, error) func extractS3ConfigSecret(s *corev1.Secret) (*storage.S3StorageConfig, error) { // Extract and validate mandatory fields - endpoint := s.Data[storage.KeyAWSEndpoint] - if len(endpoint) == 0 { - return nil, kverrors.New("missing secret field", "field", storage.KeyAWSEndpoint) - } buckets := s.Data[storage.KeyAWSBucketNames] if len(buckets) == 0 { - return nil, kverrors.New("missing secret field", "field", storage.KeyAWSBucketNames) + return nil, fmt.Errorf("%w: %s", errSecretMissingField, storage.KeyAWSBucketNames) + } + endpoint := s.Data[storage.KeyAWSEndpoint] + if len(endpoint) == 0 { + return nil, fmt.Errorf("%w: %s", errSecretMissingField, storage.KeyAWSEndpoint) } id := s.Data[storage.KeyAWSAccessKeyID] if len(id) == 0 { - return nil, kverrors.New("missing secret field", "field", storage.KeyAWSAccessKeyID) + return nil, fmt.Errorf("%w: %s", errSecretMissingField, storage.KeyAWSAccessKeyID) } secret := s.Data[storage.KeyAWSAccessKeySecret] if len(secret) == 0 { - return nil, kverrors.New("missing secret field", "field", storage.KeyAWSAccessKeySecret) + return nil, fmt.Errorf("%w: %s", errSecretMissingField, storage.KeyAWSAccessKeySecret) } // Extract and validate optional fields @@ -173,39 +179,39 @@ func extractSwiftConfigSecret(s *corev1.Secret) (*storage.SwiftStorageConfig, er // Extract and validate mandatory fields url := s.Data[storage.KeySwiftAuthURL] if len(url) == 0 { - return nil, kverrors.New("missing secret field", "field", storage.KeySwiftAuthURL) + return nil, fmt.Errorf("%w: %s", errSecretMissingField, storage.KeySwiftAuthURL) } username := s.Data[storage.KeySwiftUsername] if len(username) == 0 { - return nil, kverrors.New("missing secret field", "field", storage.KeySwiftUsername) + return nil, fmt.Errorf("%w: %s", errSecretMissingField, storage.KeySwiftUsername) } userDomainName := s.Data[storage.KeySwiftUserDomainName] if len(userDomainName) == 0 { - return nil, kverrors.New("missing secret field", "field", storage.KeySwiftUserDomainName) + return nil, fmt.Errorf("%w: %s", errSecretMissingField, storage.KeySwiftUserDomainName) } userDomainID := s.Data[storage.KeySwiftUserDomainID] if len(userDomainID) == 0 { - return nil, kverrors.New("missing secret field", "field", storage.KeySwiftUserDomainID) + return nil, fmt.Errorf("%w: %s", errSecretMissingField, storage.KeySwiftUserDomainID) } userID := s.Data[storage.KeySwiftUserID] if len(userID) == 0 { - return nil, kverrors.New("missing secret field", "field", storage.KeySwiftUserID) + return nil, fmt.Errorf("%w: %s", errSecretMissingField, storage.KeySwiftUserID) } password := s.Data[storage.KeySwiftPassword] if len(password) == 0 { - return nil, kverrors.New("missing secret field", "field", storage.KeySwiftPassword) + return nil, fmt.Errorf("%w: %s", errSecretMissingField, storage.KeySwiftPassword) } domainID := s.Data[storage.KeySwiftDomainID] if len(domainID) == 0 { - return nil, kverrors.New("missing secret field", "field", storage.KeySwiftDomainID) + return nil, fmt.Errorf("%w: %s", errSecretMissingField, storage.KeySwiftDomainID) } domainName := s.Data[storage.KeySwiftDomainName] if len(domainName) == 0 { - return nil, kverrors.New("missing secret field", "field", storage.KeySwiftDomainName) + return nil, fmt.Errorf("%w: %s", errSecretMissingField, storage.KeySwiftDomainName) } containerName := s.Data[storage.KeySwiftContainerName] if len(containerName) == 0 { - return nil, kverrors.New("missing secret field", "field", storage.KeySwiftContainerName) + return nil, fmt.Errorf("%w: %s", errSecretMissingField, storage.KeySwiftContainerName) } // Extract and validate optional fields diff --git a/operator/internal/handlers/internal/storage/secrets_test.go b/operator/internal/handlers/internal/storage/secrets_test.go index 844c253f55ef4..9d93fe7af519d 100644 --- a/operator/internal/handlers/internal/storage/secrets_test.go +++ b/operator/internal/handlers/internal/storage/secrets_test.go @@ -61,17 +61,24 @@ func TestHashSecretData(t *testing.T) { } } +func TestUnknownType(t *testing.T) { + wantError := "unknown secret type: test-unknown-type" + + _, err := extractSecret(&corev1.Secret{}, "test-unknown-type") + require.EqualError(t, err, wantError) +} + func TestAzureExtract(t *testing.T) { type test struct { - name string - secret *corev1.Secret - wantErr bool + name string + secret *corev1.Secret + wantError string } table := []test{ { - name: "missing environment", - secret: &corev1.Secret{}, - wantErr: true, + name: "missing environment", + secret: &corev1.Secret{}, + wantError: "missing secret field: environment", }, { name: "missing container", @@ -80,7 +87,7 @@ func TestAzureExtract(t *testing.T) { "environment": []byte("here"), }, }, - wantErr: true, + wantError: "missing secret field: container", }, { name: "missing account_name", @@ -90,7 +97,7 @@ func TestAzureExtract(t *testing.T) { "container": []byte("this,that"), }, }, - wantErr: true, + wantError: "missing secret field: account_name", }, { name: "missing account_key", @@ -102,7 +109,7 @@ func TestAzureExtract(t *testing.T) { "account_name": []byte("id"), }, }, - wantErr: true, + wantError: "missing secret field: account_key", }, { name: "all set", @@ -123,14 +130,13 @@ func TestAzureExtract(t *testing.T) { t.Parallel() opts, err := extractSecret(tst.secret, lokiv1.ObjectStorageSecretAzure) - if !tst.wantErr { + if tst.wantError == "" { require.NoError(t, err) require.NotEmpty(t, opts.SecretName) require.NotEmpty(t, opts.SecretSHA1) require.Equal(t, opts.SharedStore, lokiv1.ObjectStorageSecretAzure) - } - if tst.wantErr { - require.NotNil(t, err) + } else { + require.EqualError(t, err, tst.wantError) } }) } @@ -138,15 +144,15 @@ func TestAzureExtract(t *testing.T) { func TestGCSExtract(t *testing.T) { type test struct { - name string - secret *corev1.Secret - wantErr bool + name string + secret *corev1.Secret + wantError string } table := []test{ { - name: "missing bucketname", - secret: &corev1.Secret{}, - wantErr: true, + name: "missing bucketname", + secret: &corev1.Secret{}, + wantError: "missing secret field: bucketname", }, { name: "missing key.json", @@ -155,7 +161,7 @@ func TestGCSExtract(t *testing.T) { "bucketname": []byte("here"), }, }, - wantErr: true, + wantError: "missing secret field: key.json", }, { name: "all set", @@ -174,11 +180,10 @@ func TestGCSExtract(t *testing.T) { t.Parallel() _, err := extractSecret(tst.secret, lokiv1.ObjectStorageSecretGCS) - if !tst.wantErr { + if tst.wantError == "" { require.NoError(t, err) - } - if tst.wantErr { - require.NotNil(t, err) + } else { + require.EqualError(t, err, tst.wantError) } }) } @@ -186,24 +191,24 @@ func TestGCSExtract(t *testing.T) { func TestS3Extract(t *testing.T) { type test struct { - name string - secret *corev1.Secret - wantErr bool + name string + secret *corev1.Secret + wantError string } table := []test{ { - name: "missing endpoint", - secret: &corev1.Secret{}, - wantErr: true, + name: "missing bucketnames", + secret: &corev1.Secret{}, + wantError: "missing secret field: bucketnames", }, { - name: "missing bucketnames", + name: "missing endpoint", secret: &corev1.Secret{ Data: map[string][]byte{ - "endpoint": []byte("here"), + "bucketnames": []byte("this,that"), }, }, - wantErr: true, + wantError: "missing secret field: endpoint", }, { name: "missing access_key_id", @@ -213,7 +218,7 @@ func TestS3Extract(t *testing.T) { "bucketnames": []byte("this,that"), }, }, - wantErr: true, + wantError: "missing secret field: access_key_id", }, { name: "missing access_key_secret", @@ -224,7 +229,7 @@ func TestS3Extract(t *testing.T) { "access_key_id": []byte("id"), }, }, - wantErr: true, + wantError: "missing secret field: access_key_secret", }, { name: "all set", @@ -245,14 +250,13 @@ func TestS3Extract(t *testing.T) { t.Parallel() opts, err := extractSecret(tst.secret, lokiv1.ObjectStorageSecretS3) - if !tst.wantErr { + if tst.wantError == "" { require.NoError(t, err) require.NotEmpty(t, opts.SecretName) require.NotEmpty(t, opts.SecretSHA1) require.Equal(t, opts.SharedStore, lokiv1.ObjectStorageSecretS3) - } - if tst.wantErr { - require.NotNil(t, err) + } else { + require.EqualError(t, err, tst.wantError) } }) } @@ -260,15 +264,15 @@ func TestS3Extract(t *testing.T) { func TestSwiftExtract(t *testing.T) { type test struct { - name string - secret *corev1.Secret - wantErr bool + name string + secret *corev1.Secret + wantError string } table := []test{ { - name: "missing auth_url", - secret: &corev1.Secret{}, - wantErr: true, + name: "missing auth_url", + secret: &corev1.Secret{}, + wantError: "missing secret field: auth_url", }, { name: "missing username", @@ -277,7 +281,7 @@ func TestSwiftExtract(t *testing.T) { "auth_url": []byte("here"), }, }, - wantErr: true, + wantError: "missing secret field: username", }, { name: "missing user_domain_name", @@ -287,7 +291,7 @@ func TestSwiftExtract(t *testing.T) { "username": []byte("this,that"), }, }, - wantErr: true, + wantError: "missing secret field: user_domain_name", }, { name: "missing user_domain_id", @@ -298,7 +302,7 @@ func TestSwiftExtract(t *testing.T) { "user_domain_name": []byte("id"), }, }, - wantErr: true, + wantError: "missing secret field: user_domain_id", }, { name: "missing user_id", @@ -310,7 +314,7 @@ func TestSwiftExtract(t *testing.T) { "user_domain_id": []byte("secret"), }, }, - wantErr: true, + wantError: "missing secret field: user_id", }, { name: "missing password", @@ -323,7 +327,7 @@ func TestSwiftExtract(t *testing.T) { "user_id": []byte("there"), }, }, - wantErr: true, + wantError: "missing secret field: password", }, { name: "missing domain_id", @@ -337,7 +341,7 @@ func TestSwiftExtract(t *testing.T) { "password": []byte("cred"), }, }, - wantErr: true, + wantError: "missing secret field: domain_id", }, { name: "missing domain_name", @@ -352,7 +356,7 @@ func TestSwiftExtract(t *testing.T) { "domain_id": []byte("text"), }, }, - wantErr: true, + wantError: "missing secret field: domain_name", }, { name: "missing container_name", @@ -368,7 +372,7 @@ func TestSwiftExtract(t *testing.T) { "domain_name": []byte("where"), }, }, - wantErr: true, + wantError: "missing secret field: container_name", }, { name: "all set", @@ -394,14 +398,13 @@ func TestSwiftExtract(t *testing.T) { t.Parallel() opts, err := extractSecret(tst.secret, lokiv1.ObjectStorageSecretSwift) - if !tst.wantErr { + if tst.wantError == "" { require.NoError(t, err) require.NotEmpty(t, opts.SecretName) require.NotEmpty(t, opts.SecretSHA1) require.Equal(t, opts.SharedStore, lokiv1.ObjectStorageSecretSwift) - } - if tst.wantErr { - require.NotNil(t, err) + } else { + require.EqualError(t, err, tst.wantError) } }) } diff --git a/operator/internal/handlers/internal/storage/storage_test.go b/operator/internal/handlers/internal/storage/storage_test.go index 08a1db3bdd0af..5c1f001286f52 100644 --- a/operator/internal/handlers/internal/storage/storage_test.go +++ b/operator/internal/handlers/internal/storage/storage_test.go @@ -134,7 +134,7 @@ func TestBuildOptions_WhenInvalidSecret_SetDegraded(t *testing.T) { } degradedErr := &status.DegradedError{ - Message: "Invalid object storage secret contents: missing secret field", + Message: "Invalid object storage secret contents: missing secret field: bucketnames", Reason: lokiv1.ReasonInvalidObjectStorageSecret, Requeue: false, }