Skip to content
This repository was archived by the owner on Jun 1, 2022. It is now read-only.
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
40 changes: 40 additions & 0 deletions commands/base.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,11 +25,15 @@ import (
"io/ioutil"
"net/http"
"net/url"
"os/user"
"path/filepath"
"reflect"
"regexp"
"strings"
"text/template"

"gopkg.in/ini.v1"

"github.com/go-openapi/runtime"
httptransport "github.com/go-openapi/runtime/client"
inventorypb "github.com/percona/pmm/api/inventorypb/json/client"
Expand Down Expand Up @@ -69,6 +73,10 @@ type Command interface {
Run() (Result, error)
}

type ApplyDefaults interface {
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🚫 [golangci-lint] reported by reviewdog 🐶
exported type ApplyDefaults should have comment or be unexported (golint)

ApplyDefaults(cfg *ini.File)
}

// TODO remove Command above, rename CommandWithContext to Command
type CommandWithContext interface {
// TODO rename to Run
Expand Down Expand Up @@ -115,6 +123,7 @@ type globalFlagsValues struct {
ServerInsecureTLS bool
Debug bool
Trace bool
DefaultConfig string
}

// GlobalFlags contains pmm-admin core flags values.
Expand Down Expand Up @@ -184,6 +193,37 @@ func (e errFromNginx) GoString() string {
return fmt.Sprintf("errFromNginx(%q)", string(e))
}

func ConfigureDefaults(config string, cmd ApplyDefaults) error {
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🚫 [golangci-lint] reported by reviewdog 🐶
exported function ConfigureDefaults should have comment or be unexported (golint)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i think its better to rename it to configPath

if config != "" {
var err error
config, err = expandPath(config)
if err != nil {
return fmt.Errorf("fail to normalize path: %v", err)
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🚫 [golangci-lint] reported by reviewdog 🐶
non-wrapping format verb for fmt.Errorf. Use %w to format errors (errorlint)

}
cfg, err := ini.Load(config)
if err != nil {
return fmt.Errorf("fail to read config file: %v", err)
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🚫 [golangci-lint] reported by reviewdog 🐶
non-wrapping format verb for fmt.Errorf. Use %w to format errors (errorlint)

}

cmd.ApplyDefaults(cfg)
} else {
logrus.Debug("default config not provided")
}

return nil
}

func expandPath(path string) (string, error) {
if strings.HasPrefix(path, "~/") {
usr, err := user.Current()
if err != nil {
return "", err
}
return filepath.Join(usr.HomeDir, path[2:]), nil
}
return path, nil
}

// SetupClients configures local and PMM Server API clients.
func SetupClients(ctx context.Context, serverURL string) {
agentlocal.SetTransport(ctx, GlobalFlags.Debug || GlobalFlags.Trace)
Expand Down
63 changes: 63 additions & 0 deletions commands/base_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,9 +20,12 @@ import (
"fmt"
"io/ioutil"
"os"
"os/user"
"strings"
"testing"

"gopkg.in/ini.v1"

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

let's remove this blank line

"github.com/sirupsen/logrus"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
Expand Down Expand Up @@ -110,3 +113,63 @@ func TestReadFile(t *testing.T) {
require.Empty(t, certificate)
})
}

type cmdWithDefaultsApply struct {
applyDefaultCalled bool
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

used for tests only

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why don't use mock libraries?

user string
password string
}

func (c *cmdWithDefaultsApply) ApplyDefaults(cfg *ini.File) {
c.user = cfg.Section("client").Key("user").String()
c.password = cfg.Section("client").Key("password").String()
c.applyDefaultCalled = true
}

func TestConfigureDefaults(t *testing.T) {
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🚫 [golangci-lint] reported by reviewdog 🐶
Function TestConfigureDefaults missing the call to method parallel (paralleltest)

t.Run("ApplyDefaults is called if command supports it", func(t *testing.T) {
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🚫 [golangci-lint] reported by reviewdog 🐶
Function TestConfigureDefaults has missing the call to method parallel in the test run (paralleltest)

file, e := os.Open("../testdata/.my.cnf")
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@BupycHuk you were asking for it. I use real file in cmd test, but tmp files for commands/management/add_mysql.go

if e != nil {
t.Fatal(e)
}

cmd := &cmdWithDefaultsApply{}
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🚫 [golangci-lint] reported by reviewdog 🐶
applyDefaultCalled, user, password are missing in cmdWithDefaultsApply (exhaustivestruct)


if err := ConfigureDefaults(file.Name(), cmd); err != nil {
t.Fatal(err)
}

assert.Equal(t, "root", cmd.user)
assert.Equal(t, "toor", cmd.password)
})

t.Run("ApplyDefaults is not called if pass is not setup", func(t *testing.T) {
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🚫 [golangci-lint] reported by reviewdog 🐶
Function TestConfigureDefaults has missing the call to method parallel in the test run (paralleltest)

cmd := &cmdWithDefaultsApply{}
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🚫 [golangci-lint] reported by reviewdog 🐶
applyDefaultCalled, user, password are missing in cmdWithDefaultsApply (exhaustivestruct)


if err := ConfigureDefaults("", cmd); err != nil {
t.Fatal(err)
}

assert.Equal(t, "", cmd.user)
assert.Equal(t, "", cmd.password)
assert.False(t, cmd.applyDefaultCalled)
})
}

func TestExpandPath(t *testing.T) {
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🚫 [golangci-lint] reported by reviewdog 🐶
Function TestExpandPath missing the call to method parallel (paralleltest)

t.Run("relative to userhome", func(t *testing.T) {
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🚫 [golangci-lint] reported by reviewdog 🐶
Function TestExpandPath has missing the call to method parallel in the test run (paralleltest)

actual, err := expandPath("~/")
assert.NoError(t, err)
usr, err := user.Current()
assert.NoError(t, err)

assert.Equal(t, usr.HomeDir, actual)
})
t.Run("relative to userhome", func(t *testing.T) {
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🚫 [golangci-lint] reported by reviewdog 🐶
Function TestExpandPath has missing the call to method parallel in the test run (paralleltest)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it doesn't look relative to userhome

originalPath := "./test"
actual, err := expandPath(originalPath)
assert.NoError(t, err)

assert.Equal(t, originalPath, actual)
})
}
41 changes: 41 additions & 0 deletions commands/default_config.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
// pmm-admin
// Copyright 2019 Percona LLC
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.

package commands

import (
"io/ioutil"
"os"
)

func DefaultConfig(val string) (f *os.File, cleanup func(), e error) {
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🚫 [golangci-lint] reported by reviewdog 🐶
exported function DefaultConfig should have comment or be unexported (golint)

file, err := ioutil.TempFile("", "test-pmm-admin-defaults-*.cnf")
if err != nil {
return nil, nil, err
}
f = file
cleanup = func() {
_ = os.Remove(file.Name())
}

if _, err := file.WriteString(val); err != nil {
return nil, nil, err
}
if err := file.Sync(); err != nil {
return nil, nil, err
}

return
}
26 changes: 25 additions & 1 deletion commands/management/add_mysql.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,12 +21,17 @@ import (
"strconv"
"strings"

"github.com/sirupsen/logrus"

"gopkg.in/ini.v1"

"github.com/percona/pmm-admin/agentlocal"

Comment on lines +24 to +29
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

please fix imports to split them in 3 blocks

  1. go libs
  2. 3rd party libs
  3. local packages

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

here and in other files

"github.com/AlekSi/pointer"
"github.com/alecthomas/units"
"github.com/percona/pmm/api/managementpb/json/client"
mysql "github.com/percona/pmm/api/managementpb/json/client/my_sql"

"github.com/percona/pmm-admin/agentlocal"
"github.com/percona/pmm-admin/commands"
)

Expand Down Expand Up @@ -116,6 +121,25 @@ type addMySQLCommand struct {
CreateUser bool
}

func (cmd *addMySQLCommand) ApplyDefaults(cfg *ini.File) {
defaultUsername := cfg.Section("client").Key("user").String()
if defaultUsername != "" {
if cmd.Username == "" {
cmd.Username = defaultUsername
} else {
logrus.Debug("default username is not used, it is already set")
}
}
defaultPassword := cfg.Section("client").Key("password").String()
if defaultPassword != "" {
if cmd.Password == "" {
cmd.Password = defaultPassword
} else {
logrus.Debug("default password is not used, it is already set")
}
}
}

func (cmd *addMySQLCommand) GetServiceName() string {
return cmd.ServiceName
}
Expand Down
88 changes: 88 additions & 0 deletions commands/management/add_mysql_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,10 @@ import (
"strings"
"testing"

"github.com/sirupsen/logrus"

"github.com/percona/pmm-admin/commands"

mysql "github.com/percona/pmm/api/managementpb/json/client/my_sql"
"github.com/stretchr/testify/assert"
)
Expand Down Expand Up @@ -160,3 +164,87 @@ func TestRun(t *testing.T) {
}
})
}

func TestApplyDefaults(t *testing.T) {
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🚫 [golangci-lint] reported by reviewdog 🐶
Function 'TestApplyDefaults' is too long (86 > 60) (funlen)

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🚫 [golangci-lint] reported by reviewdog 🐶
Function 'TestApplyDefaults' is too long (81 > 60) (funlen)

t.Run("password and username is set", func(t *testing.T) {
file, cleanup, e := commands.DefaultConfig("[client]\nuser=root\npassword=toor\n")
if e != nil {
t.Fatal(e)
}
defer cleanup()

cmd := &addMySQLCommand{}

commands.ConfigureDefaults(file.Name(), cmd)
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🚫 [golangci-lint] reported by reviewdog 🐶
Error return value of commands.ConfigureDefaults is not checked (errcheck)


assert.Equal(t, "root", cmd.Username)
assert.Equal(t, "toor", cmd.Password)
})

t.Run("password and username from config have lower priority", func(t *testing.T) {
logrus.SetLevel(logrus.TraceLevel)
file, cleanup, e := commands.DefaultConfig("[client]\nuser=root\npassword=toor\n")
if e != nil {
t.Fatal(e)
}
defer cleanup()

cmd := &addMySQLCommand{
Username: "default-username",
Password: "default-password",
}

commands.ConfigureDefaults(file.Name(), cmd)
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🚫 [golangci-lint] reported by reviewdog 🐶
Error return value of commands.ConfigureDefaults is not checked (errcheck)


assert.Equal(t, "default-username", cmd.Username)
assert.Equal(t, "default-password", cmd.Password)
})

t.Run("not updated if not set", func(t *testing.T) {
file, cleanup, e := commands.DefaultConfig("")
if e != nil {
t.Fatal(e)
}
defer cleanup()

cmd := &addMySQLCommand{
Username: "default-username",
Password: "default-password",
}

commands.ConfigureDefaults(file.Name(), cmd)
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🚫 [golangci-lint] reported by reviewdog 🐶
Error return value of commands.ConfigureDefaults is not checked (errcheck)


assert.Equal(t, "default-username", cmd.Username)
assert.Equal(t, "default-password", cmd.Password)
})

t.Run("only username is set", func(t *testing.T) {
file, cleanup, e := commands.DefaultConfig("[client]\nuser=root\n")
if e != nil {
t.Fatal(e)
}
defer cleanup()

cmd := &addMySQLCommand{}

commands.ConfigureDefaults(file.Name(), cmd)

assert.Equal(t, "root", cmd.Username)
assert.Equal(t, "", cmd.Password)
})

t.Run("only password is set", func(t *testing.T) {
file, cleanup, e := commands.DefaultConfig("[client]\npassword=toor\n")
if e != nil {
t.Fatal(e)
}
defer cleanup()

cmd := &addMySQLCommand{}

commands.ConfigureDefaults(file.Name(), cmd)

assert.Equal(t, "", cmd.Username)
assert.Equal(t, "toor", cmd.Password)
})
}
1 change: 1 addition & 0 deletions go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ require (
github.com/stretchr/testify v1.6.1
golang.org/x/sys v0.0.0-20200722175500-76b94024e4b6
gopkg.in/alecthomas/kingpin.v2 v2.2.6
gopkg.in/ini.v1 v1.63.2
)

require (
Expand Down
2 changes: 2 additions & 0 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -387,6 +387,8 @@ gopkg.in/check.v1 v1.0.0-20180628173108-788fd7840127/go.mod h1:Co6ibVJAznAaIkqp8
gopkg.in/check.v1 v1.0.0-20200227125254-8fa46927fb4f h1:BLraFXnmrev5lT+xlilqcH8XK9/i0At2xKjWk4p6zsU=
gopkg.in/check.v1 v1.0.0-20200227125254-8fa46927fb4f/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0=
gopkg.in/errgo.v2 v2.1.0/go.mod h1:hNsd1EY+bozCKY1Ytp96fpM3vjJbqLJn88ws8XvfDNI=
gopkg.in/ini.v1 v1.63.2 h1:tGK/CyBg7SMzb60vP1M03vNZ3VDu3wGQJwn7Sxi9r3c=
gopkg.in/ini.v1 v1.63.2/go.mod h1:pNLf8WUiyNEtQjuu5G5vTm06TEv9tsIgeAvK8hOrP4k=
gopkg.in/reform.v1 v1.5.0/go.mod h1:AIv0CbDRJ0ljQwptGeaIXfpDRo02uJwTq92aMFELEeU=
gopkg.in/yaml.v2 v2.2.1/go.mod h1:hI93XBmqTisBFMUTm0b8Fm+jr3Dg1NNxqwp+5A1VGuI=
gopkg.in/yaml.v2 v2.2.2/go.mod h1:hI93XBmqTisBFMUTm0b8Fm+jr3Dg1NNxqwp+5A1VGuI=
Expand Down
9 changes: 9 additions & 0 deletions main.go
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,8 @@ func main() {

serverURLF := kingpin.Flag("server-url", "PMM Server URL in `https://username:password@pmm-server-host/` format").String()
kingpin.Flag("server-insecure-tls", "Skip PMM Server TLS certificate validation").BoolVar(&commands.GlobalFlags.ServerInsecureTLS)
kingpin.Flag("defaults-file", "Default config").StringVar(&commands.GlobalFlags.DefaultConfig)

kingpin.Flag("debug", "Enable debug logging").BoolVar(&commands.GlobalFlags.Debug)
kingpin.Flag("trace", "Enable trace logging (implies debug)").BoolVar(&commands.GlobalFlags.Trace)
jsonF := kingpin.Flag("json", "Enable JSON output").Bool()
Expand Down Expand Up @@ -152,6 +154,13 @@ func main() {
commands.SetupClients(ctx, *serverURLF)
}

if c, ok := command.(commands.ApplyDefaults); ok {
err := commands.ConfigureDefaults(commands.GlobalFlags.DefaultConfig, c)
if err != nil {
logrus.Panicf("Failed to configure defaults: %v", err)
}
}

var res commands.Result
var err error
if cc, ok := command.(commands.CommandWithContext); ok {
Expand Down
5 changes: 5 additions & 0 deletions testdata/.my.cnf
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
[client]
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

host=127.0.0.1
port=12345
user=root
password=toor