From cc48e68d10559ac29f11b24b297711a0f3257e5d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Patryk=20Ma=C5=82ek?= Date: Fri, 21 Jun 2024 15:01:31 +0200 Subject: [PATCH] refactor: encapsulate metadata in a struct (#365) --- Makefile | 10 ++-- cmd/main.go | 7 ++- modules/cli/cli.go | 16 ++++-- modules/cli/cli_test.go | 5 +- modules/manager/metadata/metadata.go | 52 ++++++++++++++++--- modules/manager/run.go | 12 +++-- scripts/apidocs-gen/config.yaml | 0 scripts/apidocs-gen/template/gv_details.tpl | 0 scripts/apidocs-gen/template/gv_list.tpl | 0 scripts/apidocs-gen/template/type.tpl | 0 scripts/apidocs-gen/template/type_members.tpl | 0 test/conformance/conformance_test.go | 1 + test/conformance/suite_test.go | 8 +-- test/integration/run_integration_test.go | 5 +- 14 files changed, 88 insertions(+), 28 deletions(-) mode change 100644 => 100755 scripts/apidocs-gen/config.yaml mode change 100644 => 100755 scripts/apidocs-gen/template/gv_details.tpl mode change 100644 => 100755 scripts/apidocs-gen/template/gv_list.tpl mode change 100644 => 100755 scripts/apidocs-gen/template/type.tpl mode change 100644 => 100755 scripts/apidocs-gen/template/type_members.tpl diff --git a/Makefile b/Makefile index 82eedfff4..36d94dc1a 100644 --- a/Makefile +++ b/Makefile @@ -28,11 +28,11 @@ LDFLAGS_COMMON ?= -extldflags=-Wl,-ld_classic endif LDFLAGS_METADATA ?= \ - -X $(REPO)/modules/manager/metadata.ProjectName=$(REPO_NAME) \ - -X $(REPO)/modules/manager/metadata.Release=$(TAG) \ - -X $(REPO)/modules/manager/metadata.Commit=$(COMMIT) \ - -X $(REPO)/modules/manager/metadata.Repo=$(REPO_INFO) \ - -X $(REPO)/modules/manager/metadata.RepoURL=$(REPO_URL) + -X $(REPO)/modules/manager/metadata.projectName=$(REPO_NAME) \ + -X $(REPO)/modules/manager/metadata.release=$(TAG) \ + -X $(REPO)/modules/manager/metadata.commit=$(COMMIT) \ + -X $(REPO)/modules/manager/metadata.repo=$(REPO_INFO) \ + -X $(REPO)/modules/manager/metadata.repoURL=$(REPO_URL) # ------------------------------------------------------------------------------ # Configuration - Tooling diff --git a/cmd/main.go b/cmd/main.go index dba207d6a..5b43c9e4d 100644 --- a/cmd/main.go +++ b/cmd/main.go @@ -25,16 +25,19 @@ import ( "github.com/kong/gateway-operator/modules/admission" "github.com/kong/gateway-operator/modules/cli" "github.com/kong/gateway-operator/modules/manager" + "github.com/kong/gateway-operator/modules/manager/metadata" "github.com/kong/gateway-operator/modules/manager/scheme" ) func main() { - cli := cli.New() + m := metadata.Metadata() + + cli := cli.New(m) cfg := cli.Parse(os.Args[1:]) ctrl.SetLogger(zap.New(zap.UseFlagOptions(cfg.LoggerOpts))) - if err := manager.Run(cfg, scheme.Get(), manager.SetupControllersShim, admission.NewRequestHandler, nil); err != nil { + if err := manager.Run(cfg, scheme.Get(), manager.SetupControllersShim, admission.NewRequestHandler, nil, m); err != nil { ctrl.Log.Error(err, "failed to run manager") os.Exit(1) } diff --git a/modules/cli/cli.go b/modules/cli/cli.go index f057cc8b5..d3200269c 100644 --- a/modules/cli/cli.go +++ b/modules/cli/cli.go @@ -16,7 +16,7 @@ import ( ) // New returns a new CLI. -func New() *CLI { +func New(m metadata.Info) *CLI { flagSet := flag.NewFlagSet("", flag.ExitOnError) var cfg manager.Config @@ -64,6 +64,7 @@ func New() *CLI { cfg: &cfg, loggerOpts: loggerOpts, deferFlagValues: &deferCfg, + metadata: m, } } @@ -76,6 +77,8 @@ type CLI struct { // logic after parsing flagSet to determine desired configuration. deferFlagValues *flagsForFurtherEvaluation cfg *manager.Config + + metadata metadata.Info } type flagsForFurtherEvaluation struct { @@ -113,6 +116,11 @@ func (c *CLI) bindEnvVarsToFlags() (err error) { return err } +// Metadata returns the metadata for the controller manager. +func (c *CLI) Metadata() metadata.Info { + return c.metadata +} + // Parse parses flag definitions from the argument list, which should not include the command name. // Must be called after all additional flags in the FlagSet() are defined and before flags are accessed // by the program. It returns config for controller manager. @@ -160,9 +168,9 @@ func (c *CLI) Parse(arguments []string) manager.Config { Commit string `json:"commit"` } out, err := json.Marshal(Version{ - Release: metadata.Release, - Repo: metadata.Repo, - Commit: metadata.Commit, + Release: c.metadata.Release, + Repo: c.metadata.Repo, + Commit: c.metadata.Commit, }) if err != nil { fmt.Printf("ERROR: failed to print version information: %v\n", err) diff --git a/modules/cli/cli_test.go b/modules/cli/cli_test.go index c2a9b3714..fd47e23ea 100644 --- a/modules/cli/cli_test.go +++ b/modules/cli/cli_test.go @@ -10,6 +10,7 @@ import ( "github.com/kong/gateway-operator/modules/manager" "github.com/kong/gateway-operator/modules/manager/logging" + "github.com/kong/gateway-operator/modules/manager/metadata" ) func TestParse(t *testing.T) { @@ -83,7 +84,7 @@ func TestParse(t *testing.T) { for k, v := range tC.envVars { t.Setenv(k, v) } - cli := New() + cli := New(metadata.Metadata()) cfg := cli.Parse(tC.args) require.Empty(t, cmp.Diff( @@ -103,7 +104,7 @@ func TestParseWithAdditionalFlags(t *testing.T) { } var additionalCfg additionalConfig - cli := New() + cli := New(metadata.Metadata()) cli.FlagSet().BoolVar(&additionalCfg.OptionBool, "additional-bool", true, "Additional bool flag") cli.FlagSet().StringVar(&additionalCfg.OptionString, "additional-string", "additional", "Additional string flag") cli.FlagSet().IntVar(&additionalCfg.OptionalInt, "additional-int", 0, "Additional integer flag") diff --git a/modules/manager/metadata/metadata.go b/modules/manager/metadata/metadata.go index 7957e9d99..0fca1acbe 100644 --- a/modules/manager/metadata/metadata.go +++ b/modules/manager/metadata/metadata.go @@ -8,22 +8,62 @@ package metadata // WARNING: moving any of these variables requires changes to both the Makefile // and the Dockerfile which modify them during the link step with -X +// Info is a struct type that holds the metadata for the controller manager. +type Info struct { + // Release returns the release version, generally a semver like v1.0.0. + Release string + + // Repo returns the git repository URL. + Repo string + + // RepoURL returns the repository URL. + RepoURL string + + // Commit returns the SHA from the current branch HEAD. + Commit string + + // ProjectName is the name of the project. + ProjectName string + + // Organization is the Kong organization + Organization string + + // Flavor is the flavor of the build. + Flavor string +} + var ( // Release returns the release version, generally a semver like v1.0.0. - Release = "NOT_SET" + release = "NOT_SET" // Repo returns the git repository URL. - Repo = "NOT_SET" + repo = "NOT_SET" // RepoURL returns the repository URL. - RepoURL = "NOT_SET" + repoURL = "NOT_SET" // Commit returns the SHA from the current branch HEAD. - Commit = "NOT_SET" + commit = "NOT_SET" // ProjectName is the name of the project. - ProjectName = "NOT_SET" + projectName = "NOT_SET" // Organization is the Kong organization - Organization = "Kong" + organization = "Kong" + + // Flavor is the flavor of the build. + flavor = "oss" ) + +// Metadata returns the metadata for the controller manager. +func Metadata() Info { + return Info{ + Release: release, + Repo: repo, + RepoURL: repoURL, + Commit: commit, + ProjectName: projectName, + Organization: organization, + Flavor: flavor, + } +} diff --git a/modules/manager/run.go b/modules/manager/run.go index 2d3f6fae0..d69f6526e 100644 --- a/modules/manager/run.go +++ b/modules/manager/run.go @@ -132,6 +132,7 @@ func Run( setupControllers SetupControllersFunc, admissionRequestHandler AdmissionRequestHandlerFunc, startedChan chan<- struct{}, + metadata metadata.Info, ) error { setupLog := ctrl.Log.WithName("setup") setupLog.Info("starting controller manager", @@ -243,7 +244,7 @@ func Run( // Enable anonnymous reporting when configured but not for development builds // to reduce the noise. if cfg.AnonymousReports && !cfg.DevelopmentMode { - stopAnonymousReports, err := setupAnonymousReports(context.Background(), cfg, setupLog) + stopAnonymousReports, err := setupAnonymousReports(context.Background(), cfg, setupLog, metadata) if err != nil { setupLog.Error(err, "failed setting up anonymous reports") } else { @@ -368,18 +369,19 @@ func getKubeconfig(apiServerPath string, kubeconfig string) (*rest.Config, error // a cleanup function and an error. // The caller is responsible to call the returned function - when the returned // error is not nil - to stop the reports sending. -func setupAnonymousReports(ctx context.Context, cfg Config, logger logr.Logger) (func(), error) { +func setupAnonymousReports(ctx context.Context, cfg Config, logger logr.Logger, metadata metadata.Info) (func(), error) { logger.Info("starting anonymous reports") restConfig, err := getKubeconfig(cfg.APIServerPath, cfg.KubeconfigPath) if err != nil { return nil, fmt.Errorf("failed to get kubeconfig: %w", err) } - telemetryPayload := telemetry.Payload{ - "v": metadata.Release, + payload := telemetry.Payload{ + "v": metadata.Release, + "flavor": metadata.Flavor, } - tMgr, err := telemetry.CreateManager(telemetry.SignalPing, restConfig, logger, telemetryPayload) + tMgr, err := telemetry.CreateManager(telemetry.SignalPing, restConfig, logger, payload) if err != nil { return nil, fmt.Errorf("failed to create anonymous reports manager: %w", err) } diff --git a/scripts/apidocs-gen/config.yaml b/scripts/apidocs-gen/config.yaml old mode 100644 new mode 100755 diff --git a/scripts/apidocs-gen/template/gv_details.tpl b/scripts/apidocs-gen/template/gv_details.tpl old mode 100644 new mode 100755 diff --git a/scripts/apidocs-gen/template/gv_list.tpl b/scripts/apidocs-gen/template/gv_list.tpl old mode 100644 new mode 100755 diff --git a/scripts/apidocs-gen/template/type.tpl b/scripts/apidocs-gen/template/type.tpl old mode 100644 new mode 100755 diff --git a/scripts/apidocs-gen/template/type_members.tpl b/scripts/apidocs-gen/template/type_members.tpl old mode 100644 new mode 100755 diff --git a/test/conformance/conformance_test.go b/test/conformance/conformance_test.go index d2a39d8b4..00b181b49 100644 --- a/test/conformance/conformance_test.go +++ b/test/conformance/conformance_test.go @@ -109,6 +109,7 @@ func TestGatewayConformance(t *testing.T) { // Currently mode only relies on the KongRouterFlavor, but in the future // we may want to add more modes. mode := string(config.KongRouterFlavor) + metadata := metadata.Metadata() reportFileName := fmt.Sprintf("standard-%s-%s-report.yaml", metadata.Release, mode) opts := conformance.DefaultOptions(t) diff --git a/test/conformance/suite_test.go b/test/conformance/suite_test.go index 61716b5c7..12f5e7ffa 100644 --- a/test/conformance/suite_test.go +++ b/test/conformance/suite_test.go @@ -22,6 +22,7 @@ import ( "github.com/kong/gateway-operator/config" "github.com/kong/gateway-operator/modules/admission" "github.com/kong/gateway-operator/modules/manager" + "github.com/kong/gateway-operator/modules/manager/metadata" "github.com/kong/gateway-operator/modules/manager/scheme" testutils "github.com/kong/gateway-operator/pkg/utils/test" "github.com/kong/gateway-operator/test" @@ -113,7 +114,8 @@ func TestMain(m *testing.M) { fmt.Println("INFO: starting the operator's controller manager") // startControllerManager will spawn the controller manager in a separate // goroutine and will report whether that succeeded. - started := startControllerManager() + metadata := metadata.Metadata() + started := startControllerManager(metadata) <-started exitOnErr(testutils.BuildMTLSCredentials(ctx, clients.K8sClient, &httpc)) @@ -160,7 +162,7 @@ func exitOnErr(err error) { // startControllerManager will configure the manager and start it in a separate goroutine. // It returns a channel which will get closed when manager.Start() gets called. -func startControllerManager() <-chan struct{} { +func startControllerManager(metadata metadata.Info) <-chan struct{} { cfg := manager.DefaultConfig() cfg.LeaderElection = false cfg.DevelopmentMode = true @@ -182,7 +184,7 @@ func startControllerManager() <-chan struct{} { startedChan := make(chan struct{}) go func() { - exitOnErr(manager.Run(cfg, scheme.Get(), manager.SetupControllersShim, admission.NewRequestHandler, startedChan)) + exitOnErr(manager.Run(cfg, scheme.Get(), manager.SetupControllersShim, admission.NewRequestHandler, startedChan, metadata)) }() return startedChan diff --git a/test/integration/run_integration_test.go b/test/integration/run_integration_test.go index 9813a0276..44cea8237 100644 --- a/test/integration/run_integration_test.go +++ b/test/integration/run_integration_test.go @@ -8,6 +8,7 @@ import ( "github.com/kong/gateway-operator/modules/admission" "github.com/kong/gateway-operator/modules/manager" + "github.com/kong/gateway-operator/modules/manager/metadata" "github.com/kong/gateway-operator/modules/manager/scheme" "github.com/kong/gateway-operator/pkg/consts" "github.com/kong/gateway-operator/test/helpers" @@ -22,8 +23,10 @@ func TestMain(m *testing.M) { testSuiteToRun = helpers.ParseGoTestFlags(TestIntegration, testSuiteToRun) cfg := integration.DefaultControllerConfigForTests() + + metadata := metadata.Metadata() managerToTest := func(startedChan chan struct{}) error { - return manager.Run(cfg, scheme.Get(), manager.SetupControllersShim, admission.NewRequestHandler, startedChan) + return manager.Run(cfg, scheme.Get(), manager.SetupControllersShim, admission.NewRequestHandler, startedChan, metadata) } integration.TestMain( m,