Skip to content
This repository has been archived by the owner on Jun 1, 2022. It is now read-only.

PMM-4879: defaults-file param #171

Open
wants to merge 15 commits into
base: main
Choose a base branch
from
Open
39 changes: 39 additions & 0 deletions commands/base.go
Original file line number Diff line number Diff line change
@@ -21,10 +21,13 @@ import (
"context"
"crypto/tls"
"fmt"
"gopkg.in/ini.v1"
"io"
"io/ioutil"
"net/http"
"net/url"
"os/user"
"path/filepath"
"reflect"
"regexp"
"strings"
@@ -69,6 +72,10 @@ type Command interface {
Run() (Result, error)
}

type ApplyDefaults interface {

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
@@ -115,6 +122,7 @@ type globalFlagsValues struct {
ServerInsecureTLS bool
Debug bool
Trace bool
DefaultConfig string
}

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

func ConfigureDefaults(config string, cmd ApplyDefaults) error {

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
Member

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)

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)

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)
63 changes: 63 additions & 0 deletions commands/base_test.go
Original file line number Diff line number Diff line change
@@ -18,8 +18,10 @@ package commands
import (
"bytes"
"fmt"
"gopkg.in/ini.v1"
"io/ioutil"
"os"
"os/user"
"strings"
"testing"

@@ -110,3 +112,64 @@ func TestReadFile(t *testing.T) {
require.Empty(t, certificate)
})
}

type cmdWithDefaultsApply struct {
applyDefaultCalled bool
Copy link
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
Member

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?

username string
password string
}

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

func TestConfigureDefaults(t *testing.T) {

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) {

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, cleanup, e := DefaultConfig("username=root\npassword=toor\n")
if e != nil {
t.Fatal(e)
}
defer cleanup()

cmd := &cmdWithDefaultsApply{}

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.username)
assert.Equal(t, "toor", cmd.password)
})

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

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{}

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.username)
assert.Equal(t, "", cmd.password)
assert.False(t, cmd.applyDefaultCalled)
})
}

func TestExpandPath(t *testing.T) {

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) {

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) {

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
Member

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)
})
}
26 changes: 26 additions & 0 deletions commands/default_config.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
package commands

import (
"io/ioutil"
"os"
)

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

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
}
14 changes: 13 additions & 1 deletion commands/management/add_mysql.go
Original file line number Diff line number Diff line change
@@ -17,6 +17,8 @@ package management

import (
"fmt"
"github.com/percona/pmm-admin/agentlocal"
"gopkg.in/ini.v1"
"os"
"strconv"
"strings"
@@ -26,7 +28,6 @@ import (
"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"
)

@@ -116,6 +117,17 @@ type addMySQLCommand struct {
CreateUser bool
}

func (cmd *addMySQLCommand) ApplyDefaults(cfg *ini.File) {
usernameOverride := cfg.Section("client").Key("username").String()
if usernameOverride != "" {
cmd.Username = usernameOverride
}
passwordOverride := cfg.Section("client").Key("password").String()
if passwordOverride != "" {
cmd.Password = passwordOverride
}
}

func (cmd *addMySQLCommand) GetServiceName() string {
return cmd.ServiceName
}
90 changes: 90 additions & 0 deletions commands/management/add_mysql_test.go
Original file line number Diff line number Diff line change
@@ -16,6 +16,7 @@
package management

import (
"github.com/percona/pmm-admin/commands"
"strings"
"testing"

@@ -160,3 +161,92 @@ func TestRun(t *testing.T) {
}
})
}

func TestApplyDefaults(t *testing.T) {

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)

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]\nusername=root\npassword=toor\n")
if e != nil {
t.Fatal(e)
}
defer cleanup()

cmd := &addMySQLCommand{}

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

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 priority", func(t *testing.T) {
file, cleanup, e := commands.DefaultConfig("[client]\nusername=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)

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("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)

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]\nusername=root\n")
if e != nil {
t.Fatal(e)
}
defer cleanup()

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

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

assert.Equal(t, "root", cmd.Username)
assert.Equal(t, "default-password", 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{
Username: "default-username",
Password: "default-password",
}

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

assert.Equal(t, "default-username", 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
@@ -14,4 +14,5 @@ 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 // indirect
)
95 changes: 95 additions & 0 deletions go.sum

Large diffs are not rendered by default.

9 changes: 9 additions & 0 deletions main.go
Original file line number Diff line number Diff line change
@@ -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()
@@ -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 {