diff --git a/cmd/kosli/config.go b/cmd/kosli/config.go index 8d64250fb..1a216cbd6 100644 --- a/cmd/kosli/config.go +++ b/cmd/kosli/config.go @@ -1,6 +1,7 @@ package main import ( + "errors" "fmt" "io" "os" @@ -17,15 +18,18 @@ type configOptions struct { unSetKeys []string } -const configShortDesc = `Config global Kosli flags values and store them in $HOME/.kosli . ` +const configShortDesc = `Config global Kosli flags values and store them in $HOME/.kosli.yml . ` const configLongDesc = configShortDesc + ` Flag values are determined in the following order (highest precedence first): - command line flags on each executed command. - environment variables. -- custom config file provided with --config-file flag. -- default config file in $HOME/.kosli +- custom config file provided with the --config-file flag or the KOSLI_CONFIG_FILE env var. +- default config file in $HOME/.kosli.yml + +A config file in the directory a command runs from is never read unless it is named +with --config-file or KOSLI_CONFIG_FILE. You can configure global Kosli flags (the ones that apply to all/most commands) using their dedicated convenience flags (e.g. --org). @@ -79,6 +83,12 @@ func newConfigCmd(out io.Writer) *cobra.Command { func (o *configOptions) run() error { path := defaultConfigFilePathFunc() + // An empty path means no home directory could be resolved. Continuing would + // write the config into the current working directory, which is never where + // the default config file belongs. + if path == "" { + return errors.New("setting default config failed. Could not determine your home directory. Set HOME, or pass --config-file to the commands you run") + } home := filepath.Dir(path) configFileName := filepath.Base(path) permissions := os.FileMode(0600) diff --git a/cmd/kosli/configWorkingDir_test.go b/cmd/kosli/configWorkingDir_test.go new file mode 100644 index 000000000..e3b2eb6b6 --- /dev/null +++ b/cmd/kosli/configWorkingDir_test.go @@ -0,0 +1,413 @@ +package main + +import ( + "os" + "path/filepath" + "runtime" + "strings" + "testing" + "time" + + "github.com/stretchr/testify/suite" +) + +// WorkingDirConfigTestSuite covers the config file the CLI must NOT load: one +// sitting in the current working directory that the user never named. Loading +// it lets the contents of a checkout set host, http-proxy or kubeconfig for a +// command run with a real API token (kosli-dev/server#6778, #6779). +type WorkingDirConfigTestSuite struct { + suite.Suite +} + +func (suite *WorkingDirConfigTestSuite) TearDownTest() { + defaultConfigFilePathFunc = (&RealConfigGetter{}).defaultConfigFilePath + global = new(GlobalOpts) +} + +// stubHomeConfig points the default config path at a file that does not exist, +// which is the state that used to trigger the working-directory fallback. +func (suite *WorkingDirConfigTestSuite) stubHomeConfig() string { + path := filepath.Join(suite.T().TempDir(), defaultConfigFilename) + mockConfigGetter := new(MockConfigGetter) + mockConfigGetter.Mock.On("defaultConfigFilePath").Return(path) + defaultConfigFilePathFunc = mockConfigGetter.defaultConfigFilePath + return path +} + +// chdirWithConfig writes content to a named config file in a temp directory and +// makes that directory the working directory for the test. +func (suite *WorkingDirConfigTestSuite) chdirWithConfig(name, content string) { + dir := suite.T().TempDir() + suite.Require().NoError(os.WriteFile(filepath.Join(dir, name), []byte(content), 0600)) + suite.T().Chdir(dir) +} + +func (suite *WorkingDirConfigTestSuite) TestDefaultIsHomePathWhenHomeConfigIsAbsent() { + path := suite.stubHomeConfig() + + _, _, _, _, err := executeCommandC("version") + + suite.Require().NoError(err) + suite.Equal(path, global.ConfigFile, + "the default config file must be the home path, not a bare name that resolves to the working directory") +} + +func (suite *WorkingDirConfigTestSuite) TestWorkingDirConfigIsNotLoaded() { + for _, name := range []string{"kosli.yml", "kosli.yaml", "kosli.json", "kosli.toml", "kosli.properties", "kosli.env"} { + suite.Run(name, func() { + defer func() { global = new(GlobalOpts) }() + suite.stubHomeConfig() + content := "host: https://attacker.example\n" + switch filepath.Ext(name) { + case ".json": + content = `{"host": "https://attacker.example"}` + case ".toml": + content = `host = "https://attacker.example"` + case ".properties", ".env": + content = "host=https://attacker.example\n" + } + suite.chdirWithConfig(name, content) + + _, _, _, _, err := executeCommandC("version") + + suite.Require().NoError(err) + suite.Equal(defaultHost, global.Host, + "a config file in the working directory must not set the host") + }) + } +} + +func (suite *WorkingDirConfigTestSuite) TestExplicitConfigFileFlagStillLoadsWorkingDirConfig() { + suite.stubHomeConfig() + suite.chdirWithConfig("kosli.yml", "host: https://named.example\n") + + _, _, _, stderr, err := executeCommandC("version --config-file kosli.yml") + + suite.Require().NoError(err) + suite.Equal("https://named.example", global.Host, + "a config file the user names must still be loaded") + suite.NotContains(stderr, "no longer loaded automatically", + "naming the file is the supported way to load it, so there is nothing to warn about") +} + +func (suite *WorkingDirConfigTestSuite) TestConfigFileEnvVarStillLoadsWorkingDirConfig() { + suite.stubHomeConfig() + suite.chdirWithConfig("kosli.yml", "host: https://named.example\n") + suite.T().Setenv("KOSLI_CONFIG_FILE", "kosli.yml") + + _, _, _, stderr, err := executeCommandC("version") + + suite.Require().NoError(err) + suite.Equal("https://named.example", global.Host, + "KOSLI_CONFIG_FILE must still be able to name a working-directory file") + suite.NotContains(stderr, "no longer loaded automatically") +} + +// TestEmptyDefaultConfigPathLoadsNothing covers the case where no home +// directory can be resolved, so defaultConfigFilePath returns no path at all. +// The working directory must not become the fallback search location. The empty +// path is injected because homedir.Dir cannot be made to fail portably: on +// macOS it falls back to dscl even with HOME unset. +func (suite *WorkingDirConfigTestSuite) TestEmptyDefaultConfigPathLoadsNothing() { + mockConfigGetter := new(MockConfigGetter) + mockConfigGetter.Mock.On("defaultConfigFilePath").Return("") + defaultConfigFilePathFunc = mockConfigGetter.defaultConfigFilePath + suite.chdirWithConfig("kosli.yml", "host: https://attacker.example\n") + + _, _, _, _, err := executeCommandC("version") + + suite.Require().NoError(err) + suite.Equal(defaultHost, global.Host, + "with no default config path the working directory must not be searched instead") +} + +func (suite *WorkingDirConfigTestSuite) TestWarnsAboutIgnoredWorkingDirConfig() { + cases := []struct { + name string + content string + wantWarn bool + }{ + {name: "host", content: "host: https://attacker.example\n", wantWarn: true}, + {name: "org", content: "org: some-org\n", wantWarn: true}, + {name: "api token", content: "api-token: abc123\n", wantWarn: true}, + {name: "documented uppercase keys", content: "ORG: some-org\nAPI-TOKEN: abc123\n", wantWarn: true}, + // The settings the advisories were about, and the ones a hand-picked + // key set was most likely to miss. + {name: "http proxy only", content: "http-proxy: http://proxy:8080\n", wantWarn: true}, + {name: "kubeconfig only", content: "kubeconfig: ./some-kubeconfig.yml\n", wantWarn: true}, + {name: "flow only", content: "flow: some-flow\n", wantWarn: true}, + // trail and artifacts are CLI flags as well as template keys. As a + // scalar they are config, so they must warn. + {name: "trail as a scalar", content: "trail: my-trail\n", wantWarn: true}, + {name: "artifacts as a scalar", content: "artifacts: my-artifact\n", wantWarn: true}, + // A kosli.yml in a repository root is far more often a flow template, + // which was never loaded as CLI config, so it must stay silent. + {name: "flow template shape", content: "version: 1\ntrail:\n attestations:\n - name: pull-request\n", wantWarn: false}, + {name: "flow template without version", content: "trail:\n artifacts:\n - name: nginx\n", wantWarn: false}, + {name: "artifacts only template", content: "artifacts:\n - name: nginx\n", wantWarn: false}, + {name: "unparsable file", content: "\tnot: [valid\n", wantWarn: false}, + {name: "empty file", content: "", wantWarn: false}, + } + for _, tc := range cases { + suite.Run(tc.name, func() { + defer func() { global = new(GlobalOpts) }() + suite.stubHomeConfig() + suite.chdirWithConfig("kosli.yml", tc.content) + + _, _, _, stderr, err := executeCommandC("version") + + suite.Require().NoError(err) + if tc.wantWarn { + suite.Contains(stderr, "kosli.yml") + suite.Contains(stderr, "no longer loaded automatically") + suite.Contains(stderr, "--config-file kosli.yml", + "the warning must name the fix, not just the problem") + } else { + suite.NotContains(stderr, "no longer loaded automatically") + } + }) + } +} + +// TestConfigCommandFailsWithoutHomeDirectory pins the other side of an empty +// default config path: `kosli config` must say so rather than silently writing +// a config file into the current working directory. +func (suite *WorkingDirConfigTestSuite) TestConfigCommandFailsWithoutHomeDirectory() { + mockConfigGetter := new(MockConfigGetter) + mockConfigGetter.Mock.On("defaultConfigFilePath").Return("") + defaultConfigFilePathFunc = mockConfigGetter.defaultConfigFilePath + suite.T().Chdir(suite.T().TempDir()) + + _, _, _, _, err := executeCommandC("config --org some-org") + + suite.Require().Error(err) + suite.Contains(err.Error(), "Could not determine your home directory") + _, statErr := os.Stat(defaultConfigFilename) + suite.Require().Error(statErr, "no config file may be written into the working directory") +} + +// TestHomeConfigIsStillLoaded pins the primary load path. Every other test in +// this suite stubs the default at a path that does not exist, so inverting the +// guard around the config read would leave the rest of the suite green. +func (suite *WorkingDirConfigTestSuite) TestHomeConfigIsStillLoaded() { + path := filepath.Join(suite.T().TempDir(), defaultConfigFilename) + suite.Require().NoError(os.WriteFile(path, []byte("host: https://home.example\n"), 0600)) + mockConfigGetter := new(MockConfigGetter) + mockConfigGetter.Mock.On("defaultConfigFilePath").Return(path) + defaultConfigFilePathFunc = mockConfigGetter.defaultConfigFilePath + + _, _, _, _, err := executeCommandC("version") + + suite.Require().NoError(err) + suite.Equal("https://home.example", global.Host, + "the home config file must still be loaded without the user naming it") +} + +// TestOversizedWorkingDirConfigIsNotParsed pins that the warning does not hand +// an arbitrarily large repository-controlled file to a parser. +func (suite *WorkingDirConfigTestSuite) TestOversizedWorkingDirConfigIsNotParsed() { + suite.stubHomeConfig() + padding := strings.Repeat("# padding\n", 200000) + suite.chdirWithConfig("kosli.yml", "org: some-org\n"+padding) + + _, _, _, stderr, err := executeCommandC("version") + + suite.Require().NoError(err) + suite.NotContains(stderr, "no longer loaded automatically", + "a file past the size ceiling must be skipped rather than parsed") +} + +// TestWarnsAboutIgnoredDotEnvConfig covers the two extensions beyond YAML/JSON +// that viper can actually decode. A kosli.env in the working directory was a +// working redirect before this change, so it has to warn. +func (suite *WorkingDirConfigTestSuite) TestWarnsAboutIgnoredDotEnvConfig() { + for _, name := range []string{"kosli.env", "kosli.dotenv"} { + suite.Run(name, func() { + defer func() { global = new(GlobalOpts) }() + suite.stubHomeConfig() + suite.chdirWithConfig(name, "host=https://attacker.example\n") + + _, _, _, stderr, err := executeCommandC("version") + + suite.Require().NoError(err) + suite.Equal(defaultHost, global.Host) + suite.Contains(stderr, name) + suite.Contains(stderr, "no longer loaded automatically") + }) + } +} + +// TestUndecodableWorkingDirConfigIsSilent pins that a format viper lists but has +// no decoder for stays quiet. Before this change such a file made every command +// fail with "failed to parse config file", so no pipeline can have depended on +// it and there is nothing to warn about. +func (suite *WorkingDirConfigTestSuite) TestUndecodableWorkingDirConfigIsSilent() { + suite.stubHomeConfig() + suite.chdirWithConfig("kosli.properties", "org=some-org\n") + + _, _, _, stderr, err := executeCommandC("version") + + suite.Require().NoError(err, "an undecodable file must no longer fail the command") + suite.NotContains(stderr, "no longer loaded automatically") +} + +// TestNoWarningWhenHomeConfigExists pins that the warning is limited to the +// population that actually lost behaviour. The old default fell back to the +// working directory only when the home config file was absent, so a user who +// has one never loaded the working-directory file, and telling them to pass +// --config-file would replace their home config rather than restore anything. +func (suite *WorkingDirConfigTestSuite) TestNoWarningWhenHomeConfigExists() { + path := filepath.Join(suite.T().TempDir(), defaultConfigFilename) + suite.Require().NoError(os.WriteFile(path, []byte("host: https://home.example\n"), 0600)) + mockConfigGetter := new(MockConfigGetter) + mockConfigGetter.Mock.On("defaultConfigFilePath").Return(path) + defaultConfigFilePathFunc = mockConfigGetter.defaultConfigFilePath + suite.chdirWithConfig("kosli.yml", "org: some-org\n") + + _, _, _, stderr, err := executeCommandC("version") + + suite.Require().NoError(err) + suite.Equal("https://home.example", global.Host) + suite.NotContains(stderr, "no longer loaded automatically", + "a user with a home config file never loaded the working-directory file") +} + +// TestNonRegularWorkingDirConfigIsNotParsed pins that the size ceiling cannot +// be walked around with a symlink. os.Stat follows one, and a checkout can ship +// kosli.yml -> /dev/zero, which reports IsDir false and Size 0 while an +// unbounded read waits behind it. The run is bounded so that a regression fails +// here instead of hanging the package. +func (suite *WorkingDirConfigTestSuite) TestNonRegularWorkingDirConfigIsNotParsed() { + if runtime.GOOS == "windows" { + suite.T().Skip("/dev/zero and os.Symlink are POSIX-only") + } + suite.stubHomeConfig() + dir := suite.T().TempDir() + // /dev/zero must sit on the first name in viper's SupportedExts order, + // because the loop stops at the first existing name: with the IsRegular + // guard gone this is the read that never returns. + suite.Require().NoError(os.Symlink("/dev/zero", filepath.Join(dir, "kosli.json"))) + suite.T().Chdir(dir) + + type result struct { + stderr string + err error + } + done := make(chan result, 1) + go func() { + _, _, _, stderr, err := executeCommandC("version") + done <- result{stderr, err} + }() + + select { + case got := <-done: + suite.Require().NoError(got.err) + suite.NotContains(got.stderr, "no longer loaded automatically") + case <-time.After(30 * time.Second): + // FailNow, not Fail: the goroutine is still inside the unbounded read, + // and letting the test continue would restore the working directory + // from under a live command. + suite.FailNow("reading a non-regular config file did not terminate") + } +} + +// TestNoWarningWhenHomeConfigIsNotYaml pins that the gate follows the config +// name the read above uses, not one filename. A home config is loaded from +// ~/.kosli.json just as happily as ~/.kosli.yml, and warning that user would +// tell them to replace a config file that was loaded moments earlier. +func (suite *WorkingDirConfigTestSuite) TestNoWarningWhenHomeConfigIsNotYaml() { + home := suite.T().TempDir() + suite.Require().NoError(os.WriteFile(filepath.Join(home, ".kosli.json"), + []byte(`{"host": "https://home.example"}`), 0600)) + mockConfigGetter := new(MockConfigGetter) + mockConfigGetter.Mock.On("defaultConfigFilePath").Return(filepath.Join(home, defaultConfigFilename)) + defaultConfigFilePathFunc = mockConfigGetter.defaultConfigFilePath + suite.chdirWithConfig("kosli.yml", "org: some-org\n") + + _, _, _, stderr, err := executeCommandC("version") + + suite.Require().NoError(err) + suite.Equal("https://home.example", global.Host, + "a home config file is loaded by config name, so .json counts") + suite.NotContains(stderr, "no longer loaded automatically", + "this user's home config was loaded, so nothing was lost") +} + +// TestWarnsWhenAnotherCommandShadowsTheConfigFileFlag pins that the warning +// follows the Kosli config file flag, not whatever flag of that name the running +// command happens to declare. snapshot k8s registers its own --config-file for +// namespace selectors, so looking the flag up on the command suppressed the +// warning on the very command #6779 was reported against. +func (suite *WorkingDirConfigTestSuite) TestWarnsWhenAnotherCommandShadowsTheConfigFileFlag() { + suite.stubHomeConfig() + dir := suite.T().TempDir() + suite.Require().NoError(os.WriteFile(filepath.Join(dir, "kosli.yml"), + []byte("org: some-org\nhost: https://attacker.example\n"), 0600)) + suite.Require().NoError(os.WriteFile(filepath.Join(dir, "k8s-envs.yml"), + []byte("environments:\n - name: prod-env\n namespaces: [default]\n"), 0600)) + suite.T().Chdir(dir) + + // The warning is emitted in PersistentPreRunE, which does not stop the run, + // and --api-token DRY_RUN suppresses only the Kosli request, not the cluster + // read. A kubeconfig that cannot exist stops it before any cluster is + // reached, on a developer machine with a current context as well as in CI. + _, _, _, stderr, err := executeCommandC( + "snapshot k8s --config-file k8s-envs.yml --kubeconfig " + + filepath.Join(dir, "no-such-kubeconfig") + " --api-token DRY_RUN --org some-org") + + suite.Require().Error(err, "the run must stop at the kubeconfig, never reaching a cluster") + suite.Contains(stderr, "no longer loaded automatically", + "a command's own --config-file must not be mistaken for the Kosli config file") + suite.Contains(stderr, "This command declares its own --config-file", + "on this command --config-file, -c and KOSLI_CONFIG_FILE all name something else, so none may be offered as the fix") + suite.NotContains(stderr, "pass --config-file kosli.yml") + suite.Equal(defaultHost, global.Host) +} + +// TestOnlyTheFileViperWouldHaveLoadedIsReported pins that the loop stops where +// viper stopped. viper took the first existing name in SupportedExts order, so +// with a kosli.json template beside a real kosli.yml it loaded the template and +// never read the yml. Skipping ahead to the yml would claim it was loaded when +// it never was, and the remedy would resolve back to the template, since +// --config-file strips the extension and searches the name again. +func (suite *WorkingDirConfigTestSuite) TestOnlyTheFileViperWouldHaveLoadedIsReported() { + suite.stubHomeConfig() + dir := suite.T().TempDir() + suite.Require().NoError(os.WriteFile(filepath.Join(dir, "kosli.json"), + []byte(`{"trail": {"artifacts": [{"name": "nginx"}]}}`), 0600)) + suite.Require().NoError(os.WriteFile(filepath.Join(dir, "kosli.yml"), + []byte("org: some-org\n"), 0600)) + suite.T().Chdir(dir) + + _, _, _, stderr, err := executeCommandC("version") + + suite.Require().NoError(err) + suite.NotContains(stderr, "no longer loaded automatically", + "the file viper loaded was the template, and a template is not a broken pipeline") + suite.NotContains(stderr, "kosli.yml", + "kosli.yml was never the loaded file, and --config-file kosli.yml would load the template anyway") +} + +// TestDirectoryNamedLikeAConfigFileIsSkipped pins viper's existence rule, which +// is !stat.IsDir() rather than a successful stat: a directory of that name was +// never the file viper loaded, so the search carried on past it. Stopping there +// instead would drop a real config below it in silence. +func (suite *WorkingDirConfigTestSuite) TestDirectoryNamedLikeAConfigFileIsSkipped() { + suite.stubHomeConfig() + dir := suite.T().TempDir() + suite.Require().NoError(os.Mkdir(filepath.Join(dir, "kosli.json"), 0700)) + suite.Require().NoError(os.WriteFile(filepath.Join(dir, "kosli.yml"), + []byte("org: some-org\n"), 0600)) + suite.T().Chdir(dir) + + _, _, _, stderr, err := executeCommandC("version") + + suite.Require().NoError(err) + suite.Contains(stderr, "no longer loaded automatically") + suite.Contains(stderr, "kosli.yml", + "viper skipped the directory and loaded this file, so this is the one that was lost") +} + +func TestWorkingDirConfigTestSuite(t *testing.T) { + suite.Run(t, new(WorkingDirConfigTestSuite)) +} diff --git a/cmd/kosli/root.go b/cmd/kosli/root.go index f2d180ce3..16654c3ea 100644 --- a/cmd/kosli/root.go +++ b/cmd/kosli/root.go @@ -122,7 +122,7 @@ The ^.kosli_ignore^ will be treated as part of the artifact like any other file, httpProxyFlag = "[optional] The HTTP proxy URL including protocol and port number. e.g. 'http://proxy-server-ip:proxy-port'" dryRunFlag = "[optional] Run in dry-run mode. When enabled, no data is sent to Kosli and the CLI exits with 0 exit code regardless of any errors." maxAPIRetryFlag = "[defaulted] How many times should API calls be retried when the API host is not reachable." - configFileFlag = "[optional] The Kosli config file path." + configFileFlag = "[optional] The Kosli config file path. Config is read from this path or the default only, never implicitly from the current directory." debugFlag = "[optional] Print debug logs to stdout." quietFlag = "[optional] Suppress non-critical warning messages. Errors and normal output are not affected. If both --quiet and --debug are set, --debug wins." artifactTypeFlag = "The type of the artifact to calculate its SHA256 fingerprint. One of: [oci, docker, file, dir]. Only required if you want Kosli to calculate the fingerprint for you (i.e. when you don't specify '--fingerprint' on commands that allow it)." @@ -369,18 +369,125 @@ func (r *RealConfigGetter) defaultConfigFilePath() string { return filepath.Join(home, defaultConfigFilename) } - return "kosli" // for backward compatibility with old default config location + // With no resolvable home directory there is no default config file. A bare + // name here would make viper search the current working directory, which is + // the whole problem getConfigFileFlagDefault exists to avoid. + return "" } // defaultConfigFilePathFunc is a variable holding the implementation of defaultConfigFilePath var defaultConfigFilePathFunc = (&RealConfigGetter{}).defaultConfigFilePath -func getConfigFileFlagDefault() string { - defaultPath := defaultConfigFilePathFunc() - if _, err := os.Stat(defaultPath); err == nil { - return defaultPath +// workingDirConfigNames are the config file names the CLI used to load +// implicitly from the current working directory, before that became +// kosli-dev/server#6778 and #6779. viper searched a "kosli" config name in +// every extension it supports and loaded the first match, so the list is +// derived from viper, in viper's own order. +var workingDirConfigNames = func() []string { + names := make([]string, 0, len(viper.SupportedExts)) + for _, ext := range viper.SupportedExts { + names = append(names, "kosli."+ext) + } + return names +}() + +// isFlowTemplate reports whether a parsed file is a flow template rather than +// CLI config. A template is passed with --template-file and was never loaded as +// CLI config, so an ignored one is not a broken pipeline. The shapes tell them +// apart rather than the key names, because trail and artifacts are CLI flags +// too: a template's trail is a mapping and its artifacts a sequence, where the +// flags of those names take a string, so `trail: my-trail` is config and warns. +func isFlowTemplate(v *viper.Viper) bool { + if _, ok := v.Get("trail").(map[string]any); ok { + return true } - return "kosli" // for backward compatibility with old default config location + _, ok := v.Get("artifacts").([]any) + return ok +} + +// maxWorkingDirConfigSize caps what the warning is willing to parse. The file is +// repository-controlled and read on every command run in that directory, and no +// real config file comes close to this. +const maxWorkingDirConfigSize = 1 << 20 + +// warnAboutIgnoredWorkingDirConfig reports a config file in the current working +// directory that an earlier CLI would have loaded, so that the change does not +// break a pipeline silently. +// +// Every parseable file is reported except a flow template. Reporting on the file +// rather than on a chosen set of keys is deliberate: any key set narrow enough +// to be worth writing down would let some real config break in silence, which is +// the one thing this warning exists to prevent. +func warnAboutIgnoredWorkingDirConfig(cmd *cobra.Command) { + // snapshot k8s declares its own --config-file for namespace selectors, and + // cobra's flag merge drops the root's, the -c shorthand with it. Neither + // --config-file nor KOSLI_CONFIG_FILE can name the Kosli config file there, + // so naming them would send the user into runMultiEnv with this file. + // Tested for the shadow rather than for the root flag, so that a caller + // reaching here before the flag merge gets the message true of every other + // command instead of one claiming a flag this command does not declare. + shadowed := false + if f := cmd.Flags().Lookup("config-file"); f != nil && f != cmd.Root().PersistentFlags().Lookup("config-file") { + shadowed = true + } + + for _, name := range workingDirConfigNames { + // viper loaded the first existing name in this order and stopped, so a + // later name was never the file it read. Everything below therefore + // decides whether to warn about this one, never whether to move on: + // skipping ahead would warn about a file that was never loaded, and name + // a remedy that resolves back to the file skipped over. + info, err := os.Stat(name) + if err != nil { + continue + } + + // viper's existence check is !stat.IsDir(), so a directory of this name + // was not the file it loaded. Keep looking, as it did. + if info.IsDir() { + continue + } + + // Only a regular file's Size says how much there is to read. os.Stat + // follows symlinks, and a checkout can ship kosli.json -> /dev/zero, + // which reports IsDir false and Size 0 with an unbounded read behind it. + if !info.Mode().IsRegular() || info.Size() > maxWorkingDirConfigSize { + return + } + + v := viper.New() + v.SetConfigFile(name) + // An unparseable file made every command fail outright before this + // change, so there is no behaviour to migrate. + if err := v.ReadInConfig(); err != nil { + return + } + if len(v.AllKeys()) == 0 { + return + } + if isFlowTemplate(v) { + return + } + + if shadowed { + logger.Warn("config file [%s] in the current directory is no longer loaded automatically. This command declares its own --config-file, so move its settings to your home config file with 'kosli config'.", name) + } else { + logger.Warn("config file [%s] in the current directory is no longer loaded automatically. To keep using it, pass --config-file %s or set KOSLI_CONFIG_FILE=%s. To apply its settings to every command, move them to your home config file with 'kosli config'.", name, name, name) + } + return + } +} + +// getConfigFileFlagDefault returns the default --config-file value, which is +// always the home config file whether or not that file exists. It used to fall +// back to the bare name "kosli", which viper resolves against the current +// working directory: a repository could then set host, http-proxy or kubeconfig +// for a command run with a real API token, redirecting the bearer token or +// executing a kubeconfig credential plugin (kosli-dev/server#6778, #6779). +// A config file in the working directory is now loaded only when the user names +// it with --config-file or KOSLI_CONFIG_FILE. +func getConfigFileFlagDefault() string { + return defaultConfigFilePathFunc() } func newRootCmd(out, errOut io.Writer, args []string) (*cobra.Command, error) { @@ -529,40 +636,64 @@ func initialize(cmd *cobra.Command, out, errOut io.Writer) error { // handle passing the config file as an env variable. // we load the config file before we bind env vars to flags, // so we check for the config file env var separately here - configFlag := cmd.Flags().Lookup("config-file") + // Asked of the root rather than of cmd, because snapshot k8s declares a + // local --config-file for its namespace selectors, and cobra's flag merge + // keeps the local one. Looking it up on cmd there answers about the wrong + // flag: it reports the Kosli config file as named when it was not, dropping + // KOSLI_CONFIG_FILE and suppressing the working-directory warning. + configFlag := cmd.Root().PersistentFlags().Lookup("config-file") + namedByUser := configFlag.Changed if !configFlag.Changed { // A variable set to the empty string reports as present, but it names no // file. Overriding the default with it loads no config file at all, // silently dropping org, api-token and every other configured default. if path, exists := os.LookupEnv("KOSLI_CONFIG_FILE"); exists && path != "" { global.ConfigFile = path + namedByUser = true } } - dir, file := filepath.Split(global.ConfigFile) - file = strings.TrimSuffix(file, filepath.Ext(file)) - // Set the base name of the config file, without the file extension. - v.SetConfigName(file) + if global.ConfigFile != "" { + dir, file := filepath.Split(global.ConfigFile) + file = strings.TrimSuffix(file, filepath.Ext(file)) - // Set as many paths as you like where viper should look for the - // config file. By default, we are looking in the current working directory. - if dir == "" { - dir = "." - } - v.AddConfigPath(dir) - - // Attempt to read the config file, gracefully ignoring errors - // caused by a config file not being found. Return an error - // if we cannot parse the config file. - logger.Debug("processing config file [%s]", global.ConfigFile) - if err := v.ReadInConfig(); err != nil { - // It's okay if there isn't a config file - if _, ok := err.(viper.ConfigFileNotFoundError); !ok { - return fmt.Errorf("failed to parse config file [%s] : %v", global.ConfigFile, err) - } else { - logger.Debug("config file [%s] not found. Skipping.", global.ConfigFile) + // Set the base name of the config file, without the file extension. + v.SetConfigName(file) + + // A relative path is resolved against the current working directory, + // which is what a user asks for by naming one. The default is absolute, + // so it never reaches that case. + if dir == "" { + dir = "." + } + v.AddConfigPath(dir) + + // Attempt to read the config file, gracefully ignoring errors + // caused by a config file not being found. Return an error + // if we cannot parse the config file. + logger.Debug("processing config file [%s]", global.ConfigFile) + if err := v.ReadInConfig(); err != nil { + // It's okay if there isn't a config file + if _, ok := err.(viper.ConfigFileNotFoundError); !ok { + return fmt.Errorf("failed to parse config file [%s] : %v", global.ConfigFile, err) + } else { + logger.Debug("config file [%s] not found. Skipping.", global.ConfigFile) + } } + } else { + logger.Debug("no default config file location could be determined. Skipping.") } + + // The old default fell back to the working directory only while no home + // config file existed, so only that population lost behaviour. A user whose + // home config was loaded never loaded the working-directory file, and + // passing --config-file would replace their home config rather than restore + // anything. Asked of viper rather than stat'ed, because the read above + // matches a config name: ~/.kosli.json is a home config too. Evaluated here + // because bindFlags can overwrite global.ConfigFile from a config file of + // its own. An unresolvable home directory reads nothing, and did fall back. + workingDirConfigWasLoadable := !namedByUser && v.ConfigFileUsed() == "" + // When we bind flags to environment variables expect that the // environment variables are prefixed, e.g. a flag like --namespace // binds to an environment variable KOSLI_NAMESPACE. This helps @@ -592,6 +723,12 @@ func initialize(cmd *cobra.Command, out, errOut io.Writer) error { logger.Debug("--quiet is ignored because --debug is set") } + // Warned after the flag binding above so that KOSLI_QUIET suppresses this + // message exactly as --quiet does. + if workingDirConfigWasLoadable { + warnAboutIgnoredWorkingDirConfig(cmd) + } + var err error kosliClient, err = requests.NewKosliClient(global.HttpProxy, global.MaxAPIRetries, global.Debug, logger) if err != nil {