From c0fd6d2a53ceee7e565827ed68f99f785828d7c0 Mon Sep 17 00:00:00 2001 From: KsaweryZietara <91937141+KsaweryZietara@users.noreply.github.com> Date: Thu, 12 Dec 2024 08:15:59 +0100 Subject: [PATCH] Use slog in API (#1568) --- cmd/broker/bindings_envtest_test.go | 6 +- cmd/broker/broker_suite_test.go | 2 +- cmd/broker/main.go | 30 +++----- internal/appinfo/runtime_info_test.go | 19 +++-- internal/broker/bind_create.go | 26 +++---- internal/broker/bind_create_test.go | 21 ++--- internal/broker/bind_delete.go | 22 +++--- internal/broker/bind_get.go | 16 ++-- internal/broker/bind_get_test.go | 7 +- internal/broker/bind_last_operation.go | 14 ++-- internal/broker/instance_create.go | 41 +++++----- .../instance_create_input_params_test.go | 12 ++- internal/broker/instance_create_test.go | 77 +++++++++++-------- internal/broker/instance_deprovision.go | 24 +++--- internal/broker/instance_deprovision_test.go | 17 ++-- internal/broker/instance_get.go | 12 +-- internal/broker/instance_get_test.go | 25 +++--- internal/broker/instance_last_operation.go | 20 ++--- .../broker/instance_last_operation_test.go | 15 ++-- internal/broker/instance_update.go | 76 +++++++++--------- internal/broker/instance_update_test.go | 25 +++--- internal/broker/services.go | 8 +- internal/broker/services_test.go | 14 ++-- internal/httputil/response.go | 11 ++- internal/httputil/response_test.go | 36 ++++++--- internal/suspension/handler.go | 38 ++++----- internal/suspension/handler_test.go | 21 +++-- 27 files changed, 339 insertions(+), 296 deletions(-) diff --git a/cmd/broker/bindings_envtest_test.go b/cmd/broker/bindings_envtest_test.go index 6ecb390f1a..0d7d829065 100644 --- a/cmd/broker/bindings_envtest_test.go +++ b/cmd/broker/bindings_envtest_test.go @@ -209,9 +209,9 @@ func TestCreateBindingEndpoint(t *testing.T) { publisher := event.NewPubSub(log) //// api handler - bindEndpoint := broker.NewBind(*bindingCfg, db, logs, skrK8sClientProvider, skrK8sClientProvider, publisher) - getBindingEndpoint := broker.NewGetBinding(logs, db) - unbindEndpoint := broker.NewUnbind(logs, db, brokerBindings.NewServiceAccountBindingsManager(skrK8sClientProvider, skrK8sClientProvider), publisher) + bindEndpoint := broker.NewBind(*bindingCfg, db, log, skrK8sClientProvider, skrK8sClientProvider, publisher) + getBindingEndpoint := broker.NewGetBinding(log, db) + unbindEndpoint := broker.NewUnbind(log, db, brokerBindings.NewServiceAccountBindingsManager(skrK8sClientProvider, skrK8sClientProvider), publisher) apiHandler := handlers.NewApiHandler(broker.KymaEnvironmentBroker{ ServicesEndpoint: nil, ProvisionEndpoint: nil, diff --git a/cmd/broker/broker_suite_test.go b/cmd/broker/broker_suite_test.go index d4dbab021e..8f74c800c1 100644 --- a/cmd/broker/broker_suite_test.go +++ b/cmd/broker/broker_suite_test.go @@ -397,7 +397,7 @@ func (s *BrokerSuiteTest) CreateAPI(inputFactory broker.PlanValidator, cfg *Conf kcBuilder := &kcMock.KcBuilder{} kcBuilder.On("Build", nil).Return("--kubeconfig file", nil) createAPI(s.router, servicesConfig, inputFactory, cfg, db, provisioningQueue, deprovisionQueue, updateQueue, - lager.NewLogger("api"), logs, log, planDefaults, kcBuilder, skrK8sClientProvider, skrK8sClientProvider, gardenerClient, fakeKcpK8sClient, eventBroker) + lager.NewLogger("api"), log, planDefaults, kcBuilder, skrK8sClientProvider, skrK8sClientProvider, gardenerClient, fakeKcpK8sClient, eventBroker) s.httpServer = httptest.NewServer(s.router) } diff --git a/cmd/broker/main.go b/cmd/broker/main.go index 42f3e2b335..11c3e40d68 100644 --- a/cmd/broker/main.go +++ b/cmd/broker/main.go @@ -57,7 +57,6 @@ import ( "github.com/kyma-project/kyma-environment-broker/internal/swagger" "github.com/prometheus/client_golang/prometheus" "github.com/prometheus/client_golang/prometheus/promhttp" - "github.com/sirupsen/logrus" "github.com/vrischmann/envconfig" apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1" "k8s.io/apimachinery/pkg/util/wait" @@ -218,14 +217,6 @@ func main() { defer cancel() // set default formatted - logrus.SetFormatter(&logrus.JSONFormatter{ - TimestampFormat: time.RFC3339Nano, - }) - logs := logrus.New() - logs.SetFormatter(&logrus.JSONFormatter{ - TimestampFormat: time.RFC3339Nano, - }) - logLevel := new(slog.LevelVar) logLevel.Set(slog.LevelInfo) log := slog.New(slog.NewJSONHandler(os.Stdout, &slog.HandlerOptions{ @@ -239,9 +230,6 @@ func main() { fatalOnError(err, log) if cfg.LogLevel != "" { - l, _ := logrus.ParseLevel(cfg.LogLevel) - logs.SetLevel(l) - logLevel.Set(cfg.getLogLevel()) } @@ -344,7 +332,7 @@ func main() { // create server router := mux.NewRouter() - createAPI(router, servicesConfig, inputFactory, &cfg, db, provisionQueue, deprovisionQueue, updateQueue, logger, logs, log, + createAPI(router, servicesConfig, inputFactory, &cfg, db, provisionQueue, deprovisionQueue, updateQueue, logger, log, inputFactory.GetPlanDefaults, kcBuilder, skrK8sClientProvider, skrK8sClientProvider, gardenerClient, kcpK8sClient, eventBroker) // create metrics endpoint @@ -423,28 +411,28 @@ func logConfiguration(logs *slog.Logger, cfg Config) { } func createAPI(router *mux.Router, servicesConfig broker.ServicesConfig, planValidator broker.PlanValidator, cfg *Config, db storage.BrokerStorage, - provisionQueue, deprovisionQueue, updateQueue *process.Queue, logger lager.Logger, logs logrus.FieldLogger, log *slog.Logger, planDefaults broker.PlanDefaults, kcBuilder kubeconfig.KcBuilder, clientProvider K8sClientProvider, kubeconfigProvider KubeconfigProvider, gardenerClient, kcpK8sClient client.Client, publisher event.Publisher) { + provisionQueue, deprovisionQueue, updateQueue *process.Queue, logger lager.Logger, logs *slog.Logger, planDefaults broker.PlanDefaults, kcBuilder kubeconfig.KcBuilder, clientProvider K8sClientProvider, kubeconfigProvider KubeconfigProvider, gardenerClient, kcpK8sClient client.Client, publisher event.Publisher) { suspensionCtxHandler := suspension.NewContextUpdateHandler(db.Operations(), provisionQueue, deprovisionQueue, logs) defaultPlansConfig, err := servicesConfig.DefaultPlansConfig() - fatalOnError(err, log) + fatalOnError(err, logs) debugSink, err := lager.NewRedactingSink(lager.NewWriterSink(os.Stdout, lager.DEBUG), []string{"instance-details"}, []string{}) - fatalOnError(err, log) + fatalOnError(err, logs) logger.RegisterSink(debugSink) errorSink, err := lager.NewRedactingSink(lager.NewWriterSink(os.Stderr, lager.ERROR), []string{"instance-details"}, []string{}) - fatalOnError(err, log) + fatalOnError(err, logs) logger.RegisterSink(errorSink) freemiumGlobalAccountIds, err := whitelist.ReadWhitelistedGlobalAccountIdsFromFile(cfg.FreemiumWhitelistedGlobalAccountsFilePath) - fatalOnError(err, log) - logs.Infof("Number of globalAccountIds for unlimited freeemium: %d\n", len(freemiumGlobalAccountIds)) + fatalOnError(err, logs) + logs.Info(fmt.Sprintf("Number of globalAccountIds for unlimited freemium: %d", len(freemiumGlobalAccountIds))) // backward compatibility for tests convergedCloudRegionProvider, err := broker.NewDefaultConvergedCloudRegionsProvider(cfg.SapConvergedCloudRegionMappingsFilePath, &broker.YamlRegionReader{}) - fatalOnError(err, log) - logs.Infof("%s plan region mappings loaded", broker.SapConvergedCloudPlanName) + fatalOnError(err, logs) + logs.Info(fmt.Sprintf("%s plan region mappings loaded", broker.SapConvergedCloudPlanName)) // create KymaEnvironmentBroker endpoints kymaEnvBroker := &broker.KymaEnvironmentBroker{ diff --git a/internal/appinfo/runtime_info_test.go b/internal/appinfo/runtime_info_test.go index f048fac06a..47c189ce35 100644 --- a/internal/appinfo/runtime_info_test.go +++ b/internal/appinfo/runtime_info_test.go @@ -3,8 +3,10 @@ package appinfo_test import ( "encoding/json" "fmt" + "log/slog" "net/http" "net/http/httptest" + "os" "testing" "time" @@ -15,7 +17,6 @@ import ( "github.com/kyma-project/kyma-environment-broker/internal/broker" "github.com/kyma-project/kyma-environment-broker/internal/fixture" "github.com/kyma-project/kyma-environment-broker/internal/httputil" - "github.com/kyma-project/kyma-environment-broker/internal/logger" "github.com/kyma-project/kyma-environment-broker/internal/storage" "github.com/kyma-project/kyma-environment-broker/internal/storage/driver/memory" "github.com/pivotal-cf/brokerapi/v8/domain" @@ -92,7 +93,7 @@ func TestRuntimeInfoHandlerSuccess(t *testing.T) { var ( fixReq = httptest.NewRequest("GET", "http://example.com/foo", nil) respSpy = httptest.NewRecorder() - writer = httputil.NewResponseWriter(logger.NewLogDummy(), true) + writer = httputil.NewResponseWriter(fixLogger(), true) memStorage = newInMemoryStorage(t, tc.instances, tc.provisionOp, tc.deprovisionOp) ) @@ -115,7 +116,7 @@ func TestRuntimeInfoHandlerFailures(t *testing.T) { var ( fixReq = httptest.NewRequest("GET", "http://example.com/foo", nil) respSpy = httptest.NewRecorder() - writer = httputil.NewResponseWriter(logger.NewLogDummy(), true) + writer = httputil.NewResponseWriter(fixLogger(), true) expBody = `{ "status": 500, "requestId": "", @@ -222,7 +223,7 @@ func TestRuntimeInfoHandlerOperationRecognition(t *testing.T) { req, err := http.NewRequest("GET", "/info/runtimes", nil) require.NoError(t, err) - responseWriter := httputil.NewResponseWriter(logger.NewLogDummy(), true) + responseWriter := httputil.NewResponseWriter(fixLogger(), true) runtimesInfoHandler := appinfo.NewRuntimeInfoHandler(instances, operations, broker.PlansConfig{}, "", responseWriter) rr := httptest.NewRecorder() @@ -332,7 +333,7 @@ func TestRuntimeInfoHandlerOperationRecognition(t *testing.T) { req, err := http.NewRequest("GET", "/info/runtimes", nil) require.NoError(t, err) - responseWriter := httputil.NewResponseWriter(logger.NewLogDummy(), true) + responseWriter := httputil.NewResponseWriter(fixLogger(), true) runtimesInfoHandler := appinfo.NewRuntimeInfoHandler(instances, operations, broker.PlansConfig{}, "", responseWriter) rr := httptest.NewRecorder() @@ -471,7 +472,7 @@ func TestRuntimeInfoHandlerOperationRecognition(t *testing.T) { req, err := http.NewRequest("GET", "/info/runtimes", nil) require.NoError(t, err) - responseWriter := httputil.NewResponseWriter(logger.NewLogDummy(), true) + responseWriter := httputil.NewResponseWriter(fixLogger(), true) runtimesInfoHandler := appinfo.NewRuntimeInfoHandler(instances, operations, broker.PlansConfig{}, "", responseWriter) rr := httptest.NewRecorder() @@ -576,3 +577,9 @@ func fixSucceededOperation(operationType internal.OperationType, idx int) intern Type: operationType, } } + +func fixLogger() *slog.Logger { + return slog.New(slog.NewTextHandler(os.Stdout, &slog.HandlerOptions{ + Level: slog.LevelInfo, + })) +} diff --git a/internal/broker/bind_create.go b/internal/broker/bind_create.go index d395b327d2..5ca9a10fb0 100644 --- a/internal/broker/bind_create.go +++ b/internal/broker/bind_create.go @@ -5,6 +5,7 @@ import ( "encoding/json" "errors" "fmt" + "log/slog" "net/http" "strings" "time" @@ -18,7 +19,6 @@ import ( "github.com/pivotal-cf/brokerapi/v8/domain/apiresponses" "github.com/pivotal-cf/brokerapi/v8/domain" - "github.com/sirupsen/logrus" ) const ( @@ -44,7 +44,7 @@ type BindEndpoint struct { serviceAccountBindingManager broker.BindingsManager publisher event.Publisher - log logrus.FieldLogger + log *slog.Logger } type BindingContext struct { @@ -69,14 +69,14 @@ type Credentials struct { Kubeconfig string `json:"kubeconfig"` } -func NewBind(cfg BindingConfig, db storage.BrokerStorage, log logrus.FieldLogger, clientProvider broker.ClientProvider, kubeconfigProvider broker.KubeconfigProvider, +func NewBind(cfg BindingConfig, db storage.BrokerStorage, log *slog.Logger, clientProvider broker.ClientProvider, kubeconfigProvider broker.KubeconfigProvider, publisher event.Publisher) *BindEndpoint { return &BindEndpoint{config: cfg, instancesStorage: db.Instances(), bindingsStorage: db.Bindings(), publisher: publisher, operationsStorage: db.Operations(), - log: log.WithField("service", "BindEndpoint"), + log: log.With("service", "BindEndpoint"), serviceAccountBindingManager: broker.NewServiceAccountBindingsManager(clientProvider, kubeconfigProvider), } } @@ -95,10 +95,10 @@ func (b *BindEndpoint) Bind(ctx context.Context, instanceID, bindingID string, d } func (b *BindEndpoint) bind(ctx context.Context, instanceID, bindingID string, details domain.BindDetails, asyncAllowed bool) (domain.Binding, error) { - b.log.Infof("Bind instanceID: %s", instanceID) - b.log.Infof("Bind parameters: %s", string(details.RawParameters)) - b.log.Infof("Bind context: %s", string(details.RawContext)) - b.log.Infof("Bind asyncAllowed: %v", asyncAllowed) + b.log.Info(fmt.Sprintf("Bind instanceID: %s", instanceID)) + b.log.Info(fmt.Sprintf("Bind parameters: %s", string(details.RawParameters))) + b.log.Info(fmt.Sprintf("Bind context: %s", string(details.RawContext))) + b.log.Info(fmt.Sprintf("Bind asyncAllowed: %v", asyncAllowed)) if !b.config.Enabled { return domain.Binding{}, fmt.Errorf("not supported") @@ -203,7 +203,7 @@ func (b *BindEndpoint) bind(ctx context.Context, instanceID, bindingID string, d bindingCount := len(bindingList) message := fmt.Sprintf("reaching the maximum (%d) number of non expired bindings for instance %s", b.config.MaxBindingsCount, instanceID) if bindingCount == b.config.MaxBindingsCount-1 { - b.log.Infof(message) + b.log.Info(message) } if bindingCount >= b.config.MaxBindingsCount { expiredCount := 0 @@ -213,11 +213,11 @@ func (b *BindEndpoint) bind(ctx context.Context, instanceID, bindingID string, d } } if (bindingCount - expiredCount) == (b.config.MaxBindingsCount - 1) { - b.log.Infof(message) + b.log.Info(message) } if (bindingCount - expiredCount) >= b.config.MaxBindingsCount { message := fmt.Sprintf("maximum number of non expired bindings reached: %d", b.config.MaxBindingsCount) - b.log.Infof(message+" for instance %s", instanceID) + b.log.Info(fmt.Sprintf(message+" for instance %s", instanceID)) return domain.Binding{}, apiresponses.NewFailureResponse(fmt.Errorf(message), http.StatusBadRequest, message) } } @@ -250,7 +250,7 @@ func (b *BindEndpoint) bind(ctx context.Context, instanceID, bindingID string, d kubeconfig, expiresAt, err = b.serviceAccountBindingManager.Create(ctx, instance, bindingID, expirationSeconds) if err != nil { message := fmt.Sprintf("failed to create a Kyma binding using service account's kubeconfig: %s", err) - b.log.Errorf("for instance %s %s", instanceID, message) + b.log.Error(fmt.Sprintf("for instance %s %s", instanceID, message)) return domain.Binding{}, apiresponses.NewFailureResponse(fmt.Errorf(message), http.StatusBadRequest, message) } @@ -262,7 +262,7 @@ func (b *BindEndpoint) bind(ctx context.Context, instanceID, bindingID string, d message := fmt.Sprintf("failed to update Kyma binding in storage: %s", err) return domain.Binding{}, apiresponses.NewFailureResponse(fmt.Errorf(message), http.StatusInternalServerError, message) } - b.log.Infof("Successfully created binding %s for instance %s", bindingID, instanceID) + b.log.Info(fmt.Sprintf("Successfully created binding %s for instance %s", bindingID, instanceID)) b.publisher.Publish(context.Background(), BindingCreated{PlanID: instance.ServicePlanID}) return domain.Binding{ diff --git a/internal/broker/bind_create_test.go b/internal/broker/bind_create_test.go index a69495f616..4350bd502b 100644 --- a/internal/broker/bind_create_test.go +++ b/internal/broker/bind_create_test.go @@ -20,7 +20,6 @@ import ( "github.com/kyma-project/kyma-environment-broker/internal/storage" "github.com/kyma-project/kyma-environment-broker/internal/storage/dberr" "github.com/pivotal-cf/brokerapi/v8/domain" - "github.com/sirupsen/logrus" "github.com/stretchr/testify/require" ) @@ -49,14 +48,8 @@ func TestCreateBindingEndpoint(t *testing.T) { Level: slog.LevelDebug, })) - logs := logrus.New() - logs.SetLevel(logrus.DebugLevel) - logs.SetFormatter(&logrus.JSONFormatter{ - TimestampFormat: time.RFC3339Nano, - }) - brokerLogger := lager.NewLogger("test") - brokerLogger.RegisterSink(lager.NewWriterSink(logs.Writer(), lager.DEBUG)) + brokerLogger.RegisterSink(lager.NewWriterSink(os.Stdout, lager.DEBUG)) //// schema @@ -83,7 +76,7 @@ func TestCreateBindingEndpoint(t *testing.T) { publisher := event.NewPubSub(log) //// api handler - bindEndpoint := NewBind(*bindingCfg, db, logs, &provider{}, &provider{}, publisher) + bindEndpoint := NewBind(*bindingCfg, db, fixLogger(), &provider{}, &provider{}, publisher) // test relies on checking if got nil on kubeconfig provider but the instance got inserted either way t.Run("should INSERT binding despite error on k8s api call", func(t *testing.T) { @@ -194,7 +187,7 @@ func TestCreateSecondBindingWithTheSameIdButDifferentParams(t *testing.T) { publisher := event.NewPubSub(log) - svc := NewBind(*bindingCfg, brokerStorage, logrus.New(), nil, nil, publisher) + svc := NewBind(*bindingCfg, brokerStorage, fixLogger(), nil, nil, publisher) params := BindingParams{ ExpirationSeconds: 601, } @@ -245,7 +238,7 @@ func TestCreateSecondBindingWithTheSameIdAndParams(t *testing.T) { publisher := event.NewPubSub(log) - svc := NewBind(*bindingCfg, brokerStorage, logrus.New(), nil, nil, publisher) + svc := NewBind(*bindingCfg, brokerStorage, fixLogger(), nil, nil, publisher) params := BindingParams{ ExpirationSeconds: 600, } @@ -297,7 +290,7 @@ func TestCreateSecondBindingWithTheSameIdAndParamsForExpired(t *testing.T) { // event publisher publisher := event.NewPubSub(log) - svc := NewBind(*bindingCfg, brokerStorage, logrus.New(), nil, nil, publisher) + svc := NewBind(*bindingCfg, brokerStorage, fixLogger(), nil, nil, publisher) params := BindingParams{ ExpirationSeconds: 600, } @@ -351,7 +344,7 @@ func TestCreateSecondBindingWithTheSameIdAndParamsForBindingInProgress(t *testin // event publisher publisher := event.NewPubSub(log) - svc := NewBind(*bindingCfg, brokerStorage, logrus.New(), nil, nil, publisher) + svc := NewBind(*bindingCfg, brokerStorage, fixLogger(), nil, nil, publisher) params := BindingParams{ ExpirationSeconds: 600, } @@ -402,7 +395,7 @@ func TestCreateSecondBindingWithTheSameIdAndParamsNotExplicitlyDefined(t *testin publisher := event.NewPubSub(log) - svc := NewBind(*bindingCfg, brokerStorage, logrus.New(), nil, nil, publisher) + svc := NewBind(*bindingCfg, brokerStorage, fixLogger(), nil, nil, publisher) // when resp, err := svc.Bind(context.Background(), instanceID, bindingID, domain.BindDetails{}, false) diff --git a/internal/broker/bind_delete.go b/internal/broker/bind_delete.go index c3432b2167..0aa6ae2be3 100644 --- a/internal/broker/bind_delete.go +++ b/internal/broker/bind_delete.go @@ -3,6 +3,7 @@ package broker import ( "context" "fmt" + "log/slog" "net/http" "time" @@ -14,11 +15,10 @@ import ( "github.com/kyma-project/kyma-environment-broker/internal/storage/dberr" "github.com/pivotal-cf/brokerapi/v8/domain" "github.com/pivotal-cf/brokerapi/v8/domain/apiresponses" - "github.com/sirupsen/logrus" ) type UnbindEndpoint struct { - log logrus.FieldLogger + log *slog.Logger bindingsStorage storage.Bindings instancesStorage storage.Instances operationsStorage storage.Operations @@ -26,8 +26,8 @@ type UnbindEndpoint struct { publisher event.Publisher } -func NewUnbind(log logrus.FieldLogger, db storage.BrokerStorage, bindingsManager broker.BindingsManager, publisher event.Publisher) *UnbindEndpoint { - return &UnbindEndpoint{log: log.WithField("service", "UnbindEndpoint"), +func NewUnbind(log *slog.Logger, db storage.BrokerStorage, bindingsManager broker.BindingsManager, publisher event.Publisher) *UnbindEndpoint { + return &UnbindEndpoint{log: log.With("service", "UnbindEndpoint"), bindingsStorage: db.Bindings(), instancesStorage: db.Instances(), bindingsManager: bindingsManager, @@ -50,16 +50,16 @@ func (b *UnbindEndpoint) Unbind(ctx context.Context, instanceID, bindingID strin func (b *UnbindEndpoint) unbind(ctx context.Context, instanceID, bindingID string, details domain.UnbindDetails, asyncAllowed bool) (domain.UnbindSpec, error) { - b.log.Infof("Unbind instanceID: %s", instanceID) - b.log.Infof("Unbind details: %+v", details) - b.log.Infof("Unbind asyncAllowed: %v", asyncAllowed) + b.log.Info(fmt.Sprintf("Unbind instanceID: %s", instanceID)) + b.log.Info(fmt.Sprintf("Unbind details: %+v", details)) + b.log.Info(fmt.Sprintf("Unbind asyncAllowed: %v", asyncAllowed)) instance, err := b.instancesStorage.GetByID(instanceID) switch { case dberr.IsNotFound(err): err = b.bindingsStorage.Delete(instanceID, bindingID) if err != nil { - b.log.Errorf("Unbind error during removal of db entity: %v", err) + b.log.Error(fmt.Sprintf("Unbind error during removal of db entity: %v", err)) return domain.UnbindSpec{}, apiresponses.NewFailureResponse(fmt.Errorf("failed to delete binding for binding %s and not existing instance %s: %v", bindingID, instanceID, err), http.StatusInternalServerError, fmt.Sprintf("failed to delete resources for binding %s and not existing instance %s: %v", bindingID, instanceID, err)) } return domain.UnbindSpec{}, apiresponses.ErrInstanceDoesNotExist @@ -83,17 +83,17 @@ func (b *UnbindEndpoint) unbind(ctx context.Context, instanceID, bindingID strin if lastOperation.Type != internal.OperationTypeDeprovision { err = b.bindingsManager.Delete(ctx, instance, bindingID) if err != nil { - b.log.Errorf("Unbind error during removal of service account resources: %s", err) + b.log.Error(fmt.Sprintf("Unbind error during removal of service account resources: %s", err)) return domain.UnbindSpec{}, apiresponses.NewFailureResponse(fmt.Errorf("failed to delete binding resources for binding %s and instance %s: %v", bindingID, instanceID, err), http.StatusInternalServerError, fmt.Sprintf("failed to delete resources for binding %s and instance %s: %v", bindingID, instanceID, err)) } } err = b.bindingsStorage.Delete(instanceID, bindingID) if err != nil { - b.log.Errorf("Unbind error during removal of db entity: %v", err) + b.log.Error(fmt.Sprintf("Unbind error during removal of db entity: %v", err)) return domain.UnbindSpec{}, apiresponses.NewFailureResponse(fmt.Errorf("failed to delete binding resources for binding %s and instance %s: %v", bindingID, instanceID, err), http.StatusInternalServerError, fmt.Sprintf("failed to delete resources for binding %s and instance %s: %v", bindingID, instanceID, err)) } - b.log.Infof("Successfully removed binding %s for instance %s", bindingID, instanceID) + b.log.Info(fmt.Sprintf("Successfully removed binding %s for instance %s", bindingID, instanceID)) return domain.UnbindSpec{ IsAsync: false, diff --git a/internal/broker/bind_get.go b/internal/broker/bind_get.go index 73bb23c092..56c9442ac9 100644 --- a/internal/broker/bind_get.go +++ b/internal/broker/bind_get.go @@ -3,6 +3,7 @@ package broker import ( "context" "fmt" + "log/slog" "net/http" "time" @@ -11,25 +12,24 @@ import ( "github.com/kyma-project/kyma-environment-broker/internal/storage" "github.com/pivotal-cf/brokerapi/v8/domain" "github.com/pivotal-cf/brokerapi/v8/domain/apiresponses" - "github.com/sirupsen/logrus" ) type GetBindingEndpoint struct { - log logrus.FieldLogger + log *slog.Logger bindings storage.Bindings operations storage.Operations } -func NewGetBinding(log logrus.FieldLogger, db storage.BrokerStorage) *GetBindingEndpoint { - return &GetBindingEndpoint{log: log.WithField("service", "GetBindingEndpoint"), bindings: db.Bindings(), operations: db.Operations()} +func NewGetBinding(log *slog.Logger, db storage.BrokerStorage) *GetBindingEndpoint { + return &GetBindingEndpoint{log: log.With("service", "GetBindingEndpoint"), bindings: db.Bindings(), operations: db.Operations()} } // GetBinding fetches an existing service binding // // GET /v2/service_instances/{instance_id}/service_bindings/{binding_id} func (b *GetBindingEndpoint) GetBinding(_ context.Context, instanceID, bindingID string, _ domain.FetchBindingDetails) (domain.GetBindingSpec, error) { - b.log.Infof("GetBinding instanceID: %s", instanceID) - b.log.Infof("GetBinding bindingID: %s", bindingID) + b.log.Info(fmt.Sprintf("GetBinding instanceID: %s", instanceID)) + b.log.Info(fmt.Sprintf("GetBinding bindingID: %s", bindingID)) lastOperation, err := b.operations.GetLastOperation(instanceID) if err != nil { @@ -48,7 +48,7 @@ func (b *GetBindingEndpoint) GetBinding(_ context.Context, instanceID, bindingID } if binding.ExpiresAt.Before(time.Now()) { - b.log.Infof("GetBinding was called for expired binding %s for instance %s", bindingID, instanceID) + b.log.Info(fmt.Sprintf("GetBinding was called for expired binding %s for instance %s", bindingID, instanceID)) message := "Binding expired" return domain.GetBindingSpec{}, apiresponses.NewFailureResponse(fmt.Errorf(message), http.StatusNotFound, message) } @@ -59,7 +59,7 @@ func (b *GetBindingEndpoint) GetBinding(_ context.Context, instanceID, bindingID } if err != nil { - b.log.Errorf("GetBinding error: %s", err) + b.log.Error(fmt.Sprintf("GetBinding error: %s", err)) message := fmt.Sprintf("Unexpected error: %s", err) return domain.GetBindingSpec{}, apiresponses.NewFailureResponse(fmt.Errorf(message), http.StatusInternalServerError, message) } diff --git a/internal/broker/bind_get_test.go b/internal/broker/bind_get_test.go index f4766acad9..928590e490 100644 --- a/internal/broker/bind_get_test.go +++ b/internal/broker/bind_get_test.go @@ -12,7 +12,6 @@ import ( "github.com/kyma-project/kyma-environment-broker/internal/storage/driver/memory" "github.com/pivotal-cf/brokerapi/v8/domain" "github.com/pivotal-cf/brokerapi/v8/domain/apiresponses" - "github.com/sirupsen/logrus" "github.com/stretchr/testify/require" ) @@ -37,7 +36,7 @@ func TestGetBinding(t *testing.T) { endpoint := &GetBindingEndpoint{ bindings: bindingsMemory, operations: operationsMemory, - log: &logrus.Logger{}, + log: fixLogger(), } // when @@ -72,7 +71,7 @@ func TestGetBinding(t *testing.T) { endpoint := &GetBindingEndpoint{ bindings: bindingsMemory, operations: operationsMemory, - log: &logrus.Logger{}, + log: fixLogger(), } // when @@ -107,7 +106,7 @@ func TestGetBinding(t *testing.T) { endpoint := &GetBindingEndpoint{ bindings: bindingsMemory, operations: operationsMemory, - log: &logrus.Logger{}, + log: fixLogger(), } // when diff --git a/internal/broker/bind_last_operation.go b/internal/broker/bind_last_operation.go index 726ab3b8de..d873a0225a 100644 --- a/internal/broker/bind_last_operation.go +++ b/internal/broker/bind_last_operation.go @@ -3,26 +3,26 @@ package broker import ( "context" "fmt" + "log/slog" "github.com/pivotal-cf/brokerapi/v8/domain" - "github.com/sirupsen/logrus" ) type LastBindingOperationEndpoint struct { - log logrus.FieldLogger + log *slog.Logger } -func NewLastBindingOperation(log logrus.FieldLogger) *LastBindingOperationEndpoint { - return &LastBindingOperationEndpoint{log: log.WithField("service", "LastBindingOperationEndpoint")} +func NewLastBindingOperation(log *slog.Logger) *LastBindingOperationEndpoint { + return &LastBindingOperationEndpoint{log: log.With("service", "LastBindingOperationEndpoint")} } // LastBindingOperation fetches last operation state for a service binding // // GET /v2/service_instances/{instance_id}/service_bindings/{binding_id}/last_operation func (b *LastBindingOperationEndpoint) LastBindingOperation(ctx context.Context, instanceID, bindingID string, details domain.PollDetails) (domain.LastOperation, error) { - b.log.Infof("LastBindingOperation instanceID: %s", instanceID) - b.log.Infof("LastBindingOperation bindingID: %s", bindingID) - b.log.Infof("LastBindingOperation details: %+v", details) + b.log.Info(fmt.Sprintf("LastBindingOperation instanceID: %s", instanceID)) + b.log.Info(fmt.Sprintf("LastBindingOperation bindingID: %s", bindingID)) + b.log.Info(fmt.Sprintf("LastBindingOperation details: %+v", details)) return domain.LastOperation{}, fmt.Errorf("not supported") } diff --git a/internal/broker/instance_create.go b/internal/broker/instance_create.go index fbd27977f3..ec384e4cf5 100644 --- a/internal/broker/instance_create.go +++ b/internal/broker/instance_create.go @@ -5,6 +5,7 @@ import ( "encoding/base64" "encoding/json" "fmt" + "log/slog" "net" "net/http" "net/netip" @@ -27,10 +28,6 @@ import ( "github.com/google/uuid" "github.com/kyma-incubator/compass/components/director/pkg/jsonschema" - "github.com/pivotal-cf/brokerapi/v8/domain" - "github.com/pivotal-cf/brokerapi/v8/domain/apiresponses" - "github.com/sirupsen/logrus" - "github.com/kyma-project/kyma-environment-broker/common/gardener" pkg "github.com/kyma-project/kyma-environment-broker/common/runtime" "github.com/kyma-project/kyma-environment-broker/internal" @@ -39,6 +36,8 @@ import ( "github.com/kyma-project/kyma-environment-broker/internal/ptr" "github.com/kyma-project/kyma-environment-broker/internal/storage" "github.com/kyma-project/kyma-environment-broker/internal/storage/dberr" + "github.com/pivotal-cf/brokerapi/v8/domain" + "github.com/pivotal-cf/brokerapi/v8/domain/apiresponses" ) //go:generate mockery --name=Queue --output=automock --outpkg=automock --case=underscore @@ -76,7 +75,7 @@ type ProvisionEndpoint struct { convergedCloudRegionsProvider ConvergedCloudRegionProvider - log logrus.FieldLogger + log *slog.Logger } const ( @@ -92,7 +91,7 @@ func NewProvision(cfg Config, builderFactory PlanValidator, plansConfig PlansConfig, planDefaults PlanDefaults, - log logrus.FieldLogger, + log *slog.Logger, dashboardConfig dashboard.Config, kcBuilder kubeconfig.KcBuilder, freemiumWhitelist whitelist.Set, @@ -111,7 +110,7 @@ func NewProvision(cfg Config, instanceArchivedStorage: instanceArchivedStorage, queue: queue, builderFactory: builderFactory, - log: log.WithField("service", "ProvisionEndpoint"), + log: log.With("service", "ProvisionEndpoint"), enabledPlanIDs: enabledPlanIDs, plansConfig: plansConfig, shootDomain: gardenerConfig.ShootDomain, @@ -130,8 +129,8 @@ func NewProvision(cfg Config, // PUT /v2/service_instances/{instance_id} func (b *ProvisionEndpoint) Provision(ctx context.Context, instanceID string, details domain.ProvisionDetails, asyncAllowed bool) (domain.ProvisionedServiceSpec, error) { operationID := uuid.New().String() - logger := b.log.WithFields(logrus.Fields{"instanceID": instanceID, "operationID": operationID, "planID": details.PlanID}) - logger.Infof("Provision called with context: %s", marshallRawContext(hideSensitiveDataFromRawContext(details.RawContext))) + logger := b.log.With("instanceID", instanceID, "operationID", operationID, "planID", details.PlanID) + logger.Info(fmt.Sprintf("Provision called with context: %s", marshallRawContext(hideSensitiveDataFromRawContext(details.RawContext)))) region, found := middleware.RegionFromContext(ctx) if !found { @@ -165,16 +164,16 @@ func (b *ProvisionEndpoint) Provision(ctx context.Context, instanceID string, de PlatformProvider: platformProvider, } - logger.Infof("Starting provisioning runtime: Name=%s, GlobalAccountID=%s, SubAccountID=%s PlatformRegion=%s, ProvisioningParameterts.Region=%s, ShootAndSeedSameRegion=%t, ProvisioningParameterts.MachineType=%s", + logger.Info(fmt.Sprintf("Starting provisioning runtime: Name=%s, GlobalAccountID=%s, SubAccountID=%s, PlatformRegion=%s, ProvisioningParameterts.Region=%s, ShootAndSeedSameRegion=%t, ProvisioningParameterts.MachineType=%s", parameters.Name, ersContext.GlobalAccountID, ersContext.SubAccountID, region, valueOfPtr(parameters.Region), - valueOfBoolPtr(parameters.ShootAndSeedSameRegion), valueOfPtr(parameters.MachineType)) + valueOfBoolPtr(parameters.ShootAndSeedSameRegion), valueOfPtr(parameters.MachineType))) logParametersWithMaskedKubeconfig(parameters, logger) // check if operation with instance ID already created existingOperation, errStorage := b.operationsStorage.GetProvisioningOperationByInstanceID(instanceID) switch { case errStorage != nil && !dberr.IsNotFound(errStorage): - logger.Errorf("cannot get existing operation from storage %s", errStorage) + logger.Error(fmt.Sprintf("cannot get existing operation from storage %s", errStorage)) return domain.ProvisionedServiceSpec{}, fmt.Errorf("cannot get existing operation from storage") case existingOperation != nil && !dberr.IsNotFound(errStorage): return b.handleExistingOperation(existingOperation, provisioningParameters) @@ -188,7 +187,7 @@ func (b *ProvisionEndpoint) Provision(ctx context.Context, instanceID string, de // create and save new operation operation, err := internal.NewProvisioningOperationWithID(operationID, instanceID, provisioningParameters) if err != nil { - logger.Errorf("cannot create new operation: %s", err) + logger.Error(fmt.Sprintf("cannot create new operation: %s", err)) return domain.ProvisionedServiceSpec{}, fmt.Errorf("cannot create new operation") } @@ -201,11 +200,11 @@ func (b *ProvisionEndpoint) Provision(ctx context.Context, instanceID string, de operation.ShootName = provisioningParameters.Parameters.ShootName operation.ShootDomain = provisioningParameters.Parameters.ShootDomain } - logger.Infof("Runtime ShootDomain: %s", operation.ShootDomain) + logger.Info(fmt.Sprintf("Runtime ShootDomain: %s", operation.ShootDomain)) err = b.operationsStorage.InsertOperation(operation.Operation) if err != nil { - logger.Errorf("cannot save operation: %s", err) + logger.Error(fmt.Sprintf("cannot save operation: %s", err)) return domain.ProvisionedServiceSpec{}, fmt.Errorf("cannot save operation") } @@ -222,7 +221,7 @@ func (b *ProvisionEndpoint) Provision(ctx context.Context, instanceID string, de } err = b.instanceStorage.Insert(instance) if err != nil { - logger.Errorf("cannot save instance in storage: %s", err) + logger.Error(fmt.Sprintf("cannot save instance in storage: %s", err)) return domain.ProvisionedServiceSpec{}, fmt.Errorf("cannot save instance") } @@ -239,9 +238,9 @@ func (b *ProvisionEndpoint) Provision(ctx context.Context, instanceID string, de }, nil } -func logParametersWithMaskedKubeconfig(parameters pkg.ProvisioningParametersDTO, logger *logrus.Entry) { +func logParametersWithMaskedKubeconfig(parameters pkg.ProvisioningParametersDTO, logger *slog.Logger) { parameters.Kubeconfig = "*****" - logger.Infof("Runtime parameters: %+v", parameters) + logger.Info(fmt.Sprintf("Runtime parameters: %+v", parameters)) } func valueOfPtr(ptr *string) string { @@ -258,7 +257,7 @@ func valueOfBoolPtr(ptr *bool) bool { return *ptr } -func (b *ProvisionEndpoint) validateAndExtract(details domain.ProvisionDetails, provider pkg.CloudProvider, ctx context.Context, l logrus.FieldLogger) (internal.ERSContext, pkg.ProvisioningParametersDTO, error) { +func (b *ProvisionEndpoint) validateAndExtract(details domain.ProvisionDetails, provider pkg.CloudProvider, ctx context.Context, l *slog.Logger) (internal.ERSContext, pkg.ProvisioningParametersDTO, error) { var ersContext internal.ERSContext var parameters pkg.ProvisioningParametersDTO @@ -270,7 +269,7 @@ func (b *ProvisionEndpoint) validateAndExtract(details domain.ProvisionDetails, } ersContext, err := b.extractERSContext(details) - logger := l.WithField("globalAccountID", ersContext.GlobalAccountID) + logger := l.With("globalAccountID", ersContext.GlobalAccountID) if err != nil { return ersContext, parameters, fmt.Errorf("while extracting ers context: %w", err) } @@ -316,7 +315,7 @@ func (b *ProvisionEndpoint) validateAndExtract(details domain.ProvisionDetails, // EU Access if isEuRestrictedAccess(ctx) { - logger.Infof("EU Access restricted instance creation") + logger.Info("EU Access restricted instance creation") } parameters.LicenceType = b.determineLicenceType(details.PlanID) diff --git a/internal/broker/instance_create_input_params_test.go b/internal/broker/instance_create_input_params_test.go index 75e0347cd1..90d004b6fe 100644 --- a/internal/broker/instance_create_input_params_test.go +++ b/internal/broker/instance_create_input_params_test.go @@ -2,17 +2,21 @@ package broker import ( "encoding/json" + "log/slog" + "os" "testing" "github.com/kyma-project/kyma-environment-broker/common/gardener" "github.com/kyma-project/kyma-environment-broker/internal/dashboard" "github.com/pivotal-cf/brokerapi/v8/domain" - "github.com/sirupsen/logrus" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) func TestShootAndSeedSameRegion(t *testing.T) { + log := slog.New(slog.NewTextHandler(os.Stdout, &slog.HandlerOptions{ + Level: slog.LevelInfo, + })) t.Run("should parse shootAndSeedSameRegion - true", func(t *testing.T) { // given @@ -30,7 +34,7 @@ func TestShootAndSeedSameRegion(t *testing.T) { nil, nil, nil, - logrus.StandardLogger(), + log, dashboard.Config{}, nil, nil, @@ -61,7 +65,7 @@ func TestShootAndSeedSameRegion(t *testing.T) { nil, nil, nil, - logrus.StandardLogger(), + log, dashboard.Config{}, nil, nil, @@ -92,7 +96,7 @@ func TestShootAndSeedSameRegion(t *testing.T) { nil, nil, nil, - logrus.StandardLogger(), + log, dashboard.Config{}, nil, nil, diff --git a/internal/broker/instance_create_test.go b/internal/broker/instance_create_test.go index b33397a586..73c30fc85e 100644 --- a/internal/broker/instance_create_test.go +++ b/internal/broker/instance_create_test.go @@ -4,7 +4,9 @@ import ( "context" "encoding/json" "fmt" + "log/slog" "net/http" + "os" "testing" "github.com/kyma-project/kyma-environment-broker/internal/whitelist" @@ -24,7 +26,6 @@ import ( "github.com/kyma-project/kyma-environment-broker/internal/ptr" "github.com/kyma-project/kyma-environment-broker/internal/storage" "github.com/pivotal-cf/brokerapi/v8/domain" - "github.com/sirupsen/logrus" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/mock" "github.com/stretchr/testify/require" @@ -53,6 +54,9 @@ const ( var dashboardConfig = dashboard.Config{LandscapeURL: "https://dashboard.example.com"} func TestProvision_Provision(t *testing.T) { + log := slog.New(slog.NewTextHandler(os.Stdout, &slog.HandlerOptions{ + Level: slog.LevelInfo, + })) t.Run("new operation will be created", func(t *testing.T) { // given // #setup memory storage @@ -85,7 +89,7 @@ func TestProvision_Provision(t *testing.T) { factoryBuilder, broker.PlansConfig{}, planDefaults, - logrus.StandardLogger(), + log, dashboardConfig, kcBuilder, whitelist.Set{}, @@ -161,7 +165,7 @@ func TestProvision_Provision(t *testing.T) { factoryBuilder, broker.PlansConfig{}, planDefaults, - logrus.StandardLogger(), + log, dashboardConfig, kcBuilder, whitelist.Set{}, @@ -241,7 +245,7 @@ func TestProvision_Provision(t *testing.T) { factoryBuilder, broker.PlansConfig{}, planDefaults, - logrus.StandardLogger(), + log, dashboardConfig, kcBuilder, whitelist.Set{}, @@ -292,7 +296,7 @@ func TestProvision_Provision(t *testing.T) { factoryBuilder, broker.PlansConfig{}, planDefaults, - logrus.StandardLogger(), + log, dashboardConfig, kcBuilder, whitelist.Set{}, @@ -368,7 +372,7 @@ func TestProvision_Provision(t *testing.T) { factoryBuilder, broker.PlansConfig{}, planDefaults, - logrus.StandardLogger(), + log, dashboardConfig, kcBuilder, whitelist.Set{}, @@ -447,7 +451,7 @@ func TestProvision_Provision(t *testing.T) { factoryBuilder, broker.PlansConfig{}, planDefaults, - logrus.StandardLogger(), + log, dashboardConfig, kcBuilder, whitelist.Set{}, @@ -498,7 +502,7 @@ func TestProvision_Provision(t *testing.T) { factoryBuilder, broker.PlansConfig{}, planDefaults, - logrus.StandardLogger(), + log, dashboardConfig, kcBuilder, whitelist.Set{}, @@ -550,7 +554,7 @@ func TestProvision_Provision(t *testing.T) { factoryBuilder, broker.PlansConfig{}, planDefaults, - logrus.StandardLogger(), + log, dashboardConfig, kcBuilder, whitelist.Set{}, @@ -616,7 +620,7 @@ func TestProvision_Provision(t *testing.T) { factoryBuilder, broker.PlansConfig{}, planDefaults, - logrus.StandardLogger(), + log, dashboardConfig, kcBuilder, whitelist.Set{}, @@ -682,7 +686,7 @@ func TestProvision_Provision(t *testing.T) { factoryBuilder, broker.PlansConfig{}, planDefaults, - logrus.StandardLogger(), + log, dashboardConfig, kcBuilder, whitelist.Set{}, @@ -728,7 +732,7 @@ func TestProvision_Provision(t *testing.T) { factoryBuilder, broker.PlansConfig{}, planDefaults, - logrus.StandardLogger(), + log, dashboardConfig, kcBuilder, whitelist.Set{}, @@ -767,7 +771,7 @@ func TestProvision_Provision(t *testing.T) { factoryBuilder, broker.PlansConfig{}, planDefaults, - logrus.StandardLogger(), + log, dashboardConfig, kcBuilder, whitelist.Set{}, @@ -810,7 +814,7 @@ func TestProvision_Provision(t *testing.T) { factoryBuilder, broker.PlansConfig{}, planDefaults, - logrus.StandardLogger(), + log, dashboardConfig, kcBuilder, whitelist.Set{}, @@ -857,7 +861,7 @@ func TestProvision_Provision(t *testing.T) { factoryBuilder, broker.PlansConfig{}, planDefaults, - logrus.StandardLogger(), + log, dashboardConfig, kcBuilder, whitelist.Set{}, @@ -910,7 +914,7 @@ func TestProvision_Provision(t *testing.T) { factoryBuilder, broker.PlansConfig{}, planDefaults, - logrus.StandardLogger(), + log, dashboardConfig, kcBuilder, whitelist.Set{}, @@ -969,7 +973,7 @@ func TestProvision_Provision(t *testing.T) { factoryBuilder, broker.PlansConfig{}, planDefaults, - logrus.StandardLogger(), + log, dashboardConfig, kcBuilder, whitelist.Set{}, @@ -1028,7 +1032,7 @@ func TestProvision_Provision(t *testing.T) { factoryBuilder, broker.PlansConfig{}, planDefaults, - logrus.StandardLogger(), + log, dashboardConfig, kcBuilder, whitelist.Set{}, @@ -1088,7 +1092,7 @@ func TestProvision_Provision(t *testing.T) { factoryBuilder, broker.PlansConfig{}, planDefaults, - logrus.StandardLogger(), + log, dashboardConfig, kcBuilder, whitelist.Set{}, @@ -1134,7 +1138,7 @@ func TestProvision_Provision(t *testing.T) { factoryBuilder, broker.PlansConfig{}, planDefaults, - logrus.StandardLogger(), + log, dashboardConfig, kcBuilder, whitelist.Set{}, @@ -1202,7 +1206,7 @@ func TestProvision_Provision(t *testing.T) { factoryBuilder, broker.PlansConfig{}, planDefaults, - logrus.StandardLogger(), + log, dashboardConfig, kcBuilder, whitelist.Set{}, @@ -1260,7 +1264,7 @@ func TestProvision_Provision(t *testing.T) { factoryBuilder, broker.PlansConfig{}, planDefaults, - logrus.StandardLogger(), + log, dashboardConfig, kcBuilder, whitelist.Set{globalAccountID: struct{}{}}, @@ -1316,7 +1320,7 @@ func TestProvision_Provision(t *testing.T) { factoryBuilder, broker.PlansConfig{}, planDefaults, - logrus.StandardLogger(), + log, dashboardConfig, kcBuilder, whitelist.Set{}, @@ -1363,7 +1367,7 @@ func TestProvision_Provision(t *testing.T) { factoryBuilder, broker.PlansConfig{}, planDefaults, - logrus.StandardLogger(), + log, dashboardConfig, kcBuilder, whitelist.Set{}, @@ -1413,7 +1417,7 @@ func TestProvision_Provision(t *testing.T) { factoryBuilder, broker.PlansConfig{}, planDefaults, - logrus.StandardLogger(), + log, dashboardConfig, kcBuilder, whitelist.Set{}, @@ -1466,7 +1470,7 @@ func TestProvision_Provision(t *testing.T) { factoryBuilder, broker.PlansConfig{}, planDefaults, - logrus.StandardLogger(), + log, dashboardConfig, kcBuilder, whitelist.Set{}, @@ -1549,6 +1553,10 @@ func TestNetworkingValidation(t *testing.T) { // #setup memory storage memoryStorage := storage.NewMemoryStorage() + log := slog.New(slog.NewTextHandler(os.Stdout, &slog.HandlerOptions{ + Level: slog.LevelInfo, + })) + queue := &automock.Queue{} queue.On("Add", mock.AnythingOfType("string")) @@ -1570,7 +1578,7 @@ func TestNetworkingValidation(t *testing.T) { factoryBuilder, broker.PlansConfig{}, planDefaults, - logrus.StandardLogger(), + log, dashboardConfig, kcBuilder, whitelist.Set{}, @@ -1647,6 +1655,10 @@ func TestRegionValidation(t *testing.T) { // #setup memory storage memoryStorage := storage.NewMemoryStorage() + log := slog.New(slog.NewTextHandler(os.Stdout, &slog.HandlerOptions{ + Level: slog.LevelInfo, + })) + queue := &automock.Queue{} queue.On("Add", mock.AnythingOfType("string")) @@ -1668,7 +1680,7 @@ func TestRegionValidation(t *testing.T) { factoryBuilder, broker.PlansConfig{}, planDefaults, - logrus.StandardLogger(), + log, dashboardConfig, kcBuilder, whitelist.Set{}, @@ -1698,6 +1710,9 @@ func TestRegionValidation(t *testing.T) { } func TestSapConvergedCloudBlocking(t *testing.T) { + log := slog.New(slog.NewTextHandler(os.Stdout, &slog.HandlerOptions{ + Level: slog.LevelInfo, + })) t.Run("Should succeed if converged cloud is enabled and converged plan selected", func(t *testing.T) { // given memoryStorage := storage.NewMemoryStorage() @@ -1728,7 +1743,7 @@ func TestSapConvergedCloudBlocking(t *testing.T) { factoryBuilder, broker.PlansConfig{}, planDefaults, - logrus.StandardLogger(), + log, dashboardConfig, kcBuilder, whitelist.Set{}, @@ -1779,7 +1794,7 @@ func TestSapConvergedCloudBlocking(t *testing.T) { factoryBuilder, broker.PlansConfig{}, planDefaults, - logrus.StandardLogger(), + log, dashboardConfig, kcBuilder, whitelist.Set{}, @@ -1829,7 +1844,7 @@ func TestSapConvergedCloudBlocking(t *testing.T) { factoryBuilder, broker.PlansConfig{}, planDefaults, - logrus.StandardLogger(), + log, dashboardConfig, kcBuilder, whitelist.Set{}, diff --git a/internal/broker/instance_deprovision.go b/internal/broker/instance_deprovision.go index 8c1dbdf990..6e5254f4d3 100644 --- a/internal/broker/instance_deprovision.go +++ b/internal/broker/instance_deprovision.go @@ -3,6 +3,7 @@ package broker import ( "context" "fmt" + "log/slog" "net/http" "github.com/kyma-project/kyma-environment-broker/internal" @@ -13,11 +14,10 @@ import ( "github.com/kyma-project/kyma-environment-broker/internal/storage/dberr" "github.com/pivotal-cf/brokerapi/v8/domain" "github.com/pivotal-cf/brokerapi/v8/domain/apiresponses" - "github.com/sirupsen/logrus" ) type DeprovisionEndpoint struct { - log logrus.FieldLogger + log *slog.Logger instancesStorage storage.Instances operationsStorage storage.Deprovisioning @@ -25,9 +25,9 @@ type DeprovisionEndpoint struct { queue Queue } -func NewDeprovision(instancesStorage storage.Instances, operationsStorage storage.Operations, q Queue, log logrus.FieldLogger) *DeprovisionEndpoint { +func NewDeprovision(instancesStorage storage.Instances, operationsStorage storage.Operations, q Queue, log *slog.Logger) *DeprovisionEndpoint { return &DeprovisionEndpoint{ - log: log.WithField("service", "DeprovisionEndpoint"), + log: log.With("service", "DeprovisionEndpoint"), instancesStorage: instancesStorage, operationsStorage: operationsStorage, @@ -39,8 +39,8 @@ func NewDeprovision(instancesStorage storage.Instances, operationsStorage storag // // DELETE /v2/service_instances/{instance_id} func (b *DeprovisionEndpoint) Deprovision(ctx context.Context, instanceID string, details domain.DeprovisionDetails, asyncAllowed bool) (domain.DeprovisionServiceSpec, error) { - logger := b.log.WithFields(logrus.Fields{"instanceID": instanceID}) - logger.Infof("Deprovisioning triggered, details: %+v", details) + logger := b.log.With("instanceID", instanceID) + logger.Info(fmt.Sprintf("Deprovisioning triggered, details: %+v", details)) instance, err := b.instancesStorage.GetByID(instanceID) switch { @@ -51,16 +51,16 @@ func (b *DeprovisionEndpoint) Deprovision(ctx context.Context, instanceID string IsAsync: false, }, nil default: - logger.Errorf("unable to get instance from a storage: %s", err) + logger.Error(fmt.Sprintf("unable to get instance from storage: %s", err)) return domain.DeprovisionServiceSpec{}, apiresponses.NewFailureResponse(fmt.Errorf("unable to get instance from the storage"), http.StatusInternalServerError, fmt.Sprintf("could not deprovision runtime, instanceID %s", instanceID)) } - logger = logger.WithFields(logrus.Fields{"runtimeID": instance.RuntimeID, "globalAccountID": instance.GlobalAccountID, "planID": instance.ServicePlanID}) + logger = logger.With("runtimeID", instance.RuntimeID, "globalAccountID", instance.GlobalAccountID, "planID", instance.ServicePlanID) // check if operation with the same instance ID is already created existingOperation, errStorage := b.operationsStorage.GetDeprovisioningOperationByInstanceID(instanceID) if errStorage != nil && !dberr.IsNotFound(errStorage) { - logger.Errorf("cannot get existing operation from storage %s", errStorage) + logger.Error(fmt.Sprintf("cannot get existing operation from storage %s", errStorage)) return domain.DeprovisionServiceSpec{}, fmt.Errorf("cannot get existing operation from storage") } @@ -86,10 +86,10 @@ func (b *DeprovisionEndpoint) Deprovision(ctx context.Context, instanceID string // create and save new operation operationID := uuid.New().String() - logger = logger.WithField("operationID", operationID) + logger = logger.With("operationID", operationID) operation, err := internal.NewDeprovisioningOperationWithID(operationID, instance) if err != nil { - logger.Errorf("cannot create new operation: %s", err) + logger.Error(fmt.Sprintf("cannot create new operation: %s", err)) return domain.DeprovisionServiceSpec{}, fmt.Errorf("cannot create new operation") } if v := ctx.Value("User-Agent"); v != nil { @@ -97,7 +97,7 @@ func (b *DeprovisionEndpoint) Deprovision(ctx context.Context, instanceID string } err = b.operationsStorage.InsertDeprovisioningOperation(operation) if err != nil { - logger.Errorf("cannot save operation: %s", err) + logger.Error(fmt.Sprintf("cannot save operation: %s", err)) return domain.DeprovisionServiceSpec{}, fmt.Errorf("cannot save operation") } diff --git a/internal/broker/instance_deprovision_test.go b/internal/broker/instance_deprovision_test.go index 419682b10d..00c6c83ea2 100644 --- a/internal/broker/instance_deprovision_test.go +++ b/internal/broker/instance_deprovision_test.go @@ -2,6 +2,8 @@ package broker import ( "context" + "log/slog" + "os" "testing" "github.com/kyma-project/kyma-environment-broker/internal" @@ -9,7 +11,6 @@ import ( "github.com/kyma-project/kyma-environment-broker/internal/fixture" "github.com/kyma-project/kyma-environment-broker/internal/storage" "github.com/pivotal-cf/brokerapi/v8/domain" - "github.com/sirupsen/logrus" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/mock" "github.com/stretchr/testify/require" @@ -27,7 +28,7 @@ func TestDeprovisionEndpoint_DeprovisionNotExistingInstance(t *testing.T) { queue := &automock.Queue{} queue.On("Add", mock.AnythingOfType("string")) - svc := NewDeprovision(memoryStorage.Instances(), memoryStorage.Operations(), queue, logrus.StandardLogger()) + svc := NewDeprovision(memoryStorage.Instances(), memoryStorage.Operations(), queue, fixLogger()) // when _, err := svc.Deprovision(context.TODO(), "inst-0001", domain.DeprovisionDetails{}, true) @@ -45,7 +46,7 @@ func TestDeprovisionEndpoint_DeprovisionExistingInstance(t *testing.T) { queue := &automock.Queue{} queue.On("Add", mock.AnythingOfType("string")) - svc := NewDeprovision(memoryStorage.Instances(), memoryStorage.Operations(), queue, logrus.StandardLogger()) + svc := NewDeprovision(memoryStorage.Instances(), memoryStorage.Operations(), queue, fixLogger()) // when _, err = svc.Deprovision(context.TODO(), instanceID, domain.DeprovisionDetails{}, true) @@ -69,7 +70,7 @@ func TestDeprovisionEndpoint_DeprovisionExistingOperationInProgress(t *testing.T queue := &automock.Queue{} queue.On("Add", mock.AnythingOfType("string")) - svc := NewDeprovision(memoryStorage.Instances(), memoryStorage.Operations(), queue, logrus.StandardLogger()) + svc := NewDeprovision(memoryStorage.Instances(), memoryStorage.Operations(), queue, fixLogger()) // when res, err := svc.Deprovision(context.TODO(), instanceID, domain.DeprovisionDetails{}, true) @@ -96,7 +97,7 @@ func TestDeprovisionEndpoint_DeprovisionExistingOperationFailed(t *testing.T) { queue := &automock.Queue{} queue.On("Add", mock.Anything) - svc := NewDeprovision(memoryStorage.Instances(), memoryStorage.Operations(), queue, logrus.StandardLogger()) + svc := NewDeprovision(memoryStorage.Instances(), memoryStorage.Operations(), queue, fixLogger()) // when res, err := svc.Deprovision(context.TODO(), instanceID, domain.DeprovisionDetails{}, true) @@ -123,3 +124,9 @@ func fixInstance() internal.Instance { return instance } + +func fixLogger() *slog.Logger { + return slog.New(slog.NewTextHandler(os.Stdout, &slog.HandlerOptions{ + Level: slog.LevelInfo, + })) +} diff --git a/internal/broker/instance_get.go b/internal/broker/instance_get.go index 318e087dc7..7c8adf84c2 100644 --- a/internal/broker/instance_get.go +++ b/internal/broker/instance_get.go @@ -3,6 +3,7 @@ package broker import ( "context" "fmt" + "log/slog" "net/http" "strings" @@ -14,7 +15,6 @@ import ( "github.com/pivotal-cf/brokerapi/v8/domain" "github.com/pivotal-cf/brokerapi/v8/domain/apiresponses" - "github.com/sirupsen/logrus" ) const allSubaccountsIDs = "all" @@ -25,29 +25,29 @@ type GetInstanceEndpoint struct { operationsStorage storage.Provisioning brokerURL string kcBuilder kubeconfig.KcBuilder - log logrus.FieldLogger + log *slog.Logger } func NewGetInstance(cfg Config, instancesStorage storage.Instances, operationsStorage storage.Provisioning, kcBuilder kubeconfig.KcBuilder, - log logrus.FieldLogger, + log *slog.Logger, ) *GetInstanceEndpoint { return &GetInstanceEndpoint{ config: cfg, instancesStorage: instancesStorage, operationsStorage: operationsStorage, kcBuilder: kcBuilder, - log: log.WithField("service", "GetInstanceEndpoint"), + log: log.With("service", "GetInstanceEndpoint"), } } // GetInstance fetches information about a service instance // GET /v2/service_instances/{instance_id} func (b *GetInstanceEndpoint) GetInstance(_ context.Context, instanceID string, _ domain.FetchInstanceDetails) (domain.GetInstanceDetailsSpec, error) { - logger := b.log.WithField("instanceID", instanceID) - logger.Infof("GetInstance called") + logger := b.log.With("instanceID", instanceID) + logger.Info("GetInstance called") instance, err := b.instancesStorage.GetByID(instanceID) if err != nil { diff --git a/internal/broker/instance_get_test.go b/internal/broker/instance_get_test.go index 71dc3a5386..bc320fe1f8 100644 --- a/internal/broker/instance_get_test.go +++ b/internal/broker/instance_get_test.go @@ -4,7 +4,9 @@ import ( "context" "encoding/json" "fmt" + "log/slog" "net/http" + "os" "testing" "time" @@ -20,7 +22,6 @@ import ( "github.com/kyma-project/kyma-environment-broker/internal/storage" "github.com/pivotal-cf/brokerapi/v8/domain" "github.com/pivotal-cf/brokerapi/v8/domain/apiresponses" - "github.com/sirupsen/logrus" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/mock" "github.com/stretchr/testify/require" @@ -30,7 +31,7 @@ func TestGetEndpoint_GetNonExistingInstance(t *testing.T) { // given st := storage.NewMemoryStorage() kcBuilder := &kcMock.KcBuilder{} - svc := broker.NewGetInstance(broker.Config{}, st.Instances(), st.Operations(), kcBuilder, logrus.New()) + svc := broker.NewGetInstance(broker.Config{}, st.Instances(), st.Operations(), kcBuilder, fixLogger()) // when _, err := svc.GetInstance(context.Background(), instanceID, domain.FetchInstanceDetails{}) @@ -65,13 +66,13 @@ func TestGetEndpoint_GetProvisioningInstance(t *testing.T) { factoryBuilder, broker.PlansConfig{}, planDefaults, - logrus.StandardLogger(), + fixLogger(), dashboardConfig, kcBuilder, whitelist.Set{}, &broker.OneForAllConvergedCloudRegionsProvider{}, ) - getSvc := broker.NewGetInstance(broker.Config{EnableKubeconfigURLLabel: true}, st.Instances(), st.Operations(), kcBuilder, logrus.New()) + getSvc := broker.NewGetInstance(broker.Config{EnableKubeconfigURLLabel: true}, st.Instances(), st.Operations(), kcBuilder, fixLogger()) // when _, err := createSvc.Provision(fixRequestContext(t, "req-region"), instanceID, domain.ProvisionDetails{ @@ -127,7 +128,7 @@ func TestGetEndpoint_DoNotReturnInstanceWhereDeletedAtIsNotZero(t *testing.T) { err = st.Instances().Insert(instance) require.NoError(t, err) - svc := broker.NewGetInstance(cfg, st.Instances(), st.Operations(), kcBuilder, logrus.New()) + svc := broker.NewGetInstance(cfg, st.Instances(), st.Operations(), kcBuilder, fixLogger()) // when _, err = svc.GetInstance(context.Background(), instanceID, domain.FetchInstanceDetails{}) @@ -169,7 +170,7 @@ func TestGetEndpoint_GetExpiredInstanceWithExpirationDetails(t *testing.T) { err = st.Instances().Insert(instance) require.NoError(t, err) - svc := broker.NewGetInstance(cfg, st.Instances(), st.Operations(), kcBuilder, logrus.New()) + svc := broker.NewGetInstance(cfg, st.Instances(), st.Operations(), kcBuilder, fixLogger()) // when response, err := svc.GetInstance(context.Background(), instanceID, domain.FetchInstanceDetails{}) @@ -214,7 +215,7 @@ func TestGetEndpoint_GetExpiredInstanceWithExpirationDetailsAllSubaccountsIDs(t err = st.Instances().Insert(instance) require.NoError(t, err) - svc := broker.NewGetInstance(cfg, st.Instances(), st.Operations(), kcBuilder, logrus.New()) + svc := broker.NewGetInstance(cfg, st.Instances(), st.Operations(), kcBuilder, fixLogger()) // when response, err := svc.GetInstance(context.Background(), instanceID, domain.FetchInstanceDetails{}) @@ -260,7 +261,7 @@ func TestGetEndpoint_GetExpiredInstanceWithoutExpirationInfo(t *testing.T) { err = st.Instances().Insert(instance) require.NoError(t, err) - svc := broker.NewGetInstance(cfg, st.Instances(), st.Operations(), kcBuilder, logrus.New()) + svc := broker.NewGetInstance(cfg, st.Instances(), st.Operations(), kcBuilder, fixLogger()) // when response, err := svc.GetInstance(context.Background(), instanceID, domain.FetchInstanceDetails{}) @@ -304,7 +305,7 @@ func TestGetEndpoint_GetExpiredFreeInstanceWithExpirationDetails(t *testing.T) { err = st.Instances().Insert(instance) require.NoError(t, err) - svc := broker.NewGetInstance(cfg, st.Instances(), st.Operations(), kcBuilder, logrus.New()) + svc := broker.NewGetInstance(cfg, st.Instances(), st.Operations(), kcBuilder, fixLogger()) // when response, err := svc.GetInstance(context.Background(), instanceID, domain.FetchInstanceDetails{}) @@ -318,3 +319,9 @@ func TestGetEndpoint_GetExpiredFreeInstanceWithExpirationDetails(t *testing.T) { assert.Contains(t, response.Metadata.Labels, "Free plan expiration details") assert.Contains(t, response.Metadata.Labels, "Available plans documentation") } + +func fixLogger() *slog.Logger { + return slog.New(slog.NewTextHandler(os.Stdout, &slog.HandlerOptions{ + Level: slog.LevelInfo, + })) +} diff --git a/internal/broker/instance_last_operation.go b/internal/broker/instance_last_operation.go index 5f765340d4..ae462f1872 100644 --- a/internal/broker/instance_last_operation.go +++ b/internal/broker/instance_last_operation.go @@ -3,6 +3,7 @@ package broker import ( "context" "fmt" + "log/slog" "net/http" "github.com/kyma-project/kyma-environment-broker/common/orchestration" @@ -11,21 +12,20 @@ import ( "github.com/kyma-project/kyma-environment-broker/internal/storage/dberr" "github.com/pivotal-cf/brokerapi/v8/domain" "github.com/pivotal-cf/brokerapi/v8/domain/apiresponses" - "github.com/sirupsen/logrus" ) type LastOperationEndpoint struct { operationStorage storage.Operations instancesArchived storage.InstancesArchived - log logrus.FieldLogger + log *slog.Logger } -func NewLastOperation(os storage.Operations, ia storage.InstancesArchived, log logrus.FieldLogger) *LastOperationEndpoint { +func NewLastOperation(os storage.Operations, ia storage.InstancesArchived, log *slog.Logger) *LastOperationEndpoint { return &LastOperationEndpoint{ operationStorage: os, instancesArchived: ia, - log: log.WithField("service", "LastOperationEndpoint"), + log: log.With("service", "LastOperationEndpoint"), } } @@ -33,7 +33,7 @@ func NewLastOperation(os storage.Operations, ia storage.InstancesArchived, log l // // GET /v2/service_instances/{instance_id}/last_operation func (b *LastOperationEndpoint) LastOperation(ctx context.Context, instanceID string, details domain.PollDetails) (domain.LastOperation, error) { - logger := b.log.WithField("instanceID", instanceID).WithField("operationID", details.OperationData) + logger := b.log.With("instanceID", instanceID).With("operationID", details.OperationData) if details.OperationData == "" { lastOp, err := b.operationStorage.GetLastOperationByTypes( @@ -49,7 +49,7 @@ func (b *LastOperationEndpoint) LastOperation(ctx context.Context, instanceID st if dberr.IsNotFound(err) { return b.responseFromInstanceArchived(instanceID, logger) } - logger.Errorf("cannot get operation from storage: %s", err) + logger.Error(fmt.Sprintf("cannot get operation from storage: %s", err)) return domain.LastOperation{}, apiresponses.NewFailureResponse(err, statusCode, fmt.Sprintf("while getting last operation from storage")) } @@ -65,14 +65,14 @@ func (b *LastOperationEndpoint) LastOperation(ctx context.Context, instanceID st if dberr.IsNotFound(err) { return b.responseFromInstanceArchived(instanceID, logger) } - logger.Errorf("cannot get operation from storage: %s", err) + logger.Error(fmt.Sprintf("cannot get operation from storage: %s", err)) return domain.LastOperation{}, apiresponses.NewFailureResponse(err, statusCode, fmt.Sprintf("while getting operation from storage")) } if operation.InstanceID != instanceID { err := fmt.Errorf("operation exists, but instanceID is invalid") - logger.Errorf("%s", err.Error()) + logger.Error(fmt.Sprintf("%s", err.Error())) return domain.LastOperation{}, apiresponses.NewFailureResponse(err, http.StatusBadRequest, err.Error()) } @@ -82,7 +82,7 @@ func (b *LastOperationEndpoint) LastOperation(ctx context.Context, instanceID st }, nil } -func (b *LastOperationEndpoint) responseFromInstanceArchived(instanceID string, logger *logrus.Entry) (domain.LastOperation, error) { +func (b *LastOperationEndpoint) responseFromInstanceArchived(instanceID string, logger *slog.Logger) (domain.LastOperation, error) { _, err := b.instancesArchived.GetByInstanceID(instanceID) switch { @@ -94,7 +94,7 @@ func (b *LastOperationEndpoint) responseFromInstanceArchived(instanceID string, case dberr.IsNotFound(err): return domain.LastOperation{}, apiresponses.NewFailureResponse(fmt.Errorf("Operation not found"), http.StatusNotFound, "Instance not found") default: - logger.Errorf("unable to get instance from archived storage: %s", err.Error()) + logger.Error(fmt.Sprintf("unable to get instance from archived storage: %s", err.Error())) return domain.LastOperation{}, apiresponses.NewFailureResponse(err, http.StatusInternalServerError, "") } } diff --git a/internal/broker/instance_last_operation_test.go b/internal/broker/instance_last_operation_test.go index 8ce5d99379..571c105478 100644 --- a/internal/broker/instance_last_operation_test.go +++ b/internal/broker/instance_last_operation_test.go @@ -11,7 +11,6 @@ import ( "github.com/kyma-project/kyma-environment-broker/internal/fixture" "github.com/kyma-project/kyma-environment-broker/internal/storage" "github.com/pivotal-cf/brokerapi/v8/domain" - "github.com/sirupsen/logrus" "github.com/stretchr/testify/assert" ) @@ -28,7 +27,7 @@ func TestLastOperation_LastOperation(t *testing.T) { err := memoryStorage.Operations().InsertOperation(fixOperation()) assert.NoError(t, err) - lastOperationEndpoint := broker.NewLastOperation(memoryStorage.Operations(), memoryStorage.InstancesArchived(), logrus.StandardLogger()) + lastOperationEndpoint := broker.NewLastOperation(memoryStorage.Operations(), memoryStorage.InstancesArchived(), fixLogger()) // when response, err := lastOperationEndpoint.LastOperation(context.TODO(), instID, domain.PollDetails{OperationData: operationID}) @@ -46,7 +45,7 @@ func TestLastOperation_LastOperation(t *testing.T) { err := memoryStorage.Operations().InsertOperation(fixOperation()) assert.NoError(t, err) - lastOperationEndpoint := broker.NewLastOperation(memoryStorage.Operations(), memoryStorage.InstancesArchived(), logrus.StandardLogger()) + lastOperationEndpoint := broker.NewLastOperation(memoryStorage.Operations(), memoryStorage.InstancesArchived(), fixLogger()) // when response, err := lastOperationEndpoint.LastOperation(context.TODO(), instID, domain.PollDetails{OperationData: ""}) @@ -66,7 +65,7 @@ func TestLastOperation_LastOperation(t *testing.T) { err := memoryStorage.Operations().InsertUpdatingOperation(updateOp) assert.NoError(t, err) - lastOperationEndpoint := broker.NewLastOperation(memoryStorage.Operations(), memoryStorage.InstancesArchived(), logrus.StandardLogger()) + lastOperationEndpoint := broker.NewLastOperation(memoryStorage.Operations(), memoryStorage.InstancesArchived(), fixLogger()) // when response, err := lastOperationEndpoint.LastOperation(context.TODO(), instID, domain.PollDetails{OperationData: ""}) @@ -94,7 +93,7 @@ func TestLastOperation_LastOperation(t *testing.T) { err := memoryStorage.Operations().InsertUpdatingOperation(updateOp) assert.NoError(t, err) - lastOperationEndpoint := broker.NewLastOperation(memoryStorage.Operations(), memoryStorage.InstancesArchived(), logrus.StandardLogger()) + lastOperationEndpoint := broker.NewLastOperation(memoryStorage.Operations(), memoryStorage.InstancesArchived(), fixLogger()) // when response, err := lastOperationEndpoint.LastOperation(context.TODO(), instID, domain.PollDetails{OperationData: ""}) @@ -125,7 +124,7 @@ func TestLastOperation_LastOperation(t *testing.T) { err := memoryStorage.Operations().InsertUpdatingOperation(updateOp) assert.NoError(t, err) - lastOperationEndpoint := broker.NewLastOperation(memoryStorage.Operations(), memoryStorage.InstancesArchived(), logrus.StandardLogger()) + lastOperationEndpoint := broker.NewLastOperation(memoryStorage.Operations(), memoryStorage.InstancesArchived(), fixLogger()) // when response, err := lastOperationEndpoint.LastOperation(context.TODO(), instID, domain.PollDetails{OperationData: ""}) @@ -155,7 +154,7 @@ func TestLastOperation_LastOperation(t *testing.T) { err := memoryStorage.Operations().InsertUpdatingOperation(updateOp) assert.NoError(t, err) - lastOperationEndpoint := broker.NewLastOperation(memoryStorage.Operations(), memoryStorage.InstancesArchived(), logrus.StandardLogger()) + lastOperationEndpoint := broker.NewLastOperation(memoryStorage.Operations(), memoryStorage.InstancesArchived(), fixLogger()) // when response, err := lastOperationEndpoint.LastOperation(context.TODO(), instID, domain.PollDetails{OperationData: ""}) @@ -190,7 +189,7 @@ func TestLastOperation_LastOperation(t *testing.T) { err := memoryStorage.Operations().InsertOperation(provisioning) assert.NoError(t, err) - lastOperationEndpoint := broker.NewLastOperation(memoryStorage.Operations(), memoryStorage.InstancesArchived(), logrus.StandardLogger()) + lastOperationEndpoint := broker.NewLastOperation(memoryStorage.Operations(), memoryStorage.InstancesArchived(), fixLogger()) // when response, err := lastOperationEndpoint.LastOperation(context.TODO(), instID, domain.PollDetails{OperationData: ""}) diff --git a/internal/broker/instance_update.go b/internal/broker/instance_update.go index 964762e1d5..4c68186999 100644 --- a/internal/broker/instance_update.go +++ b/internal/broker/instance_update.go @@ -4,6 +4,7 @@ import ( "context" "encoding/json" "fmt" + "log/slog" "net/http" "strings" "time" @@ -19,7 +20,6 @@ import ( "github.com/google/uuid" "github.com/pivotal-cf/brokerapi/v8/domain" "github.com/pivotal-cf/brokerapi/v8/domain/apiresponses" - "github.com/sirupsen/logrus" "k8s.io/apimachinery/pkg/util/wait" "github.com/kyma-project/kyma-environment-broker/internal" @@ -35,7 +35,7 @@ type ContextUpdateHandler interface { type UpdateEndpoint struct { config Config - log logrus.FieldLogger + log *slog.Logger instanceStorage storage.Instances runtimeStates storage.RuntimeStates @@ -71,7 +71,7 @@ func NewUpdate(cfg Config, queue Queue, plansConfig PlansConfig, planDefaults PlanDefaults, - log logrus.FieldLogger, + log *slog.Logger, dashboardConfig dashboard.Config, kcBuilder kubeconfig.KcBuilder, convergedCloudRegionsProvider ConvergedCloudRegionProvider, @@ -79,7 +79,7 @@ func NewUpdate(cfg Config, ) *UpdateEndpoint { return &UpdateEndpoint{ config: cfg, - log: log.WithField("service", "UpdateEndpoint"), + log: log.With("service", "UpdateEndpoint"), instanceStorage: instanceStorage, runtimeStates: runtimeStates, operationStorage: operationStorage, @@ -101,27 +101,27 @@ func NewUpdate(cfg Config, // // PATCH /v2/service_instances/{instance_id} func (b *UpdateEndpoint) Update(_ context.Context, instanceID string, details domain.UpdateDetails, asyncAllowed bool) (domain.UpdateServiceSpec, error) { - logger := b.log.WithField("instanceID", instanceID) - logger.Infof("Updating instanceID: %s", instanceID) - logger.Infof("Updating asyncAllowed: %v", asyncAllowed) - logger.Infof("Parameters: '%s'", string(details.RawParameters)) + logger := b.log.With("instanceID", instanceID) + logger.Info(fmt.Sprintf("Updating instanceID: %s", instanceID)) + logger.Info(fmt.Sprintf("Updating asyncAllowed: %v", asyncAllowed)) + logger.Info(fmt.Sprintf("Parameters: '%s'", string(details.RawParameters))) instance, err := b.instanceStorage.GetByID(instanceID) if err != nil && dberr.IsNotFound(err) { - logger.Errorf("unable to get instance: %s", err.Error()) + logger.Error(fmt.Sprintf("unable to get instance: %s", err.Error())) return domain.UpdateServiceSpec{}, apiresponses.NewFailureResponse(err, http.StatusNotFound, fmt.Sprintf("could not execute update for instanceID %s", instanceID)) } else if err != nil { - logger.Errorf("unable to get instance: %s", err.Error()) + logger.Error(fmt.Sprintf("unable to get instance: %s", err.Error())) return domain.UpdateServiceSpec{}, fmt.Errorf("unable to get instance") } - logger.Infof("Plan ID/Name: %s/%s", instance.ServicePlanID, PlanNamesMapping[instance.ServicePlanID]) + logger.Info(fmt.Sprintf("Plan ID/Name: %s/%s", instance.ServicePlanID, PlanNamesMapping[instance.ServicePlanID])) var ersContext internal.ERSContext err = json.Unmarshal(details.RawContext, &ersContext) if err != nil { - logger.Errorf("unable to decode context: %s", err.Error()) + logger.Error(fmt.Sprintf("unable to decode context: %s", err.Error())) return domain.UpdateServiceSpec{}, fmt.Errorf("unable to unmarshal context") } - logger.Infof("Global account ID: %s active: %s", instance.GlobalAccountID, ptr.BoolAsString(ersContext.Active)) - logger.Infof("Received context: %s", marshallRawContext(hideSensitiveDataFromRawContext(details.RawContext))) + logger.Info(fmt.Sprintf("Global account ID: %s active: %s", instance.GlobalAccountID, ptr.BoolAsString(ersContext.Active))) + logger.Info(fmt.Sprintf("Received context: %s", marshallRawContext(hideSensitiveDataFromRawContext(details.RawContext)))) // validation of incoming input if err := b.validateWithJsonSchemaValidator(details, instance); err != nil { return domain.UpdateServiceSpec{}, apiresponses.NewFailureResponse(err, http.StatusBadRequest, "validation failed") @@ -135,7 +135,7 @@ func (b *UpdateEndpoint) Update(_ context.Context, instanceID string, details do } lastProvisioningOperation, err := b.operationStorage.GetProvisioningOperationByInstanceID(instance.InstanceID) if err != nil { - logger.Errorf("cannot fetch provisioning lastProvisioningOperation for instance with ID: %s : %s", instance.InstanceID, err.Error()) + logger.Error(fmt.Sprintf("cannot fetch provisioning lastProvisioningOperation for instance with ID: %s : %s", instance.InstanceID, err.Error())) return domain.UpdateServiceSpec{}, fmt.Errorf("unable to process the update") } if lastProvisioningOperation.State == domain.Failed { @@ -144,13 +144,13 @@ func (b *UpdateEndpoint) Update(_ context.Context, instanceID string, details do lastDeprovisioningOperation, err := b.operationStorage.GetDeprovisioningOperationByInstanceID(instance.InstanceID) if err != nil && !dberr.IsNotFound(err) { - logger.Errorf("cannot fetch deprovisioning for instance with ID: %s : %s", instance.InstanceID, err.Error()) + logger.Error(fmt.Sprintf("cannot fetch deprovisioning for instance with ID: %s : %s", instance.InstanceID, err.Error())) return domain.UpdateServiceSpec{}, fmt.Errorf("unable to process the update") } if err == nil { if !lastDeprovisioningOperation.Temporary { // it is not a suspension, but real deprovisioning - logger.Warnf("Cannot process update, the instance has started deprovisioning process (operationID=%s)", lastDeprovisioningOperation.Operation.ID) + logger.Warn(fmt.Sprintf("Cannot process update, the instance has started deprovisioning process (operationID=%s)", lastDeprovisioningOperation.Operation.ID)) return domain.UpdateServiceSpec{}, apiresponses.NewFailureResponse(fmt.Errorf("Unable to process an update of a deprovisioned instance"), http.StatusUnprocessableEntity, "") } } @@ -207,9 +207,9 @@ func shouldUpdate(instance *internal.Instance, details domain.UpdateDetails, ers return ersContext.ERSUpdate() } -func (b *UpdateEndpoint) processUpdateParameters(instance *internal.Instance, details domain.UpdateDetails, lastProvisioningOperation *internal.ProvisioningOperation, asyncAllowed bool, ersContext internal.ERSContext, logger logrus.FieldLogger) (domain.UpdateServiceSpec, error) { +func (b *UpdateEndpoint) processUpdateParameters(instance *internal.Instance, details domain.UpdateDetails, lastProvisioningOperation *internal.ProvisioningOperation, asyncAllowed bool, ersContext internal.ERSContext, logger *slog.Logger) (domain.UpdateServiceSpec, error) { if !shouldUpdate(instance, details, ersContext) { - logger.Debugf("Parameters not provided, skipping processing update parameters") + logger.Debug("Parameters not provided, skipping processing update parameters") return domain.UpdateServiceSpec{ IsAsync: false, DashboardURL: instance.DashboardURL, @@ -227,23 +227,23 @@ func (b *UpdateEndpoint) processUpdateParameters(instance *internal.Instance, de if len(details.RawParameters) != 0 { err := json.Unmarshal(details.RawParameters, ¶ms) if err != nil { - logger.Errorf("unable to unmarshal parameters: %s", err.Error()) + logger.Error(fmt.Sprintf("unable to unmarshal parameters: %s", err.Error())) return domain.UpdateServiceSpec{}, fmt.Errorf("unable to unmarshal parameters") } - logger.Debugf("Updating with params: %+v", params) + logger.Debug(fmt.Sprintf("Updating with params: %+v", params)) } if params.OIDC.IsProvided() { if err := params.OIDC.Validate(); err != nil { - logger.Errorf("invalid OIDC parameters: %s", err.Error()) + logger.Error(fmt.Sprintf("invalid OIDC parameters: %s", err.Error())) return domain.UpdateServiceSpec{}, apiresponses.NewFailureResponse(err, http.StatusUnprocessableEntity, err.Error()) } } operationID := uuid.New().String() - logger = logger.WithField("operationID", operationID) + logger = logger.With("operationID", operationID) - logger.Debugf("creating update operation %v", params) + logger.Debug(fmt.Sprintf("creating update operation %v", params)) operation := internal.NewUpdateOperation(operationID, instance, params) planID := instance.Parameters.PlanID if len(details.PlanID) != 0 { @@ -251,7 +251,7 @@ func (b *UpdateEndpoint) processUpdateParameters(instance *internal.Instance, de } defaults, err := b.planDefaults(planID, instance.Provider, &instance.Provider) if err != nil { - logger.Errorf("unable to obtain plan defaults: %s", err.Error()) + logger.Error(fmt.Sprintf("unable to obtain plan defaults: %s", err.Error())) return domain.UpdateServiceSpec{}, fmt.Errorf("unable to obtain plan defaults") } var autoscalerMin, autoscalerMax int @@ -260,7 +260,7 @@ func (b *UpdateEndpoint) processUpdateParameters(instance *internal.Instance, de autoscalerMin, autoscalerMax = p.AutoScalerMin, p.AutoScalerMax } if err := operation.ProvisioningParameters.Parameters.AutoScalerParameters.Validate(autoscalerMin, autoscalerMax); err != nil { - logger.Errorf("invalid autoscaler parameters: %s", err.Error()) + logger.Error(fmt.Sprintf("invalid autoscaler parameters: %s", err.Error())) return domain.UpdateServiceSpec{}, apiresponses.NewFailureResponse(err, http.StatusBadRequest, err.Error()) } err = b.operationStorage.InsertOperation(operation) @@ -292,7 +292,7 @@ func (b *UpdateEndpoint) processUpdateParameters(instance *internal.Instance, de instance, err = b.instanceStorage.Update(*instance) if err != nil { params := strings.Join(updateStorage, ", ") - logger.Warnf("unable to update instance with new %v (%s), retrying", params, err.Error()) + logger.Warn(fmt.Sprintf("unable to update instance with new %v (%s), retrying", params, err.Error())) return false, nil } return true, nil @@ -301,7 +301,7 @@ func (b *UpdateEndpoint) processUpdateParameters(instance *internal.Instance, de return domain.UpdateServiceSpec{}, response } } - logger.Debugf("Adding update operation to the processing queue") + logger.Debug("Adding update operation to the processing queue") b.updatingQueue.Add(operationID) return domain.UpdateServiceSpec{ @@ -314,18 +314,18 @@ func (b *UpdateEndpoint) processUpdateParameters(instance *internal.Instance, de }, nil } -func (b *UpdateEndpoint) processContext(instance *internal.Instance, details domain.UpdateDetails, lastProvisioningOperation *internal.ProvisioningOperation, logger logrus.FieldLogger) (*internal.Instance, bool, error) { +func (b *UpdateEndpoint) processContext(instance *internal.Instance, details domain.UpdateDetails, lastProvisioningOperation *internal.ProvisioningOperation, logger *slog.Logger) (*internal.Instance, bool, error) { var ersContext internal.ERSContext err := json.Unmarshal(details.RawContext, &ersContext) if err != nil { - logger.Errorf("unable to decode context: %s", err.Error()) + logger.Error(fmt.Sprintf("unable to decode context: %s", err.Error())) return nil, false, fmt.Errorf("unable to unmarshal context") } - logger.Infof("Global account ID: %s active: %s", instance.GlobalAccountID, ptr.BoolAsString(ersContext.Active)) + logger.Info(fmt.Sprintf("Global account ID: %s active: %s", instance.GlobalAccountID, ptr.BoolAsString(ersContext.Active))) lastOp, err := b.operationStorage.GetLastOperation(instance.InstanceID) if err != nil { - logger.Errorf("unable to get last operation: %s", err.Error()) + logger.Error(fmt.Sprintf("unable to get last operation: %s", err.Error())) return nil, false, fmt.Errorf("failed to process ERS context") } @@ -344,7 +344,7 @@ func (b *UpdateEndpoint) processContext(instance *internal.Instance, details dom changed, err := b.contextUpdateHandler.Handle(instance, ersContext) if err != nil { - logger.Errorf("processing context updated failed: %s", err.Error()) + logger.Error(fmt.Sprintf("processing context updated failed: %s", err.Error())) return nil, changed, fmt.Errorf("unable to process the update") } @@ -360,12 +360,12 @@ func (b *UpdateEndpoint) processContext(instance *internal.Instance, details dom } instance.GlobalAccountID = ersContext.GlobalAccountID needUpdateCustomResources = true - logger.Infof("Global account ID changed to: %s. need update labels", instance.GlobalAccountID) + logger.Info(fmt.Sprintf("Global account ID changed to: %s. need update labels", instance.GlobalAccountID)) } newInstance, err := b.instanceStorage.Update(*instance) if err != nil { - logger.Errorf("processing context updated failed: %s", err.Error()) + logger.Error(fmt.Sprintf("processing context updated failed: %s", err.Error())) return nil, changed, fmt.Errorf("unable to process the update") } else if b.updateCustomResourcesLabelsOnAccountMove && needUpdateCustomResources { logger.Info("updating labels on related CRs") @@ -373,7 +373,7 @@ func (b *UpdateEndpoint) processContext(instance *internal.Instance, details dom labeler := NewLabeler(b.kcpClient) err = labeler.UpdateLabels(newInstance.RuntimeID, newInstance.GlobalAccountID) if err != nil { - logger.Errorf("unable to update global account label on CRs while doing account move: %s", err.Error()) + logger.Error(fmt.Sprintf("unable to update global account label on CRs while doing account move: %s", err.Error())) response := apiresponses.NewFailureResponse(fmt.Errorf("Update CRs label failed"), http.StatusInternalServerError, err.Error()) return newInstance, changed, response } @@ -386,7 +386,7 @@ func (b *UpdateEndpoint) processContext(instance *internal.Instance, details dom func (b *UpdateEndpoint) extractActiveValue(id string, provisioning internal.ProvisioningOperation) (*bool, error) { deprovisioning, dErr := b.operationStorage.GetDeprovisioningOperationByInstanceID(id) if dErr != nil && !dberr.IsNotFound(dErr) { - b.log.Errorf("Unable to get deprovisioning operation for the instance %s to check the active flag: %s", id, dErr.Error()) + b.log.Error(fmt.Sprintf("Unable to get deprovisioning operation for the instance %s to check the active flag: %s", id, dErr.Error())) return nil, dErr } // there was no any deprovisioning in the past (any suspension) @@ -399,7 +399,7 @@ func (b *UpdateEndpoint) extractActiveValue(id string, provisioning internal.Pro func (b *UpdateEndpoint) getJsonSchemaValidator(provider pkg.CloudProvider, planID string, platformRegion string) (JSONSchemaValidator, error) { // shootAndSeedSameRegion is never enabled for update - b.log.Printf("region is: %s", platformRegion) + b.log.Info(fmt.Sprintf("region is: %s", platformRegion)) plans := Plans(b.plansConfig, provider, b.config.IncludeAdditionalParamsInSchema, euaccess.IsEURestrictedAccess(platformRegion), b.config.UseSmallerMachineTypes, false, b.convergedCloudRegionsProvider.GetRegions(platformRegion), assuredworkloads.IsKSA(platformRegion)) plan := plans[planID] schema := string(Marshal(plan.Schemas.Instance.Update.Parameters)) diff --git a/internal/broker/instance_update_test.go b/internal/broker/instance_update_test.go index 70ce9585af..15c8952348 100644 --- a/internal/broker/instance_update_test.go +++ b/internal/broker/instance_update_test.go @@ -32,7 +32,6 @@ import ( "github.com/kyma-project/kyma-environment-broker/internal/storage" "github.com/pivotal-cf/brokerapi/v8/domain" "github.com/pivotal-cf/brokerapi/v8/domain/apiresponses" - "github.com/sirupsen/logrus" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" "sigs.k8s.io/controller-runtime/pkg/client" @@ -96,7 +95,7 @@ func TestUpdateEndpoint_UpdateSuspension(t *testing.T) { q, PlansConfig{}, planDefaults, - logrus.New(), + fixLogger(), dashboardConfig, kcBuilder, &OneForAllConvergedCloudRegionsProvider{}, @@ -157,7 +156,7 @@ func TestUpdateEndpoint_UpdateOfExpiredTrial(t *testing.T) { } kcBuilder := &kcMock.KcBuilder{} svc := NewUpdate(Config{}, st.Instances(), st.RuntimeStates(), st.Operations(), handler, true, false, true, q, PlansConfig{}, - planDefaults, logrus.New(), dashboardConfig, kcBuilder, &OneForAllConvergedCloudRegionsProvider{}, fakeKcpK8sClient) + planDefaults, fixLogger(), dashboardConfig, kcBuilder, &OneForAllConvergedCloudRegionsProvider{}, fakeKcpK8sClient) // when response, err := svc.Update(context.Background(), instanceID, domain.UpdateDetails{ @@ -207,7 +206,7 @@ func TestUpdateEndpoint_UpdateAutoscalerParams(t *testing.T) { } kcBuilder := &kcMock.KcBuilder{} svc := NewUpdate(Config{}, st.Instances(), st.RuntimeStates(), st.Operations(), handler, true, false, true, q, PlansConfig{}, - planDefaults, logrus.New(), dashboardConfig, kcBuilder, &OneForAllConvergedCloudRegionsProvider{}, fakeKcpK8sClient) + planDefaults, fixLogger(), dashboardConfig, kcBuilder, &OneForAllConvergedCloudRegionsProvider{}, fakeKcpK8sClient) t.Run("Should fail on invalid (too low) autoScalerMin and autoScalerMax", func(t *testing.T) { @@ -301,7 +300,7 @@ func TestUpdateEndpoint_UpdateUnsuspension(t *testing.T) { } kcBuilder := &kcMock.KcBuilder{} svc := NewUpdate(Config{}, st.Instances(), st.RuntimeStates(), st.Operations(), handler, true, false, true, q, PlansConfig{}, - planDefaults, logrus.New(), dashboardConfig, kcBuilder, &OneForAllConvergedCloudRegionsProvider{}, fakeKcpK8sClient) + planDefaults, fixLogger(), dashboardConfig, kcBuilder, &OneForAllConvergedCloudRegionsProvider{}, fakeKcpK8sClient) // when _, err = svc.Update(context.Background(), instanceID, domain.UpdateDetails{ @@ -352,7 +351,7 @@ func TestUpdateEndpoint_UpdateInstanceWithWrongActiveValue(t *testing.T) { } kcBuilder := &kcMock.KcBuilder{} svc := NewUpdate(Config{}, st.Instances(), st.RuntimeStates(), st.Operations(), handler, true, false, true, q, PlansConfig{}, - planDefaults, logrus.New(), dashboardConfig, kcBuilder, &OneForAllConvergedCloudRegionsProvider{}, fakeKcpK8sClient) + planDefaults, fixLogger(), dashboardConfig, kcBuilder, &OneForAllConvergedCloudRegionsProvider{}, fakeKcpK8sClient) // when _, err = svc.Update(context.Background(), instanceID, domain.UpdateDetails{ @@ -384,7 +383,7 @@ func TestUpdateEndpoint_UpdateNonExistingInstance(t *testing.T) { } kcBuilder := &kcMock.KcBuilder{} svc := NewUpdate(Config{}, st.Instances(), st.RuntimeStates(), st.Operations(), handler, true, false, true, q, PlansConfig{}, - planDefaults, logrus.New(), dashboardConfig, kcBuilder, &OneForAllConvergedCloudRegionsProvider{}, fakeKcpK8sClient) + planDefaults, fixLogger(), dashboardConfig, kcBuilder, &OneForAllConvergedCloudRegionsProvider{}, fakeKcpK8sClient) // when _, err := svc.Update(context.Background(), instanceID, domain.UpdateDetails{ @@ -451,7 +450,7 @@ func TestUpdateEndpoint_UpdateGlobalAccountID(t *testing.T) { kcBuilder := &kcMock.KcBuilder{} svc := NewUpdate(Config{}, st.Instances(), st.RuntimeStates(), st.Operations(), handler, true, true, false, q, PlansConfig{}, - planDefaults, logrus.New(), dashboardConfig, kcBuilder, &OneForAllConvergedCloudRegionsProvider{}, fakeKcpK8sClient) + planDefaults, fixLogger(), dashboardConfig, kcBuilder, &OneForAllConvergedCloudRegionsProvider{}, fakeKcpK8sClient) // when response, err := svc.Update(context.Background(), instanceID, domain.UpdateDetails{ @@ -495,7 +494,7 @@ func TestUpdateEndpoint_UpdateParameters(t *testing.T) { } kcBuilder := &kcMock.KcBuilder{} svc := NewUpdate(Config{}, st.Instances(), st.RuntimeStates(), st.Operations(), handler, true, true, false, q, PlansConfig{}, - planDefaults, logrus.New(), dashboardConfig, kcBuilder, &OneForAllConvergedCloudRegionsProvider{}, fakeKcpK8sClient) + planDefaults, fixLogger(), dashboardConfig, kcBuilder, &OneForAllConvergedCloudRegionsProvider{}, fakeKcpK8sClient) t.Run("Should fail on invalid OIDC params", func(t *testing.T) { // given @@ -626,7 +625,7 @@ func TestUpdateEndpoint_UpdateWithEnabledDashboard(t *testing.T) { } kcBuilder := &kcMock.KcBuilder{} svc := NewUpdate(Config{AllowUpdateExpiredInstanceWithContext: true}, st.Instances(), st.RuntimeStates(), st.Operations(), handler, true, false, true, q, PlansConfig{}, - planDefaults, logrus.New(), dashboardConfig, kcBuilder, &OneForAllConvergedCloudRegionsProvider{}, fakeKcpK8sClient) + planDefaults, fixLogger(), dashboardConfig, kcBuilder, &OneForAllConvergedCloudRegionsProvider{}, fakeKcpK8sClient) createFakeCRs(t) // when response, err := svc.Update(context.Background(), instanceID, domain.UpdateDetails{ @@ -682,7 +681,7 @@ func TestUpdateExpiredInstance(t *testing.T) { } svc := NewUpdate(Config{AllowUpdateExpiredInstanceWithContext: true}, storage.Instances(), storage.RuntimeStates(), storage.Operations(), handler, true, false, true, queue, PlansConfig{}, - planDefaults, logrus.New(), dashboardConfig, kcBuilder, &OneForAllConvergedCloudRegionsProvider{}, fakeKcpK8sClient) + planDefaults, fixLogger(), dashboardConfig, kcBuilder, &OneForAllConvergedCloudRegionsProvider{}, fakeKcpK8sClient) t.Run("should accept if it is same as previous", func(t *testing.T) { _, err = svc.Update(context.Background(), instanceID, domain.UpdateDetails{ @@ -770,7 +769,7 @@ func TestSubaccountMovement(t *testing.T) { } svc := NewUpdate(Config{SubaccountMovementEnabled: true}, storage.Instances(), storage.RuntimeStates(), storage.Operations(), handler, true, true, true, queue, PlansConfig{}, - planDefaults, logrus.New(), dashboardConfig, kcBuilder, &OneForAllConvergedCloudRegionsProvider{}, fakeKcpK8sClient) + planDefaults, fixLogger(), dashboardConfig, kcBuilder, &OneForAllConvergedCloudRegionsProvider{}, fakeKcpK8sClient) t.Run("no move performed so subscription should be empty", func(t *testing.T) { _, err = svc.Update(context.Background(), instanceID, domain.UpdateDetails{ @@ -861,7 +860,7 @@ func TestLabelChangeWhenMovingSubaccount(t *testing.T) { } svc := NewUpdate(Config{SubaccountMovementEnabled: true}, storage.Instances(), storage.RuntimeStates(), storage.Operations(), handler, true, true, true, queue, PlansConfig{}, - planDefaults, logrus.New(), dashboardConfig, kcBuilder, &OneForAllConvergedCloudRegionsProvider{}, fakeKcpK8sClient) + planDefaults, fixLogger(), dashboardConfig, kcBuilder, &OneForAllConvergedCloudRegionsProvider{}, fakeKcpK8sClient) t.Run("simulate flow of moving account with labels on CRs", func(t *testing.T) { // initial state of instance - moving account was never donex diff --git a/internal/broker/services.go b/internal/broker/services.go index 643e5f56d9..ec42ab7c04 100644 --- a/internal/broker/services.go +++ b/internal/broker/services.go @@ -3,6 +3,7 @@ package broker import ( "context" "fmt" + "log/slog" "github.com/kyma-project/kyma-environment-broker/internal/assuredworkloads" @@ -11,7 +12,6 @@ import ( "github.com/kyma-project/kyma-environment-broker/internal/middleware" "github.com/pivotal-cf/brokerapi/v8/domain" - "github.com/sirupsen/logrus" ) const ( @@ -20,7 +20,7 @@ const ( ) type ServicesEndpoint struct { - log logrus.FieldLogger + log *slog.Logger cfg Config servicesConfig ServicesConfig @@ -28,7 +28,7 @@ type ServicesEndpoint struct { convergedCloudRegionsProvider ConvergedCloudRegionProvider } -func NewServices(cfg Config, servicesConfig ServicesConfig, log logrus.FieldLogger, convergedCloudRegionsProvider ConvergedCloudRegionProvider) *ServicesEndpoint { +func NewServices(cfg Config, servicesConfig ServicesConfig, log *slog.Logger, convergedCloudRegionsProvider ConvergedCloudRegionProvider) *ServicesEndpoint { enabledPlanIDs := map[string]struct{}{} for _, planName := range cfg.EnablePlans { id := PlanIDsMapping[planName] @@ -36,7 +36,7 @@ func NewServices(cfg Config, servicesConfig ServicesConfig, log logrus.FieldLogg } return &ServicesEndpoint{ - log: log.WithField("service", "ServicesEndpoint"), + log: log.With("service", "ServicesEndpoint"), cfg: cfg, servicesConfig: servicesConfig, enabledPlanIDs: enabledPlanIDs, diff --git a/internal/broker/services_test.go b/internal/broker/services_test.go index 6d4d0b45ca..b2e473143a 100644 --- a/internal/broker/services_test.go +++ b/internal/broker/services_test.go @@ -2,17 +2,21 @@ package broker_test import ( "context" + "log/slog" + "os" "strings" "testing" "github.com/kyma-project/kyma-environment-broker/internal/broker" "github.com/pivotal-cf/brokerapi/v8/domain" - "github.com/sirupsen/logrus" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) func TestServices_Services(t *testing.T) { + log := slog.New(slog.NewTextHandler(os.Stdout, &slog.HandlerOptions{ + Level: slog.LevelInfo, + })) t.Run("should get service and plans without OIDC", func(t *testing.T) { // given @@ -32,7 +36,7 @@ func TestServices_Services(t *testing.T) { }, }, } - servicesEndpoint := broker.NewServices(cfg, servicesConfig, logrus.StandardLogger(), &broker.OneForAllConvergedCloudRegionsProvider{}) + servicesEndpoint := broker.NewServices(cfg, servicesConfig, log, &broker.OneForAllConvergedCloudRegionsProvider{}) // when services, err := servicesEndpoint.Services(context.TODO()) @@ -64,7 +68,7 @@ func TestServices_Services(t *testing.T) { }, }, } - servicesEndpoint := broker.NewServices(cfg, servicesConfig, logrus.StandardLogger(), &broker.OneForAllConvergedCloudRegionsProvider{}) + servicesEndpoint := broker.NewServices(cfg, servicesConfig, log, &broker.OneForAllConvergedCloudRegionsProvider{}) // when services, err := servicesEndpoint.Services(context.TODO()) @@ -100,7 +104,7 @@ func TestServices_Services(t *testing.T) { }, }, } - servicesEndpoint := broker.NewServices(cfg, servicesConfig, logrus.StandardLogger(), &broker.OneForAllConvergedCloudRegionsProvider{}) + servicesEndpoint := broker.NewServices(cfg, servicesConfig, log, &broker.OneForAllConvergedCloudRegionsProvider{}) // when services, err := servicesEndpoint.Services(context.TODO()) @@ -139,7 +143,7 @@ func TestServices_Services(t *testing.T) { }, }, } - servicesEndpoint := broker.NewServices(cfg, servicesConfig, logrus.StandardLogger(), &broker.OneForAllConvergedCloudRegionsProvider{}) + servicesEndpoint := broker.NewServices(cfg, servicesConfig, log, &broker.OneForAllConvergedCloudRegionsProvider{}) // when services, err := servicesEndpoint.Services(context.TODO()) diff --git a/internal/httputil/response.go b/internal/httputil/response.go index 37d30296b8..efeb4375d3 100644 --- a/internal/httputil/response.go +++ b/internal/httputil/response.go @@ -2,9 +2,8 @@ package httputil import ( "fmt" + "log/slog" "net/http" - - "github.com/sirupsen/logrus" ) // Writer provides syntactic sugar for writing http responses. @@ -12,12 +11,12 @@ import ( // - devMode: true - returns a given error in the response under `details` field // - devMode: false - only log the given error in context of the requestID but do not return it in response type Writer struct { - log logrus.FieldLogger + log *slog.Logger devMode bool } // NewResponseWriter returns new instance of Writer -func NewResponseWriter(log logrus.FieldLogger, devMode bool) *Writer { +func NewResponseWriter(log *slog.Logger, devMode bool) *Writer { return &Writer{ log: log, devMode: devMode, @@ -45,12 +44,12 @@ func (w *Writer) InternalServerError(rw http.ResponseWriter, r *http.Request, er func (w *Writer) writeError(rw http.ResponseWriter, r *http.Request, errDTO ErrorDTO) { errDTO.RequestID = r.Header.Get("X-Request-Id") if !w.devMode { - w.log.WithField("request-id", errDTO.RequestID).Error(errDTO.Details) + w.log.With("request-id", errDTO.RequestID).Error(errDTO.Details) errDTO.Details = "" } if err := JSONEncodeWithCode(rw, errDTO, errDTO.Status); err != nil { - w.log.WithField("request-id", errDTO.RequestID).Errorf("while encoding error DTO: %s", err) + w.log.With("request-id", errDTO.RequestID).Error(fmt.Sprintf("while encoding error DTO: %s", err)) rw.WriteHeader(http.StatusInternalServerError) return } diff --git a/internal/httputil/response_test.go b/internal/httputil/response_test.go index c926700f86..ace500af3c 100644 --- a/internal/httputil/response_test.go +++ b/internal/httputil/response_test.go @@ -1,21 +1,23 @@ package httputil_test import ( + "bytes" "fmt" + "log/slog" "net/http" "net/http/httptest" "testing" "github.com/kyma-project/kyma-environment-broker/internal/httputil" - "github.com/kyma-project/kyma-environment-broker/internal/logger" - "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) func TestResponseWriterDevModeOff(t *testing.T) { - logSpy := logger.NewLogSpy() - writer := httputil.NewResponseWriter(logSpy.Logger, false) + cw := &captureWriter{buf: &bytes.Buffer{}} + handler := slog.NewTextHandler(cw, nil) + log := slog.New(handler) + writer := httputil.NewResponseWriter(log, false) for testName, testCase := range map[string]struct { testedFn func(rw http.ResponseWriter, r *http.Request, err error, context string) @@ -43,7 +45,7 @@ func TestResponseWriterDevModeOff(t *testing.T) { } { t.Run(testName, func(t *testing.T) { // given - logSpy.Reset() + cw.Reset() var ( respSpy = httptest.NewRecorder() fixReq = fixRequest(t) @@ -56,7 +58,7 @@ func TestResponseWriterDevModeOff(t *testing.T) { // then - logSpy.AssertErrorLogged(t, fmt.Errorf("generated by testing scenario: %w", fixErr)) + assert.Contains(t, cw.buf.String(), fmt.Sprintf("generated by testing scenario: %v", fixErr)) assert.Equal(t, testCase.status, respSpy.Code) assert.JSONEq(t, testCase.body, respSpy.Body.String()) @@ -65,8 +67,10 @@ func TestResponseWriterDevModeOff(t *testing.T) { } func TestResponseWriterDevModeOn(t *testing.T) { - logSpy := logger.NewLogSpy() - writer := httputil.NewResponseWriter(logSpy.Logger, true) + cw := &captureWriter{buf: &bytes.Buffer{}} + handler := slog.NewTextHandler(cw, nil) + log := slog.New(handler) + writer := httputil.NewResponseWriter(log, true) for testName, testCase := range map[string]struct { testedFn func(rw http.ResponseWriter, r *http.Request, err error, context string) @@ -96,7 +100,7 @@ func TestResponseWriterDevModeOn(t *testing.T) { } { t.Run(testName, func(t *testing.T) { // given - logSpy.Reset() + cw.Reset() var ( respSpy = httptest.NewRecorder() fixReq = fixRequest(t) @@ -108,7 +112,7 @@ func TestResponseWriterDevModeOn(t *testing.T) { testCase.testedFn(respSpy, fixReq, fixErr, fixErrContext) // then - assert.Empty(t, logSpy.DumpAll()) + assert.Empty(t, cw.buf.String()) assert.Equal(t, testCase.status, respSpy.Code) assert.JSONEq(t, testCase.body, respSpy.Body.String()) @@ -123,3 +127,15 @@ func fixRequest(t *testing.T) *http.Request { return req } + +type captureWriter struct { + buf *bytes.Buffer +} + +func (c *captureWriter) Write(p []byte) (n int, err error) { + return c.buf.Write(p) +} + +func (c *captureWriter) Reset() { + c.buf.Reset() +} diff --git a/internal/suspension/handler.go b/internal/suspension/handler.go index 3d9f5d95a4..231a7b9d6a 100644 --- a/internal/suspension/handler.go +++ b/internal/suspension/handler.go @@ -2,6 +2,7 @@ package suspension import ( "fmt" + "log/slog" "net/http" "github.com/google/uuid" @@ -12,7 +13,6 @@ import ( "github.com/kyma-project/kyma-environment-broker/internal/storage/dberr" "github.com/pivotal-cf/brokerapi/v8/domain" "github.com/pivotal-cf/brokerapi/v8/domain/apiresponses" - "github.com/sirupsen/logrus" ) type ContextUpdateHandler struct { @@ -20,14 +20,14 @@ type ContextUpdateHandler struct { provisioningQueue Adder deprovisioningQueue Adder - log logrus.FieldLogger + log *slog.Logger } type Adder interface { Add(processId string) } -func NewContextUpdateHandler(operations storage.Operations, provisioningQueue Adder, deprovisioningQueue Adder, l logrus.FieldLogger) *ContextUpdateHandler { +func NewContextUpdateHandler(operations storage.Operations, provisioningQueue Adder, deprovisioningQueue Adder, l *slog.Logger) *ContextUpdateHandler { return &ContextUpdateHandler{ operations: operations, provisioningQueue: provisioningQueue, @@ -39,11 +39,11 @@ func NewContextUpdateHandler(operations storage.Operations, provisioningQueue Ad // Handle performs suspension/unsuspension for given instance. // Applies only when 'Active' parameter has changes and ServicePlanID is `Trial` func (h *ContextUpdateHandler) Handle(instance *internal.Instance, newCtx internal.ERSContext) (bool, error) { - l := h.log.WithFields(logrus.Fields{ - "instanceID": instance.InstanceID, - "runtimeID": instance.RuntimeID, - "globalAccountID": instance.GlobalAccountID, - }) + l := h.log.With( + "instanceID", instance.InstanceID, + "runtimeID", instance.RuntimeID, + "globalAccountID", instance.GlobalAccountID, + ) if !broker.IsTrialPlan(instance.ServicePlanID) { l.Info("Context update for non-trial instance, skipping") @@ -53,7 +53,7 @@ func (h *ContextUpdateHandler) Handle(instance *internal.Instance, newCtx intern return h.handleContextChange(newCtx, instance, l) } -func (h *ContextUpdateHandler) handleContextChange(newCtx internal.ERSContext, instance *internal.Instance, l logrus.FieldLogger) (bool, error) { +func (h *ContextUpdateHandler) handleContextChange(newCtx internal.ERSContext, instance *internal.Instance, l *slog.Logger) (bool, error) { isActivated := true if instance.Parameters.ErsContext.Active != nil { isActivated = *instance.Parameters.ErsContext.Active @@ -66,20 +66,20 @@ func (h *ContextUpdateHandler) handleContextChange(newCtx internal.ERSContext, i } if newCtx.Active == nil || isActivated == *newCtx.Active { - l.Debugf("Context.Active flag was not changed, the current value: %v", isActivated) + l.Debug(fmt.Sprintf("Context.Active flag was not changed, the current value: %v", isActivated)) if isActivated { // instance is marked as Active and incoming context update is unsuspension // TODO: consider retriggering failed unsuspension here - l.Infof("Context.Active flag is true - not triggering suspension for instance ID %s", instance.InstanceID) + l.Info(fmt.Sprintf("Context.Active flag is true - not triggering suspension for instance ID %s", instance.InstanceID)) return false, nil } if !isActivated { // instance is inactive and incoming context update is suspension - verify if KEB should retrigger the operation if lastDeprovisioning.State == domain.Failed { - l.Infof("triggering suspension again for instance id %s", instance.InstanceID) + l.Info(fmt.Sprintf("triggering suspension again for instance id %s", instance.InstanceID)) return true, h.suspend(instance, l) } - l.Infof("last deprovisioning is not in Failed state - not triggering suspension for instance ID %s", instance.InstanceID) + l.Info(fmt.Sprintf("last deprovisioning is not in Failed state - not triggering suspension for instance ID %s", instance.InstanceID)) return false, nil } } @@ -90,7 +90,7 @@ func (h *ContextUpdateHandler) handleContextChange(newCtx internal.ERSContext, i return false, nil } if lastDeprovisioning != nil && !lastDeprovisioning.Temporary { - l.Infof("Instance has a deprovisioning operation %s (%s), skipping unsuspension.", lastDeprovisioning.ID, lastDeprovisioning.State) + l.Info(fmt.Sprintf("Instance has a deprovisioning operation %s (%s), skipping unsuspension.", lastDeprovisioning.ID, lastDeprovisioning.State)) return false, nil } if lastDeprovisioning != nil && lastDeprovisioning.State == domain.Failed { @@ -103,7 +103,7 @@ func (h *ContextUpdateHandler) handleContextChange(newCtx internal.ERSContext, i } } -func (h *ContextUpdateHandler) suspend(instance *internal.Instance, log logrus.FieldLogger) error { +func (h *ContextUpdateHandler) suspend(instance *internal.Instance, log *slog.Logger) error { lastDeprovisioning, err := h.operations.GetDeprovisioningOperationByInstanceID(instance.InstanceID) // there was an error - fail if err != nil && !dberr.IsNotFound(err) { @@ -112,7 +112,7 @@ func (h *ContextUpdateHandler) suspend(instance *internal.Instance, log logrus.F // no error, operation exists and is in progress if err == nil && (lastDeprovisioning.State == domain.InProgress || lastDeprovisioning.State == orchestration.Pending) { - log.Infof("Suspension already started") + log.Info("Suspension already started") return nil } @@ -126,7 +126,7 @@ func (h *ContextUpdateHandler) suspend(instance *internal.Instance, log logrus.F return nil } -func (h *ContextUpdateHandler) unsuspend(instance *internal.Instance, log logrus.FieldLogger) error { +func (h *ContextUpdateHandler) unsuspend(instance *internal.Instance, log *slog.Logger) error { if instance.IsExpired() { log.Info("Expired instance cannot be unsuspended") return nil @@ -139,11 +139,11 @@ func (h *ContextUpdateHandler) unsuspend(instance *internal.Instance, log logrus operation.KimDeprovisionsOnly = nil if err != nil { - h.log.Errorf("unable to extract shoot name: %s", err.Error()) + h.log.Error(fmt.Sprintf("unable to extract shoot name: %s", err.Error())) return err } operation.State = orchestration.Pending - log.Infof("Starting unsuspension: shootName=%s shootDomain=%s", operation.ShootName, operation.ShootDomain) + log.Info(fmt.Sprintf("Starting unsuspension: shootName=%s shootDomain=%s", operation.ShootName, operation.ShootDomain)) // RuntimeID must be cleaned - this mean that there is no runtime in the provisioner/director operation.RuntimeID = "" operation.DashboardURL = instance.DashboardURL diff --git a/internal/suspension/handler_test.go b/internal/suspension/handler_test.go index 86185b3e73..a4b7bc3299 100644 --- a/internal/suspension/handler_test.go +++ b/internal/suspension/handler_test.go @@ -1,6 +1,8 @@ package suspension import ( + "log/slog" + "os" "testing" "time" @@ -14,7 +16,6 @@ import ( "github.com/kyma-project/kyma-environment-broker/internal/ptr" "github.com/kyma-project/kyma-environment-broker/internal/storage" "github.com/pivotal-cf/brokerapi/v8/domain" - "github.com/sirupsen/logrus" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) @@ -25,7 +26,7 @@ func TestSuspension(t *testing.T) { deprovisioning := NewDummyQueue() st := storage.NewMemoryStorage() - svc := NewContextUpdateHandler(st.Operations(), provisioning, deprovisioning, logrus.New()) + svc := NewContextUpdateHandler(st.Operations(), provisioning, deprovisioning, fixLogger()) instance := fixInstance(fixActiveErsContext()) err := st.Instances().Insert(*instance) require.NoError(t, err) @@ -51,7 +52,7 @@ func TestSuspension_Retrigger(t *testing.T) { deprovisioning := NewDummyQueue() st := storage.NewMemoryStorage() - svc := NewContextUpdateHandler(st.Operations(), provisioning, deprovisioning, logrus.New()) + svc := NewContextUpdateHandler(st.Operations(), provisioning, deprovisioning, fixLogger()) instance := fixInstance(fixInactiveErsContext()) err := st.Instances().Insert(*instance) require.NoError(t, err) @@ -89,7 +90,7 @@ func TestSuspension_Retrigger(t *testing.T) { deprovisioning := NewDummyQueue() st := storage.NewMemoryStorage() - svc := NewContextUpdateHandler(st.Operations(), provisioning, deprovisioning, logrus.New()) + svc := NewContextUpdateHandler(st.Operations(), provisioning, deprovisioning, fixLogger()) instance := fixInstance(fixInactiveErsContext()) err := st.Instances().Insert(*instance) require.NoError(t, err) @@ -138,7 +139,7 @@ func TestUnsuspension(t *testing.T) { deprovisioning := NewDummyQueue() st := storage.NewMemoryStorage() - svc := NewContextUpdateHandler(st.Operations(), provisioning, deprovisioning, logrus.New()) + svc := NewContextUpdateHandler(st.Operations(), provisioning, deprovisioning, fixLogger()) instance := fixInstance(fixInactiveErsContext()) instance.InstanceDetails.ShootName = "c-012345" instance.InstanceDetails.ShootDomain = "c-012345.sap.com" @@ -174,7 +175,7 @@ func TestUnsuspensionForDeprovisioningInstance(t *testing.T) { deprovisioning := NewDummyQueue() st := storage.NewMemoryStorage() - svc := NewContextUpdateHandler(st.Operations(), provisioning, deprovisioning, logrus.New()) + svc := NewContextUpdateHandler(st.Operations(), provisioning, deprovisioning, fixLogger()) instance := fixInstance(fixInactiveErsContext()) instance.InstanceDetails.ShootName = "c-012345" instance.InstanceDetails.ShootDomain = "c-012345.sap.com" @@ -204,7 +205,7 @@ func TestUnsuspensionForExpiredInstance(t *testing.T) { deprovisioning := NewDummyQueue() st := storage.NewMemoryStorage() - svc := NewContextUpdateHandler(st.Operations(), provisioning, deprovisioning, logrus.New()) + svc := NewContextUpdateHandler(st.Operations(), provisioning, deprovisioning, fixLogger()) instance := fixInstance(fixInactiveErsContext()) instance.InstanceDetails.ShootName = "c-012345" instance.InstanceDetails.ShootDomain = "c-012345.sap.com" @@ -256,3 +257,9 @@ func NewDummyQueue() *dummyQueue { func (q *dummyQueue) Add(id string) { q.IDs = append(q.IDs, id) } + +func fixLogger() *slog.Logger { + return slog.New(slog.NewTextHandler(os.Stdout, &slog.HandlerOptions{ + Level: slog.LevelInfo, + })) +}