Skip to content

Commit

Permalink
Merge pull request #7181 from dolthub/steph/config
Browse files Browse the repository at this point in the history 
Warning for nonsense variables in config
  • Loading branch information
@stephkyou
stephkyou authored Dec 19, 2023
2 parents 10cac7a + d06e827 commit 2d717c6
Show file tree
Hide file tree
Showing 6 changed files with 168 additions and 80 deletions.
8 changes: 7 additions & 1 deletion go/cmd/dolt/commands/config.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -207,7 +207,13 @@ func addOperation(dEnv *env.DoltEnv, setCfgTypes *set.StrSet, args []string, usa

updates := make(map[string]string)
for i := 0; i < len(args); i += 2 {
updates[strings.ToLower(args[i])] = args[i+1]
option := strings.ToLower(args[i])
value := args[i+1]
if _, ok := config.ConfigOptions[option]; !ok && !strings.HasPrefix(option, env.SqlServerGlobalsPrefix) {
cli.Println("error: invalid config option, use dolt config --help to check valid configuration variables")
return 1
}
updates[option] = value
}

var cfgType string
Expand Down
60 changes: 30 additions & 30 deletions go/cmd/dolt/commands/config_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -51,25 +51,25 @@ func TestConfigAdd(t *testing.T) {
Name: "local",
CfgSet: localCfg,
Scope: env.LocalConfig,
Args: []string{"title", "senior dufus"},
Args: []string{"user.name", "senior dufus"},
},
{
Name: "global",
CfgSet: globalCfg,
Scope: env.GlobalConfig,
Args: []string{"title", "senior dufus"},
Args: []string{"user.name", "senior dufus"},
},
{
Name: "default",
CfgSet: &set.StrSet{},
Scope: env.LocalConfig,
Args: []string{"title", "senior dufus"},
Args: []string{"user.name", "senior dufus"},
},
{
Name: "multi error",
CfgSet: multiCfg,
Scope: env.LocalConfig,
Args: []string{"title", "senior dufus"},
Args: []string{"user.name", "senior dufus"},
Code: 1,
},
{
Expand All @@ -83,7 +83,7 @@ func TestConfigAdd(t *testing.T) {
Name: "odd args",
CfgSet: multiCfg,
Scope: env.LocalConfig,
Args: []string{"title"},
Args: []string{"user.name"},
Code: 1,
},
}
Expand All @@ -97,7 +97,7 @@ func TestConfigAdd(t *testing.T) {
assert.Equal(t, tt.Code, resCode)

} else if cfg, ok := dEnv.Config.GetConfig(tt.Scope); ok {
resVal := cfg.GetStringOrDefault("title", "")
resVal := cfg.GetStringOrDefault("user.name", "")
assert.Equal(t, "senior dufus", resVal)
} else {
t.Error("comparison config not found")
Expand Down Expand Up @@ -280,12 +280,12 @@ func TestConfig(t *testing.T) {

// test setting global config with --add
configCmd := ConfigCmd{}
ret := configCmd.Exec(ctx, "dolt config", []string{"-global", "--add", "name", "bheni"}, dEnv, nil)
ret += configCmd.Exec(ctx, "dolt config", []string{"-global", "--add", "title", "dufus"}, dEnv, nil)
ret := configCmd.Exec(ctx, "dolt config", []string{"-global", "--add", "user.name", "bheni"}, dEnv, nil)
ret += configCmd.Exec(ctx, "dolt config", []string{"-global", "--add", "user.email", "dufus@example.com"}, dEnv, nil)

expectedGlobal := map[string]string{
"name": "bheni",
"title": "dufus",
"user.name": "bheni",
"user.email": "dufus@example.com",
}

if ret != 0 {
Expand All @@ -295,11 +295,11 @@ func TestConfig(t *testing.T) {
}

// test setting global config with --set
ret = configCmd.Exec(ctx, "dolt config", []string{"-global", "--set", "name", "steph"}, dEnv, nil)
ret = configCmd.Exec(ctx, "dolt config", []string{"-global", "--set", "user.name", "steph"}, dEnv, nil)

expectedGlobal = map[string]string{
"name": "steph",
"title": "dufus",
"user.name": "steph",
"user.email": "dufus@example.com",
}

if ret != 0 {
Expand All @@ -309,40 +309,40 @@ func TestConfig(t *testing.T) {
}

// test setting local config with --add
ret = configCmd.Exec(ctx, "dolt config", []string{"-local", "--add", "title", "senior dufus"}, dEnv, nil)
ret = configCmd.Exec(ctx, "dolt config", []string{"-local", "--add", "user.name", "senior dufus"}, dEnv, nil)

expectedLocal := map[string]string{
"title": "senior dufus",
"user.name": "senior dufus",
}

if ret != 0 {
t.Error("Failed to set local config")
} else if cfg, ok := dEnv.Config.GetConfig(env.LocalConfig); !ok || !config.Equals(cfg, expectedLocal) {
t.Error("config -add did not yield expected local results")
} else if val, err := cfg.GetString("title"); err != nil || val != "senior dufus" {
t.Error("Unexpected value of \"title\" retrieved from the config hierarchy")
} else if val, err := cfg.GetString("user.name"); err != nil || val != "senior dufus" {
t.Error("Unexpected value of \"user.name\" retrieved from the config hierarchy")
}

// test setting local config with --set
ret = configCmd.Exec(ctx, "dolt config", []string{"-local", "--set", "name", "steph"}, dEnv, nil)
ret = configCmd.Exec(ctx, "dolt config", []string{"-local", "--set", "user.email", "dufus@example.com"}, dEnv, nil)

expectedLocal = map[string]string{
"name": "steph",
"title": "senior dufus",
"user.name": "senior dufus",
"user.email": "dufus@example.com",
}

if ret != 0 {
t.Error("Failed to set local config")
} else if cfg, ok := dEnv.Config.GetConfig(env.LocalConfig); !ok || !config.Equals(cfg, expectedLocal) {
t.Error("config -set did not yield expected local results")
} else if val, err := cfg.GetString("name"); err != nil || val != "steph" {
t.Error("Unexpected value of \"name\" retrieved from the config hierarchy")
} else if val, err := cfg.GetString("user.email"); err != nil || val != "dufus@example.com" {
t.Error("Unexpected value of \"user.email\" retrieved from the config hierarchy")
}

ret = configCmd.Exec(ctx, "dolt config", []string{"-global", "--unset", "name"}, dEnv, nil)
ret = configCmd.Exec(ctx, "dolt config", []string{"-global", "--unset", "user.name"}, dEnv, nil)

expectedGlobal = map[string]string{
"title": "dufus",
"user.email": "dufus@example.com",
}

if ret != 0 {
Expand All @@ -351,7 +351,7 @@ func TestConfig(t *testing.T) {
t.Error("config -add did not yield expected global results")
}

expectedGlobal = map[string]string{"title": "dufus"}
expectedGlobal = map[string]string{"user.email": "dufus@example.com"}
globalProperties := map[string]string{}
ret = listOperation(dEnv, globalCfg, []string{}, func() {}, func(k string, v string) {
globalProperties[k] = v
Expand All @@ -363,7 +363,7 @@ func TestConfig(t *testing.T) {
t.Error("listOperation did not yield expected global results")
}

expectedLocal = map[string]string{"name": "steph", "title": "senior dufus"}
expectedLocal = map[string]string{"user.name": "senior dufus", "user.email": "dufus@example.com"}
localProperties := map[string]string{}
ret = listOperation(dEnv, localCfg, []string{}, func() {}, func(k string, v string) {
localProperties[k] = v
Expand All @@ -375,8 +375,8 @@ func TestConfig(t *testing.T) {
t.Error("listOperation did not yield expected local results")
}

ret = getOperation(dEnv, globalCfg, []string{"title"}, func(k string, v *string) {
if v == nil || *v != "dufus" {
ret = getOperation(dEnv, globalCfg, []string{"user.email"}, func(k string, v *string) {
if v == nil || *v != "dufus@example.com" {
t.Error("Failed to get expected value for title.")
}
})
Expand All @@ -385,9 +385,9 @@ func TestConfig(t *testing.T) {
t.Error("get operation failed")
}

ret = getOperation(dEnv, globalCfg, []string{"name"}, func(k string, v *string) {
ret = getOperation(dEnv, globalCfg, []string{"user.name"}, func(k string, v *string) {
if v != nil {
t.Error("Failed to get expected value for \"name\" which should not be set in the config.")
t.Error("Failed to get expected value for \"user.name\" which should not be set in the config.")
}
})

Expand Down
21 changes: 21 additions & 0 deletions go/cmd/dolt/dolt.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ import (
"os/exec"
"path/filepath"
"strconv"
"strings"
"time"

"github.com/dolthub/go-mysql-server/sql"
Expand Down Expand Up @@ -461,6 +462,26 @@ func runMain() int {
return 1
}

globalConfig.Iter(func(name, val string) (stop bool) {
option := strings.ToLower(name)
if _, ok := config.ConfigOptions[option]; !ok && !strings.HasPrefix(option, env.SqlServerGlobalsPrefix) {
cli.Println(color.YellowString("Warning: Unknown global config option '%s'. Use `dolt config --global --unset %s` to remove.", name, name))
}
return false
})

// try verifying contents of local config
localConfig, ok := dEnv.Config.GetConfig(env.LocalConfig)
if ok {
localConfig.Iter(func(name, val string) (stop bool) {
option := strings.ToLower(name)
if _, ok := config.ConfigOptions[option]; !ok && !strings.HasPrefix(option, env.SqlServerGlobalsPrefix) {
cli.Println(color.YellowString("Warning: Unknown local config option '%s'. Use `dolt config --local --unset %s` to remove.", name, name))
}
return false
})
}

apr, remainingArgs, subcommandName, err := parseGlobalArgsAndSubCommandName(globalConfig, args)
if err == argparser.ErrHelp {
doltCommand.PrintUsage("dolt")
Expand Down
20 changes: 20 additions & 0 deletions go/libraries/utils/config/keys.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,24 @@

package config

var ConfigOptions = map[string]struct{}{
UserEmailKey: {},
UserNameKey: {},
UserCreds: {},
DoltEditor: {},
InitBranchName: {},
RemotesApiHostKey: {},
RemotesApiHostPortKey: {},
AddCredsUrlKey: {},
DoltLabInsecureKey: {},
MetricsDisabled: {},
MetricsHost: {},
MetricsPort: {},
MetricsInsecure: {},
PushAutoSetupRemote: {},
ProfileKey: {},
}

const UserEmailKey = "user.email"

const UserNameKey = "user.name"
Expand Down Expand Up @@ -41,3 +59,5 @@ const MetricsPort = "metrics.port"
const MetricsInsecure = "metrics.insecure"

const PushAutoSetupRemote = "push.autosetupremote"

const ProfileKey = "profile"
28 changes: 14 additions & 14 deletions integration-tests/bats/config-home.bats
View file
Original file line number Diff line number Diff line change
Expand Up @@ -37,41 +37,41 @@ teardown() {
HOME="$BATS_TMPDIR/config-test$$/homeA"

HOME=$HOME dolt config --global --add metrics.disabled true > /dev/null 2>&1
HOME=$HOME run dolt config --global --add test testA
HOME=$HOME run dolt config --global --add user.name "Your Name"
[ "$status" -eq 0 ]
[[ "$output" =~ "Config successfully updated" ]] || false
HOME=$HOME run dolt config --list
[ "$status" -eq 0 ]
[[ "$output" =~ "test = testA" ]] || false
HOME=$HOME run dolt config --get test
[[ "$output" =~ "user.name = Your Name" ]] || false
HOME=$HOME run dolt config --get user.name
[ "$status" -eq 0 ]
[ "$output" = "testA" ]
[ "$output" = "Your Name" ]

mkdir "$BATS_TMPDIR/config-test$$/homeB"
HOME="$BATS_TMPDIR/config-test$$/homeB"

HOME=$HOME dolt config --global --add metrics.disabled true > /dev/null 2>&1
HOME=$HOME run dolt config --global --add test testB
HOME=$HOME run dolt config --global --add core.editor foo
[ "$status" -eq 0 ]
[[ "$output" =~ "Config successfully updated" ]] || false
HOME=$HOME run dolt config --list
[ "$status" -eq 0 ]
[[ "$output" =~ "test = testB" ]] || false
[[ ! "$output" =~ "test = testA" ]] || false
HOME=$HOME run dolt config --get test
[[ "$output" =~ "core.editor = foo" ]] || false
[[ ! "$output" =~ "user.name = Your Name" ]] || false
HOME=$HOME run dolt config --get core.editor
[ "$status" -eq 0 ]
[ "$output" = "testB" ]
[ "$output" = "foo" ]

HOME="$BATS_TMPDIR/config-test$$/homeA"

HOME=$HOME run dolt config --global --add test testA
HOME=$HOME run dolt config --global --add user.email "you@example.com"
[ "$status" -eq 0 ]
[[ "$output" =~ "Config successfully updated" ]] || false
HOME=$HOME run dolt config --list
[ "$status" -eq 0 ]
[[ "$output" =~ "test = testA" ]] || false
[[ ! "$output" =~ "test = testB" ]] || false
HOME=$HOME run dolt config --get test
[[ "$output" =~ "user.email = you@example.com" ]] || false
[[ ! "$output" =~ "core.editor = foo" ]] || false
HOME=$HOME run dolt config --get user.email
[ "$status" -eq 0 ]
[ "$output" = "testA" ]
[ "$output" = "you@example.com" ]
}
Loading

0 comments on commit 2d717c6

Please sign in to comment.