Deprecate config.yaml as valid config source; Add unit regression for correct config paths (#1640)

Warn user of future deprecation of ./config.yaml for v1.0.0 release

---------

Signed-off-by: Aidan Delaney <adelaney21@bloomberg.net>
Signed-off-by: Christopher Phillips <christopher.phillips@anchore.com>
Co-authored-by: Christopher Phillips <christopher.phillips@anchore.com>

internal/config/application.go

@@ -3,6 +3,7 @@ package config
 import (
 	"errors"
 	"fmt"
+	"os"
 	"path"
 	"reflect"
 	"sort"
@@ -208,6 +209,7 @@ func (cfg Application) String() string {
 	return string(appaStr)
 }
 
+// nolint:funlen
 func loadConfig(v *viper.Viper, configPath string) error {
 	var err error
 	// use explicitly the given user config
@@ -223,13 +225,26 @@ func loadConfig(v *viper.Viper, configPath string) error {
 
 	// start searching for valid configs in order...
 	// 1. look for .<appname>.yaml (in the current directory)
+	confFilePath := "." + internal.ApplicationName
+
+	// TODO: Remove this before v1.0.0
+	// See syft #1634
 	v.AddConfigPath(".")
-	v.SetConfigName("." + internal.ApplicationName)
+	v.SetConfigName(confFilePath)
-	if err = v.ReadInConfig(); err == nil {
+
-		v.Set("config", v.ConfigFileUsed())
+	// check if config.yaml exists in the current directory
-		return nil
+	// DEPRECATED: this will be removed in v1.0.0
-	} else if !errors.As(err, &viper.ConfigFileNotFoundError{}) {
+	if _, err := os.Stat("config.yaml"); err == nil {
-		return fmt.Errorf("unable to parse config=%q: %w", v.ConfigFileUsed(), err)
+		log.Warn("DEPRECATED: ./config.yaml as a configuration file is deprecated and will be removed as an option in v1.0.0, please rename to .syft.yaml")
+	}
+
+	if _, err := os.Stat(confFilePath + ".yaml"); err == nil {
+		if err = v.ReadInConfig(); err == nil {
+			v.Set("config", v.ConfigFileUsed())
+			return nil
+		} else if !errors.As(err, &viper.ConfigFileNotFoundError{}) {
+			return fmt.Errorf("unable to parse config=%q: %w", v.ConfigFileUsed(), err)
+		}
 	}
 
 	// 2. look for .<appname>/config.yaml (in the current directory)
@@ -255,12 +270,14 @@ func loadConfig(v *viper.Viper, configPath string) error {
 		}
 	}
 
-	// 4. look for <appname>/config.yaml in xdg locations (starting with xdg home config dir, then moving upwards)
+	// 4. look for .<appname>/config.yaml in xdg locations (starting with xdg home config dir, then moving upwards)
-	v.AddConfigPath(path.Join(xdg.ConfigHome, internal.ApplicationName))
+
-	for _, dir := range xdg.ConfigDirs {
-		v.AddConfigPath(path.Join(dir, internal.ApplicationName))
-	}
 	v.SetConfigName("config")
+	configPath = path.Join(xdg.ConfigHome, "."+internal.ApplicationName)
+	v.AddConfigPath(configPath)
+	for _, dir := range xdg.ConfigDirs {
+		v.AddConfigPath(path.Join(dir, "."+internal.ApplicationName))
+	}
 	if err = v.ReadInConfig(); err == nil {
 		v.Set("config", v.ConfigFileUsed())
 		return nil

internal/config/application_test.go

@@ -0,0 +1,121 @@
+package config
+
+import (
+	"os"
+	"path"
+	"testing"
+
+	"github.com/adrg/xdg"
+	"github.com/spf13/viper"
+	"github.com/stretchr/testify/assert"
+)
+
+// TODO: set negative case when config.yaml is no longer a valid option
+func TestApplicationConfig(t *testing.T) {
+	// config is picked up at desired configuration paths
+	// VALID: .syft.yaml, .syft/config.yaml, ~/.syft.yaml, <XDG_CONFIG_HOME>/syft/config.yaml
+	// DEPRECATED: config.yaml is currently supported by
+	tests := []struct {
+		name       string
+		setup      func(t *testing.T) string
+		assertions func(t *testing.T, app *Application)
+		Cleanup    func(t *testing.T)
+	}{
+		{
+			name: "explicit config",
+			setup: func(t *testing.T) string {
+				return "./test-fixtures/.syft.yaml"
+			}, // no-op for explicit config
+			assertions: func(t *testing.T, app *Application) {
+				assert.Equal(t, "test-explicit-config", app.File)
+			},
+			Cleanup: func(t *testing.T) {},
+		},
+		{
+			name: "current working directory named config",
+			setup: func(t *testing.T) string {
+				err := os.Chdir("./test-fixtures/config-wd-file") // change application cwd to test-fixtures
+				if err != nil {
+					t.Fatalf("%s failed to change cwd: %+v", t.Name(), err)
+				}
+				return ""
+			},
+			assertions: func(t *testing.T, app *Application) {
+				assert.Equal(t, "test-wd-named-config", app.File)
+			},
+			Cleanup: func(t *testing.T) {},
+		},
+		{
+			name: "current working directory syft dir config",
+			setup: func(t *testing.T) string {
+				err := os.Chdir("./test-fixtures/config-dir-test") // change application cwd to test-fixtures
+				if err != nil {
+					t.Fatalf("%s failed to change cwd: %+v", t.Name(), err)
+				}
+				return ""
+			},
+			assertions: func(t *testing.T, app *Application) {
+				assert.Equal(t, "test-dir-config", app.File)
+			},
+			Cleanup: func(t *testing.T) {},
+		},
+		{
+			name: "home directory file config",
+			setup: func(t *testing.T) string {
+				// Because Setenv affects the whole process, it cannot be used in parallel tests or
+				// tests with parallel ancestors: see separate XDG test for consequence of this
+				t.Setenv("HOME", "./test-fixtures/config-home-test")
+				err := os.Link("./test-fixtures/config-home-test/config-file/.syft.yaml", "./test-fixtures/config-home-test/.syft.yaml")
+				if err != nil {
+					t.Fatalf("%s failed to link home config: %+v", t.Name(), err)
+				}
+				return ""
+			},
+			assertions: func(t *testing.T, app *Application) {
+				assert.Equal(t, "test-home-config", app.File)
+			},
+			Cleanup: func(t *testing.T) {
+				err := os.Remove("./test-fixtures/config-home-test/.syft.yaml") //
+				if err != nil {
+					t.Fatalf("%s failed to remove home config link: %+v", t.Name(), err)
+				}
+			},
+		},
+		{
+			name: "XDG file config",
+			setup: func(t *testing.T) string {
+				wd, err := os.Getwd()
+				if err != nil {
+					t.Fatalf("%s: failed to get working directory: %+v", t.Name(), err)
+				}
+				configDir := path.Join(wd, "./test-fixtures/config-home-test") // set HOME to testdata
+				t.Setenv("XDG_CONFIG_DIRS", configDir)
+				xdg.Reload()
+				return ""
+			},
+			assertions: func(t *testing.T, app *Application) {
+				assert.Equal(t, "test-home-XDG-config", app.File)
+			},
+			Cleanup: func(t *testing.T) {},
+		},
+	}
+	for _, test := range tests {
+		t.Run(test.name, func(t *testing.T) {
+			defer test.Cleanup(t)
+			wd, err := os.Getwd()
+			if err != nil {
+				t.Fatalf("failed to get working directory: %+v", err)
+			}
+			defer os.Chdir(wd) // reset working directory after test
+			application := &Application{}
+			viperInstance := viper.New()
+
+			configPath := test.setup(t)
+			err = application.LoadAllValues(viperInstance, configPath)
+			if err != nil {
+				t.Fatalf("failed to load application config: %+v", err)
+			}
+			test.assertions(t, application)
+		})
+	}
+}

internal/config/test-fixtures/.syft.yaml

@@ -0,0 +1,7 @@
+# same as --file; write output report to a file (default is to write to stdout)
+file: "test-explicit-config"
+package:
+  cataloger:
+    scope: "squashed"
+
+    # same as --scope; limit the scope of the cataloger to only the specified types

internal/config/test-fixtures/config-dir-test/.syft/config.yaml

@@ -0,0 +1,5 @@
+# same as --file; write output report to a file (default is to write to stdout)
+file: "test-dir-config"
+package:
+  cataloger:
+    scope: "squashed"

internal/config/test-fixtures/config-home-test/.syft/config.yaml

@@ -0,0 +1,5 @@
+# same as --file; write output report to a file (default is to write to stdout)
+file: "test-home-XDG-config"
+package:
+  cataloger:
+    scope: "squashed"

internal/config/test-fixtures/config-home-test/config-file/.syft.yaml

@@ -0,0 +1,5 @@
+# same as --file; write output report to a file (default is to write to stdout)
+file: "test-home-config"
+package:
+  cataloger:
+    scope: "squashed"

internal/config/test-fixtures/config-wd-file/.syft.yaml

@@ -0,0 +1,5 @@
+# same as --file; write output report to a file (default is to write to stdout)
+file: "test-wd-named-config"
+package:
+  cataloger:
+    scope: "squashed"