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

Secrets: Simplify CanReference interface to only pass secure value names (#109030)

Browse Source
This commit is contained in:
Matheus Macabu
2025-08-01 14:00:01 +02:00
committed by GitHub
parent 7374df7945
commit 988439e0b8
3 changed files with 27 additions and 106 deletions
Download Patch File Download Diff File Expand all files Collapse all files
pkg/registry/apis/secret/contracts/inline.go
+2 -2
View File
@@ -7,8 +7,8 @@ import (
)
type InlineSecureValueSupport interface {
// Check that the request user can reference a secret in the context of a given resource (owner)
CanReference(ctx context.Context, owner common.ObjectReference, values common.InlineSecureValues) error
// Check that the request user can reference secure value names in the context of a given resource (owner)
CanReference(ctx context.Context, owner common.ObjectReference, names ...string) error
// CreateInline creates a secret that is owned by the referenced object
// returns the name of the created secret or an error
pkg/registry/apis/secret/service/inline_secure_value.go
+10 -17
View File
@@ -38,13 +38,14 @@ func ProvideInlineSecureValueService(
}
}
func (s *inlineSecureValueService) CanReference(ctx context.Context, owner common.ObjectReference, values common.InlineSecureValues) error {
func (s *inlineSecureValueService) CanReference(ctx context.Context, owner common.ObjectReference, names ...string) error {
ctx, span := s.tracer.Start(ctx, "InlineSecureValueService.CanReference", trace.WithAttributes(
attribute.String("owner.namespace", owner.Namespace),
attribute.String("owner.apiGroup", owner.APIGroup),
attribute.String("owner.apiVersion", owner.APIVersion),
attribute.String("owner.kind", owner.Kind),
attribute.String("owner.name", owner.Name),
attribute.StringSlice("secureValueNames", names),
))
defer span.End()
@@ -61,31 +62,23 @@ func (s *inlineSecureValueService) CanReference(ctx context.Context, owner commo
return fmt.Errorf("owner reference must have a valid API group, API version, kind and name")
}
if len(values) == 0 {
if len(names) == 0 {
return fmt.Errorf("no inline secure values provided")
}
for field, value := range values {
if value.Name == "" {
return fmt.Errorf("field %s has an empty secure value name", field)
for _, name := range names {
if name == "" {
return fmt.Errorf("empty secure value name")
}
if !value.Create.IsZero() {
return fmt.Errorf("field %s has 'create' set, which is not allowed", field)
}
if value.Remove {
return fmt.Errorf("field %s has 'remove' set, which is not allowed", field)
}
owned, err := s.isSecureValueOwnedByResource(ctx, owner, value.Name)
owned, err := s.isSecureValueOwnedByResource(ctx, owner, name)
if err != nil {
return fmt.Errorf("field %s had an error checking secure value ownership: %w", field, err)
return err
}
if !owned {
if err := s.canIdentityReadSecureValue(ctx, xkube.Namespace(owner.Namespace), value.Name); err != nil {
return fmt.Errorf("field %s: identity cannot read secure value %s: %w", field, value.Name, err)
if err := s.canIdentityReadSecureValue(ctx, xkube.Namespace(owner.Namespace), name); err != nil {
return err
}
}
}
pkg/registry/apis/secret/service/inline_secure_value_test.go
+15 -87
View File
@@ -48,18 +48,13 @@ func TestIntegration_InlineSecureValue_CanReference(t *testing.T) {
require.NoError(t, err)
require.NotNil(t, createdSv2)
values := common.InlineSecureValues{
"fieldA": {Name: sv1},
"fieldB": {Name: sv2},
}
ctx := testutils.CreateUserAuthContext(t.Context(), defaultNs, map[string][]string{
"securevalues:read": {"securevalues:uid:" + sv2},
})
svc := service.ProvideInlineSecureValueService(tracer, tu.SecureValueService, tu.AccessClient)
err = svc.CanReference(ctx, owner, values)
err = svc.CanReference(ctx, owner, sv1, sv2)
require.NoError(t, err)
})
@@ -67,7 +62,7 @@ func TestIntegration_InlineSecureValue_CanReference(t *testing.T) {
t.Parallel()
svc := service.ProvideInlineSecureValueService(tracer, nil, nil)
err := svc.CanReference(t.Context(), common.ObjectReference{}, common.InlineSecureValues{})
err := svc.CanReference(t.Context(), common.ObjectReference{})
require.Error(t, err)
})
@@ -79,7 +74,7 @@ func TestIntegration_InlineSecureValue_CanReference(t *testing.T) {
reqNs := "org-2345"
ctx := testutils.CreateUserAuthContext(t.Context(), reqNs, map[string][]string{})
err := svc.CanReference(ctx, owner, common.InlineSecureValues{})
err := svc.CanReference(ctx, owner)
require.Error(t, err)
})
@@ -90,7 +85,7 @@ func TestIntegration_InlineSecureValue_CanReference(t *testing.T) {
ctx := testutils.CreateUserAuthContext(t.Context(), defaultNs, map[string][]string{})
err := svc.CanReference(ctx, common.ObjectReference{}, common.InlineSecureValues{})
err := svc.CanReference(ctx, common.ObjectReference{})
require.Error(t, err)
})
@@ -105,23 +100,23 @@ func TestIntegration_InlineSecureValue_CanReference(t *testing.T) {
ctx := testutils.CreateUserAuthContext(t.Context(), defaultNs, map[string][]string{})
err := svc.CanReference(ctx, owner, common.InlineSecureValues{})
err := svc.CanReference(ctx, owner)
require.Error(t, err)
owner.APIGroup = "prometheus.datasource.grafana.app"
require.Error(t, svc.CanReference(ctx, owner, common.InlineSecureValues{}))
require.Error(t, svc.CanReference(ctx, owner))
owner.APIGroup = ""
owner.APIVersion = "v1alpha1"
require.Error(t, svc.CanReference(ctx, owner, common.InlineSecureValues{}))
require.Error(t, svc.CanReference(ctx, owner))
owner.APIVersion = ""
owner.Kind = "DataSourceConfig"
require.Error(t, svc.CanReference(ctx, owner, common.InlineSecureValues{}))
require.Error(t, svc.CanReference(ctx, owner))
owner.Kind = ""
owner.Name = "test-datasource"
require.Error(t, svc.CanReference(ctx, owner, common.InlineSecureValues{}))
require.Error(t, svc.CanReference(ctx, owner))
owner.Name = ""
})
@@ -132,58 +127,7 @@ func TestIntegration_InlineSecureValue_CanReference(t *testing.T) {
ctx := testutils.CreateUserAuthContext(t.Context(), defaultNs, map[string][]string{})
err := svc.CanReference(ctx, owner, common.InlineSecureValues{})
require.Error(t, err)
})
t.Run("when one of the secure values does not have a `name`, it returns an error", func(t *testing.T) {
t.Parallel()
svc := service.ProvideInlineSecureValueService(tracer, nil, nil)
values := common.InlineSecureValues{
"fieldA": {Name: ""},
}
ctx := testutils.CreateUserAuthContext(t.Context(), defaultNs, map[string][]string{})
err := svc.CanReference(ctx, owner, values)
require.Error(t, err)
})
t.Run("when one of the secure values has `create` field set, it returns an error", func(t *testing.T) {
t.Parallel()
svc := service.ProvideInlineSecureValueService(tracer, nil, nil)
values := common.InlineSecureValues{
"fieldA": {
Name: "test-sv",
Create: common.NewSecretValue("test"),
},
}
ctx := testutils.CreateUserAuthContext(t.Context(), defaultNs, map[string][]string{})
err := svc.CanReference(ctx, owner, values)
require.Error(t, err)
})
t.Run("when one of the secure values has `remove` field set, it returns an error", func(t *testing.T) {
t.Parallel()
svc := service.ProvideInlineSecureValueService(tracer, nil, nil)
values := common.InlineSecureValues{
"fieldA": {
Name: "test-sv",
Remove: true,
},
}
ctx := testutils.CreateUserAuthContext(t.Context(), defaultNs, map[string][]string{})
err := svc.CanReference(ctx, owner, values)
err := svc.CanReference(ctx, owner)
require.Error(t, err)
})
@@ -193,13 +137,9 @@ func TestIntegration_InlineSecureValue_CanReference(t *testing.T) {
tu := testutils.Setup(t)
svc := service.ProvideInlineSecureValueService(tracer, tu.SecureValueService, nil)
values := common.InlineSecureValues{
"fieldA": {Name: "non-existent-sv"},
}
ctx := testutils.CreateUserAuthContext(t.Context(), defaultNs, map[string][]string{})
err := svc.CanReference(ctx, owner, values)
err := svc.CanReference(ctx, owner, "non-existent-sv")
require.Error(t, err)
})
@@ -225,15 +165,11 @@ func TestIntegration_InlineSecureValue_CanReference(t *testing.T) {
require.NoError(t, err)
require.NotNil(t, createdSv1)
values := common.InlineSecureValues{
"fieldA": {Name: sv1},
}
ctx := testutils.CreateUserAuthContext(t.Context(), defaultNs, map[string][]string{})
svc := service.ProvideInlineSecureValueService(tracer, tu.SecureValueService, nil)
err = svc.CanReference(ctx, owner, values)
err = svc.CanReference(ctx, owner, sv1)
require.Error(t, err)
})
@@ -249,15 +185,11 @@ func TestIntegration_InlineSecureValue_CanReference(t *testing.T) {
})
require.NoError(t, err)
values := common.InlineSecureValues{
"fieldA": {Name: sv1},
}
ctx := identity.WithServiceIdentityContext(t.Context(), 1234)
svc := service.ProvideInlineSecureValueService(tracer, tu.SecureValueService, nil)
err = svc.CanReference(ctx, owner, values)
err = svc.CanReference(ctx, owner, sv1)
require.Error(t, err)
})
@@ -273,22 +205,18 @@ func TestIntegration_InlineSecureValue_CanReference(t *testing.T) {
})
require.NoError(t, err)
values := common.InlineSecureValues{
"fieldA": {Name: sv1},
}
svc := service.ProvideInlineSecureValueService(tracer, tu.SecureValueService, tu.AccessClient)
ctx := testutils.CreateUserAuthContext(t.Context(), defaultNs, map[string][]string{
"securevalues:read": {"securevalues:uid:another-sv"}, // can read, but another resource!
})
err = svc.CanReference(ctx, owner, values)
err = svc.CanReference(ctx, owner, sv1)
require.Error(t, err)
ctx = testutils.CreateUserAuthContext(t.Context(), defaultNs, nil)
err = svc.CanReference(ctx, owner, values)
err = svc.CanReference(ctx, owner, sv1)
require.Error(t, err)
})
}