From 6fb628c07de327e6e0c6960f20047b22bc5abb89 Mon Sep 17 00:00:00 2001
From: Nick Irvine <115657443+nfi-hashicorp@users.noreply.github.com>
Date: Wed, 4 Jan 2023 16:57:00 -0800
Subject: [PATCH] fix: return error when config file with unknown extension is
 passed (#15107)

---
 .changelog/15107.txt         |  3 ++
 agent/config/builder.go      |  7 ++--
 agent/config/builder_test.go | 70 +++++++++++++++++-------------------
 3 files changed, 41 insertions(+), 39 deletions(-)
 create mode 100644 .changelog/15107.txt

diff --git a/.changelog/15107.txt b/.changelog/15107.txt
new file mode 100644
index 0000000000..33e3600823
--- /dev/null
+++ b/.changelog/15107.txt
@@ -0,0 +1,3 @@
+```release-note:bug
+cli: fatal error if config file does not have HCL or JSON extension, instead of warn and skip
+```
\ No newline at end of file
diff --git a/agent/config/builder.go b/agent/config/builder.go
index ad0b050ba2..d88d52fc35 100644
--- a/agent/config/builder.go
+++ b/agent/config/builder.go
@@ -56,6 +56,10 @@ type LoadOpts struct {
 
 	// ConfigFiles is a slice of paths to config files and directories that will
 	// be loaded.
+	//
+	// It is an error for any config files to have an extension other than `hcl`
+	// or `json`, unless ConfigFormat is also set. However, non-HCL/JSON files in
+	// a config directory are merely skipped, with a warning.
 	ConfigFiles []string
 
 	// ConfigFormat forces all config files to be interpreted as this format
@@ -228,8 +232,7 @@ func (b *builder) sourcesFromPath(path string, format string) ([]Source, error)
 
 	if !fi.IsDir() {
 		if !shouldParseFile(path, format) {
-			b.warn("skipping file %v, extension must be .hcl or .json, or config format must be set", path)
-			return nil, nil
+			return nil, fmt.Errorf("file %v has unknown extension; must be .hcl or .json, or config format must be set", path)
 		}
 
 		src, err := newSourceFromFile(path, format)
diff --git a/agent/config/builder_test.go b/agent/config/builder_test.go
index 1258dd8a6c..9fdd085b1d 100644
--- a/agent/config/builder_test.go
+++ b/agent/config/builder_test.go
@@ -68,41 +68,6 @@ func TestShouldParseFile(t *testing.T) {
 }
 
 func TestNewBuilder_PopulatesSourcesFromConfigFiles(t *testing.T) {
-	paths := setupConfigFiles(t)
-
-	b, err := newBuilder(LoadOpts{ConfigFiles: paths})
-	require.NoError(t, err)
-
-	expected := []Source{
-		FileSource{Name: paths[0], Format: "hcl", Data: "content a"},
-		FileSource{Name: paths[1], Format: "json", Data: "content b"},
-		FileSource{Name: filepath.Join(paths[3], "a.hcl"), Format: "hcl", Data: "content a"},
-		FileSource{Name: filepath.Join(paths[3], "b.json"), Format: "json", Data: "content b"},
-	}
-	require.Equal(t, expected, b.Sources)
-	require.Len(t, b.Warnings, 2)
-}
-
-func TestNewBuilder_PopulatesSourcesFromConfigFiles_WithConfigFormat(t *testing.T) {
-	paths := setupConfigFiles(t)
-
-	b, err := newBuilder(LoadOpts{ConfigFiles: paths, ConfigFormat: "hcl"})
-	require.NoError(t, err)
-
-	expected := []Source{
-		FileSource{Name: paths[0], Format: "hcl", Data: "content a"},
-		FileSource{Name: paths[1], Format: "hcl", Data: "content b"},
-		FileSource{Name: paths[2], Format: "hcl", Data: "content c"},
-		FileSource{Name: filepath.Join(paths[3], "a.hcl"), Format: "hcl", Data: "content a"},
-		FileSource{Name: filepath.Join(paths[3], "b.json"), Format: "hcl", Data: "content b"},
-		FileSource{Name: filepath.Join(paths[3], "c.yaml"), Format: "hcl", Data: "content c"},
-	}
-	require.Equal(t, expected, b.Sources)
-}
-
-// TODO: this would be much nicer with gotest.tools/fs
-func setupConfigFiles(t *testing.T) []string {
-	t.Helper()
 	path, err := os.MkdirTemp("", t.Name())
 	require.NoError(t, err)
 	t.Cleanup(func() { os.RemoveAll(path) })
@@ -121,12 +86,43 @@ func setupConfigFiles(t *testing.T) []string {
 		err = os.WriteFile(filepath.Join(dir, "c.yaml"), []byte("content c"), 0644)
 		require.NoError(t, err)
 	}
-	return []string{
+	paths := []string{
 		filepath.Join(path, "a.hcl"),
 		filepath.Join(path, "b.json"),
 		filepath.Join(path, "c.yaml"),
-		subpath,
 	}
+
+	t.Run("fail on unknown files", func(t *testing.T) {
+		_, err := newBuilder(LoadOpts{ConfigFiles: append(paths, subpath)})
+		require.Error(t, err)
+	})
+
+	t.Run("skip on unknown files in dir", func(t *testing.T) {
+		b, err := newBuilder(LoadOpts{ConfigFiles: []string{subpath}})
+		require.NoError(t, err)
+
+		expected := []Source{
+			FileSource{Name: filepath.Join(subpath, "a.hcl"), Format: "hcl", Data: "content a"},
+			FileSource{Name: filepath.Join(subpath, "b.json"), Format: "json", Data: "content b"},
+		}
+		require.Equal(t, expected, b.Sources)
+		require.Len(t, b.Warnings, 1)
+	})
+
+	t.Run("force config format", func(t *testing.T) {
+		b, err := newBuilder(LoadOpts{ConfigFiles: append(paths, subpath), ConfigFormat: "hcl"})
+		require.NoError(t, err)
+
+		expected := []Source{
+			FileSource{Name: paths[0], Format: "hcl", Data: "content a"},
+			FileSource{Name: paths[1], Format: "hcl", Data: "content b"},
+			FileSource{Name: paths[2], Format: "hcl", Data: "content c"},
+			FileSource{Name: filepath.Join(subpath, "a.hcl"), Format: "hcl", Data: "content a"},
+			FileSource{Name: filepath.Join(subpath, "b.json"), Format: "hcl", Data: "content b"},
+			FileSource{Name: filepath.Join(subpath, "c.yaml"), Format: "hcl", Data: "content c"},
+		}
+		require.Equal(t, expected, b.Sources)
+	})
 }
 
 func TestLoad_NodeName(t *testing.T) {