From e6a6fc6f74ed36439cfa9fb2e1a0a49b72d2fd09 Mon Sep 17 00:00:00 2001 From: Mariell Hoversholm Date: Thu, 12 Dec 2024 15:13:06 +0100 Subject: [PATCH] Tests: Require GrafanaOpts when creating Grafana dir (#97825) I don't see a reason to accept a variable amount here, as we never use it. The only use I can see is optionally including the opts, which isn't necessary and only complicates matters when an empty struct would do just as well: the options are all created to be assumed zero-values already, in case a test doesn't need that option set. --- .../commands/install_command_test.go | 2 +- pkg/tests/testinfra/testinfra.go | 262 +++++++++--------- pkg/tests/web/index_view_test.go | 4 +- 3 files changed, 134 insertions(+), 134 deletions(-) diff --git a/pkg/cmd/grafana-cli/commands/install_command_test.go b/pkg/cmd/grafana-cli/commands/install_command_test.go index 6d9c35c1f8f..bfb286f58d5 100644 --- a/pkg/cmd/grafana-cli/commands/install_command_test.go +++ b/pkg/cmd/grafana-cli/commands/install_command_test.go @@ -124,7 +124,7 @@ func TestValidatePluginRepoConfig(t *testing.T) { } // GrafanaComApiUrl is set to the default path https://grafana.com/api - grafDir, cfgPath := testinfra.CreateGrafDir(t) + grafDir, cfgPath := testinfra.CreateGrafDir(t, testinfra.GrafanaOpts{}) // overriding the GrafanaComApiUrl to https://grafana-dev.com c, err := commandstest.NewCliContext(map[string]string{ diff --git a/pkg/tests/testinfra/testinfra.go b/pkg/tests/testinfra/testinfra.go index 60f43905176..3120ee859a1 100644 --- a/pkg/tests/testinfra/testinfra.go +++ b/pkg/tests/testinfra/testinfra.go @@ -149,7 +149,7 @@ func StartGrafanaEnv(t *testing.T, grafDir, cfgPath string) (string, *server.Tes // CreateGrafDir creates the Grafana directory. // The log by default is muted in the regression test, to activate it, pass option EnableLog = true -func CreateGrafDir(t *testing.T, opts ...GrafanaOpts) (string, string) { +func CreateGrafDir(t *testing.T, opts GrafanaOpts) (string, string) { t.Helper() tmpDir := t.TempDir() @@ -319,147 +319,145 @@ func CreateGrafDir(t *testing.T, opts ...GrafanaOpts) (string, string) { } queryRetries := 3 - for _, o := range opts { - if o.EnableCSP { - securitySect, err := cfg.NewSection("security") - require.NoError(t, err) - _, err = securitySect.NewKey("content_security_policy", "true") - require.NoError(t, err) - } - if len(o.EnableFeatureToggles) > 0 { - featureSection, err := cfg.NewSection("feature_toggles") - require.NoError(t, err) - _, err = featureSection.NewKey("enable", strings.Join(o.EnableFeatureToggles, " ")) - require.NoError(t, err) - } - if o.NGAlertAdminConfigPollInterval != 0 { - ngalertingSection, err := cfg.NewSection("unified_alerting") - require.NoError(t, err) - _, err = ngalertingSection.NewKey("admin_config_poll_interval", o.NGAlertAdminConfigPollInterval.String()) - require.NoError(t, err) - } - if o.NGAlertAlertmanagerConfigPollInterval != 0 { - ngalertingSection, err := getOrCreateSection("unified_alerting") - require.NoError(t, err) - _, err = ngalertingSection.NewKey("alertmanager_config_poll_interval", o.NGAlertAlertmanagerConfigPollInterval.String()) - require.NoError(t, err) - } - if o.AppModeProduction { - _, err = dfltSect.NewKey("app_mode", "production") - require.NoError(t, err) - } - if o.AnonymousUserRole != "" { - _, err = anonSect.NewKey("org_role", string(o.AnonymousUserRole)) - require.NoError(t, err) - } - if o.EnableQuota { - quotaSection, err := cfg.NewSection("quota") - require.NoError(t, err) - _, err = quotaSection.NewKey("enabled", "true") - require.NoError(t, err) - dashboardQuota := int64(100) - if o.DashboardOrgQuota != nil { - dashboardQuota = *o.DashboardOrgQuota - } - _, err = quotaSection.NewKey("org_dashboard", strconv.FormatInt(dashboardQuota, 10)) - require.NoError(t, err) - } - if o.DisableAnonymous { - anonSect, err := cfg.GetSection("auth.anonymous") - require.NoError(t, err) - _, err = anonSect.NewKey("enabled", "false") - require.NoError(t, err) - } - if o.PluginAdminEnabled { - anonSect, err := cfg.NewSection("plugins") - require.NoError(t, err) - _, err = anonSect.NewKey("plugin_admin_enabled", "true") - require.NoError(t, err) - } - if o.PluginAdminExternalManageEnabled { - anonSect, err := cfg.NewSection("plugins") - require.NoError(t, err) - _, err = anonSect.NewKey("plugin_admin_external_manage_enabled", "true") - require.NoError(t, err) - } - if o.ViewersCanEdit { - usersSection, err := cfg.NewSection("users") - require.NoError(t, err) - _, err = usersSection.NewKey("viewers_can_edit", "true") - require.NoError(t, err) - } - if o.EnableUnifiedAlerting { - unifiedAlertingSection, err := getOrCreateSection("unified_alerting") - require.NoError(t, err) - _, err = unifiedAlertingSection.NewKey("enabled", "true") - require.NoError(t, err) - } - if len(o.UnifiedAlertingDisabledOrgs) > 0 { - unifiedAlertingSection, err := getOrCreateSection("unified_alerting") - require.NoError(t, err) - disableOrgStr := strings.Join(strings.Split(strings.Trim(fmt.Sprint(o.UnifiedAlertingDisabledOrgs), "[]"), " "), ",") - _, err = unifiedAlertingSection.NewKey("disabled_orgs", disableOrgStr) - require.NoError(t, err) - } - if !o.EnableLog { - logSection, err := getOrCreateSection("log") - require.NoError(t, err) - _, err = logSection.NewKey("enabled", "false") - require.NoError(t, err) - } else { - serverSection, err := getOrCreateSection("server") - require.NoError(t, err) - _, err = serverSection.NewKey("router_logging", "true") - require.NoError(t, err) + if opts.EnableCSP { + securitySect, err := cfg.NewSection("security") + require.NoError(t, err) + _, err = securitySect.NewKey("content_security_policy", "true") + require.NoError(t, err) + } + if len(opts.EnableFeatureToggles) > 0 { + featureSection, err := cfg.NewSection("feature_toggles") + require.NoError(t, err) + _, err = featureSection.NewKey("enable", strings.Join(opts.EnableFeatureToggles, " ")) + require.NoError(t, err) + } + if opts.NGAlertAdminConfigPollInterval != 0 { + ngalertingSection, err := cfg.NewSection("unified_alerting") + require.NoError(t, err) + _, err = ngalertingSection.NewKey("admin_config_poll_interval", opts.NGAlertAdminConfigPollInterval.String()) + require.NoError(t, err) + } + if opts.NGAlertAlertmanagerConfigPollInterval != 0 { + ngalertingSection, err := getOrCreateSection("unified_alerting") + require.NoError(t, err) + _, err = ngalertingSection.NewKey("alertmanager_config_poll_interval", opts.NGAlertAlertmanagerConfigPollInterval.String()) + require.NoError(t, err) + } + if opts.AppModeProduction { + _, err = dfltSect.NewKey("app_mode", "production") + require.NoError(t, err) + } + if opts.AnonymousUserRole != "" { + _, err = anonSect.NewKey("org_role", string(opts.AnonymousUserRole)) + require.NoError(t, err) + } + if opts.EnableQuota { + quotaSection, err := cfg.NewSection("quota") + require.NoError(t, err) + _, err = quotaSection.NewKey("enabled", "true") + require.NoError(t, err) + dashboardQuota := int64(100) + if opts.DashboardOrgQuota != nil { + dashboardQuota = *opts.DashboardOrgQuota } + _, err = quotaSection.NewKey("org_dashboard", strconv.FormatInt(dashboardQuota, 10)) + require.NoError(t, err) + } + if opts.DisableAnonymous { + anonSect, err := cfg.GetSection("auth.anonymous") + require.NoError(t, err) + _, err = anonSect.NewKey("enabled", "false") + require.NoError(t, err) + } + if opts.PluginAdminEnabled { + anonSect, err := cfg.NewSection("plugins") + require.NoError(t, err) + _, err = anonSect.NewKey("plugin_admin_enabled", "true") + require.NoError(t, err) + } + if opts.PluginAdminExternalManageEnabled { + anonSect, err := cfg.NewSection("plugins") + require.NoError(t, err) + _, err = anonSect.NewKey("plugin_admin_external_manage_enabled", "true") + require.NoError(t, err) + } + if opts.ViewersCanEdit { + usersSection, err := cfg.NewSection("users") + require.NoError(t, err) + _, err = usersSection.NewKey("viewers_can_edit", "true") + require.NoError(t, err) + } + if opts.EnableUnifiedAlerting { + unifiedAlertingSection, err := getOrCreateSection("unified_alerting") + require.NoError(t, err) + _, err = unifiedAlertingSection.NewKey("enabled", "true") + require.NoError(t, err) + } + if len(opts.UnifiedAlertingDisabledOrgs) > 0 { + unifiedAlertingSection, err := getOrCreateSection("unified_alerting") + require.NoError(t, err) + disableOrgStr := strings.Join(strings.Split(strings.Trim(fmt.Sprint(opts.UnifiedAlertingDisabledOrgs), "[]"), " "), ",") + _, err = unifiedAlertingSection.NewKey("disabled_orgs", disableOrgStr) + require.NoError(t, err) + } + if !opts.EnableLog { + logSection, err := getOrCreateSection("log") + require.NoError(t, err) + _, err = logSection.NewKey("enabled", "false") + require.NoError(t, err) + } else { + serverSection, err := getOrCreateSection("server") + require.NoError(t, err) + _, err = serverSection.NewKey("router_logging", "true") + require.NoError(t, err) + } - if o.APIServerStorageType != "" { - section, err := getOrCreateSection("grafana-apiserver") - require.NoError(t, err) - _, err = section.NewKey("storage_type", string(o.APIServerStorageType)) - require.NoError(t, err) + if opts.APIServerStorageType != "" { + section, err := getOrCreateSection("grafana-apiserver") + require.NoError(t, err) + _, err = section.NewKey("storage_type", string(opts.APIServerStorageType)) + require.NoError(t, err) - // Hardcoded local etcd until this is needed to run in CI - if o.APIServerStorageType == "etcd" { - _, err = section.NewKey("etcd_servers", "localhost:2379") - require.NoError(t, err) - } + // Hardcoded local etcd until this is needed to run in CI + if opts.APIServerStorageType == "etcd" { + _, err = section.NewKey("etcd_servers", "localhost:2379") + require.NoError(t, err) } + } - if o.GRPCServerAddress != "" { - logSection, err := getOrCreateSection("grpc_server") - require.NoError(t, err) - _, err = logSection.NewKey("address", o.GRPCServerAddress) - require.NoError(t, err) - } - // retry queries 3 times by default - if o.QueryRetries != 0 { - queryRetries = int(o.QueryRetries) - } + if opts.GRPCServerAddress != "" { + logSection, err := getOrCreateSection("grpc_server") + require.NoError(t, err) + _, err = logSection.NewKey("address", opts.GRPCServerAddress) + require.NoError(t, err) + } + // retry queries 3 times by default + if opts.QueryRetries != 0 { + queryRetries = int(opts.QueryRetries) + } - if o.NGAlertSchedulerBaseInterval > 0 { - unifiedAlertingSection, err := getOrCreateSection("unified_alerting") - require.NoError(t, err) - _, err = unifiedAlertingSection.NewKey("scheduler_tick_interval", o.NGAlertSchedulerBaseInterval.String()) - require.NoError(t, err) - _, err = unifiedAlertingSection.NewKey("min_interval", o.NGAlertSchedulerBaseInterval.String()) - require.NoError(t, err) - } + if opts.NGAlertSchedulerBaseInterval > 0 { + unifiedAlertingSection, err := getOrCreateSection("unified_alerting") + require.NoError(t, err) + _, err = unifiedAlertingSection.NewKey("scheduler_tick_interval", opts.NGAlertSchedulerBaseInterval.String()) + require.NoError(t, err) + _, err = unifiedAlertingSection.NewKey("min_interval", opts.NGAlertSchedulerBaseInterval.String()) + require.NoError(t, err) + } - if o.GrafanaComAPIURL != "" { - grafanaComSection, err := getOrCreateSection("grafana_com") + if opts.GrafanaComAPIURL != "" { + grafanaComSection, err := getOrCreateSection("grafana_com") + require.NoError(t, err) + _, err = grafanaComSection.NewKey("api_url", opts.GrafanaComAPIURL) + require.NoError(t, err) + } + if opts.UnifiedStorageConfig != nil { + for k, v := range opts.UnifiedStorageConfig { + section, err := getOrCreateSection(fmt.Sprintf("unified_storage.%s", k)) require.NoError(t, err) - _, err = grafanaComSection.NewKey("api_url", o.GrafanaComAPIURL) + _, err = section.NewKey("dualWriterMode", fmt.Sprintf("%d", v.DualWriterMode)) require.NoError(t, err) } - if o.UnifiedStorageConfig != nil { - for k, v := range o.UnifiedStorageConfig { - section, err := getOrCreateSection(fmt.Sprintf("unified_storage.%s", k)) - require.NoError(t, err) - _, err = section.NewKey("dualWriterMode", fmt.Sprintf("%d", v.DualWriterMode)) - require.NoError(t, err) - } - } } dbSection, err := getOrCreateSection("database") diff --git a/pkg/tests/web/index_view_test.go b/pkg/tests/web/index_view_test.go index 6ea42b35135..ec9e8f431e6 100644 --- a/pkg/tests/web/index_view_test.go +++ b/pkg/tests/web/index_view_test.go @@ -47,7 +47,9 @@ func TestIntegrationIndexView(t *testing.T) { }) t.Run("CSP disabled", func(t *testing.T) { - grafDir, cfgPath := testinfra.CreateGrafDir(t) + grafDir, cfgPath := testinfra.CreateGrafDir(t, testinfra.GrafanaOpts{ + EnableCSP: false, + }) addr, _ := testinfra.StartGrafana(t, grafDir, cfgPath) // nolint:bodyclose