public/grafana
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Secrets: Add more details about decryption when there is an error (#111741)

Browse Source
This commit is contained in:
Matheus Macabu
2025-09-29 18:29:52 +02:00
committed by GitHub
parent 117e357cc9
commit ffa9444256
6 changed files with 75 additions and 60 deletions
Download Patch File Download Diff File Expand all files Collapse all files
pkg/storage/secret/metadata/decrypt_store.go
+19 -9
View File
@@ -1,7 +1,9 @@
package metadata
import (
"cmp"
"context"
"errors"
"fmt"
"time"
@@ -83,22 +85,30 @@ func (s *decryptStorage) Decrypt(ctx context.Context, namespace xkube.Namespace,
}
}
decryptResultLabel := metrics.DecryptResultLabel(decryptErr)
if decryptErr == nil {
span.SetStatus(codes.Ok, "Decrypt succeeded")
args = append(args, "operation", "decrypt_secret_success")
} else {
span.SetStatus(codes.Error, "Decrypt failed")
span.RecordError(decryptErr)
args = append(args, "operation", "decrypt_secret_error", "error", decryptErr.Error(), "result", metrics.DecryptResultLabel(decryptErr))
args = append(args, "operation", "decrypt_secret_error", "error", decryptErr.Error(), "result", decryptResultLabel)
}
logging.FromContext(ctx).Info("Secrets Audit Log", args...)
s.metrics.DecryptDuration.WithLabelValues(metrics.DecryptResultLabel(decryptErr)).Observe(time.Since(start).Seconds())
s.metrics.DecryptDuration.WithLabelValues(decryptResultLabel).Observe(time.Since(start).Seconds())
// Do not leak error details to caller, return only the wrapped domain errors.
if decryptErr != nil {
decryptErr = cmp.Or(errors.Unwrap(decryptErr), contracts.ErrDecryptFailed)
}
}()
// Basic authn check before reading a secure value metadata, it is here on purpose.
if _, ok := claims.AuthInfoFrom(ctx); !ok {
return "", contracts.ErrDecryptNotAuthorized
return "", fmt.Errorf("no auth info in context (%w)", contracts.ErrDecryptNotAuthorized)
}
// The auth token will not necessarily have the permission to read the secure value metadata,
@@ -106,27 +116,27 @@ func (s *decryptStorage) Decrypt(ctx context.Context, namespace xkube.Namespace,
// function call happens after this.
sv, err := s.secureValueMetadataStorage.Read(ctx, namespace, name, contracts.ReadOpts{})
if err != nil {
return "", contracts.ErrDecryptNotFound
return "", fmt.Errorf("failed to read secure value metadata storage: %v (%w)", err, contracts.ErrDecryptNotFound)
}
decrypterIdentity, authorized := s.decryptAuthorizer.Authorize(ctx, namespace, name, sv.Spec.Decrypters, sv.OwnerReferences)
decrypterIdentity, authorized, reason := s.decryptAuthorizer.Authorize(ctx, namespace, name, sv.Spec.Decrypters, sv.OwnerReferences)
if !authorized {
return "", contracts.ErrDecryptNotAuthorized
return "", fmt.Errorf("failed to authorize decryption with reason %v (%w)", reason, contracts.ErrDecryptNotAuthorized)
}
keeperConfig, err := s.keeperMetadataStorage.GetKeeperConfig(ctx, namespace.String(), sv.Spec.Keeper, contracts.ReadOpts{})
if err != nil {
return "", contracts.ErrDecryptFailed
return "", fmt.Errorf("failed to read keeper config metadata storage: %v (%w)", err, contracts.ErrDecryptFailed)
}
keeper, err := s.keeperService.KeeperForConfig(keeperConfig)
if err != nil {
return "", contracts.ErrDecryptFailed
return "", fmt.Errorf("failed to get keeper for config: %v (%w)", err, contracts.ErrDecryptFailed)
}
exposedValue, err := keeper.Expose(ctx, keeperConfig, namespace.String(), name, sv.Status.Version)
if err != nil {
return "", contracts.ErrDecryptFailed
return "", fmt.Errorf("failed to expose secret: %v (%w)", err, contracts.ErrDecryptFailed)
}
return exposedValue, nil
pkg/storage/secret/metadata/decrypt_store_test.go
+7 -8
View File
@@ -32,7 +32,7 @@ func TestIntegrationDecrypt(t *testing.T) {
sut := testutils.Setup(t)
exposed, err := sut.DecryptStorage.Decrypt(ctx, "default", "name")
require.Error(t, err)
require.Equal(t, err.Error(), contracts.ErrDecryptNotAuthorized.Error()) // make sure we are stripping the error details
require.Empty(t, exposed)
})
@@ -48,7 +48,7 @@ func TestIntegrationDecrypt(t *testing.T) {
sut := testutils.Setup(t)
exposed, err := sut.DecryptStorage.Decrypt(authCtx, "default", "non-existent-value")
require.ErrorIs(t, err, contracts.ErrDecryptNotFound)
require.Equal(t, err.Error(), contracts.ErrDecryptNotFound.Error()) // make sure we are stripping the error details
require.Empty(t, exposed)
})
@@ -114,7 +114,7 @@ func TestIntegrationDecrypt(t *testing.T) {
require.NoError(t, err)
exposed, err := sut.DecryptStorage.Decrypt(authCtx, "default", svName)
require.ErrorIs(t, err, contracts.ErrDecryptNotAuthorized)
require.Equal(t, err.Error(), contracts.ErrDecryptNotAuthorized.Error()) // make sure we are stripping the error details
require.Empty(t, exposed)
})
@@ -146,7 +146,7 @@ func TestIntegrationDecrypt(t *testing.T) {
require.NoError(t, err)
exposed, err := sut.DecryptStorage.Decrypt(authCtx, "default", "sv-test")
require.ErrorIs(t, err, contracts.ErrDecryptNotAuthorized)
require.Equal(t, err.Error(), contracts.ErrDecryptNotAuthorized.Error()) // make sure we are stripping the error details
require.Empty(t, exposed)
})
@@ -179,7 +179,7 @@ func TestIntegrationDecrypt(t *testing.T) {
require.NoError(t, err)
exposed, err := sut.DecryptStorage.Decrypt(authCtx, "default", svName)
require.ErrorIs(t, err, contracts.ErrDecryptNotAuthorized)
require.Equal(t, err.Error(), contracts.ErrDecryptNotAuthorized.Error()) // make sure we are stripping the error details)
require.Empty(t, exposed)
})
@@ -211,7 +211,7 @@ func TestIntegrationDecrypt(t *testing.T) {
require.NoError(t, err)
exposed, err := sut.DecryptStorage.Decrypt(authCtx, "default", "sv-test")
require.ErrorIs(t, err, contracts.ErrDecryptNotAuthorized)
require.Equal(t, err.Error(), contracts.ErrDecryptNotAuthorized.Error()) // make sure we are stripping the error details
require.Empty(t, exposed)
})
@@ -244,8 +244,7 @@ func TestIntegrationDecrypt(t *testing.T) {
require.NoError(t, err)
exposed, err := sut.DecryptStorage.Decrypt(authCtx, "default", svName)
require.Error(t, err)
require.Equal(t, err.Error(), "not authorized")
require.Equal(t, err.Error(), contracts.ErrDecryptNotAuthorized.Error()) // make sure we are stripping the error details
require.Empty(t, exposed)
})