From 07fbb8b8f7d7e8747461996f0b8fbe942d2f974f Mon Sep 17 00:00:00 2001 From: Harshavardhana Date: Fri, 4 Jun 2021 11:35:55 -0700 Subject: [PATCH] rewrite logging in console (#788) - enhance logging throughout the codebase - all packages at pkg/ should never log or perform log.Fatal() instead packages should return errors through functions. - simplified various user, group mapping and removed redundant functions. - deprecate older flags like --tls-certificate --tls-key and --tls-ca as we do not use them anymore, keep them for backward compatibility for some time. --- cmd/console/server.go | 127 ++++++++++---------- pkg/auth/idp/oauth2/provider.go | 24 ++-- pkg/auth/operator.go | 17 +-- pkg/auth/operator_test.go | 12 +- pkg/auth/token.go | 19 +-- pkg/certs/certs.go | 14 +-- restapi/admin_arns.go | 2 +- restapi/admin_config.go | 6 +- restapi/admin_console.go | 8 +- restapi/admin_groups.go | 38 +++--- restapi/admin_groups_test.go | 8 +- restapi/admin_heal.go | 15 +-- restapi/admin_health_info.go | 2 - restapi/admin_info.go | 81 +++++-------- restapi/admin_notification_endpoints.go | 4 +- restapi/admin_parity.go | 4 +- restapi/admin_policies.go | 39 +++---- restapi/admin_profiling.go | 15 +-- restapi/admin_remote_buckets.go | 46 ++++---- restapi/admin_service.go | 2 +- restapi/admin_subscription.go | 9 +- restapi/admin_tenants.go | 26 ++--- restapi/admin_tenants_helper.go | 9 +- restapi/admin_tiers.go | 8 +- restapi/admin_trace.go | 8 +- restapi/admin_users.go | 102 ++++++++-------- restapi/client-admin.go | 2 +- restapi/configure_console.go | 17 +-- restapi/error.go | 7 +- restapi/integrations.go | 4 +- restapi/server.go | 149 +++++++++++++++--------- restapi/user_account.go | 4 +- restapi/user_bucket_quota.go | 4 +- restapi/user_buckets.go | 26 ++--- restapi/user_log_search.go | 21 ++-- restapi/user_log_search_test.go | 3 +- restapi/user_login.go | 9 +- restapi/user_objects.go | 16 +-- restapi/user_service_accounts.go | 6 +- restapi/user_watch.go | 9 +- restapi/ws_handle.go | 41 +++---- 41 files changed, 456 insertions(+), 507 deletions(-) diff --git a/cmd/console/server.go b/cmd/console/server.go index 195473468..2b4e2b37f 100644 --- a/cmd/console/server.go +++ b/cmd/console/server.go @@ -20,10 +20,7 @@ import ( "context" "fmt" "io/ioutil" - "log" - "os" "path/filepath" - "strconv" "time" "github.com/go-openapi/loads" @@ -38,18 +35,18 @@ import ( var serverCmd = cli.Command{ Name: "server", Aliases: []string{"srv"}, - Usage: "starts Console server", + Usage: "Start MinIO Console server", Action: StartServer, Flags: []cli.Flag{ cli.StringFlag{ Name: "host", Value: restapi.GetHostname(), - Usage: "hostname", + Usage: "bind to a specific HOST, HOST can be an IP or hostname", }, cli.IntFlag{ Name: "port", Value: restapi.GetPort(), - Usage: "HTTP port", + Usage: "bind to specific HTTP port", }, // This is kept here for backward compatibility, // hostname's do not have HTTP or HTTPs @@ -58,13 +55,17 @@ var serverCmd = cli.Command{ cli.StringFlag{ Name: "tls-host", Value: restapi.GetHostname(), - Usage: "HTTPS hostname", Hidden: true, }, + cli.StringFlag{ + Name: "certs-dir", + Value: certs.GlobalCertsCADir.Get(), + Usage: "path to certs directory", + }, cli.IntFlag{ Name: "tls-port", Value: restapi.GetTLSPort(), - Usage: "HTTPS port", + Usage: "bind to specific HTTPS port", }, cli.StringFlag{ Name: "tls-redirect", @@ -72,38 +73,34 @@ var serverCmd = cli.Command{ Usage: "toggle HTTP->HTTPS redirect", }, cli.StringFlag{ - Name: "certs-dir", - Value: certs.GlobalCertsCADir.Get(), - Usage: "path to certs directory", + Name: "tls-certificate", + Value: "", + Usage: "path to TLS public certificate", + Hidden: true, }, cli.StringFlag{ - Name: "tls-certificate", - Value: "", - Usage: "path to TLS public certificate", + Name: "tls-key", + Value: "", + Usage: "path to TLS private key", + Hidden: true, }, cli.StringFlag{ - Name: "tls-key", - Value: "", - Usage: "path to TLS private key", - }, - cli.StringFlag{ - Name: "tls-ca", - Value: "", - Usage: "path to TLS Certificate Authority", + Name: "tls-ca", + Value: "", + Usage: "path to TLS Certificate Authority", + Hidden: true, }, }, } -// StartServer starts the console service -func StartServer(ctx *cli.Context) error { +func buildServer() (*restapi.Server, error) { swaggerSpec, err := loads.Embedded(restapi.SwaggerJSON, restapi.FlatSwaggerJSON) if err != nil { - log.Fatalln(err) + return nil, err } api := operations.NewConsoleAPI(swaggerSpec) server := restapi.NewServer(api) - defer server.Shutdown() parser := flags.NewParser(server, flags.Default) parser.ShortDescription = "MinIO Console Server" @@ -114,33 +111,31 @@ func StartServer(ctx *cli.Context) error { for _, optsGroup := range api.CommandLineOptionsGroups { _, err := parser.AddGroup(optsGroup.ShortDescription, optsGroup.LongDescription, optsGroup.Options) if err != nil { - log.Fatalln(err) + return nil, err } } if _, err := parser.Parse(); err != nil { - code := 1 - if fe, ok := err.(*flags.Error); ok { - if fe.Type == flags.ErrHelp { - code = 0 - } - } - os.Exit(code) + return nil, err } - server.Host = ctx.String("host") - server.Port = ctx.Int("port") - restapi.Hostname = server.Host - restapi.Port = strconv.Itoa(server.Port) + return server, nil +} +func loadAllCerts(ctx *cli.Context) error { + var err error // Set all certs and CAs directories path - certs.GlobalCertsDir, _ = certs.NewConfigDirFromCtx(ctx, "certs-dir", certs.DefaultCertsDir.Get) - certs.GlobalCertsCADir = &certs.ConfigDir{Path: filepath.Join(certs.GlobalCertsDir.Get(), certs.CertsCADir)} - - // check if certs and CAs directories exists or can be created - if err := certs.MkdirAllIgnorePerm(certs.GlobalCertsCADir.Get()); err != nil { - log.Println(fmt.Sprintf("Unable to create certs CA directory at %s", certs.GlobalCertsCADir.Get())) + certs.GlobalCertsDir, _, err = certs.NewConfigDirFromCtx(ctx, "certs-dir", certs.DefaultCertsDir.Get) + if err != nil { + return err } + + certs.GlobalCertsCADir = &certs.ConfigDir{Path: filepath.Join(certs.GlobalCertsDir.Get(), certs.CertsCADir)} + // check if certs and CAs directories exists or can be created + if err = certs.MkdirAllIgnorePerm(certs.GlobalCertsCADir.Get()); err != nil { + return fmt.Errorf("unable to create certs CA directory at %s: with %w", certs.GlobalCertsCADir.Get(), err) + } + // load the certificates and the CAs restapi.GlobalRootCAs, restapi.GlobalPublicCerts, restapi.GlobalTLSCertsManager = certs.GetAllCertificatesAndCAs() @@ -153,7 +148,7 @@ func StartServer(ctx *cli.Context) error { if swaggerServerCertificate != "" && swaggerServerCertificateKey != "" { if err = certs.AddCertificate(context.Background(), restapi.GlobalTLSCertsManager, swaggerServerCertificate, swaggerServerCertificateKey); err != nil { - log.Fatalln(err) + return err } if x509Certs, err := certs.ParsePublicCertFile(swaggerServerCertificate); err == nil { restapi.GlobalPublicCerts = append(restapi.GlobalPublicCerts, x509Certs...) @@ -169,25 +164,40 @@ func StartServer(ctx *cli.Context) error { } } - if len(restapi.GlobalPublicCerts) > 0 { - // If TLS certificates are provided enforce the HTTPS schema, meaning console will redirect - // plain HTTP connections to HTTPS server - server.EnabledListeners = []string{"http", "https"} - server.TLSPort = ctx.Int("tls-port") - // Need to store tls-port, tls-host un config variables so secure.middleware can read from there - restapi.TLSPort = strconv.Itoa(server.TLSPort) - restapi.Hostname = ctx.String("host") - restapi.TLSRedirect = ctx.String("tls-redirect") + return nil +} + +// StartServer starts the console service +func StartServer(ctx *cli.Context) error { + if err := loadAllCerts(ctx); err != nil { + restapi.LogError("Unable to load certs: %v", err) + return err } - server.ConfigureAPI() + var rctx restapi.Context + if err := rctx.Load(ctx); err != nil { + restapi.LogError("argument validation failed: %v", err) + return err + } + + server, err := buildServer() + if err != nil { + restapi.LogError("Unable to initialize console server: %v", err) + return err + } + + s := server.Configure(rctx) + defer s.Shutdown() // subnet license refresh process go func() { + // start refreshing subnet license after 5 seconds.. + time.Sleep(time.Second * 5) + failedAttempts := 0 for { if err := restapi.RefreshLicense(); err != nil { - log.Println(err) + restapi.LogError("Refreshing subnet license failed: %v", err) failedAttempts++ // end license refresh after 3 consecutive failed attempts if failedAttempts >= 3 { @@ -204,8 +214,5 @@ func StartServer(ctx *cli.Context) error { } }() - if err := server.Serve(); err != nil { - log.Fatalln(err) - } - return nil + return s.Serve() } diff --git a/pkg/auth/idp/oauth2/provider.go b/pkg/auth/idp/oauth2/provider.go index d381e7aec..ca10c6cea 100644 --- a/pkg/auth/idp/oauth2/provider.go +++ b/pkg/auth/idp/oauth2/provider.go @@ -22,7 +22,6 @@ import ( "encoding/base64" "errors" "fmt" - "log" "net/http" "net/url" "strconv" @@ -37,10 +36,6 @@ import ( xoauth2 "golang.org/x/oauth2" ) -var ( - errGeneric = errors.New("an error occurred, please try again") -) - type Configuration interface { Exchange(ctx context.Context, code string, opts ...xoauth2.AuthCodeOption) (*xoauth2.Token, error) AuthCodeURL(state string, opts ...xoauth2.AuthCodeOption) string @@ -168,8 +163,8 @@ type User struct { // VerifyIdentity will contact the configured IDP and validate the user identity based on the authorization code func (client *Provider) VerifyIdentity(ctx context.Context, code, state string) (*credentials.Credentials, error) { // verify the provided state is valid (prevents CSRF attacks) - if !validateOauth2State(state) { - return nil, errGeneric + if err := validateOauth2State(state); err != nil { + return nil, err } getWebTokenExpiry := func() (*credentials.WebIdentityToken, error) { oauth2Token, err := client.oauth2Config.Exchange(ctx, code) @@ -210,29 +205,30 @@ func (client *Provider) VerifyIdentity(ctx context.Context, code, state string) // validateOauth2State validates the provided state was originated using the same // instance (or one configured using the same secrets) of Console, this is basically used to prevent CSRF attacks // https://security.stackexchange.com/questions/20187/oauth2-cross-site-request-forgery-and-state-parameter -func validateOauth2State(state string) bool { +func validateOauth2State(state string) error { // state contains a base64 encoded string that may ends with "==", the browser encodes that to "%3D%3D" // query unescape is need it before trying to decode the base64 string encodedMessage, err := url.QueryUnescape(state) if err != nil { - log.Println(err) - return false + return err } // decode the state parameter value message, err := base64.StdEncoding.DecodeString(encodedMessage) if err != nil { - log.Println(err) - return false + return err } s := strings.Split(string(message), ":") // Validate that the decoded message has the right format "message:hmac" if len(s) != 2 { - return false + return fmt.Errorf("invalid number of tokens, expected only 2, got %d instead", len(s)) } // extract the state and hmac incomingState, incomingHmac := s[0], s[1] // validate that hmac(incomingState + pbkdf2(secret, salt)) == incomingHmac - return utils.ComputeHmac256(incomingState, derivedKey) == incomingHmac + if calculatedHmac := utils.ComputeHmac256(incomingState, derivedKey); calculatedHmac != incomingHmac { + return fmt.Errorf("oauth2 state is invalid, expected %s, got %s", calculatedHmac, incomingHmac) + } + return nil } // GetRandomStateWithHMAC computes message + hmac(message, pbkdf2(key, salt)) to be used as state during the oauth authorization diff --git a/pkg/auth/operator.go b/pkg/auth/operator.go index 9d1226b0b..456835294 100644 --- a/pkg/auth/operator.go +++ b/pkg/auth/operator.go @@ -18,7 +18,6 @@ package auth import ( "context" - "log" "github.com/minio/console/cluster" "github.com/minio/minio-go/v7/pkg/credentials" @@ -64,16 +63,12 @@ func (c *operatorClient) Authenticate(ctx context.Context) ([]byte, error) { return c.client.RESTClient().Verb("GET").RequestURI("/api").DoRaw(ctx) } -// isServiceAccountTokenValid will make an authenticated request against kubernetes api, if the +// checkServiceAccountTokenValid will make an authenticated request against kubernetes api, if the // request success means the provided jwt its a valid service account token and the console user can use it for future // requests until it expires -func isServiceAccountTokenValid(ctx context.Context, operatorClient OperatorClient) bool { +func checkServiceAccountTokenValid(ctx context.Context, operatorClient OperatorClient) error { _, err := operatorClient.Authenticate(ctx) - if err != nil { - log.Println(err) - return false - } - return true + return err } // GetConsoleCredentialsForOperator will validate the provided JWT (service account token) and return it in the form of credentials.Login @@ -86,8 +81,8 @@ func GetConsoleCredentialsForOperator(jwt string) (*credentials.Credentials, err opClient := &operatorClient{ client: opClientClientSet, } - if isServiceAccountTokenValid(ctx, opClient) { - return credentials.New(operatorCredentialsProvider{serviceAccountJWT: jwt}), nil + if err = checkServiceAccountTokenValid(ctx, opClient); err != nil { + return nil, errInvalidCredentials } - return nil, errInvalidCredentials + return credentials.New(operatorCredentialsProvider{serviceAccountJWT: jwt}), nil } diff --git a/pkg/auth/operator_test.go b/pkg/auth/operator_test.go index 1264ed202..c2533e1ad 100644 --- a/pkg/auth/operator_test.go +++ b/pkg/auth/operator_test.go @@ -20,8 +20,7 @@ func (c *operatorClientTest) Authenticate(ctx context.Context) ([]byte, error) { return operatorAuthenticateMock(ctx) } -func Test_isServiceAccountTokenValid(t *testing.T) { - +func Test_checkServiceAccountTokenValid(t *testing.T) { successResponse := func() { operatorAuthenticateMock = func(ctx context.Context) ([]byte, error) { return nil, nil @@ -70,12 +69,17 @@ func Test_isServiceAccountTokenValid(t *testing.T) { }, } for _, tt := range tests { + tt := tt t.Run(tt.name, func(t *testing.T) { if tt.args.mockFunction != nil { tt.args.mockFunction() } - if got := isServiceAccountTokenValid(tt.args.ctx, tt.args.operatorClient); got != tt.want { - t.Errorf("isServiceAccountTokenValid() = %v, want %v", got, tt.want) + got := checkServiceAccountTokenValid(tt.args.ctx, tt.args.operatorClient) + if got != nil && tt.want { + t.Errorf("checkServiceAccountTokenValid() = expected success but got %s", got) + } + if got == nil && !tt.want { + t.Error("checkServiceAccountTokenValid() = expected failure but got success") } }) } diff --git a/pkg/auth/token.go b/pkg/auth/token.go index 7da8fa04f..11a8f52c1 100644 --- a/pkg/auth/token.go +++ b/pkg/auth/token.go @@ -29,7 +29,6 @@ import ( "fmt" "io" "io/ioutil" - "log" "net/http" "strings" "time" @@ -48,8 +47,6 @@ var ( ErrNoAuthToken = errors.New("session token missing") errTokenExpired = errors.New("session token has expired") errReadingToken = errors.New("session token internal data is malformed") - errClaimsFormat = errors.New("encrypted session token claims not in the right format") - errorGeneric = errors.New("an error has occurred") ) // derivedKey is the key used to encrypt the session token claims, its derived using pbkdf on CONSOLE_PBKDF_PASSPHRASE with CONSOLE_PBKDF_SALT @@ -90,7 +87,6 @@ func SessionTokenAuthenticate(token string) (*TokenClaims, error) { claimTokens, err := decryptClaims(token) if err != nil { // we print decryption token error information for debugging purposes - log.Println(err) // we return a generic error that doesn't give any information to attackers return nil, errReadingToken } @@ -126,8 +122,7 @@ func encryptClaims(credentials *TokenClaims) (string, error) { } ciphertext, err := encrypt(payload, []byte{}) if err != nil { - log.Println(err) - return "", errorGeneric + return "", err } return base64.StdEncoding.EncodeToString(ciphertext), nil } @@ -136,19 +131,15 @@ func encryptClaims(credentials *TokenClaims) (string, error) { func decryptClaims(ciphertext string) (*TokenClaims, error) { decoded, err := base64.StdEncoding.DecodeString(ciphertext) if err != nil { - log.Println(err) - return nil, errClaimsFormat + return nil, err } plaintext, err := decrypt(decoded, []byte{}) if err != nil { - log.Println(err) - return nil, errClaimsFormat + return nil, err } tokenClaims := &TokenClaims{} - err = json.Unmarshal(plaintext, tokenClaims) - if err != nil { - log.Println(err) - return nil, errClaimsFormat + if err = json.Unmarshal(plaintext, tokenClaims); err != nil { + return nil, err } return tokenClaims, nil } diff --git a/pkg/certs/certs.go b/pkg/certs/certs.go index cf4358e96..630e87e8c 100644 --- a/pkg/certs/certs.go +++ b/pkg/certs/certs.go @@ -131,7 +131,7 @@ func MkdirAllIgnorePerm(path string) error { return err } -func NewConfigDirFromCtx(ctx *cli.Context, option string, getDefaultDir func() string) (*ConfigDir, bool) { +func NewConfigDirFromCtx(ctx *cli.Context, option string, getDefaultDir func() string) (*ConfigDir, bool, error) { var dir string var dirSet bool @@ -155,23 +155,23 @@ func NewConfigDirFromCtx(ctx *cli.Context, option string, getDefaultDir func() s // default directory. dir = getDefaultDir() if dir == "" { - log.Fatalln(fmt.Sprintf("invalid arguments specified, %s option must be provided", option)) + return nil, false, fmt.Errorf("invalid arguments specified, %s option must be provided", option) } } if dir == "" { - log.Fatalln(fmt.Sprintf("empty directory, %s directory cannot be empty", option)) + return nil, false, fmt.Errorf("empty directory, %s directory cannot be empty", option) } // Disallow relative paths, figure out absolute paths. dirAbs, err := filepath.Abs(dir) if err != nil { - log.Fatalf("%s: Unable to fetch absolute path for %s=%s", err, option, dir) + return nil, false, fmt.Errorf("%w: Unable to fetch absolute path for %s=%s", err, option, dir) } if err = MkdirAllIgnorePerm(dirAbs); err != nil { - log.Fatalf("%s: Unable to create directory specified %s=%s", err, option, dir) + return nil, false, fmt.Errorf("%w: Unable to create directory specified %s=%s", err, option, dir) } - return &ConfigDir{Path: dirAbs}, dirSet + return &ConfigDir{Path: dirAbs}, dirSet, nil } func getPublicCertFile() string { @@ -307,7 +307,7 @@ func GetTLSConfig() (x509Certs []*x509.Certificate, manager *xcerts.Manager, err continue } if err = manager.AddCertificate(certFile, keyFile); err != nil { - log.Fatalln(fmt.Errorf("unable to load TLS certificate '%s,%s': %w", certFile, keyFile, err)) + return nil, nil, fmt.Errorf("unable to load TLS certificate '%s,%s': %w", certFile, keyFile, err) } } return x509Certs, manager, nil diff --git a/restapi/admin_arns.go b/restapi/admin_arns.go index 5f7647883..1a53beeb7 100644 --- a/restapi/admin_arns.go +++ b/restapi/admin_arns.go @@ -52,7 +52,7 @@ func getArns(ctx context.Context, client MinioAdmin) (*models.ArnsResponse, erro // getArnsResponse returns a list of active arns in the instance func getArnsResponse(session *models.Principal) (*models.ArnsResponse, *models.Error) { - mAdmin, err := newMAdminClient(session) + mAdmin, err := newAdminClient(session) if err != nil { return nil, prepareError(err) } diff --git a/restapi/admin_config.go b/restapi/admin_config.go index dfd9723dc..615b85035 100644 --- a/restapi/admin_config.go +++ b/restapi/admin_config.go @@ -77,7 +77,7 @@ func listConfig(client MinioAdmin) ([]*models.ConfigDescription, error) { // getListConfigResponse performs listConfig() and serializes it to the handler's output func getListConfigResponse(session *models.Principal) (*models.ListConfigResponse, *models.Error) { - mAdmin, err := newMAdminClient(session) + mAdmin, err := newAdminClient(session) if err != nil { return nil, prepareError(err) } @@ -125,7 +125,7 @@ func getConfig(ctx context.Context, client MinioAdmin, name string) ([]*models.C // getConfigResponse performs getConfig() and serializes it to the handler's output func getConfigResponse(session *models.Principal, params admin_api.ConfigInfoParams) (*models.Configuration, *models.Error) { ctx := context.Background() - mAdmin, err := newMAdminClient(session) + mAdmin, err := newAdminClient(session) if err != nil { return nil, prepareError(err) } @@ -177,7 +177,7 @@ func buildConfig(configName *string, kvs []*models.ConfigurationKV) *string { // setConfigResponse implements setConfig() to be used by handler func setConfigResponse(session *models.Principal, name string, configRequest *models.SetConfigRequest) (*models.SetConfigResponse, *models.Error) { - mAdmin, err := newMAdminClient(session) + mAdmin, err := newAdminClient(session) if err != nil { return nil, prepareError(err) } diff --git a/restapi/admin_console.go b/restapi/admin_console.go index 3c54a048b..37fb34060 100644 --- a/restapi/admin_console.go +++ b/restapi/admin_console.go @@ -19,8 +19,6 @@ package restapi import ( "context" "encoding/json" - "fmt" - "log" "strings" "time" @@ -52,21 +50,21 @@ func startConsoleLog(ctx context.Context, conn WSConn, client MinioAdmin) error return nil } if logInfo.Err != nil { - log.Println("error on console logs:", logInfo.Err) + LogError("error on console logs: %v", logInfo.Err) return logInfo.Err } // Serialize message to be sent bytes, err := json.Marshal(serializeConsoleLogInfo(&logInfo)) if err != nil { - fmt.Println("error on json.Marshal:", err) + LogError("error on json.Marshal: %v", err) return err } // Send Message through websocket connection err = conn.writeMessage(websocket.TextMessage, bytes) if err != nil { - log.Println("error writeMessage:", err) + LogError("error writeMessage: %v", err) return err } } diff --git a/restapi/admin_groups.go b/restapi/admin_groups.go index 602b6374f..9e34bc213 100644 --- a/restapi/admin_groups.go +++ b/restapi/admin_groups.go @@ -18,7 +18,6 @@ package restapi import ( "context" - "log" "github.com/go-openapi/errors" "github.com/go-openapi/runtime/middleware" @@ -71,19 +70,10 @@ func registerGroupsHandlers(api *operations.ConsoleAPI) { }) } -// listGroups calls MinIO server to list all groups names present on the server. -func listGroups(ctx context.Context, client MinioAdmin) (*[]string, error) { - groupList, err := client.listGroups(ctx) - if err != nil { - return nil, err - } - return &groupList, nil -} - // getListGroupsResponse performs listGroups() and serializes it to the handler's output func getListGroupsResponse(session *models.Principal) (*models.ListGroupsResponse, *models.Error) { ctx := context.Background() - mAdmin, err := newMAdminClient(session) + mAdmin, err := newAdminClient(session) if err != nil { return nil, prepareError(err) } @@ -91,15 +81,17 @@ func getListGroupsResponse(session *models.Principal) (*models.ListGroupsRespons // defining the client to be used adminClient := adminClient{client: mAdmin} - groups, err := listGroups(ctx, adminClient) + groups, err := adminClient.listGroups(ctx) if err != nil { return nil, prepareError(err) } + // serialize output listGroupsResponse := &models.ListGroupsResponse{ - Groups: *groups, - Total: int64(len(*groups)), + Groups: groups, + Total: int64(len(groups)), } + return listGroupsResponse, nil } @@ -115,7 +107,7 @@ func groupInfo(ctx context.Context, client MinioAdmin, group string) (*madmin.Gr // getGroupInfoResponse performs groupInfo() and serializes it to the handler's output func getGroupInfoResponse(session *models.Principal, params admin_api.GroupInfoParams) (*models.Group, *models.Error) { ctx := context.Background() - mAdmin, err := newMAdminClient(session) + mAdmin, err := newAdminClient(session) if err != nil { return nil, prepareError(err) } @@ -159,7 +151,7 @@ func getAddGroupResponse(session *models.Principal, params *models.AddGroupReque return prepareError(errGroupBodyNotInRequest) } - mAdmin, err := newMAdminClient(session) + mAdmin, err := newAdminClient(session) if err != nil { return prepareError(err) } @@ -194,11 +186,11 @@ func getRemoveGroupResponse(session *models.Principal, params admin_api.RemoveGr if params.Name == "" { return prepareError(errGroupNameNotInRequest) } - mAdmin, err := newMAdminClient(session) + mAdmin, err := newAdminClient(session) if err != nil { return prepareError(err) } - // create a MinIO Admin Client interface implementation + // createad a MinIO Admin Client interface implementation // defining the client to be used adminClient := adminClient{client: mAdmin} @@ -277,7 +269,7 @@ func getUpdateGroupResponse(session *models.Principal, params admin_api.UpdateGr expectedGroupUpdate := params.Body groupName := params.Name - mAdmin, err := newMAdminClient(session) + mAdmin, err := newAdminClient(session) if err != nil { return nil, prepareError(err) } @@ -307,27 +299,27 @@ func groupUpdate(ctx context.Context, client MinioAdmin, groupName string, expec // get current members and status groupDescription, err := groupInfo(ctx, client, groupName) if err != nil { - log.Println("error getting group info:", err) + LogInfo("error getting group info: %v", err) return nil, err } // update group members err = addOrDeleteMembers(ctx, client, groupDescription, expectedMembers) if err != nil { - log.Println("error updating group:", err) + LogInfo("error updating group: %v", err) return nil, err } // update group status only if different from current status if expectedStatus != groupDescription.Status { err = setGroupStatus(ctx, client, groupDescription.Name, expectedStatus) if err != nil { - log.Println("error updating group's status:", err) + LogInfo("error updating group's status: %v", err) return nil, err } } // return latest group info to verify that changes were applied correctly groupDescription, err = groupInfo(ctx, client, groupName) if err != nil { - log.Println("error getting group info:", err) + LogInfo("error getting group info: %v", err) return nil, err } return groupDescription, nil diff --git a/restapi/admin_groups_test.go b/restapi/admin_groups_test.go index 3a7ae4452..37fa6b35d 100644 --- a/restapi/admin_groups_test.go +++ b/restapi/admin_groups_test.go @@ -70,14 +70,14 @@ func TestListGroups(t *testing.T) { // get list Groups response this response should have Name, CreationDate, Size and Access // as part of of each Groups function := "listGroups()" - groupsList, err := listGroups(ctx, adminClient) + groupsList, err := adminClient.listGroups(ctx) if err != nil { t.Errorf("Failed on %s:, error occurred: %s", function, err.Error()) } // verify length of Groupss is correct - assert.Equal(len(mockGroupsList), len(*groupsList), fmt.Sprintf("Failed on %s: length of Groups's lists is not the same", function)) + assert.Equal(len(mockGroupsList), len(groupsList), fmt.Sprintf("Failed on %s: length of Groups's lists is not the same", function)) - for i, g := range *groupsList { + for i, g := range groupsList { assert.Equal(mockGroupsList[i], g) } @@ -85,7 +85,7 @@ func TestListGroups(t *testing.T) { minioListGroupsMock = func() ([]string, error) { return nil, errors.New("error") } - _, err = listGroups(ctx, adminClient) + _, err = adminClient.listGroups(ctx) if assert.Error(err) { assert.Equal("error", err.Error()) } diff --git a/restapi/admin_heal.go b/restapi/admin_heal.go index 04527a897..edd76f553 100644 --- a/restapi/admin_heal.go +++ b/restapi/admin_heal.go @@ -20,7 +20,6 @@ import ( "context" "encoding/json" "fmt" - "log" "net/http" "regexp" "strconv" @@ -115,11 +114,10 @@ func startHeal(ctx context.Context, conn WSConn, client MinioAdmin, hOpts *healO // Initialize heal healStart, _, err := client.heal(ctx, hOpts.BucketName, hOpts.Prefix, hOpts.HealOpts, "", hOpts.ForceStart, hOpts.ForceStop) if err != nil { - log.Println("error initializing healing:", err) + LogError("error initializing healing: %v", err) return err } if hOpts.ForceStop { - log.Println("heal stopped successfully") return nil } clientToken := healStart.ClientToken @@ -134,21 +132,20 @@ func startHeal(ctx context.Context, conn WSConn, client MinioAdmin, hOpts *healO default: _, res, err := client.heal(ctx, hOpts.BucketName, hOpts.Prefix, hOpts.HealOpts, clientToken, hOpts.ForceStart, hOpts.ForceStop) if err != nil { - log.Println("error on heal:", err) + LogError("error on heal: %v", err) return err } hs.writeStatus(&res, conn) if res.Summary == "finished" { - log.Println("heal finished") return nil } if res.Summary == "stopped" { - log.Println("heal stopped") return fmt.Errorf("heal had an error - %s", res.FailureDetail) } + time.Sleep(time.Second) } } @@ -160,7 +157,7 @@ func (h *healStatus) writeStatus(s *madmin.HealTaskStatus, conn WSConn) error { for _, item := range s.Items { err := h.updateStats(item) if err != nil { - fmt.Println("error on updateStats:", err) + LogError("error on updateStats: %v", err) return err } } @@ -168,13 +165,13 @@ func (h *healStatus) writeStatus(s *madmin.HealTaskStatus, conn WSConn) error { // Serialize message to be sent infoBytes, err := json.Marshal(h) if err != nil { - fmt.Println("error on json.Marshal:", err) + LogError("error on json.Marshal: %v", err) return err } // Send Message through websocket connection err = conn.writeMessage(websocket.TextMessage, infoBytes) if err != nil { - log.Println("error writeMessage:", err) + LogError("error writeMessage: %v", err) return err } return nil diff --git a/restapi/admin_health_info.go b/restapi/admin_health_info.go index cb584923c..6718d3cb2 100644 --- a/restapi/admin_health_info.go +++ b/restapi/admin_health_info.go @@ -19,7 +19,6 @@ package restapi import ( "context" "encoding/json" - "log" "net/http" "time" @@ -60,7 +59,6 @@ func startHealthInfo(ctx context.Context, conn WSConn, client MinioAdmin, deadli // Serialize message to be sent bytes, err := json.Marshal(healthInfo) if err != nil { - log.Println("error on json.Marshal:", err) return err } diff --git a/restapi/admin_info.go b/restapi/admin_info.go index 90d46a7fa..05d3d718f 100644 --- a/restapi/admin_info.go +++ b/restapi/admin_info.go @@ -21,7 +21,6 @@ import ( "encoding/json" "fmt" "io/ioutil" - "log" "net/http" "net/url" "regexp" @@ -791,7 +790,7 @@ func getAdminInfoResponse(session *models.Principal) (*models.AdminInfoResponse, prometheusURL := getPrometheusURL() if prometheusURL == "" { - mAdmin, err := newMAdminClient(session) + mAdmin, err := newAdminClient(session) if err != nil { return nil, prepareError(err) } @@ -842,6 +841,33 @@ func getAdminInfoResponse(session *models.Principal) (*models.AdminInfoResponse, return sessionResp, nil } +func unmarshalPrometheus(endpoint string, data interface{}) bool { + resp, err := http.Get(endpoint) + if err != nil { + LogError("Unable to fetch labels from prometheus %s, %v", endpoint, err) + return true + } + + body, err := ioutil.ReadAll(resp.Body) + resp.Body.Close() + if err != nil { + LogError("Unexpected error reading response from prometheus %s, %v", endpoint, err) + return true + } + + if resp.StatusCode != 200 { + LogError("Unexpected error from prometheus %s, %s (%s)", endpoint, string(body), resp.Status) + return true + } + + if err = json.Unmarshal(body, data); err != nil { + LogError("Unexpected error reading response from prometheus %s, %v", endpoint, err) + return true + } + + return false +} + func getAdminInfoWidgetResponse(params admin_api.DashboardWidgetDetailsParams) (*models.WidgetDetails, *models.Error) { prometheusURL := getPrometheusURL() prometheusJobID := getPrometheusJobID() @@ -852,30 +878,8 @@ func getAdminInfoWidgetResponse(params admin_api.DashboardWidgetDetailsParams) ( go func(lbl WidgetLabel) { endpoint := fmt.Sprintf("%s/api/v1/label/%s/values", prometheusURL, lbl.Name) - resp, err := http.Get(endpoint) - if err != nil { - log.Println(err) - return - } - if resp.StatusCode != 200 { - body, err := ioutil.ReadAll(resp.Body) - resp.Body.Close() - if err != nil { - log.Println(err) - return - } - log.Println(endpoint) - log.Println(resp.StatusCode) - log.Println(string(body)) - return - } - var response LabelResponse - jd := json.NewDecoder(resp.Body) - err = jd.Decode(&response) - resp.Body.Close() - if err != nil { - log.Println(err) + if unmarshalPrometheus(endpoint, &response) { return } @@ -945,33 +949,9 @@ LabelsWaitLoop: queryExpr = strings.Replace(queryExpr, "${jobid}", prometheusJobID, -1) endpoint := fmt.Sprintf("%s/api/v1/%s?query=%s%s", prometheusURL, apiType, url.QueryEscape(queryExpr), extraParamters) - resp, err := http.Get(endpoint) - if err != nil { - log.Println(err) - return - } - defer func() { - if err := resp.Body.Close(); err != nil { - log.Println(err) - } - }() - - if resp.StatusCode != 200 { - body, err := ioutil.ReadAll(resp.Body) - if err != nil { - log.Println(err) - return - } - log.Println(endpoint) - log.Println(resp.StatusCode) - log.Println(string(body)) - return - } var response PromResp - jd := json.NewDecoder(resp.Body) - if err = jd.Decode(&response); err != nil { - log.Println(err) + if unmarshalPrometheus(endpoint, &response) { return } @@ -979,6 +959,7 @@ LabelsWaitLoop: LegendFormat: target.LegendFormat, ResultType: response.Data.ResultType, } + for _, r := range response.Data.Result { targetResult.Result = append(targetResult.Result, &models.WidgetResult{ Metric: r.Metric, diff --git a/restapi/admin_notification_endpoints.go b/restapi/admin_notification_endpoints.go index bb0372573..b5e27fe4c 100644 --- a/restapi/admin_notification_endpoints.go +++ b/restapi/admin_notification_endpoints.go @@ -77,7 +77,7 @@ func getNotificationEndpoints(ctx context.Context, client MinioAdmin) (*models.N // getNotificationEndpointsResponse returns a list of notification endpoints in the instance func getNotificationEndpointsResponse(session *models.Principal) (*models.NotifEndpointResponse, *models.Error) { - mAdmin, err := newMAdminClient(session) + mAdmin, err := newAdminClient(session) if err != nil { return nil, prepareError(err) } @@ -148,7 +148,7 @@ func addNotificationEndpoint(ctx context.Context, client MinioAdmin, params *adm // getNotificationEndpointsResponse returns a list of notification endpoints in the instance func getAddNotificationEndpointResponse(session *models.Principal, params *admin_api.AddNotificationEndpointParams) (*models.SetNotificationEndpointResponse, *models.Error) { - mAdmin, err := newMAdminClient(session) + mAdmin, err := newAdminClient(session) if err != nil { return nil, prepareError(err) } diff --git a/restapi/admin_parity.go b/restapi/admin_parity.go index fefd3bf43..dcfca9c53 100644 --- a/restapi/admin_parity.go +++ b/restapi/admin_parity.go @@ -18,7 +18,6 @@ package restapi import ( "fmt" - "log" "github.com/minio/console/pkg/utils" @@ -53,9 +52,8 @@ func getParityResponse(params admin_api.GetParityParams) (models.ParityResponse, disksPerNode := params.DisksPerNode parityValues, err := GetParityInfo(nodes, disksPerNode) - if err != nil { - log.Println("error getting parity info:", err) + LogError("error getting parity info: %v", err) return nil, prepareError(err) } diff --git a/restapi/admin_policies.go b/restapi/admin_policies.go index 19970ed78..69ee9daa6 100644 --- a/restapi/admin_policies.go +++ b/restapi/admin_policies.go @@ -20,7 +20,7 @@ import ( "bytes" "context" "encoding/json" - "log" + "sort" "github.com/go-openapi/runtime/middleware" "github.com/minio/console/models" @@ -93,7 +93,7 @@ func registersPoliciesHandler(api *operations.ConsoleAPI) { func getListPoliciesWithBucketResponse(session *models.Principal, bucket string) (*models.ListPoliciesResponse, *models.Error) { ctx := context.Background() - mAdmin, err := newMAdminClient(session) + mAdmin, err := newAdminClient(session) if err != nil { return nil, prepareError(err) } @@ -171,7 +171,7 @@ func listPolicies(ctx context.Context, client MinioAdmin) ([]*models.Policy, err // getListPoliciesResponse performs listPolicies() and serializes it to the handler's output func getListPoliciesResponse(session *models.Principal) (*models.ListPoliciesResponse, *models.Error) { ctx := context.Background() - mAdmin, err := newMAdminClient(session) + mAdmin, err := newAdminClient(session) if err != nil { return nil, prepareError(err) } @@ -194,7 +194,7 @@ func getListPoliciesResponse(session *models.Principal) (*models.ListPoliciesRes // getListUsersForPoliciesResponse performs lists users affected by a given policy. func getListUsersForPolicyResponse(session *models.Principal, policy string) ([]string, *models.Error) { ctx := context.Background() - mAdmin, err := newMAdminClient(session) + mAdmin, err := newAdminClient(session) if err != nil { return nil, prepareError(err) } @@ -206,20 +206,18 @@ func getListUsersForPolicyResponse(session *models.Principal, policy string) ([] if err != nil { return nil, prepareError(err) } - userArray := []string{} - for i := 0; i < len(users); i++ { - if err == nil { - for j := 0; j < len(users[i].Policy); j++ { - if users[i].Policy[j] == policy { - userArray = append(userArray, users[i].AccessKey) - break - } + + var filteredUsers []string + for _, user := range users { + for _, upolicy := range user.Policy { + if upolicy == policy { + filteredUsers = append(filteredUsers, user.AccessKey) + break } - } else { - log.Println(err) } } - return userArray, nil + sort.Strings(filteredUsers) + return filteredUsers, nil } // removePolicy() calls MinIO server to remove a policy based on name. @@ -237,7 +235,7 @@ func getRemovePolicyResponse(session *models.Principal, params admin_api.RemoveP if params.Name == "" { return prepareError(errPolicyNameNotInRequest) } - mAdmin, err := newMAdminClient(session) + mAdmin, err := newAdminClient(session) if err != nil { return prepareError(err) } @@ -274,11 +272,10 @@ func addPolicy(ctx context.Context, client MinioAdmin, name, policy string) (*mo func getAddPolicyResponse(session *models.Principal, params *models.AddPolicyRequest) (*models.Policy, *models.Error) { ctx := context.Background() if params == nil { - log.Println("error AddPolicy body not in request") return nil, prepareError(errPolicyBodyNotInRequest) } - mAdmin, err := newMAdminClient(session) + mAdmin, err := newAdminClient(session) if err != nil { return nil, prepareError(err) } @@ -311,7 +308,7 @@ func policyInfo(ctx context.Context, client MinioAdmin, name string) (*models.Po // getPolicyInfoResponse performs policyInfo() and serializes it to the handler's output func getPolicyInfoResponse(session *models.Principal, params admin_api.PolicyInfoParams) (*models.Policy, *models.Error) { ctx := context.Background() - mAdmin, err := newMAdminClient(session) + mAdmin, err := newAdminClient(session) if err != nil { return nil, prepareError(err) } @@ -343,7 +340,7 @@ func getSetPolicyResponse(session *models.Principal, name string, params *models if name == "" { return prepareError(errPolicyNameNotInRequest) } - mAdmin, err := newMAdminClient(session) + mAdmin, err := newAdminClient(session) if err != nil { return prepareError(err) } @@ -359,7 +356,7 @@ func getSetPolicyResponse(session *models.Principal, name string, params *models func getSetPolicyMultipleResponse(session *models.Principal, name string, params *models.SetPolicyMultipleRequest) *models.Error { ctx := context.Background() - mAdmin, err := newMAdminClient(session) + mAdmin, err := newAdminClient(session) if err != nil { return prepareError(err) } diff --git a/restapi/admin_profiling.go b/restapi/admin_profiling.go index 5c52c403e..09b79bd0a 100644 --- a/restapi/admin_profiling.go +++ b/restapi/admin_profiling.go @@ -19,7 +19,6 @@ package restapi import ( "context" "io" - "log" "net/http" "github.com/go-openapi/runtime" @@ -48,15 +47,11 @@ func registerProfilingHandler(api *operations.ConsoleAPI) { // Custom response writer to set the content-disposition header to tell the // HTTP client the name and extension of the file we are returning return middleware.ResponderFunc(func(w http.ResponseWriter, _ runtime.Producer) { + defer profilingStopResponse.Close() + w.Header().Set("Content-Type", "application/octet-stream") w.Header().Set("Content-Disposition", "attachment; filename=profile.zip") - if _, err := io.Copy(w, profilingStopResponse); err != nil { - log.Println(err) - } else { - if err := profilingStopResponse.Close(); err != nil { - log.Println(err) - } - } + io.Copy(w, profilingStopResponse) }) }) } @@ -93,7 +88,7 @@ func getProfilingStartResponse(session *models.Principal, params *models.Profili if params == nil { return nil, prepareError(errPolicyBodyNotInRequest) } - mAdmin, err := newMAdminClient(session) + mAdmin, err := newAdminClient(session) if err != nil { return nil, prepareError(err) } @@ -124,7 +119,7 @@ func stopProfiling(ctx context.Context, client MinioAdmin) (io.ReadCloser, error // getProfilingStopResponse() performs setPolicy() and serializes it to the handler's output func getProfilingStopResponse(session *models.Principal) (io.ReadCloser, *models.Error) { ctx := context.Background() - mAdmin, err := newMAdminClient(session) + mAdmin, err := newAdminClient(session) if err != nil { return nil, prepareError(err) } diff --git a/restapi/admin_remote_buckets.go b/restapi/admin_remote_buckets.go index 962616bdf..a7a9631ab 100644 --- a/restapi/admin_remote_buckets.go +++ b/restapi/admin_remote_buckets.go @@ -20,7 +20,6 @@ import ( "context" "errors" "fmt" - "log" "net/url" "strconv" "time" @@ -113,15 +112,15 @@ func registerAdminBucketRemoteHandlers(api *operations.ConsoleAPI) { func getListRemoteBucketsResponse(session *models.Principal) (*models.ListRemoteBucketsResponse, error) { ctx := context.Background() - mAdmin, err := newMAdminClient(session) + mAdmin, err := newAdminClient(session) if err != nil { - log.Println("error creating Madmin Client:", err) + LogError("error creating Madmin Client: %v", err) return nil, err } adminClient := adminClient{client: mAdmin} buckets, err := listRemoteBuckets(ctx, adminClient) if err != nil { - log.Println("error listing remote buckets:", err) + LogError("error listing remote buckets: %v", err) return nil, err } return &models.ListRemoteBucketsResponse{ @@ -132,15 +131,15 @@ func getListRemoteBucketsResponse(session *models.Principal) (*models.ListRemote func getRemoteBucketDetailsResponse(session *models.Principal, params user_api.RemoteBucketDetailsParams) (*models.RemoteBucket, error) { ctx := context.Background() - mAdmin, err := newMAdminClient(session) + mAdmin, err := newAdminClient(session) if err != nil { - log.Println("error creating Madmin Client:", err) + LogError("error creating Madmin Client: %v", err) return nil, err } adminClient := adminClient{client: mAdmin} bucket, err := getRemoteBucket(ctx, adminClient, params.Name) if err != nil { - log.Println("error getting remote bucket details:", err) + LogError("error getting remote bucket details: %v", err) return nil, err } return bucket, nil @@ -148,15 +147,15 @@ func getRemoteBucketDetailsResponse(session *models.Principal, params user_api.R func getDeleteRemoteBucketResponse(session *models.Principal, params user_api.DeleteRemoteBucketParams) error { ctx := context.Background() - mAdmin, err := newMAdminClient(session) + mAdmin, err := newAdminClient(session) if err != nil { - log.Println("error creating Madmin Client:", err) + LogError("error creating Madmin Client: %v", err) return err } adminClient := adminClient{client: mAdmin} err = deleteRemoteBucket(ctx, adminClient, params.SourceBucketName, params.Arn) if err != nil { - log.Println("error deleting remote bucket: ", err) + LogError("error deleting remote bucket: %v", err) return err } return err @@ -164,15 +163,15 @@ func getDeleteRemoteBucketResponse(session *models.Principal, params user_api.De func getAddRemoteBucketResponse(session *models.Principal, params user_api.AddRemoteBucketParams) error { ctx := context.Background() - mAdmin, err := newMAdminClient(session) + mAdmin, err := newAdminClient(session) if err != nil { - log.Println("error creating Madmin Client:", err) + LogError("error creating Madmin Client: %v", err) return err } adminClient := adminClient{client: mAdmin} _, err = addRemoteBucket(ctx, adminClient, *params.Body) if err != nil { - log.Println("error adding remote bucket: ", err) + LogError("error adding remote bucket: %v", err) return err } return err @@ -274,7 +273,7 @@ func addBucketReplicationItem(ctx context.Context, session *models.Principal, mi // we will tolerate this call failing cfg, err := minClient.getBucketReplication(ctx, bucketName) if err != nil { - log.Println("error versioning bucket:", err) + LogError("error fetching replication configuration for bucket %s: %v", bucketName, err) } // add rule @@ -288,7 +287,7 @@ func addBucketReplicationItem(ctx context.Context, session *models.Principal, mi s3Client, err := newS3BucketClient(session, bucketName, prefix) if err != nil { - log.Println("error creating S3Client:", err) + LogError("error creating S3Client: %v", err) return err } // create a mc S3Client interface implementation @@ -309,7 +308,6 @@ func addBucketReplicationItem(ctx context.Context, session *models.Principal, mi if repMeta { repMetaStatus = "enable" } - log.Println("repMetaStatus is not yet implemented", repMetaStatus) opts := replication.Options{ RoleArn: arn, @@ -320,12 +318,12 @@ func addBucketReplicationItem(ctx context.Context, session *models.Principal, mi TagString: tags, ReplicateDeleteMarkers: repDelMarkStatus, ReplicateDeletes: repDelsStatus, - //ReplicaSync: repMetaStatus, + ReplicaSync: repMetaStatus, } err2 := mcClient.setReplication(ctx, &cfg, opts) if err2 != nil { - log.Println("error creating replication for bucket:", err2.Cause) + LogError("error creating replication for bucket:", err2.Cause) return err2.Cause } return nil @@ -409,16 +407,16 @@ func setMultiBucketReplication(ctx context.Context, session *models.Principal, c func setMultiBucketReplicationResponse(session *models.Principal, params user_api.SetMultiBucketReplicationParams) (*models.MultiBucketResponseState, *models.Error) { ctx := context.Background() - mAdmin, err := newMAdminClient(session) + mAdmin, err := newAdminClient(session) if err != nil { - log.Println("error creating Madmin Client:", err) + LogError("error creating Madmin Client:", err) return nil, prepareError(err) } adminClient := adminClient{client: mAdmin} mClient, err := newMinioClient(session) if err != nil { - log.Println("error creating MinIO Client:", err) + LogError("error creating MinIO Client:", err) return nil, prepareError(err) } // create a minioClient interface implementation @@ -477,7 +475,7 @@ func listExternalBucketsResponse(params user_api.ListExternalBucketsParams) (*mo func deleteReplicationRule(ctx context.Context, session *models.Principal, bucketName, ruleID string) error { mClient, err := newMinioClient(session) if err != nil { - log.Println("error creating MinIO Client:", err) + LogError("error creating MinIO Client: %v", err) return err } // create a minioClient interface implementation @@ -486,12 +484,12 @@ func deleteReplicationRule(ctx context.Context, session *models.Principal, bucke cfg, err := minClient.getBucketReplication(ctx, bucketName) if err != nil { - log.Println("error versioning bucket:", err) + LogError("error versioning bucket: %v", err) } s3Client, err := newS3BucketClient(session, bucketName, "") if err != nil { - log.Println("error creating S3Client:", err) + LogError("error creating S3Client: %v", err) return err } // create a mc S3Client interface implementation diff --git a/restapi/admin_service.go b/restapi/admin_service.go index 2359687fe..2e119ac79 100644 --- a/restapi/admin_service.go +++ b/restapi/admin_service.go @@ -61,7 +61,7 @@ func serviceRestart(ctx context.Context, client MinioAdmin) error { // getRestartServiceResponse performs serviceRestart() func getRestartServiceResponse(session *models.Principal) *models.Error { ctx := context.Background() - mAdmin, err := newMAdminClient(session) + mAdmin, err := newAdminClient(session) if err != nil { return prepareError(err) } diff --git a/restapi/admin_subscription.go b/restapi/admin_subscription.go index 1353370e4..c1e615073 100644 --- a/restapi/admin_subscription.go +++ b/restapi/admin_subscription.go @@ -21,7 +21,6 @@ import ( "context" "errors" "fmt" - "log" "time" miniov2 "github.com/minio/operator/pkg/apis/minio.min.io/v2" @@ -190,7 +189,7 @@ func saveSubscriptionLicense(ctx context.Context, clientSet K8sClientI, license err := clientSet.deleteSecret(ctx, cluster.Namespace, OperatorSubnetLicenseSecretName, metav1.DeleteOptions{}) if err != nil { // log the error if any and continue - log.Println(err) + LogError("unable to delete secret %s: %v", OperatorSubnetLicenseSecretName, err) } // Save subnet license in k8s secrets imm := true @@ -225,7 +224,7 @@ func updateTenantLicenseAndRestartConsole(ctx context.Context, clientSet K8sClie err = clientSet.deleteSecret(ctx, namespace, consoleSecretName, metav1.DeleteOptions{}) if err != nil { // log the error if any and continue - log.Println(err) + LogError("unable to delete secret %s: %v", consoleSecretName, err) } // Save subnet license in k8s secrets imm := true @@ -305,7 +304,7 @@ func getSubscriptionLicense(ctx context.Context, clientSet K8sClientI, namespace } license, ok := licenseSecret.Data[ConsoleSubnetLicense] if !ok { - log.Println("subnet secret doesn't contain jwt license") + LogError("subnet secret does not contain a valid subnet license") return "", errorGeneric } return string(license), nil @@ -386,7 +385,7 @@ func getSubscriptionRefreshResponse(session *models.Principal) (*models.License, // iterate over all tenants, update console configuration and restart console pods for _, tenant := range tenants.Tenants { if err := updateTenantLicenseAndRestartConsole(ctx, &k8sClient, licenseRaw, tenant.Namespace, tenant.Name); err != nil { - log.Println(err) + LogError("unable to updateTenantLicenseAndRestartConsole: %v", err) } } diff --git a/restapi/admin_tenants.go b/restapi/admin_tenants.go index b32018480..1407f22b4 100644 --- a/restapi/admin_tenants.go +++ b/restapi/admin_tenants.go @@ -23,7 +23,6 @@ import ( "encoding/json" "errors" "fmt" - "log" "net" "net/http" "os" @@ -256,8 +255,7 @@ func GetTenantServiceURL(mi *miniov2.Tenant) (svcURL string) { scheme = "https" port = miniov2.MinIOTLSPortLoadBalancerSVC } - svc := fmt.Sprintf("%s.%s.svc.cluster.local", mi.MinIOCIServiceName(), mi.Namespace) - return fmt.Sprintf("%s://%s", scheme, net.JoinHostPort(svc, strconv.Itoa(port))) + return fmt.Sprintf("%s://%s", scheme, net.JoinHostPort(mi.MinIOFQDNServiceName(), strconv.Itoa(port))) } func getTenantAdminClient(ctx context.Context, client K8sClientI, tenant *miniov2.Tenant, svcURL string) (*madmin.AdminClient, error) { @@ -289,12 +287,12 @@ func getTenantCreds(ctx context.Context, client K8sClientI, tenant *miniov2.Tena } tenantAccessKey, ok := creds.Data["accesskey"] if !ok { - log.Println("tenant's secret doesn't contain accesskey") + LogError("tenant's secret doesn't contain accesskey") return nil, errorGeneric } tenantSecretKey, ok := creds.Data["secretkey"] if !ok { - log.Println("tenant's secret doesn't contain secretkey") + LogError("tenant's secret doesn't contain secretkey") return nil, errorGeneric } // TODO: @@ -405,7 +403,7 @@ func getTenantInfoResponse(session *models.Principal, params admin_api.TenantInf consoleSecret, err := clientSet.CoreV1().Secrets(minTenant.Namespace).Get(ctx, minTenant.Name, metav1.GetOptions{}) // we can tolerate not getting this secret if err != nil { - log.Println(err) + LogError("unable to fetch existing secrets for %s: %v", minTenant.Name, err) } if consoleSecret != nil { if _, ok := consoleSecret.Data["CONSOLE_IDP_URL"]; ok { @@ -449,13 +447,13 @@ func getTenantInfoResponse(session *models.Principal, params admin_api.TenantInf minSvc, err := k8sClient.getService(ctx, minTenant.Namespace, minTenant.MinIOCIServiceName(), metav1.GetOptions{}) if err != nil { // we can tolerate this error - log.Println(err) + LogError("Unable to get MinIO service name: %v, continuing", err) } //console service conSvc, err := k8sClient.getService(ctx, minTenant.Namespace, minTenant.ConsoleCIServiceName(), metav1.GetOptions{}) if err != nil { // we can tolerate this error - log.Println(err) + LogError("Unable to get MinIO console service name: %v, continuing", err) } schema := "http" @@ -649,13 +647,13 @@ func getTenantCreatedResponse(session *models.Principal, params admin_api.Create // delete secrets created if an error occurred during tenant creation, defer func() { if mError != nil { - log.Printf("deleting secrets created for failed tenant: %s if any\n", tenantName) + LogError("deleting secrets created for failed tenant: %s if any: %v", tenantName, mError) opts := metav1.ListOptions{ LabelSelector: fmt.Sprintf("%s=%s", miniov2.TenantLabel, tenantName), } err = clientSet.CoreV1().Secrets(ns).DeleteCollection(ctx, metav1.DeleteOptions{}, opts) if err != nil { - log.Println("error deleting tenant's secrets:", err) + LogError("error deleting tenant's secrets: %v", err) } } }() @@ -944,7 +942,7 @@ func getTenantCreatedResponse(session *models.Principal, params admin_api.Create for _, pool := range tenantReq.Pools { pool, err := parseTenantPoolRequest(pool) if err != nil { - log.Println("parseTenantPoolRequest", err) + LogError("parseTenantPoolRequest failed: %v", err) return nil, prepareError(err) } minInst.Spec.Pools = append(minInst.Spec.Pools, *pool) @@ -1085,7 +1083,7 @@ func getTenantCreatedResponse(session *models.Principal, params admin_api.Create _, err = opClient.MinioV2().Tenants(ns).Create(context.Background(), &minInst, metav1.CreateOptions{}) if err != nil { - log.Println("Create", err) + LogError("Creating new tenant failed with: %v", err) return nil, prepareError(err) } @@ -1188,7 +1186,7 @@ func updateTenantAction(ctx context.Context, operatorClient OperatorClientI, cli } else { // update the image pull secret content if _, err := setImageRegistry(ctx, imageRegistryReq, clientset, namespace, params.Tenant); err != nil { - log.Println("error setting image registry secret:", err) + LogError("error setting image registry secret: %v", err) return err } } @@ -1842,7 +1840,7 @@ func getTenantUpdatePoolResponse(session *models.Principal, params admin_api.Ten t, err := updateTenantPools(ctx, opClient, params.Namespace, params.Tenant, params.Body.Pools) if err != nil { - log.Println("error updating Tenant's pools:", err) + LogError("error updating Tenant's pools: %v", err) return nil, prepareError(err) } diff --git a/restapi/admin_tenants_helper.go b/restapi/admin_tenants_helper.go index d9101a533..a330ac658 100644 --- a/restapi/admin_tenants_helper.go +++ b/restapi/admin_tenants_helper.go @@ -22,7 +22,6 @@ import ( "encoding/base64" "encoding/hex" "fmt" - "log" "strconv" "time" @@ -249,7 +248,7 @@ func createOrReplaceSecrets(ctx context.Context, clientSet K8sClientI, ns string err := clientSet.deleteSecret(ctx, ns, secret.Name, metav1.DeleteOptions{}) if err != nil { // log the error if any and continue - log.Println(err) + LogError("deleting secret name %s failed: %v, continuing..", secret.Name, err) } imm := true k8sSecret := &corev1.Secret{ @@ -289,7 +288,7 @@ func createOrReplaceExternalCertSecrets(ctx context.Context, clientSet K8sClient err := clientSet.deleteSecret(ctx, ns, keyPairSecretName, metav1.DeleteOptions{}) if err != nil { // log the error if any and continue - log.Println(err) + LogError("deleting secret name %s failed: %v, continuing..", keyPairSecretName, err) } imm := true tlsCrt, err := base64.StdEncoding.DecodeString(*keyPair.Crt) @@ -331,12 +330,12 @@ func createOrReplaceKesConfigurationSecrets(ctx context.Context, clientSet K8sCl // delete KES configuration secret if exists if err := clientSet.deleteSecret(ctx, ns, kesConfigurationSecretName, metav1.DeleteOptions{}); err != nil { // log the error if any and continue - log.Println(err) + LogError("deleting secret name %s failed: %v, continuing..", kesConfigurationSecretName, err) } // delete KES client cert secret if exists if err := clientSet.deleteSecret(ctx, ns, kesClientCertSecretName, metav1.DeleteOptions{}); err != nil { // log the error if any and continue - log.Println(err) + LogError("deleting secret name %s failed: %v, continuing..", kesClientCertSecretName, err) } // if autoCert is enabled then Operator will generate the client certificates, calculate the client cert identity // and pass it to KES via the ${MINIO_KES_IDENTITY} variable diff --git a/restapi/admin_tiers.go b/restapi/admin_tiers.go index b0388f2c9..356f80a94 100644 --- a/restapi/admin_tiers.go +++ b/restapi/admin_tiers.go @@ -131,7 +131,7 @@ func getTiers(ctx context.Context, client MinioAdmin) (*models.TierListResponse, // getTiersResponse returns a response with a list of tiers func getTiersResponse(session *models.Principal) (*models.TierListResponse, *models.Error) { - mAdmin, err := newMAdminClient(session) + mAdmin, err := newAdminClient(session) if err != nil { return nil, prepareError(err) } @@ -221,7 +221,7 @@ func addTier(ctx context.Context, client MinioAdmin, params *admin_api.AddTierPa // getAddTierResponse returns the response of admin tier func getAddTierResponse(session *models.Principal, params *admin_api.AddTierParams) *models.Error { - mAdmin, err := newMAdminClient(session) + mAdmin, err := newAdminClient(session) if err != nil { return prepareError(err) } @@ -304,7 +304,7 @@ func getTier(ctx context.Context, client MinioAdmin, params *admin_api.GetTierPa // getGetTierResponse returns a tier func getGetTierResponse(session *models.Principal, params *admin_api.GetTierParams) (*models.Tier, *models.Error) { - mAdmin, err := newMAdminClient(session) + mAdmin, err := newAdminClient(session) if err != nil { return nil, prepareError(err) } @@ -340,7 +340,7 @@ func editTierCredentials(ctx context.Context, client MinioAdmin, params *admin_a // getEditTierCredentialsResponse returns the result of editing credentials for a tier func getEditTierCredentialsResponse(session *models.Principal, params *admin_api.EditTierCredentialsParams) *models.Error { - mAdmin, err := newMAdminClient(session) + mAdmin, err := newAdminClient(session) if err != nil { return prepareError(err) } diff --git a/restapi/admin_trace.go b/restapi/admin_trace.go index 4d0ebc638..cce9a476f 100644 --- a/restapi/admin_trace.go +++ b/restapi/admin_trace.go @@ -19,8 +19,6 @@ package restapi import ( "context" "encoding/json" - "fmt" - "log" "net/http" "strings" "time" @@ -68,19 +66,19 @@ func startTraceInfo(ctx context.Context, conn WSConn, client MinioAdmin, opts se return nil } if traceInfo.Err != nil { - log.Println("error on serviceTrace:", traceInfo.Err) + LogError("error on serviceTrace: %v", traceInfo.Err) return traceInfo.Err } // Serialize message to be sent traceInfoBytes, err := json.Marshal(shortTrace(&traceInfo)) if err != nil { - fmt.Println("error on json.Marshal:", err) + LogError("error on json.Marshal: %v", err) return err } // Send Message through websocket connection err = conn.writeMessage(websocket.TextMessage, traceInfoBytes) if err != nil { - log.Println("error writeMessage:", err) + LogError("error writeMessage: %v", err) return err } } diff --git a/restapi/admin_users.go b/restapi/admin_users.go index 99a14c5a8..c32b8bc07 100644 --- a/restapi/admin_users.go +++ b/restapi/admin_users.go @@ -19,7 +19,6 @@ package restapi import ( "context" "fmt" - "log" "sort" "strings" @@ -144,7 +143,7 @@ func listUsers(ctx context.Context, client MinioAdmin) ([]*models.User, error) { // getListUsersResponse performs listUsers() and serializes it to the handler's output func getListUsersResponse(session *models.Principal) (*models.ListUsersResponse, *models.Error) { ctx := context.Background() - mAdmin, err := newMAdminClient(session) + mAdmin, err := newAdminClient(session) if err != nil { return nil, prepareError(err) } @@ -190,7 +189,7 @@ func addUser(ctx context.Context, client MinioAdmin, accessKey, secretKey *strin func getUserAddResponse(session *models.Principal, params admin_api.AddUserParams) (*models.User, *models.Error) { ctx := context.Background() - mAdmin, err := newMAdminClient(session) + mAdmin, err := newAdminClient(session) if err != nil { return nil, prepareError(err) } @@ -216,7 +215,7 @@ func removeUser(ctx context.Context, client MinioAdmin, accessKey string) error func getRemoveUserResponse(session *models.Principal, params admin_api.RemoveUserParams) *models.Error { ctx := context.Background() - mAdmin, err := newMAdminClient(session) + mAdmin, err := newAdminClient(session) if err != nil { return prepareError(err) } @@ -233,7 +232,6 @@ func getRemoveUserResponse(session *models.Principal, params admin_api.RemoveUse return prepareError(err) } - log.Println("User removed successfully:", params.Name) return nil } @@ -250,7 +248,7 @@ func getUserInfo(ctx context.Context, client MinioAdmin, accessKey string) (*mad func getUserInfoResponse(session *models.Principal, params admin_api.GetUserInfoParams) (*models.User, *models.Error) { ctx := context.Background() - mAdmin, err := newMAdminClient(session) + mAdmin, err := newAdminClient(session) if err != nil { return nil, prepareError(err) } @@ -365,7 +363,7 @@ func updateUserGroups(ctx context.Context, client MinioAdmin, user string, group func getUpdateUserGroupsResponse(session *models.Principal, params admin_api.UpdateUserGroupsParams) (*models.User, *models.Error) { ctx := context.Background() - mAdmin, err := newMAdminClient(session) + mAdmin, err := newAdminClient(session) if err != nil { return nil, prepareError(err) } @@ -404,7 +402,7 @@ func setUserStatus(ctx context.Context, client MinioAdmin, user string, status s func getUpdateUserResponse(session *models.Principal, params admin_api.UpdateUserInfoParams) (*models.User, *models.Error) { ctx := context.Background() - mAdmin, err := newMAdminClient(session) + mAdmin, err := newAdminClient(session) if err != nil { return nil, prepareError(err) } @@ -475,7 +473,7 @@ func addUsersListToGroups(ctx context.Context, client MinioAdmin, usersToUpdate func getAddUsersListToGroupsResponse(session *models.Principal, params admin_api.BulkUpdateUsersGroupsParams) *models.Error { ctx := context.Background() - mAdmin, err := newMAdminClient(session) + mAdmin, err := newAdminClient(session) if err != nil { return prepareError(err) } @@ -496,7 +494,7 @@ func getAddUsersListToGroupsResponse(session *models.Principal, params admin_api func getListUsersWithAccessToBucketResponse(session *models.Principal, bucket string) ([]string, *models.Error) { ctx := context.Background() - mAdmin, err := newMAdminClient(session) + mAdmin, err := newAdminClient(session) if err != nil { return nil, prepareError(err) } @@ -530,64 +528,66 @@ func listUsersWithAccessToBucket(ctx context.Context, adminClient MinioAdmin, bu return nil, prepareError(err) } var retval []string - akHasAccess := make(map[string]bool) - akIsDenied := make(map[string]bool) + akHasAccess := make(map[string]struct{}) + akIsDenied := make(map[string]struct{}) for k, v := range users { for _, policyName := range strings.Split(v.PolicyName, ",") { policyName = strings.TrimSpace(policyName) + if policyName == "" { + continue + } policy, err := adminClient.getPolicy(ctx, policyName) - if err == nil { - if !akIsDenied[k] { - switch policyAllowsAndMatchesBucket(policy, bucket) { - case Allow: - if !akHasAccess[k] { - akHasAccess[k] = true - } - case Deny: - akIsDenied[k] = true - akHasAccess[k] = false + if err != nil { + LogError("unable to fetch policy %s: %v", policyName, err) + continue + } + if _, ok := akIsDenied[k]; !ok { + switch policyAllowsAndMatchesBucket(policy, bucket) { + case Allow: + if _, ok := akHasAccess[k]; !ok { + akHasAccess[k] = struct{}{} } + case Deny: + akIsDenied[k] = struct{}{} + delete(akHasAccess, k) } - } else { - log.Println(err) } } } - groups, err := listGroups(ctx, adminClient) + groups, err := adminClient.listGroups(ctx) if err != nil { - log.Println(err) + LogError("unable to list groups: %v", err) return retval, nil } - for i := 0; i < len(*groups); i++ { - info, err := groupInfo(ctx, adminClient, (*groups)[i]) - if err == nil { - policy, err2 := adminClient.getPolicy(ctx, info.Policy) - if err2 == nil { - for j := 0; j < len(info.Members); j++ { - if !akIsDenied[info.Members[j]] { - switch policyAllowsAndMatchesBucket(policy, bucket) { - case Allow: - if !akHasAccess[info.Members[j]] { - akHasAccess[info.Members[j]] = true - } - case Deny: - akIsDenied[info.Members[j]] = true - akHasAccess[info.Members[j]] = false - } + + for _, groupName := range groups { + info, err := groupInfo(ctx, adminClient, groupName) + if err != nil { + LogError("unable to fetch group info %s: %v", groupName, err) + continue + } + policy, err := adminClient.getPolicy(ctx, info.Policy) + if err != nil { + LogError("unable to fetch group policy %s: %v", info.Policy, err) + continue + } + for _, member := range info.Members { + if _, ok := akIsDenied[member]; !ok { + switch policyAllowsAndMatchesBucket(policy, bucket) { + case Allow: + if _, ok := akHasAccess[member]; !ok { + akHasAccess[member] = struct{}{} } + case Deny: + akIsDenied[member] = struct{}{} + delete(akHasAccess, member) } - } else { - log.Println(err2) } - } else { - log.Println(err) } } - for k, v := range akHasAccess { - if v { - retval = append(retval, k) - } + for k := range akHasAccess { + retval = append(retval, k) } sort.Strings(retval) return retval, nil @@ -604,7 +604,7 @@ func changeUserPassword(ctx context.Context, client MinioAdmin, selectedUser str // getChangeUserPasswordResponse will change the password of selctedUser to newSecretKey func getChangeUserPasswordResponse(session *models.Principal, params admin_api.ChangeUserPasswordParams) *models.Error { ctx := context.Background() - mAdmin, err := newMAdminClient(session) + mAdmin, err := newAdminClient(session) if err != nil { return prepareError(err) } diff --git a/restapi/client-admin.go b/restapi/client-admin.go index c1b37c95c..42cac20ae 100644 --- a/restapi/client-admin.go +++ b/restapi/client-admin.go @@ -389,7 +389,7 @@ func (ac adminClient) editTierCreds(ctx context.Context, tierName string, creds return ac.client.EditTier(ctx, tierName, creds) } -func newMAdminClient(sessionClaims *models.Principal) (*madmin.AdminClient, error) { +func newAdminClient(sessionClaims *models.Principal) (*madmin.AdminClient, error) { adminClient, err := newAdminFromClaims(sessionClaims) if err != nil { return nil, err diff --git a/restapi/configure_console.go b/restapi/configure_console.go index 3df07d128..aa359038c 100644 --- a/restapi/configure_console.go +++ b/restapi/configure_console.go @@ -23,7 +23,6 @@ import ( "crypto/tls" "io" "io/fs" - "log" "net/http" "strings" "time" @@ -31,7 +30,6 @@ import ( portal_ui "github.com/minio/console/portal-ui" "github.com/go-openapi/errors" - "github.com/go-openapi/runtime" "github.com/go-openapi/swag" "github.com/minio/console/models" "github.com/minio/console/pkg/auth" @@ -55,26 +53,13 @@ func configureFlags(api *operations.ConsoleAPI) { } func configureAPI(api *operations.ConsoleAPI) http.Handler { - // configure the api here - api.ServeError = errors.ServeError - - // Set your custom logger if needed. Default one is log.Printf - // Expected interface func(string, ...interface{}) - // - // Example: - // api.Logger = log.Printf - - api.JSONConsumer = runtime.JSONConsumer() - - api.JSONProducer = runtime.JSONProducer() // Applies when the "x-token" header is set - api.KeyAuth = func(token string, scopes []string) (*models.Principal, error) { // we are validating the session token by decrypting the claims inside, if the operation succeed that means the jwt // was generated and signed by us in the first place claims, err := auth.SessionTokenAuthenticate(token) if err != nil { - log.Println(err) + api.Logger("Unable to validate the session token %s: %v", token, err) return nil, errors.New(401, "incorrect api key auth") } return &models.Principal{ diff --git a/restapi/error.go b/restapi/error.go index 1ce615d56..a90fec530 100644 --- a/restapi/error.go +++ b/restapi/error.go @@ -2,7 +2,6 @@ package restapi import ( "errors" - "log" "runtime" "strings" @@ -49,7 +48,7 @@ func prepareError(err ...error) *models.Error { if len(err) > 0 { frame := getFrame(2) fileParts := strings.Split(frame.File, "/") - log.Printf("%s:%d: original error: %s", fileParts[len(fileParts)-1], frame.Line, err[0].Error()) + LogError("original error -> (%s:%d: %v)", fileParts[len(fileParts)-1], frame.Line, err[0]) if k8sErrors.IsUnauthorized(err[0]) { errorCode = 401 errorMessage = errorGenericUnauthorized.Error() @@ -146,12 +145,12 @@ func prepareError(err ...error) *models.Error { } // if we received a second error take that as friendly message but dont override the code if len(err) > 1 && err[1] != nil { - log.Print("friendly error: ", err[1].Error()) + LogError("friendly error: %v", err[1].Error()) errorMessage = err[1].Error() } // if we receive third error we just print that as debugging if len(err) > 2 && err[2] != nil { - log.Print("debugging error: ", err[2].Error()) + LogError("debugging error: %v", err[2].Error()) } errRemoteTierExists := errors.New("Specified remote tier already exists") //nolint diff --git a/restapi/integrations.go b/restapi/integrations.go index afdfbd350..bf61effd2 100644 --- a/restapi/integrations.go +++ b/restapi/integrations.go @@ -19,7 +19,6 @@ package restapi import ( "context" "fmt" - "log" "strings" "time" @@ -65,9 +64,8 @@ func gkeIntegration(clientset *kubernetes.Clientset, tenantName string, namespac }) go podInformer.Run(doneCh) - //block until the informer exits + // block until the informer exits <-doneCh - log.Println("informer closed") tenantDomain := fmt.Sprintf("%s.cloud.min.dev", tenantName) tenantConsoleDomain := fmt.Sprintf("console.%s.cloud.min.dev", tenantName) diff --git a/restapi/server.go b/restapi/server.go index 2ae6fa282..50fc5c14a 100644 --- a/restapi/server.go +++ b/restapi/server.go @@ -40,6 +40,7 @@ import ( "github.com/go-openapi/runtime/flagext" flags "github.com/jessevdk/go-flags" + "github.com/minio/cli" "github.com/minio/console/restapi/operations" ) @@ -48,29 +49,59 @@ const ( schemeHTTPS = "https" ) -var defaultSchemes []string - -func init() { - defaultSchemes = []string{ - schemeHTTP, - } +var defaultSchemes = []string{ + schemeHTTP, } +var infoLog = log.New(os.Stdout, "I: ", log.LstdFlags|log.Lshortfile) +var errorLog = log.New(os.Stdout, "E: ", log.LstdFlags|log.Lshortfile) + +func logInfo(msg string, data ...interface{}) { + infoLog.Printf(msg+"\n", data...) +} + +func logError(msg string, data ...interface{}) { + errorLog.Printf(msg+"\n", data...) +} + +var ( + LogInfo = logInfo + LogError = logError +) + // NewServer creates a new api console server but does not configure it func NewServer(api *operations.ConsoleAPI) *Server { s := new(Server) - s.shutdown = make(chan struct{}) s.api = api + s.shutdown = make(chan struct{}) s.interrupt = make(chan os.Signal, 1) return s } -// ConfigureAPI configures the API and handlers. -func (s *Server) ConfigureAPI() { +func (s *Server) Configure(ctx Context) *Server { + s.Host = ctx.Host + s.Port = ctx.HTTPPort + Port = strconv.Itoa(s.Port) + Hostname = s.Host + + if len(GlobalPublicCerts) > 0 { + // If TLS certificates are provided enforce the HTTPS schema, meaning console will redirect + // plain HTTP connections to HTTPS server + s.EnabledListeners = []string{"http", "https"} + s.TLSPort = ctx.HTTPSPort + // Need to store tls-port, tls-host un config variables so secure.middleware can read from there + TLSPort = strconv.Itoa(s.TLSPort) + Hostname = ctx.Host + TLSRedirect = ctx.TLSRedirect + } + + // configure the API handlers.. if s.api != nil { s.handler = configureAPI(s.api) } + + return s } // ConfigureFlags configures the additional flags defined by the handlers. Needs to be called before the parser.Parse @@ -80,6 +111,38 @@ func (s *Server) ConfigureFlags() { } } +// Context captures all command line flags values +type Context struct { + Host string + HTTPPort, HTTPSPort int + TLSRedirect string + // Legacy options, TODO: remove in future + TLSCertificate, TLSKey, TLSca string +} + +func (c *Context) Load(ctx *cli.Context) error { + *c = Context{ + Host: ctx.String("host"), + HTTPPort: ctx.Int("port"), + HTTPSPort: ctx.Int("tls-port"), + TLSRedirect: ctx.String("tls-redirect"), + // Legacy options to be removed. + TLSCertificate: ctx.String("tls-certificate"), + TLSKey: ctx.String("tls-key"), + TLSca: ctx.String("tls-ca"), + } + if c.HTTPPort > 65535 { + return errors.New("invalid argument --port out of range - ports can range from 1-65535") + } + if c.HTTPSPort > 65535 { + return errors.New("invalid argument --tls-port out of range - ports can range from 1-65535") + } + if c.TLSRedirect != "on" && c.TLSRedirect != "off" { + return errors.New("invalid argument --tls-redirect only accepts either 'on' or 'off'") + } + return nil +} + // Server for the console API type Server struct { EnabledListeners []string `long:"scheme" description:"the listeners to enable, this can be repeated and defaults to the schemes in the swagger spec"` @@ -107,36 +170,16 @@ type Server struct { interrupt chan os.Signal } -// Logf logs message either via defined user logger or via system one if no user logger is defined. -func (s *Server) Logf(f string, args ...interface{}) { - if s.api != nil && s.api.Logger != nil { - s.api.Logger(f, args...) - } else { - log.Printf(f, args...) - } +// Log logs message either via defined user logger or via system one if no user logger is defined. +func (s *Server) Log(f string, args ...interface{}) { + logInfo(f, args...) } -// Fatalf logs message either via defined user logger or via system one if no user logger is defined. +// Fatal logs message either via defined user logger or via system one if no user logger is defined. // Exits with non-zero status after printing -func (s *Server) Fatalf(f string, args ...interface{}) { - if s.api != nil && s.api.Logger != nil { - s.api.Logger(f, args...) - os.Exit(1) - } else { - log.Fatalf(f, args...) - } -} - -// SetAPI configures the server with the specified API. Needs to be called before Serve -func (s *Server) SetAPI(api *operations.ConsoleAPI) { - if api == nil { - s.api = nil - s.handler = nil - return - } - - s.api = api - s.handler = configureAPI(api) +func (s *Server) Fatal(f string, args ...interface{}) { + logError(f, args) + os.Exit(1) } func (s *Server) hasScheme(scheme string) bool { @@ -175,7 +218,7 @@ func (s *Server) Serve() (err error) { signalNotify(s.interrupt) go handleInterrupt(once, s) - servers := []*http.Server{} + var servers []*http.Server if s.hasScheme(schemeHTTP) { httpServer := new(http.Server) @@ -186,13 +229,11 @@ func (s *Server) Serve() (err error) { servers = append(servers, httpServer) wg.Add(1) - s.Logf("Serving console at http://%s", s.httpServerL.Addr()) + s.Log("Serving console at http://%s", s.httpServerL.Addr()) go func(l net.Listener) { defer wg.Done() - if err := httpServer.Serve(l); err != nil && err != http.ErrServerClosed { - s.Fatalf("%v", err) - } - s.Logf("Stopped serving console at http://%s", l.Addr()) + httpServer.Serve(l) + s.Log("Stopped serving console at http://%s", l.Addr()) }(s.httpServerL) } @@ -257,15 +298,15 @@ func (s *Server) Serve() (err error) { // after standard and custom config are passed, this ends up with no certificate if s.TLSCertificate == "" { if s.TLSCertificateKey == "" { - s.Fatalf("the required flags `--tls-certificate` and `--tls-key` were not specified") + s.Fatal("the required flags `--tls-certificate` and `--tls-key` were not specified") } - s.Fatalf("the required flag `--tls-certificate` was not specified") + s.Fatal("the required flag `--tls-certificate` was not specified") } if s.TLSCertificateKey == "" { - s.Fatalf("the required flag `--tls-key` was not specified") + s.Fatal("the required flag `--tls-key` was not specified") } // this happens with a wrong custom TLS configurator - s.Fatalf("no certificate was configured for TLS") + s.Fatal("no certificate was configured for TLS") } // must have at least one certificate or panics @@ -273,13 +314,11 @@ func (s *Server) Serve() (err error) { servers = append(servers, httpsServer) wg.Add(1) - s.Logf("Serving console at https://%s", s.httpsServerL.Addr()) + s.Log("Serving console at https://%s", s.httpsServerL.Addr()) go func(l net.Listener) { defer wg.Done() - if err := httpsServer.Serve(l); err != nil && err != http.ErrServerClosed { - s.Fatalf("%v", err) - } - s.Logf("Stopped serving console at https://%s", l.Addr()) + httpsServer.Serve(l) + s.Log("Stopped serving console at https://%s", l.Addr()) }(tls.NewListener(s.httpsServerL, httpsServer.TLSConfig)) } @@ -346,7 +385,7 @@ func (s *Server) handleShutdown(wg *sync.WaitGroup, servers []*http.Server) { }() if err := server.Shutdown(ctx); err != nil { // Error from closing listeners, or context timeout: - s.Logf("HTTP server Shutdown: %v", err) + s.Log("HTTP server Shutdown: %v", err) } else { success = true } @@ -397,13 +436,13 @@ func handleInterrupt(once *sync.Once, s *Server) { once.Do(func() { for range s.interrupt { if s.interrupted { - s.Logf("Server already shutting down") + s.Log("Server already shutting down") continue } s.interrupted = true - s.Logf("Shutting down... ") + s.Log("Shutting down... ") if err := s.Shutdown(); err != nil { - s.Logf("HTTP server Shutdown: %v", err) + s.Log("HTTP server Shutdown: %v", err) } } }) diff --git a/restapi/user_account.go b/restapi/user_account.go index eb6907be2..6205b966e 100644 --- a/restapi/user_account.go +++ b/restapi/user_account.go @@ -71,7 +71,7 @@ func getChangePasswordResponse(session *models.Principal, params user_api.Accoun // changePassword operations requires an AdminClient initialized with parent account credentials not // STS credentials - parentAccountClient, err := newMAdminClient(&models.Principal{ + parentAccountClient, err := newAdminClient(&models.Principal{ STSAccessKeyID: session.AccountAccessKey, STSSecretAccessKey: *params.Body.CurrentSecretKey, }) @@ -109,7 +109,7 @@ func getUserHasPermissionsResponse(session *models.Principal, params user_api.Ha ctx, cancel := context.WithTimeout(context.Background(), time.Second*20) defer cancel() - mAdmin, err := newMAdminClient(session) + mAdmin, err := newAdminClient(session) if err != nil { return nil, prepareError(err) } diff --git a/restapi/user_bucket_quota.go b/restapi/user_bucket_quota.go index eb347acd2..463656301 100644 --- a/restapi/user_bucket_quota.go +++ b/restapi/user_bucket_quota.go @@ -52,7 +52,7 @@ func registerBucketQuotaHandlers(api *operations.ConsoleAPI) { } func setBucketQuotaResponse(session *models.Principal, params user_api.SetBucketQuotaParams) *models.Error { - mAdmin, err := newMAdminClient(session) + mAdmin, err := newAdminClient(session) if err != nil { return prepareError(err) } @@ -96,7 +96,7 @@ func setBucketQuota(ctx context.Context, ac *adminClient, bucket *string, bucket } func getBucketQuotaResponse(session *models.Principal, params user_api.GetBucketQuotaParams) (*models.BucketQuota, *models.Error) { - mAdmin, err := newMAdminClient(session) + mAdmin, err := newAdminClient(session) if err != nil { return nil, prepareError(err) } diff --git a/restapi/user_buckets.go b/restapi/user_buckets.go index 24a174df7..705e5e64f 100644 --- a/restapi/user_buckets.go +++ b/restapi/user_buckets.go @@ -20,7 +20,6 @@ import ( "context" "encoding/json" "fmt" - "log" "strings" "time" @@ -162,7 +161,7 @@ const ( func doSetVersioning(client MCClient, state VersionState) error { err := client.setVersioning(context.Background(), string(state)) if err != nil { - log.Println("error setting versioning for bucket:", err.Cause) + LogError("error setting versioning for bucket: %s", err.Cause) return err.Cause } @@ -196,7 +195,7 @@ func getBucketReplicationdResponse(session *models.Principal, bucketName string) mClient, err := newMinioClient(session) if err != nil { - log.Println("error creating MinIO Client:", err) + LogError("error creating MinIO Client: %v", err) return nil, err } // create a minioClient interface implementation @@ -206,13 +205,12 @@ func getBucketReplicationdResponse(session *models.Principal, bucketName string) // we will tolerate this call failing res, err := minioClient.getBucketReplication(ctx, bucketName) if err != nil { - log.Println("error versioning bucket:", err) + LogError("error versioning bucket: %v", err) } var rules []*models.BucketReplicationRule for _, rule := range res.Rules { - repDelMarkerStatus := false if rule.DeleteMarkerReplication.Status == "enable" { repDelMarkerStatus = true @@ -247,7 +245,7 @@ func getBucketVersionedResponse(session *models.Principal, bucketName string) (* mClient, err := newMinioClient(session) if err != nil { - log.Println("error creating MinIO Client:", err) + LogError("error creating MinIO Client: %v", err) return nil, err } // create a minioClient interface implementation @@ -257,7 +255,7 @@ func getBucketVersionedResponse(session *models.Principal, bucketName string) (* // we will tolerate this call failing res, err := minioClient.getBucketVersioning(ctx, bucketName) if err != nil { - log.Println("error versioning bucket:", err) + LogError("error versioning bucket: %v", err) } // serialize output @@ -287,7 +285,7 @@ func getListBucketsResponse(session *models.Principal) (*models.ListBucketsRespo ctx, cancel := context.WithTimeout(context.Background(), time.Second*20) defer cancel() - mAdmin, err := newMAdminClient(session) + mAdmin, err := newAdminClient(session) if err != nil { return nil, prepareError(err) } @@ -344,9 +342,9 @@ func getMakeBucketResponse(session *models.Principal, br *models.MakeBucketReque // make sure to delete bucket if an error occurs after bucket was created defer func() { if err != nil { - log.Println("error creating bucket:", err) + LogError("error creating bucket: %v", err) if err := removeBucket(minioClient, *br.Name); err != nil { - log.Println("error removing bucket:", err) + LogError("error removing bucket: %v", err) } } }() @@ -368,7 +366,7 @@ func getMakeBucketResponse(session *models.Principal, br *models.MakeBucketReque // if it has support for if br.Quota != nil && br.Quota.Enabled != nil && *br.Quota.Enabled { - mAdmin, err := newMAdminClient(session) + mAdmin, err := newAdminClient(session) if err != nil { return prepareError(err) } @@ -377,7 +375,7 @@ func getMakeBucketResponse(session *models.Principal, br *models.MakeBucketReque adminClient := adminClient{client: mAdmin} // we will tolerate this call failing if err := setBucketQuota(ctx, &adminClient, br.Name, br.Quota); err != nil { - log.Println("error versioning bucket:", err) + LogError("error versioning bucket:", err) } } @@ -731,7 +729,7 @@ func getBucketObLockingResponse(session *models.Principal, bucketName string) (* mClient, err := newMinioClient(session) if err != nil { - log.Println("error creating MinIO Client:", err) + LogError("error creating MinIO Client: %v", err) return nil, err } // create a minioClient interface implementation @@ -746,7 +744,7 @@ func getBucketObLockingResponse(session *models.Principal, bucketName string) (* ObjectLockingEnabled: false, }, nil } - log.Println("error object locking bucket:", err) + LogError("error object locking bucket: %v", err) } // serialize output diff --git a/restapi/user_log_search.go b/restapi/user_log_search.go index d4a3985ab..6f0e79235 100644 --- a/restapi/user_log_search.go +++ b/restapi/user_log_search.go @@ -72,26 +72,25 @@ func logSearch(endpoint string) (*models.LogSearchResponse, *models.Error) { if err != nil { return nil, prepareError(err) } - defer resp.Body.Close() + + body, err := ioutil.ReadAll(resp.Body) + resp.Body.Close() + if err != nil { + return nil, prepareError(err) + } if resp.StatusCode != 200 { - log.Println("Error Status Code", resp.StatusCode) - _, err := ioutil.ReadAll(resp.Body) - if err != nil { - return nil, prepareError(err) - } - return nil, &models.Error{ - Code: 500, - Message: swag.String("Error retrieving logs"), + Code: int32(resp.StatusCode), + Message: swag.String(fmt.Sprintf("error retrieving logs: %s", http.StatusText(resp.StatusCode))), } } var results []logsearchServer.ReqInfoRow - - if err := json.NewDecoder(resp.Body).Decode(&results); err != nil { + if err = json.Unmarshal(body, &results); err != nil { return nil, prepareError(err) } + response := models.LogSearchResponse{ Results: results, } diff --git a/restapi/user_log_search_test.go b/restapi/user_log_search_test.go index b11de1bd9..f481235d7 100644 --- a/restapi/user_log_search_test.go +++ b/restapi/user_log_search_test.go @@ -98,12 +98,13 @@ func TestLogSearch(t *testing.T) { expectedResponse: nil, expectedError: &models.Error{ Code: 500, - Message: swag.String("Error retrieving logs"), + Message: swag.String(fmt.Sprintf("error retrieving logs: %s", http.StatusText(500))), }, }, } for _, tt := range tests { + tt := tt t.Run(tt.name, func(t *testing.T) { testRequest := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { w.WriteHeader(tt.args.apiResponseCode) diff --git a/restapi/user_login.go b/restapi/user_login.go index 243f2b30b..e4f658f75 100644 --- a/restapi/user_login.go +++ b/restapi/user_login.go @@ -19,7 +19,6 @@ package restapi import ( "bytes" "context" - "log" "net/http" "time" @@ -96,7 +95,7 @@ func login(credentials ConsoleCredentialsI) (*string, error) { // if we made it here, the consoleCredentials work, generate a jwt with claims token, err := auth.NewEncryptedTokenForClient(&tokens, credentials.GetAccountAccessKey(), credentials.GetActions()) if err != nil { - log.Println("error authenticating user", err) + LogError("error authenticating user: %v", err) return nil, errInvalidCredentials } return &token, nil @@ -130,7 +129,7 @@ func getConsoleCredentials(ctx context.Context, accessKey, secretKey string) (*c return nil, err } // initialize admin client - mAdminClient, err := newMAdminClient(&models.Principal{ + mAdminClient, err := newAdminClient(&models.Principal{ STSAccessKeyID: tokens.AccessKeyID, STSSecretAccessKey: tokens.SecretAccessKey, STSSessionToken: tokens.SessionToken, @@ -209,7 +208,7 @@ func getLoginDetailsResponse() (*models.LoginDetails, *models.Error) { func verifyUserAgainstIDP(ctx context.Context, provider auth.IdentityProviderI, code, state string) (*credentials.Credentials, error) { userCredentials, err := provider.VerifyIdentity(ctx, code, state) if err != nil { - log.Println("error validating user identity against idp:", err) + LogError("error validating user identity against idp: %v", err) return nil, errInvalidCredentials } return userCredentials, nil @@ -251,7 +250,7 @@ func getLoginOauth2AuthResponse(lr *models.LoginOauth2AuthRequest) (*models.Logi return nil, prepareError(errInvalidCredentials, nil, err) } // initialize admin client - mAdminClient, err := newMAdminClient(&models.Principal{ + mAdminClient, err := newAdminClient(&models.Principal{ STSAccessKeyID: creds.AccessKeyID, STSSecretAccessKey: creds.SecretAccessKey, STSSessionToken: creds.SessionToken, diff --git a/restapi/user_objects.go b/restapi/user_objects.go index 261686990..6d204ca77 100644 --- a/restapi/user_objects.go +++ b/restapi/user_objects.go @@ -20,7 +20,6 @@ import ( "context" "fmt" "io" - "log" "net/http" "path/filepath" "regexp" @@ -69,13 +68,8 @@ func registerObjectsHandlers(api *operations.ConsoleAPI) { return user_api.NewDownloadObjectDefault(int(err.Code)).WithPayload(err) } return middleware.ResponderFunc(func(rw http.ResponseWriter, _ runtime.Producer) { - if _, err := io.Copy(rw, resp); err != nil { - log.Println(err) - } else { - if err := resp.Close(); err != nil { - log.Println(err) - } - } + io.Copy(rw, resp) + resp.Close() }) }) // upload object @@ -185,7 +179,7 @@ func listBucketObjects(ctx context.Context, client MinioClient, bucketName strin if err != nil { errResp := minio.ToErrorResponse(probe.NewError(err).ToGoError()) if errResp.Code != "InvalidRequest" && errResp.Code != "NoSuchObjectLockConfiguration" { - log.Printf("error getting legal hold status for %s : %s", lsObj.VersionID, err) + LogError("error getting legal hold status for %s : %v", lsObj.VersionID, err) } } else { if legalHoldStatus != nil { @@ -197,7 +191,7 @@ func listBucketObjects(ctx context.Context, client MinioClient, bucketName strin if err != nil { errResp := minio.ToErrorResponse(probe.NewError(err).ToGoError()) if errResp.Code != "NoSuchObjectLockConfiguration" { - log.Printf("error getting retention status for %s : %s", lsObj.VersionID, err) + LogError("error getting retention status for %s : %v", lsObj.VersionID, err) } } else { if retention != nil && retUntilDate != nil { @@ -208,7 +202,7 @@ func listBucketObjects(ctx context.Context, client MinioClient, bucketName strin } tags, err := client.getObjectTagging(ctx, bucketName, lsObj.Key, minio.GetObjectTaggingOptions{VersionID: lsObj.VersionID}) if err != nil { - log.Printf("error getting object tags for %s : %s", lsObj.VersionID, err) + LogError("error getting object tags for %s : %v", lsObj.VersionID, err) } else { obj.Tags = tags.ToMap() } diff --git a/restapi/user_service_accounts.go b/restapi/user_service_accounts.go index 86c1980f7..e114b5f7d 100644 --- a/restapi/user_service_accounts.go +++ b/restapi/user_service_accounts.go @@ -95,7 +95,7 @@ func getCreateServiceAccountResponse(session *models.Principal, serviceAccount * ctx, cancel := context.WithTimeout(context.Background(), time.Second*20) defer cancel() - userAdmin, err := newMAdminClient(session) + userAdmin, err := newAdminClient(session) if err != nil { return nil, prepareError(err) } @@ -129,7 +129,7 @@ func getUserServiceAccountsResponse(session *models.Principal, user string) (mod ctx, cancel := context.WithTimeout(context.Background(), time.Second*20) defer cancel() - userAdmin, err := newMAdminClient(session) + userAdmin, err := newAdminClient(session) if err != nil { return nil, prepareError(err) } @@ -154,7 +154,7 @@ func getDeleteServiceAccountResponse(session *models.Principal, accessKey string ctx, cancel := context.WithTimeout(context.Background(), time.Second*20) defer cancel() - userAdmin, err := newMAdminClient(session) + userAdmin, err := newAdminClient(session) if err != nil { return prepareError(err) } diff --git a/restapi/user_watch.go b/restapi/user_watch.go index 492ea866c..b09e2d2dc 100644 --- a/restapi/user_watch.go +++ b/restapi/user_watch.go @@ -20,7 +20,6 @@ import ( "context" "encoding/json" "fmt" - "log" "net/http" "regexp" "strings" @@ -37,7 +36,7 @@ type watchOptions struct { func startWatch(ctx context.Context, conn WSConn, wsc MCClient, options *watchOptions) error { wo, pErr := wsc.watch(ctx, options.WatchOptions) if pErr != nil { - fmt.Println("error initializing watch:", pErr.Cause) + LogError("error initializing watch: %v", pErr.Cause) return pErr.Cause } for { @@ -54,13 +53,13 @@ func startWatch(ctx context.Context, conn WSConn, wsc MCClient, options *watchOp // Serialize message to be sent bytes, err := json.Marshal(event) if err != nil { - log.Println("error on json.Marshal:", err) + LogError("error on json.Marshal: %v", err) return err } // Send Message through websocket connection err = conn.writeMessage(websocket.TextMessage, bytes) if err != nil { - log.Println("error writeMessage:", err) + LogError("error writeMessage: %v", err) return err } } @@ -70,7 +69,7 @@ func startWatch(ctx context.Context, conn WSConn, wsc MCClient, options *watchOp return nil } if pErr != nil { - log.Println("error on watch:", pErr.Cause) + LogError("error on watch: %v", pErr.Cause) return pErr.Cause } diff --git a/restapi/ws_handle.go b/restapi/ws_handle.go index 8d1211594..d3d7cbb99 100644 --- a/restapi/ws_handle.go +++ b/restapi/ws_handle.go @@ -18,7 +18,6 @@ package restapi import ( "context" - "log" "net" "net/http" "strings" @@ -104,7 +103,6 @@ func serveWS(w http.ResponseWriter, req *http.Request) { // authenticate WS connection with Console session, err := auth.GetClaimsFromTokenInRequest(req) if err != nil { - log.Print("error on ws authentication: ", err) errors.ServeError(w, req, errors.New(http.StatusUnauthorized, err.Error())) return } @@ -112,7 +110,6 @@ func serveWS(w http.ResponseWriter, req *http.Request) { // upgrades the HTTP server connection to the WebSocket protocol. conn, err := upgrader.Upgrade(w, req, nil) if err != nil { - log.Print("error on upgrade: ", err) errors.ServeError(w, req, err) return } @@ -136,7 +133,7 @@ func serveWS(w http.ResponseWriter, req *http.Request) { case strings.HasPrefix(wsPath, `/health-info`): deadline, err := getHealthInfoOptionsFromReq(req) if err != nil { - log.Println("error getting health info options:", err) + LogError("error getting health info options: %v", err) closeWsConn(conn) return } @@ -149,7 +146,7 @@ func serveWS(w http.ResponseWriter, req *http.Request) { case strings.HasPrefix(wsPath, `/heal`): hOptions, err := getHealOptionsFromReq(req) if err != nil { - log.Println("error getting heal options:", err) + LogError("error getting heal options: %v", err) closeWsConn(conn) return } @@ -162,7 +159,7 @@ func serveWS(w http.ResponseWriter, req *http.Request) { case strings.HasPrefix(wsPath, `/watch`): wOptions, err := getWatchOptionsFromReq(req) if err != nil { - log.Println("error getting watch options:", err) + LogError("error getting watch options: %v", err) closeWsConn(conn) return } @@ -184,7 +181,7 @@ func newWebSocketAdminClient(conn *websocket.Conn, autClaims *models.Principal) // authenticated with MinIO mAdmin, err := newAdminFromClaims(autClaims) if err != nil { - log.Println("error creating Madmin Client:", err) + LogError("error creating madmin client: %v", err) return nil, err } // create a websocket connection interface implementation @@ -204,7 +201,7 @@ func newWebSocketS3Client(conn *websocket.Conn, claims *models.Principal, bucket // authenticated with MinIO s3Client, err := newS3BucketClient(claims, bucketName, "") if err != nil { - log.Println("error creating S3Client:", err) + LogError("error creating S3Client:", err) return nil, err } // create a websocket connection interface implementation @@ -232,16 +229,16 @@ func wsReadClientCtx(conn WSConn) context.Context { if err != nil { // if error of type websocket.CloseError and is Unexpected if websocket.IsUnexpectedCloseError(err, websocket.CloseGoingAway, websocket.CloseNormalClosure) { - log.Println("error unexpected CloseError on ReadMessage:", err) + LogError("error unexpected CloseError on ReadMessage: %v", err) return } // Not all errors are of type websocket.CloseError. if _, ok := err.(*websocket.CloseError); !ok { - log.Println("error on ReadMessage:", err) + LogError("error on ReadMessage: %v", err) return } // else is an expected Close Error - log.Println("closed conn.ReadMessage:", err) + LogError("closed conn.ReadMessage: %v", err) return } } @@ -259,11 +256,11 @@ func closeWsConn(conn *websocket.Conn) { // on a Websocket connection. func (wsc *wsAdminClient) trace() { defer func() { - log.Println("trace stopped") + LogInfo("trace stopped") // close connection after return wsc.conn.close() }() - log.Println("trace started") + LogInfo("trace started") ctx := wsReadClientCtx(wsc.conn) @@ -279,11 +276,11 @@ func (wsc *wsAdminClient) trace() { // on a Websocket connection. func (wsc *wsAdminClient) console() { defer func() { - log.Println("console logs stopped") + LogInfo("console logs stopped") // close connection after return wsc.conn.close() }() - log.Println("console logs started") + LogInfo("console logs started") ctx := wsReadClientCtx(wsc.conn) @@ -294,11 +291,11 @@ func (wsc *wsAdminClient) console() { func (wsc *wsS3Client) watch(params *watchOptions) { defer func() { - log.Println("watch stopped") + LogInfo("watch stopped") // close connection after return wsc.conn.close() }() - log.Println("watch started") + LogInfo("watch started") ctx := wsReadClientCtx(wsc.conn) @@ -309,11 +306,11 @@ func (wsc *wsS3Client) watch(params *watchOptions) { func (wsc *wsAdminClient) heal(opts *healOptions) { defer func() { - log.Println("heal stopped") + LogInfo("heal stopped") // close connection after return wsc.conn.close() }() - log.Println("heal started") + LogInfo("heal started") ctx := wsReadClientCtx(wsc.conn) @@ -324,11 +321,11 @@ func (wsc *wsAdminClient) heal(opts *healOptions) { func (wsc *wsAdminClient) healthInfo(deadline *time.Duration) { defer func() { - log.Println("health info stopped") + LogInfo("health info stopped") // close connection after return wsc.conn.close() }() - log.Println("health info started") + LogInfo("health info started") ctx := wsReadClientCtx(wsc.conn) @@ -341,7 +338,7 @@ func (wsc *wsAdminClient) healthInfo(deadline *time.Duration) { // see https://tools.ietf.org/html/rfc6455#page-45 func sendWsCloseMessage(conn WSConn, err error) { if err != nil { - log.Print("original ws error: ", err.Error()) + LogError("original ws error: %v", err) // If connection exceeded read deadline send Close // Message Policy Violation code since we don't want // to let the receiver figure out the read deadline.