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

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 {

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

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
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?

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

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, e := os.Open("../testdata/.my.cnf")
Copy link
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{}

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

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.user)
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)
})
}
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) {

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
Member

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
Member

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

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]\nuser=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 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)

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)

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