Better tracing during extSvcAcc registration (#84719)

This commit is contained in:
Xavi Lacasa
2024-03-21 16:41:10 +01:00
committed by GitHub
parent b848d8709c
commit a813046f3d
8 changed files with 95 additions and 46 deletions

View File

@ -3,10 +3,12 @@ package serverlock
import ( import (
"context" "context"
"errors" "errors"
"fmt"
"math/rand" "math/rand"
"time" "time"
"go.opentelemetry.io/otel/attribute" "go.opentelemetry.io/otel/attribute"
"go.opentelemetry.io/otel/codes"
"github.com/grafana/grafana/pkg/infra/db" "github.com/grafana/grafana/pkg/infra/db"
"github.com/grafana/grafana/pkg/infra/log" "github.com/grafana/grafana/pkg/infra/log"
@ -48,6 +50,7 @@ func (sl *ServerLockService) LockAndExecute(ctx context.Context, actionName stri
rowLock, err := sl.getOrCreate(ctx, actionName) rowLock, err := sl.getOrCreate(ctx, actionName)
if err != nil { if err != nil {
span.RecordError(err) span.RecordError(err)
span.SetStatus(codes.Error, fmt.Sprintf("failed to getOrCreate serverlock: %v", err))
return err return err
} }
@ -60,6 +63,7 @@ func (sl *ServerLockService) LockAndExecute(ctx context.Context, actionName stri
acquiredLock, err := sl.acquireLock(ctx, rowLock) acquiredLock, err := sl.acquireLock(ctx, rowLock)
if err != nil { if err != nil {
span.RecordError(err) span.RecordError(err)
span.SetStatus(codes.Error, fmt.Sprintf("failed to acquire serverlock: %v", err))
return err return err
} }
@ -147,6 +151,7 @@ func (sl *ServerLockService) LockExecuteAndRelease(ctx context.Context, actionNa
// could not get the lock, returning // could not get the lock, returning
if err != nil { if err != nil {
span.RecordError(err) span.RecordError(err)
span.SetStatus(codes.Error, fmt.Sprintf("failed to acquire serverlock: %v", err))
return err return err
} }
@ -155,6 +160,7 @@ func (sl *ServerLockService) LockExecuteAndRelease(ctx context.Context, actionNa
err = sl.releaseLock(ctx, actionName) err = sl.releaseLock(ctx, actionName)
if err != nil { if err != nil {
span.RecordError(err) span.RecordError(err)
span.SetStatus(codes.Error, fmt.Sprintf("failed to release serverlock: %v", err))
ctxLogger.Error("Failed to release the lock", "error", err) ctxLogger.Error("Failed to release the lock", "error", err)
} }
@ -207,6 +213,7 @@ func (sl *ServerLockService) LockExecuteAndReleaseWithRetries(ctx context.Contex
continue continue
} }
span.RecordError(err) span.RecordError(err)
span.SetStatus(codes.Error, fmt.Sprintf("failed to acquire serverlock: %v", err))
return err return err
} }
@ -218,6 +225,7 @@ func (sl *ServerLockService) LockExecuteAndReleaseWithRetries(ctx context.Contex
if err := sl.releaseLock(ctx, actionName); err != nil { if err := sl.releaseLock(ctx, actionName); err != nil {
span.RecordError(err) span.RecordError(err)
span.SetStatus(codes.Error, fmt.Sprintf("failed to release serverlock: %v", err))
ctxLogger.Error("Failed to release the lock", "error", err) ctxLogger.Error("Failed to release the lock", "error", err)
} }

View File

@ -126,9 +126,10 @@ func (r *Registry) RemoveExternalService(ctx context.Context, name string) error
// associated service account has the correct permissions. // associated service account has the correct permissions.
func (r *Registry) SaveExternalService(ctx context.Context, cmd *extsvcauth.ExternalServiceRegistration) (*extsvcauth.ExternalService, error) { func (r *Registry) SaveExternalService(ctx context.Context, cmd *extsvcauth.ExternalServiceRegistration) (*extsvcauth.ExternalService, error) {
var ( var (
errSave error errSave error
extSvc *extsvcauth.ExternalService extSvc *extsvcauth.ExternalService
lockName = "ext-svc-save-" + cmd.Name lockName = "ext-svc-save-" + cmd.Name
ctxLogger = r.logger.FromContext(ctx)
) )
err := r.serverLock.LockExecuteAndReleaseWithRetries(ctx, lockName, lockTimeConfig, func(ctx context.Context) { err := r.serverLock.LockExecuteAndReleaseWithRetries(ctx, lockName, lockTimeConfig, func(ctx context.Context) {
@ -140,10 +141,10 @@ func (r *Registry) SaveExternalService(ctx context.Context, cmd *extsvcauth.Exte
switch cmd.AuthProvider { switch cmd.AuthProvider {
case extsvcauth.ServiceAccounts: case extsvcauth.ServiceAccounts:
if !r.features.IsEnabled(ctx, featuremgmt.FlagExternalServiceAccounts) { if !r.features.IsEnabled(ctx, featuremgmt.FlagExternalServiceAccounts) {
r.logger.Warn("Skipping External Service authentication, flag disabled", "service", cmd.Name, "flag", featuremgmt.FlagExternalServiceAccounts) ctxLogger.Warn("Skipping External Service authentication, flag disabled", "service", cmd.Name, "flag", featuremgmt.FlagExternalServiceAccounts)
return return
} }
r.logger.Debug("Routing the External Service registration to the External Service Account service", "service", cmd.Name) ctxLogger.Debug("Routing the External Service registration to the External Service Account service", "service", cmd.Name)
extSvc, errSave = r.saReg.SaveExternalService(ctx, cmd) extSvc, errSave = r.saReg.SaveExternalService(ctx, cmd)
default: default:
errSave = extsvcauth.ErrUnknownProvider.Errorf("unknown provider '%v'", cmd.AuthProvider) errSave = extsvcauth.ErrUnknownProvider.Errorf("unknown provider '%v'", cmd.AuthProvider)

View File

@ -11,6 +11,7 @@ import (
"github.com/google/go-cmp/cmp/cmpopts" "github.com/google/go-cmp/cmp/cmpopts"
"github.com/stretchr/testify/require" "github.com/stretchr/testify/require"
"github.com/grafana/grafana/pkg/infra/tracing"
"github.com/grafana/grafana/pkg/plugins" "github.com/grafana/grafana/pkg/plugins"
"github.com/grafana/grafana/pkg/plugins/auth" "github.com/grafana/grafana/pkg/plugins/auth"
"github.com/grafana/grafana/pkg/plugins/backendplugin" "github.com/grafana/grafana/pkg/plugins/backendplugin"
@ -1474,7 +1475,7 @@ func newLoader(t *testing.T, cfg *config.PluginManagementCfg, reg registry.Servi
finder.NewLocalFinder(false), reg), finder.NewLocalFinder(false), reg),
pipeline.ProvideBootstrapStage(cfg, signature.DefaultCalculator(cfg), assets), pipeline.ProvideBootstrapStage(cfg, signature.DefaultCalculator(cfg), assets),
pipeline.ProvideValidationStage(cfg, signature.NewValidator(signature.NewUnsignedAuthorizer(cfg)), angularInspector, sigErrTracker), pipeline.ProvideValidationStage(cfg, signature.NewValidator(signature.NewUnsignedAuthorizer(cfg)), angularInspector, sigErrTracker),
pipeline.ProvideInitializationStage(cfg, reg, backendFactory, proc, &fakes.FakeAuthService{}, fakes.NewFakeRoleRegistry(), fakes.NewFakePluginEnvProvider()), pipeline.ProvideInitializationStage(cfg, reg, backendFactory, proc, &fakes.FakeAuthService{}, fakes.NewFakeRoleRegistry(), fakes.NewFakePluginEnvProvider(), tracing.InitializeTracerForTest()),
terminate) terminate)
} }
@ -1506,7 +1507,7 @@ func newLoaderWithOpts(t *testing.T, cfg *config.PluginManagementCfg, opts loade
finder.NewLocalFinder(false), reg), finder.NewLocalFinder(false), reg),
pipeline.ProvideBootstrapStage(cfg, signature.DefaultCalculator(cfg), assets), pipeline.ProvideBootstrapStage(cfg, signature.DefaultCalculator(cfg), assets),
pipeline.ProvideValidationStage(cfg, signature.NewValidator(signature.NewUnsignedAuthorizer(cfg)), angularInspector, sigErrTracker), pipeline.ProvideValidationStage(cfg, signature.NewValidator(signature.NewUnsignedAuthorizer(cfg)), angularInspector, sigErrTracker),
pipeline.ProvideInitializationStage(cfg, reg, backendFactoryProvider, proc, authServiceRegistry, fakes.NewFakeRoleRegistry(), fakes.NewFakePluginEnvProvider()), pipeline.ProvideInitializationStage(cfg, reg, backendFactoryProvider, proc, authServiceRegistry, fakes.NewFakeRoleRegistry(), fakes.NewFakePluginEnvProvider(), tracing.InitializeTracerForTest()),
terminate) terminate)
} }

View File

@ -3,6 +3,7 @@ package pipeline
import ( import (
"context" "context"
"github.com/grafana/grafana/pkg/infra/tracing"
"github.com/grafana/grafana/pkg/plugins" "github.com/grafana/grafana/pkg/plugins"
"github.com/grafana/grafana/pkg/plugins/auth" "github.com/grafana/grafana/pkg/plugins/auth"
"github.com/grafana/grafana/pkg/plugins/config" "github.com/grafana/grafana/pkg/plugins/config"
@ -61,10 +62,10 @@ func ProvideValidationStage(cfg *config.PluginManagementCfg, sv signature.Valida
func ProvideInitializationStage(cfg *config.PluginManagementCfg, pr registry.Service, bp plugins.BackendFactoryProvider, func ProvideInitializationStage(cfg *config.PluginManagementCfg, pr registry.Service, bp plugins.BackendFactoryProvider,
pm process.Manager, externalServiceRegistry auth.ExternalServiceRegistry, pm process.Manager, externalServiceRegistry auth.ExternalServiceRegistry,
roleRegistry plugins.RoleRegistry, pluginEnvProvider envvars.Provider) *initialization.Initialize { roleRegistry plugins.RoleRegistry, pluginEnvProvider envvars.Provider, tracer tracing.Tracer) *initialization.Initialize {
return initialization.New(cfg, initialization.Opts{ return initialization.New(cfg, initialization.Opts{
InitializeFuncs: []initialization.InitializeFunc{ InitializeFuncs: []initialization.InitializeFunc{
ExternalServiceRegistrationStep(cfg, externalServiceRegistry), ExternalServiceRegistrationStep(cfg, externalServiceRegistry, tracer),
initialization.BackendClientInitStep(pluginEnvProvider, bp), initialization.BackendClientInitStep(pluginEnvProvider, bp),
initialization.PluginRegistrationStep(pr), initialization.PluginRegistrationStep(pr),
initialization.BackendProcessStartStep(pm), initialization.BackendProcessStartStep(pm),

View File

@ -3,9 +3,14 @@ package pipeline
import ( import (
"context" "context"
"errors" "errors"
"fmt"
"slices" "slices"
"go.opentelemetry.io/otel/attribute"
"go.opentelemetry.io/otel/codes"
"github.com/grafana/grafana/pkg/infra/metrics" "github.com/grafana/grafana/pkg/infra/metrics"
"github.com/grafana/grafana/pkg/infra/tracing"
"github.com/grafana/grafana/pkg/plugins" "github.com/grafana/grafana/pkg/plugins"
"github.com/grafana/grafana/pkg/plugins/auth" "github.com/grafana/grafana/pkg/plugins/auth"
"github.com/grafana/grafana/pkg/plugins/config" "github.com/grafana/grafana/pkg/plugins/config"
@ -23,31 +28,43 @@ type ExternalServiceRegistration struct {
cfg *config.PluginManagementCfg cfg *config.PluginManagementCfg
externalServiceRegistry auth.ExternalServiceRegistry externalServiceRegistry auth.ExternalServiceRegistry
log log.Logger log log.Logger
tracer tracing.Tracer
} }
// ExternalServiceRegistrationStep returns an InitializeFunc for registering external services. // ExternalServiceRegistrationStep returns an InitializeFunc for registering external services.
func ExternalServiceRegistrationStep(cfg *config.PluginManagementCfg, externalServiceRegistry auth.ExternalServiceRegistry) initialization.InitializeFunc { func ExternalServiceRegistrationStep(cfg *config.PluginManagementCfg, externalServiceRegistry auth.ExternalServiceRegistry, tracer tracing.Tracer) initialization.InitializeFunc {
return newExternalServiceRegistration(cfg, externalServiceRegistry).Register return newExternalServiceRegistration(cfg, externalServiceRegistry, tracer).Register
} }
func newExternalServiceRegistration(cfg *config.PluginManagementCfg, serviceRegistry auth.ExternalServiceRegistry) *ExternalServiceRegistration { func newExternalServiceRegistration(cfg *config.PluginManagementCfg, serviceRegistry auth.ExternalServiceRegistry, tracer tracing.Tracer) *ExternalServiceRegistration {
return &ExternalServiceRegistration{ return &ExternalServiceRegistration{
cfg: cfg, cfg: cfg,
externalServiceRegistry: serviceRegistry, externalServiceRegistry: serviceRegistry,
log: log.New("plugins.external.registration"), log: log.New("plugins.external.registration"),
tracer: tracer,
} }
} }
// Register registers the external service with the external service registry, if the feature is enabled. // Register registers the external service with the external service registry, if the feature is enabled.
func (r *ExternalServiceRegistration) Register(ctx context.Context, p *plugins.Plugin) (*plugins.Plugin, error) { func (r *ExternalServiceRegistration) Register(ctx context.Context, p *plugins.Plugin) (*plugins.Plugin, error) {
if p.IAM != nil { if p.IAM == nil {
s, err := r.externalServiceRegistry.RegisterExternalService(ctx, p.ID, pfs.Type(p.Type), p.IAM) return p, nil
if err != nil {
r.log.Error("Could not register an external service. Initialization skipped", "pluginId", p.ID, "error", err)
return nil, err
}
p.ExternalService = s
} }
ctx, span := r.tracer.Start(ctx, "ExternalServiceRegistration.Register")
span.SetAttributes(attribute.String("register.pluginId", p.ID))
defer span.End()
ctxLogger := r.log.FromContext(ctx)
s, err := r.externalServiceRegistry.RegisterExternalService(ctx, p.ID, pfs.Type(p.Type), p.IAM)
if err != nil {
ctxLogger.Error("Could not register an external service. Initialization skipped", "pluginId", p.ID, "error", err)
span.SetStatus(codes.Error, fmt.Sprintf("could not register external service: %v", err))
return nil, err
}
p.ExternalService = s
return p, nil return p, nil
} }

View File

@ -41,8 +41,10 @@ func (s *Service) HasExternalService(ctx context.Context, pluginID string) (bool
// RegisterExternalService is a simplified wrapper around SaveExternalService for the plugin use case. // RegisterExternalService is a simplified wrapper around SaveExternalService for the plugin use case.
func (s *Service) RegisterExternalService(ctx context.Context, pluginID string, pType pfs.Type, svc *pfs.IAM) (*auth.ExternalService, error) { func (s *Service) RegisterExternalService(ctx context.Context, pluginID string, pType pfs.Type, svc *pfs.IAM) (*auth.ExternalService, error) {
ctxLogger := s.log.FromContext(ctx)
if !s.featureEnabled { if !s.featureEnabled {
s.log.Warn("Skipping External Service Registration. The feature is behind a feature toggle and needs to be enabled.") ctxLogger.Warn("Skipping External Service Registration. The feature is behind a feature toggle and needs to be enabled.")
return nil, nil return nil, nil
} }

View File

@ -5,6 +5,7 @@ import (
"github.com/stretchr/testify/require" "github.com/stretchr/testify/require"
"github.com/grafana/grafana/pkg/infra/tracing"
"github.com/grafana/grafana/pkg/plugins" "github.com/grafana/grafana/pkg/plugins"
"github.com/grafana/grafana/pkg/plugins/backendplugin" "github.com/grafana/grafana/pkg/plugins/backendplugin"
"github.com/grafana/grafana/pkg/plugins/backendplugin/coreplugin" "github.com/grafana/grafana/pkg/plugins/backendplugin/coreplugin"
@ -54,7 +55,7 @@ func CreateIntegrationTestCtx(t *testing.T, cfg *setting.Cfg, coreRegistry *core
disc := pipeline.ProvideDiscoveryStage(pCfg, finder.NewLocalFinder(true), reg) disc := pipeline.ProvideDiscoveryStage(pCfg, finder.NewLocalFinder(true), reg)
boot := pipeline.ProvideBootstrapStage(pCfg, signature.ProvideService(pCfg, statickey.New()), assetpath.ProvideService(pCfg, cdn)) boot := pipeline.ProvideBootstrapStage(pCfg, signature.ProvideService(pCfg, statickey.New()), assetpath.ProvideService(pCfg, cdn))
valid := pipeline.ProvideValidationStage(pCfg, signature.NewValidator(signature.NewUnsignedAuthorizer(pCfg)), angularInspector, errTracker) valid := pipeline.ProvideValidationStage(pCfg, signature.NewValidator(signature.NewUnsignedAuthorizer(pCfg)), angularInspector, errTracker)
init := pipeline.ProvideInitializationStage(pCfg, reg, provider.ProvideService(coreRegistry), proc, &fakes.FakeAuthService{}, fakes.NewFakeRoleRegistry(), nil) init := pipeline.ProvideInitializationStage(pCfg, reg, provider.ProvideService(coreRegistry), proc, &fakes.FakeAuthService{}, fakes.NewFakeRoleRegistry(), nil, tracing.InitializeTracerForTest())
term, err := pipeline.ProvideTerminationStage(pCfg, reg, proc) term, err := pipeline.ProvideTerminationStage(pCfg, reg, proc)
require.NoError(t, err) require.NoError(t, err)
@ -100,7 +101,7 @@ func CreateTestLoader(t *testing.T, cfg *pluginsCfg.PluginManagementCfg, opts Lo
if opts.Initializer == nil { if opts.Initializer == nil {
reg := registry.ProvideService() reg := registry.ProvideService()
coreRegistry := coreplugin.NewRegistry(make(map[string]backendplugin.PluginFactoryFunc)) coreRegistry := coreplugin.NewRegistry(make(map[string]backendplugin.PluginFactoryFunc))
opts.Initializer = pipeline.ProvideInitializationStage(cfg, reg, provider.ProvideService(coreRegistry), process.ProvideService(), &fakes.FakeAuthService{}, fakes.NewFakeRoleRegistry(), nil) opts.Initializer = pipeline.ProvideInitializationStage(cfg, reg, provider.ProvideService(coreRegistry), process.ProvideService(), &fakes.FakeAuthService{}, fakes.NewFakeRoleRegistry(), nil, tracing.InitializeTracerForTest())
} }
if opts.Terminator == nil { if opts.Terminator == nil {

View File

@ -110,14 +110,16 @@ func (esa *ExtSvcAccountsService) GetExternalServiceNames(ctx context.Context) (
ctx, span := esa.tracer.Start(ctx, "ExtSvcAccountsService.GetExternalServiceNames") ctx, span := esa.tracer.Start(ctx, "ExtSvcAccountsService.GetExternalServiceNames")
defer span.End() defer span.End()
esa.logger.Debug("Get external service names from store") ctxLogger := esa.logger.FromContext(ctx)
ctxLogger.Debug("Get external service names from store")
sas, err := esa.saSvc.SearchOrgServiceAccounts(ctx, &sa.SearchOrgServiceAccountsQuery{ sas, err := esa.saSvc.SearchOrgServiceAccounts(ctx, &sa.SearchOrgServiceAccountsQuery{
OrgID: extsvcauth.TmpOrgID, OrgID: extsvcauth.TmpOrgID,
Filter: sa.FilterOnlyExternal, Filter: sa.FilterOnlyExternal,
SignedInUser: extsvcuser, SignedInUser: extsvcuser,
}) })
if err != nil { if err != nil {
esa.logger.Error("Could not fetch external service accounts from store", "error", err.Error()) ctxLogger.Error("Could not fetch external service accounts from store", "error", err.Error())
return nil, err return nil, err
} }
if sas == nil { if sas == nil {
@ -134,15 +136,17 @@ func (esa *ExtSvcAccountsService) GetExternalServiceNames(ctx context.Context) (
func (esa *ExtSvcAccountsService) SaveExternalService(ctx context.Context, cmd *extsvcauth.ExternalServiceRegistration) (*extsvcauth.ExternalService, error) { func (esa *ExtSvcAccountsService) SaveExternalService(ctx context.Context, cmd *extsvcauth.ExternalServiceRegistration) (*extsvcauth.ExternalService, error) {
// This is double proofing, we should never reach here anyway the flags have already been checked. // This is double proofing, we should never reach here anyway the flags have already been checked.
if !esa.features.IsEnabled(ctx, featuremgmt.FlagExternalServiceAccounts) { if !esa.features.IsEnabled(ctx, featuremgmt.FlagExternalServiceAccounts) {
esa.logger.Warn("This feature is behind a feature flag, please set it if you want to save external services") esa.logger.FromContext(ctx).Warn("This feature is behind a feature flag, please set it if you want to save external services")
return nil, nil return nil, nil
} }
ctx, span := esa.tracer.Start(ctx, "ExtSvcAccountsService.SaveExternalService") ctx, span := esa.tracer.Start(ctx, "ExtSvcAccountsService.SaveExternalService")
defer span.End() defer span.End()
ctxLogger := esa.logger.FromContext(ctx)
if cmd == nil { if cmd == nil {
esa.logger.Warn("Received no input") ctxLogger.Warn("Received no input")
return nil, nil return nil, nil
} }
@ -160,13 +164,13 @@ func (esa *ExtSvcAccountsService) SaveExternalService(ctx context.Context, cmd *
// No need for a token if we don't have a service account // No need for a token if we don't have a service account
if saID <= 0 { if saID <= 0 {
esa.logger.Debug("Skipping service account token creation", "service", slug) ctxLogger.Debug("Skipping service account token creation", "service", slug)
return nil, nil return nil, nil
} }
token, err := esa.getExtSvcAccountToken(ctx, extsvcauth.TmpOrgID, saID, slug) token, err := esa.getExtSvcAccountToken(ctx, extsvcauth.TmpOrgID, saID, slug)
if err != nil { if err != nil {
esa.logger.Error("Could not get the external svc token", ctxLogger.Error("Could not get the external svc token",
"service", slug, "service", slug,
"saID", saID, "saID", saID,
"error", err.Error()) "error", err.Error())
@ -192,18 +196,20 @@ func (esa *ExtSvcAccountsService) RemoveExtSvcAccount(ctx context.Context, orgID
ctx, span := esa.tracer.Start(ctx, "ExtSvcAccountsService.RemoveExtSvcAccount") ctx, span := esa.tracer.Start(ctx, "ExtSvcAccountsService.RemoveExtSvcAccount")
defer span.End() defer span.End()
ctxLogger := esa.logger.FromContext(ctx)
saID, errRetrieve := esa.saSvc.RetrieveServiceAccountIdByName(ctx, orgID, sa.ExtSvcPrefix+extSvcSlug) saID, errRetrieve := esa.saSvc.RetrieveServiceAccountIdByName(ctx, orgID, sa.ExtSvcPrefix+extSvcSlug)
if errRetrieve != nil && !errors.Is(errRetrieve, sa.ErrServiceAccountNotFound) { if errRetrieve != nil && !errors.Is(errRetrieve, sa.ErrServiceAccountNotFound) {
return errRetrieve return errRetrieve
} }
if saID <= 0 { if saID <= 0 {
esa.logger.Debug("No external service account associated with this service", "service", extSvcSlug, "orgID", orgID) ctxLogger.Debug("No external service account associated with this service", "service", extSvcSlug, "orgID", orgID)
return nil return nil
} }
if err := esa.deleteExtSvcAccount(ctx, orgID, extSvcSlug, saID); err != nil { if err := esa.deleteExtSvcAccount(ctx, orgID, extSvcSlug, saID); err != nil {
esa.logger.Error("Error occurred while deleting service account", ctxLogger.Error("Error occurred while deleting service account",
"service", extSvcSlug, "service", extSvcSlug,
"saID", saID, "saID", saID,
"error", err.Error()) "error", err.Error())
@ -217,15 +223,17 @@ func (esa *ExtSvcAccountsService) RemoveExtSvcAccount(ctx context.Context, orgID
func (esa *ExtSvcAccountsService) ManageExtSvcAccount(ctx context.Context, cmd *sa.ManageExtSvcAccountCmd) (int64, error) { func (esa *ExtSvcAccountsService) ManageExtSvcAccount(ctx context.Context, cmd *sa.ManageExtSvcAccountCmd) (int64, error) {
// This is double proofing, we should never reach here anyway the flags have already been checked. // This is double proofing, we should never reach here anyway the flags have already been checked.
if !esa.features.IsEnabled(ctx, featuremgmt.FlagExternalServiceAccounts) { if !esa.features.IsEnabled(ctx, featuremgmt.FlagExternalServiceAccounts) {
esa.logger.Warn("This feature is behind a feature flag, please set it if you want to save external services") esa.logger.FromContext(ctx).Warn("This feature is behind a feature flag, please set it if you want to save external services")
return 0, nil return 0, nil
} }
ctx, span := esa.tracer.Start(ctx, "ExtSvcAccountsService.ManageExtSvcAccount") ctx, span := esa.tracer.Start(ctx, "ExtSvcAccountsService.ManageExtSvcAccount")
defer span.End() defer span.End()
ctxLogger := esa.logger.FromContext(ctx)
if cmd == nil { if cmd == nil {
esa.logger.Warn("Received no input") ctxLogger.Warn("Received no input")
return 0, nil return 0, nil
} }
@ -237,14 +245,14 @@ func (esa *ExtSvcAccountsService) ManageExtSvcAccount(ctx context.Context, cmd *
if len(cmd.Permissions) == 0 { if len(cmd.Permissions) == 0 {
if saID > 0 { if saID > 0 {
if err := esa.deleteExtSvcAccount(ctx, cmd.OrgID, cmd.ExtSvcSlug, saID); err != nil { if err := esa.deleteExtSvcAccount(ctx, cmd.OrgID, cmd.ExtSvcSlug, saID); err != nil {
esa.logger.Error("Error occurred while deleting service account", ctxLogger.Error("Error occurred while deleting service account",
"service", cmd.ExtSvcSlug, "service", cmd.ExtSvcSlug,
"saID", saID, "saID", saID,
"error", err.Error()) "error", err.Error())
return 0, err return 0, err
} }
} }
esa.logger.Info("Skipping service account creation, no permission", ctxLogger.Info("Skipping service account creation, no permission",
"service", cmd.ExtSvcSlug, "service", cmd.ExtSvcSlug,
"permission count", len(cmd.Permissions), "permission count", len(cmd.Permissions),
"saID", saID) "saID", saID)
@ -259,7 +267,7 @@ func (esa *ExtSvcAccountsService) ManageExtSvcAccount(ctx context.Context, cmd *
SaID: saID, SaID: saID,
}) })
if errSave != nil { if errSave != nil {
esa.logger.Error("Could not save service account", "service", cmd.ExtSvcSlug, "error", errSave.Error()) ctxLogger.Error("Could not save service account", "service", cmd.ExtSvcSlug, "error", errSave.Error())
return 0, errSave return 0, errSave
} }
return saID, nil return saID, nil
@ -270,9 +278,11 @@ func (esa *ExtSvcAccountsService) saveExtSvcAccount(ctx context.Context, cmd *sa
ctx, span := esa.tracer.Start(ctx, "ExtSvcAccountsService.saveExtSvcAccount") ctx, span := esa.tracer.Start(ctx, "ExtSvcAccountsService.saveExtSvcAccount")
defer span.End() defer span.End()
ctxLogger := esa.logger.FromContext(ctx)
if cmd.SaID <= 0 { if cmd.SaID <= 0 {
// Create a service account // Create a service account
esa.logger.Info("Create service account", "service", cmd.ExtSvcSlug, "orgID", cmd.OrgID) ctxLogger.Info("Create service account", "service", cmd.ExtSvcSlug, "orgID", cmd.OrgID)
sa, err := esa.saSvc.CreateServiceAccount(ctx, cmd.OrgID, &sa.CreateServiceAccountForm{ sa, err := esa.saSvc.CreateServiceAccount(ctx, cmd.OrgID, &sa.CreateServiceAccountForm{
Name: sa.ExtSvcPrefix + cmd.ExtSvcSlug, Name: sa.ExtSvcPrefix + cmd.ExtSvcSlug,
Role: newRole(roletype.RoleNone), Role: newRole(roletype.RoleNone),
@ -285,13 +295,13 @@ func (esa *ExtSvcAccountsService) saveExtSvcAccount(ctx context.Context, cmd *sa
} }
// Enable or disable the service account // Enable or disable the service account
esa.logger.Debug("Set service account state", "service", cmd.ExtSvcSlug, "saID", cmd.SaID, "enabled", cmd.Enabled) ctxLogger.Debug("Set service account state", "service", cmd.ExtSvcSlug, "saID", cmd.SaID, "enabled", cmd.Enabled)
if err := esa.saSvc.EnableServiceAccount(ctx, cmd.OrgID, cmd.SaID, cmd.Enabled); err != nil { if err := esa.saSvc.EnableServiceAccount(ctx, cmd.OrgID, cmd.SaID, cmd.Enabled); err != nil {
return 0, err return 0, err
} }
// update the service account's permissions // update the service account's permissions
esa.logger.Debug("Update role permissions", "service", cmd.ExtSvcSlug, "saID", cmd.SaID) ctxLogger.Debug("Update role permissions", "service", cmd.ExtSvcSlug, "saID", cmd.SaID)
if err := esa.acSvc.SaveExternalServiceRole(ctx, ac.SaveExternalServiceRoleCommand{ if err := esa.acSvc.SaveExternalServiceRole(ctx, ac.SaveExternalServiceRoleCommand{
AssignmentOrgID: cmd.OrgID, AssignmentOrgID: cmd.OrgID,
ExternalServiceID: cmd.ExtSvcSlug, ExternalServiceID: cmd.ExtSvcSlug,
@ -311,7 +321,9 @@ func (esa *ExtSvcAccountsService) deleteExtSvcAccount(ctx context.Context, orgID
ctx, span := esa.tracer.Start(ctx, "ExtSvcAccountsService.deleteExtSvcAccount") ctx, span := esa.tracer.Start(ctx, "ExtSvcAccountsService.deleteExtSvcAccount")
defer span.End() defer span.End()
esa.logger.Info("Delete service account", "service", slug, "orgID", orgID, "saID", saID) ctxLogger := esa.logger.FromContext(ctx)
ctxLogger.Info("Delete service account", "service", slug, "orgID", orgID, "saID", saID)
if err := esa.saSvc.DeleteServiceAccount(ctx, orgID, saID); err != nil { if err := esa.saSvc.DeleteServiceAccount(ctx, orgID, saID); err != nil {
return err return err
} }
@ -330,6 +342,8 @@ func (esa *ExtSvcAccountsService) getExtSvcAccountToken(ctx context.Context, org
ctx, span := esa.tracer.Start(ctx, "ExtSvcAccountsService.getExtSvcAccountToken") ctx, span := esa.tracer.Start(ctx, "ExtSvcAccountsService.getExtSvcAccountToken")
defer span.End() defer span.End()
ctxLogger := esa.logger.FromContext(ctx)
// Get credentials from store // Get credentials from store
credentials, err := esa.GetExtSvcCredentials(ctx, orgID, extSvcSlug) credentials, err := esa.GetExtSvcCredentials(ctx, orgID, extSvcSlug)
if err != nil && !errors.Is(err, ErrCredentialsNotFound) { if err != nil && !errors.Is(err, ErrCredentialsNotFound) {
@ -340,13 +354,13 @@ func (esa *ExtSvcAccountsService) getExtSvcAccountToken(ctx context.Context, org
} }
// Generate token // Generate token
esa.logger.Info("Generate new service account token", "service", extSvcSlug, "orgID", orgID) ctxLogger.Info("Generate new service account token", "service", extSvcSlug, "orgID", orgID)
newKeyInfo, err := satokengen.New(extSvcSlug) newKeyInfo, err := satokengen.New(extSvcSlug)
if err != nil { if err != nil {
return "", err return "", err
} }
esa.logger.Debug("Add service account token", "service", extSvcSlug, "orgID", orgID) ctxLogger.Debug("Add service account token", "service", extSvcSlug, "orgID", orgID)
if _, err := esa.saSvc.AddServiceAccountToken(ctx, saID, &sa.AddServiceAccountTokenCommand{ if _, err := esa.saSvc.AddServiceAccountToken(ctx, saID, &sa.AddServiceAccountTokenCommand{
Name: tokenNamePrefix + "-" + extSvcSlug, Name: tokenNamePrefix + "-" + extSvcSlug,
OrgId: orgID, OrgId: orgID,
@ -368,7 +382,8 @@ func (esa *ExtSvcAccountsService) getExtSvcAccountToken(ctx context.Context, org
// GetExtSvcCredentials get the credentials of an External Service from an encrypted storage // GetExtSvcCredentials get the credentials of an External Service from an encrypted storage
func (esa *ExtSvcAccountsService) GetExtSvcCredentials(ctx context.Context, orgID int64, extSvcSlug string) (*Credentials, error) { func (esa *ExtSvcAccountsService) GetExtSvcCredentials(ctx context.Context, orgID int64, extSvcSlug string) (*Credentials, error) {
esa.logger.Debug("Get service account token from skv", "service", extSvcSlug, "orgID", orgID) ctxLogger := esa.logger.FromContext(ctx)
ctxLogger.Debug("Get service account token from skv", "service", extSvcSlug, "orgID", orgID)
token, ok, err := esa.skvStore.Get(ctx, orgID, extSvcSlug, kvStoreType) token, ok, err := esa.skvStore.Get(ctx, orgID, extSvcSlug, kvStoreType)
if err != nil { if err != nil {
return nil, err return nil, err
@ -381,18 +396,21 @@ func (esa *ExtSvcAccountsService) GetExtSvcCredentials(ctx context.Context, orgI
// SaveExtSvcCredentials stores the credentials of an External Service in an encrypted storage // SaveExtSvcCredentials stores the credentials of an External Service in an encrypted storage
func (esa *ExtSvcAccountsService) SaveExtSvcCredentials(ctx context.Context, cmd *SaveCredentialsCmd) error { func (esa *ExtSvcAccountsService) SaveExtSvcCredentials(ctx context.Context, cmd *SaveCredentialsCmd) error {
esa.logger.Debug("Save service account token in skv", "service", cmd.ExtSvcSlug, "orgID", cmd.OrgID) ctxLogger := esa.logger.FromContext(ctx)
ctxLogger.Debug("Save service account token in skv", "service", cmd.ExtSvcSlug, "orgID", cmd.OrgID)
return esa.skvStore.Set(ctx, cmd.OrgID, cmd.ExtSvcSlug, kvStoreType, cmd.Secret) return esa.skvStore.Set(ctx, cmd.OrgID, cmd.ExtSvcSlug, kvStoreType, cmd.Secret)
} }
// DeleteExtSvcCredentials removes the credentials of an External Service from an encrypted storage // DeleteExtSvcCredentials removes the credentials of an External Service from an encrypted storage
func (esa *ExtSvcAccountsService) DeleteExtSvcCredentials(ctx context.Context, orgID int64, extSvcSlug string) error { func (esa *ExtSvcAccountsService) DeleteExtSvcCredentials(ctx context.Context, orgID int64, extSvcSlug string) error {
esa.logger.Debug("Delete service account token from skv", "service", extSvcSlug, "orgID", orgID) ctxLogger := esa.logger.FromContext(ctx)
ctxLogger.Debug("Delete service account token from skv", "service", extSvcSlug, "orgID", orgID)
return esa.skvStore.Del(ctx, orgID, extSvcSlug, kvStoreType) return esa.skvStore.Del(ctx, orgID, extSvcSlug, kvStoreType)
} }
func (esa *ExtSvcAccountsService) handlePluginStateChanged(ctx context.Context, event *pluginsettings.PluginStateChangedEvent) error { func (esa *ExtSvcAccountsService) handlePluginStateChanged(ctx context.Context, event *pluginsettings.PluginStateChangedEvent) error {
esa.logger.Debug("Plugin state changed", "pluginId", event.PluginId, "enabled", event.Enabled) ctxLogger := esa.logger.FromContext(ctx)
ctxLogger.Debug("Plugin state changed", "pluginId", event.PluginId, "enabled", event.Enabled)
errEnable := esa.EnableExtSvcAccount(ctx, &sa.EnableExtSvcAccountCmd{ errEnable := esa.EnableExtSvcAccount(ctx, &sa.EnableExtSvcAccountCmd{
ExtSvcSlug: event.PluginId, ExtSvcSlug: event.PluginId,
@ -402,7 +420,7 @@ func (esa *ExtSvcAccountsService) handlePluginStateChanged(ctx context.Context,
// Ignore service account not found error // Ignore service account not found error
if errors.Is(errEnable, sa.ErrServiceAccountNotFound) { if errors.Is(errEnable, sa.ErrServiceAccountNotFound) {
esa.logger.Debug("No ext svc account with this plugin", "pluginId", event.PluginId, "orgId", event.OrgId) ctxLogger.Debug("No ext svc account with this plugin", "pluginId", event.PluginId, "orgId", event.OrgId)
return nil return nil
} }
return errEnable return errEnable