From 0f482a102e4738e9c2196f7c00fb467ae561f616 Mon Sep 17 00:00:00 2001
From: Luke Massa <lmassa@tripadvisor.com>
Date: Sun, 28 Jan 2024 12:12:26 -0500
Subject: [PATCH] Implementation for preventing unused flags

---
 cmd/server_test.go    | 23 ++++++++---------------
 server/user_config.go |  2 +-
 2 files changed, 9 insertions(+), 16 deletions(-)

diff --git a/cmd/server_test.go b/cmd/server_test.go
index 7795ada4e1..ba3f2064da 100644
--- a/cmd/server_test.go
+++ b/cmd/server_test.go
@@ -18,7 +18,6 @@ import (
 	"os"
 	"path/filepath"
 	"reflect"
-	"slices"
 	"strings"
 	"testing"
 
@@ -149,13 +148,6 @@ var testFlags = map[string]interface{}{
 	EnableDiffMarkdownFormat:         false,
 }
 
-// Settings used by userConfig that do not have flags
-var nonFlagSettings = []string{
-	"require-undiverged",
-	"silence-vcs-status-no-projects",
-	"webhooks",
-}
-
 func TestExecute_Defaults(t *testing.T) {
 	t.Log("Should set the defaults for all unspecified flags.")
 
@@ -228,15 +220,16 @@ func TestUserConfigAllTested(t *testing.T) {
 
 		userConfigKey := u.Field(i).Tag.Get("mapstructure")
 		t.Run(userConfigKey, func(t *testing.T) {
-			_, ok := testFlags[userConfigKey]
-			if ok {
-				return
-			}
-			if slices.Contains(nonFlagSettings, userConfigKey) {
+			// By default, we expect all fields in UserConfig to have flags defined in server.go and tested here in server_test.go
+			// Some fields are too complicated to have flags, so are only expressible in the yaml
+			flagKey := u.Field(i).Tag.Get("flag")
+			if flagKey == "false" {
 				return
 			}
-			if !ok {
-				t.Errorf("server.UserConfig has field with mapstructure %s that is either not tested. Either add it to server_test.testFlags, or remove it from server.UserConfig", userConfigKey)
+			// If a setting is configured in server.UserConfig, it should be tested here. If there is no corresponding const
+			// for specifying the flag, that probably means one *also* needs to be added to server.go
+			if _, ok := testFlags[userConfigKey]; !ok {
+				t.Errorf("server.UserConfig has field with mapstructure %s that is either not tested or not configured as a flag. Either add it to server_test.testFlags, or remove it from server.UserConfig", userConfigKey)
 			}
 		})
 
diff --git a/server/user_config.go b/server/user_config.go
index cf2366fb76..977b008610 100644
--- a/server/user_config.go
+++ b/server/user_config.go
@@ -108,7 +108,7 @@ type UserConfig struct {
 	VarFileAllowlist           string          `mapstructure:"var-file-allowlist"`
 	VCSStatusName              string          `mapstructure:"vcs-status-name"`
 	DefaultTFVersion           string          `mapstructure:"default-tf-version"`
-	Webhooks                   []WebhookConfig `mapstructure:"webhooks"`
+	Webhooks                   []WebhookConfig `mapstructure:"webhooks" flag:"false"`
 	WebBasicAuth               bool            `mapstructure:"web-basic-auth"`
 	WebUsername                string          `mapstructure:"web-username"`
 	WebPassword                string          `mapstructure:"web-password"`