Work on nil normalization

coder · ammario · Apr 7, 2023 · Mar 30, 2023 · Mar 30, 2023 · Mar 30, 2023
commit fe93d7f1280da26fa52f7b03541b63387faad6c3
diff --git a/cli/clibase/option.go b/cli/clibase/option.go
@@ -194,3 +194,15 @@ func (s *OptionSet) SetDefaults() error {
 	}
 	return merr.ErrorOrNil()
 }
+
+// ByName returns the Option with the given name, or nil if no such option
+// exists.
+func (s *OptionSet) ByName(name string) *Option {
+	for i := range *s {
+		opt := &(*s)[i]
+		if opt.Name == name {
+			return opt
+		}
+	}
+	return nil
+}
diff --git a/cli/clibase/yaml.go b/cli/clibase/yaml.go
@@ -66,7 +66,7 @@ func (s *OptionSet) MarshalYAML() (any, error) {
 			defValue = "<unset>"
 		}
 		comment := wordwrap.WrapString(
-			fmt.Sprintf("%s (default: %s)", opt.Description, defValue),
+			fmt.Sprintf("%s (default: %s, type: %s)", opt.Description, defValue, opt.Value.Type()),
 			80,
 		)
 		nameNode := yaml.Node{
@@ -75,7 +75,12 @@ func (s *OptionSet) MarshalYAML() (any, error) {
 			HeadComment: wordwrap.WrapString(comment, 80),
 		}
 		var valueNode yaml.Node
-		if m, ok := opt.Value.(yaml.Marshaler); ok {
+		if opt.Value == nil {
+			valueNode = yaml.Node{
+				Kind:  yaml.ScalarNode,
+				Value: "null",
+			}
+		} else if m, ok := opt.Value.(yaml.Marshaler); ok {
 			v, err := m.MarshalYAML()
 			if err != nil {
 				return nil, xerrors.Errorf(

diff --git a/cli/root.go b/cli/root.go
@@ -682,7 +682,7 @@ func (r *RootCmd) checkVersions(i *clibase.Invocation, client *codersdk.Client)
 func (r *RootCmd) checkWarnings(i *clibase.Invocation, client *codersdk.Client) error {
 	if r.noFeatureWarning {
 		return nil
-}
+	}
 
 	ctx, cancel := context.WithTimeout(i.Context(), 10*time.Second)
 	defer cancel()

diff --git a/cli/server.go b/cli/server.go
@@ -195,8 +195,6 @@ func (r *RootCmd) Server(newAPI func(context.Context, *coderd.Options) (*coderd.
 				cliui.Warnf(inv.Stderr, "YAML support is experimental and offers no compatibility guarantees.")
 			}
 
-			println("HERE1")
-
 			// Print deprecation warnings.
 			for _, opt := range opts {
 				if opt.UseInstead == nil {

diff --git a/cli/server_test.go b/cli/server_test.go
@@ -21,6 +21,7 @@ import (
 	"net/url"
 	"os"
 	"path/filepath"
+	"reflect"
 	"runtime"
 	"strconv"
 	"strings"
@@ -35,6 +36,7 @@ import (
 	"gopkg.in/yaml.v3"
 
 	"github.com/coder/coder/cli"
+	"github.com/coder/coder/cli/clibase"
 	"github.com/coder/coder/cli/clitest"
 	"github.com/coder/coder/cli/config"
 	"github.com/coder/coder/coderd/coderdtest"
@@ -1441,12 +1443,13 @@ func TestServer(t *testing.T) {
 			client := codersdk.New(gotURL)
 
 			_ = coderdtest.CreateFirstUser(t, client)
-			wantValues, err := client.DeploymentConfig(ctx)
+			wantConfig, err := client.DeploymentConfig(ctx)
 			require.NoError(t, err)
 			cancel()
 			w.RequireSuccess()
 
-			// Next, we instruct the same server to display config.
+			// Next, we instruct the same server to display the YAML config
+			// and then save it.
 			inv = inv.WithContext(testutil.Context(t, testutil.WaitMedium))
 			inv.Args = append(args, "--write-config")
 			fi, err := os.OpenFile(testutil.TempFile(t, "", "coder-config-test-*"), os.O_WRONLY|os.O_CREATE, 0o600)
@@ -1458,19 +1461,55 @@ func TestServer(t *testing.T) {
 			err = inv.Run()
 			require.NoError(t, err)
 
-			// Finally, we read the config back in and ensure it matches the
-			// original.
-			inv.Args = append(args, "--config="+fi.Name())
+			// Reset the context.
+			ctx = testutil.Context(t, testutil.WaitMedium)
+			// Finally, we restart the server with just the config and no flags
+			// and ensure that the live configuration is equivalent.
+			inv, cfg = clitest.New(t, "server", "--config="+fi.Name())
 			w = clitest.StartWithWaiter(t, inv)
-			// The same client should work.
-			gotValues, err := client.DeploymentConfig(ctx)
-			require.NoError(t, err, "config:\n%s", conf.String())
-			require.Equal(t, wantValues, gotValues)
+			client = codersdk.New(waitAccessURL(t, cfg))
+			_ = coderdtest.CreateFirstUser(t, client)
+			gotConfig, err := client.DeploymentConfig(ctx)
+			normalizeNilsInOptionSet(&wantConfig.Options)
+			normalizeNilsInOptionSet(&gotConfig.Options)
+			require.NoError(t, err, "config:\n%s\nargs: %+v", conf.String(), inv.Args)
+			gotConfig.Options.ByName("Config Path").Value.Set("")
+			require.EqualValues(t, wantConfig.Options, gotConfig.Options)
 			w.RequireSuccess()
 		})
 	})
 }
 
+// The YAML process sets slice values to nil if they are empty, leading to
+// false negatives when comparing equality.
+func normalizeNilsInOptionSet(s *clibase.OptionSet) {
+	for i := range *s {
+		opt := &(*s)[i]
+		if opt.Value == nil {
+			continue
+		}
+
+		var (
+			v    = reflect.Indirect(reflect.ValueOf(opt.Value))
+			kind = v.Type().Kind()
+		)
+
+		if opt.YAML == "supportLinks" {
+			fmt.Printf("supportLinks: %v, %v", kind, v.IsZero())
+		}
+
+		if kind == reflect.Slice && v.Len() > 0 {
+			continue
+		}
+
+		if kind == reflect.Struct && !v.IsZero() {
+			continue
+		}
+
+		opt.Value = nil
+	}
+}
+
 func generateTLSCertificate(t testing.TB, commonName ...string) (certPath, keyPath string) {
 	dir := t.TempDir()
 
@@ -1542,7 +1581,7 @@ func TestServerYAMLConfig(t *testing.T) {
 	require.NoError(t, err)
 
 	// Sanity-check that we can read the config back in.
-	err = opts.UnmarshalYAML(n)
+	err = opts.UnmarshalYAML(n.(*yaml.Node))
 	require.NoError(t, err)
 
 	wantByt, err := yaml.Marshal(n)