Move flag parsing to commands

Author: bjee19

.goreleaser.yml

@@ -15,7 +15,7 @@ builds:
     asmflags:
       - all=-trimpath={{.Env.GOPATH}}
     ldflags:
-      - -s -w -X main.version={{.Version}} -X main.commit={{.Commit}} -X main.date={{.Date}} -X main.telemetryReportPeriod=15s
+      - -s -w -X main.version={{.Version}} -X main.commit={{.Commit}} -X main.date={{.Date}} -X main.telemetryReportPeriod=24h
     main: ./cmd/gateway/
     binary: gateway
 

Makefile

@@ -8,7 +8,7 @@ NGINX_CONF_DIR = internal/mode/static/nginx/conf
 NJS_DIR = internal/mode/static/nginx/modules/src
 NGINX_DOCKER_BUILD_PLUS_ARGS = --secret id=nginx-repo.crt,src=nginx-repo.crt --secret id=nginx-repo.key,src=nginx-repo.key
 BUILD_AGENT=local
-TELEMETRY_REPORT_PERIOD = 15s # also configured in goreleaser.yml
+TELEMETRY_REPORT_PERIOD = 24h # also configured in goreleaser.yml
 GW_API_VERSION = 1.0.0
 INSTALL_WEBHOOK = false
 

cmd/gateway/commands.go

@@ -7,6 +7,7 @@ import (
 	"time"
 
 	"github.com/spf13/cobra"
+	"github.com/spf13/pflag"
 	"go.uber.org/zap"
 	"k8s.io/apimachinery/pkg/types"
 	utilruntime "k8s.io/apimachinery/pkg/util/runtime"
@@ -178,6 +179,8 @@ func createStaticModeCommand() *cobra.Command {
 				}
 			}
 
+			flagKeys, flagValues := parseFlagKeysAndValues(cmd.Flags())
+
 			conf := config.Config{
 				GatewayCtlrName:          gatewayCtlrName.value,
 				ConfigName:               configName.String(),
@@ -215,7 +218,10 @@ func createStaticModeCommand() *cobra.Command {
 				Version:              version,
 				ExperimentalFeatures: gwExperimentalFeatures,
 				ImageSource:          imageSource,
-				Flags:                cmd.Flags(),
+				FlagKeyValues: config.FlagKeyValues{
+					FlagKeys:   flagKeys,
+					FlagValues: flagValues,
+				},
 			}
 
 			if err := static.StartManager(conf); err != nil {
@@ -451,3 +457,29 @@ func createSleepCommand() *cobra.Command {
 
 	return cmd
 }
+
+func parseFlagKeysAndValues(flags *pflag.FlagSet) (flagKeys, flagValues []string) {
+	flagKeys = []string{}
+	flagValues = []string{}
+
+	flags.VisitAll(
+		func(flag *pflag.Flag) {
+			flagKeys = append(flagKeys, flag.Name)
+			switch flag.Value.Type() {
+			case "bool":
+				flagValues = append(flagValues, flag.Value.String())
+			default:
+				var val string
+				if flag.Value.String() == flag.DefValue {
+					val = "default"
+				} else {
+					val = "user-defined"
+				}
+
+				flagValues = append(flagValues, val)
+			}
+		},
+	)
+
+	return flagKeys, flagValues
+}

cmd/gateway/commands_test.go

@@ -6,6 +6,8 @@ import (
 
 	. "github.com/onsi/gomega"
 	"github.com/spf13/cobra"
+	"github.com/spf13/pflag"
+	"k8s.io/apimachinery/pkg/types"
 )
 
 type flagTestCase struct {
@@ -454,3 +456,134 @@ func TestSleepCmdFlagValidation(t *testing.T) {
 		})
 	}
 }
+
+func TestParseFlagKeysAndValues(t *testing.T) {
+	g := NewWithT(t)
+
+	flagSet := pflag.NewFlagSet("flagSet", 0)
+	// set SortFlags to false for testing purposes so when parseFlagKeysAndValues loops over the flagSet it
+	// goes off of primordial order.
+	flagSet.SortFlags = false
+
+	var boolFlagTrue bool
+	flagSet.BoolVar(
+		&boolFlagTrue,
+		"boolFlagTrue",
+		true,
+		"boolean true test flag",
+	)
+
+	var boolFlagFalse bool
+	flagSet.BoolVar(
+		&boolFlagFalse,
+		"boolFlagFalse",
+		false,
+		"boolean false test flag",
+	)
+
+	intFlagDefault := intValidatingValue{
+		validator: validatePort,
+		value:     8080,
+	}
+	flagSet.Var(
+		&intFlagDefault,
+		"intFlagDefault",
+		"default int test flag",
+	)
+
+	intFlagUserDefined := intValidatingValue{
+		validator: validatePort,
+		value:     8080,
+	}
+	flagSet.Var(
+		&intFlagUserDefined,
+		"intFlagUserDefined",
+		"user defined int test flag",
+	)
+	err := flagSet.Set("intFlagUserDefined", "8081")
+	g.Expect(err).To(Not(HaveOccurred()))
+
+	stringFlagDefault := stringValidatingValue{
+		validator: validateResourceName,
+		value:     "default-string-test-flag",
+	}
+	flagSet.Var(
+		&stringFlagDefault,
+		"stringFlagDefault",
+		"default string test flag",
+	)
+
+	stringFlagUserDefined := stringValidatingValue{
+		validator: validateResourceName,
+		value:     "user-defined-string-test-flag",
+	}
+	flagSet.Var(
+		&stringFlagUserDefined,
+		"stringFlagUserDefined",
+		"user defined string test flag",
+	)
+	err = flagSet.Set("stringFlagUserDefined", "changed-test-flag-value")
+	g.Expect(err).To(Not(HaveOccurred()))
+
+	namespacedNameFlagDefault := namespacedNameValue{
+		value: types.NamespacedName{
+			Namespace: "test",
+			Name:      "test-flag",
+		},
+	}
+	flagSet.Var(
+		&namespacedNameFlagDefault,
+		"namespacedNameFlagDefault",
+		"default namespacedName test flag",
+	)
+
+	namespacedNameFlagUserDefined := namespacedNameValue{
+		value: types.NamespacedName{
+			Namespace: "test",
+			Name:      "test-flag",
+		},
+	}
+	flagSet.Var(
+		&namespacedNameFlagUserDefined,
+		"namespacedNameFlagUserDefined",
+		"user defined namespacedName test flag",
+	)
+	userDefinedNamespacedName := types.NamespacedName{
+		Namespace: "changed-namespace",
+		Name:      "changed-name",
+	}
+	err = flagSet.Set("namespacedNameFlagUserDefined", userDefinedNamespacedName.String())
+	g.Expect(err).To(Not(HaveOccurred()))
+
+	expectedKeys := []string{
+		"boolFlagTrue",
+		"boolFlagFalse",
+
+		"intFlagDefault",
+		"intFlagUserDefined",
+
+		"stringFlagDefault",
+		"stringFlagUserDefined",
+
+		"namespacedNameFlagDefault",
+		"namespacedNameFlagUserDefined",
+	}
+	expectedValues := []string{
+		"true",
+		"false",
+
+		"default",
+		"user-defined",
+
+		"default",
+		"user-defined",
+
+		"default",
+		"user-defined",
+	}
+
+	flagKeys, flagValues := parseFlagKeysAndValues(flagSet)
+
+	g.Expect(flagKeys).Should(Equal(expectedKeys))
+	g.Expect(flagValues).Should(Equal(expectedValues))
+}

internal/mode/static/config/config.go

@@ -4,7 +4,6 @@ import (
 	"time"
 
 	"github.com/go-logr/logr"
-	"github.com/spf13/pflag"
 	"go.uber.org/zap"
 	"k8s.io/apimachinery/pkg/types"
 )
@@ -16,8 +15,8 @@ type Config struct {
 	ImageSource string
 	// AtomicLevel is an atomically changeable, dynamic logging level.
 	AtomicLevel zap.AtomicLevel
-	// Flags are all the NGF flags.
-	Flags *pflag.FlagSet
+	// FlagKeyValues holds the parsed NGF flag keys and values.
+	FlagKeyValues FlagKeyValues
 	// GatewayNsName is the namespaced name of a Gateway resource that the Gateway will use.
 	// The Gateway will ignore all other Gateway resources.
 	GatewayNsName *types.NamespacedName
@@ -108,3 +107,13 @@ type UsageReportConfig struct {
 	// InsecureSkipVerify controls whether the client verifies the server cert.
 	InsecureSkipVerify bool
 }
+
+// FlagKeyValues holds the parsed NGF flag keys and values.
+// Flag Key and Value are paired based off of index in slice.
+type FlagKeyValues struct {
+	// FlagKeys holds the name of the flag.
+	FlagKeys []string
+	// FlagValues holds the value of the flag in string form.
+	// Value will be either true or false for boolean flags and default or user-defined for non-boolean flags.
+	FlagValues []string
+}

internal/mode/static/manager.go

@@ -252,8 +252,8 @@ func StartManager(cfg config.Config) error {
 				Namespace: cfg.GatewayPodConfig.Namespace,
 				Name:      cfg.GatewayPodConfig.Name,
 			},
-			ImageSource: cfg.ImageSource,
-			Flags:       cfg.Flags,
+			ImageSource:   cfg.ImageSource,
+			FlagKeyValues: cfg.FlagKeyValues,
 		})
 		if err = mgr.Add(createTelemetryJob(cfg, dataCollector, nginxChecker.getReadyCh())); err != nil {
 			return fmt.Errorf("cannot register telemetry job: %w", err)