From 5dc6ae469af6288b53b494b03fefb2f0a0af5075 Mon Sep 17 00:00:00 2001 From: Samir Faci Date: Wed, 25 Oct 2023 13:10:13 -0700 Subject: [PATCH 1/6] Adds support for Org based override of Watched Folders Fixes #193 --- config/importer-example.yml | 9 ++++++-- config/testing.yml | 1 + internal/config/config_model.go | 15 ++++++++++++ internal/config/config_test.go | 41 +++++++++++++++++++++++++++++++++ internal/config/types.go | 26 +++++++++++++-------- 5 files changed, 80 insertions(+), 12 deletions(-) diff --git a/config/importer-example.yml b/config/importer-example.yml index 561362ce..2303235d 100644 --- a/config/importer-example.yml +++ b/config/importer-example.yml @@ -5,7 +5,7 @@ storage_engine: kind: cloud cloud_type: s3 bucket_name: "" -## The configuration below is mainly intended for OSS alternatives like ceph and minio. If you use a known cloud provider + ## The configuration below is mainly intended for OSS alternatives like ceph and minio. If you use a known cloud provider ## like aws, gcs, azure please setup the auth using the provided tooling from the cloud provider. # For example, having a valid AWS bucket configured in ~/.aws/credentials will be sufficient without needing to provide the auth in the config. ### valid boolean values can be represented as true, "true", or "1" @@ -18,7 +18,7 @@ storage_engine: access_id: "" ## this value can also be read from: AWS_ACCESS_KEY. config file is given precedence secret_key: "" ## same as above, can be read from: AWS_SECRET_KEY with config file is given precedence. init_bucket: "true" ## Only supported for custom workflows. Will attempt to create a bucket if one does not exist. - endpoint: "http://localhost:9000" + endpoint: "http://localhost:9000" ssl_enabled: "false" contexts: @@ -115,6 +115,11 @@ contexts: watched: - Folder1 - Folder2 + watched_folders_override: + - organization_id: 0 + folders: + - General + - SpecialFolder global: debug: true diff --git a/config/testing.yml b/config/testing.yml index b9353213..ff700f7f 100644 --- a/config/testing.yml +++ b/config/testing.yml @@ -83,6 +83,7 @@ contexts: - Folder1 - Folder2 + global: debug: true ignore_ssl_errors: false ##when set to true will ignore invalid SSL errors diff --git a/internal/config/config_model.go b/internal/config/config_model.go index 9d0c2926..8a76f40f 100644 --- a/internal/config/config_model.go +++ b/internal/config/config_model.go @@ -188,8 +188,23 @@ func (s *GrafanaConfig) GetTeamOutput() string { return path.Join(s.OutputPath, TeamResource) } +// GetOrgMonitoredFolders return the OrganizationMonitoredFolders that override a given Org +func (s *GrafanaConfig) GetOrgMonitoredFolders(orgId int64) []string { + for _, item := range s.MonitoredFoldersOverride { + if item.OrganizationId == orgId && len(item.Folders) > 0 { + return item.Folders + } + } + + return nil +} + // GetMonitoredFolders return a list of the monitored folders alternatively returns the "General" folder. func (s *GrafanaConfig) GetMonitoredFolders() []string { + orgFolders := s.GetOrgMonitoredFolders(s.OrganizationId) + if len(orgFolders) > 0 { + return orgFolders + } if len(s.MonitoredFolders) == 0 { return []string{"General"} } diff --git a/internal/config/config_test.go b/internal/config/config_test.go index 1ed8c0e5..5b1ae019 100644 --- a/internal/config/config_test.go +++ b/internal/config/config_test.go @@ -5,6 +5,7 @@ import ( "github.com/esnet/gdg/internal/config" "github.com/esnet/grafana-swagger-api-golang/goclient/models" log "github.com/sirupsen/logrus" + "golang.org/x/exp/slices" "os" "strings" "testing" @@ -59,6 +60,46 @@ func TestSetup(t *testing.T) { validateGrafanaQA(t, grafanaConf) } +func TestWatchedFoldersConfig(t *testing.T) { + os.Setenv("GDG_CONTEXT_NAME", "qa") + //clear all ENV values + for _, key := range os.Environ() { + if strings.Contains(key, "GDG_") { + os.Unsetenv(key) + } + } + + os.Setenv("GDG_CONTEXT_NAME", "qa") + config.InitConfig("testing.yml", "") + conf := config.Config().ViperConfig() + log.Info(conf.ConfigFileUsed()) + + confobj := config.Config().GetAppConfig() + _ = confobj + log.Infof(confobj.ContextName) + assert.NotNil(t, conf) + context := conf.GetString("context_name") + assert.Equal(t, context, "qa") + grafanaConf := config.Config().GetDefaultGrafanaConfig() + assert.NotNil(t, grafanaConf) + grafanaConf.MonitoredFoldersOverride = []config.MonitoredOrgFolders{{ + OrganizationId: 0, + Folders: []string{"General", "SpecialFolder"}, + }} + folders := grafanaConf.GetMonitoredFolders() + assert.True(t, slices.Contains(folders, "SpecialFolder")) + grafanaConf.OrganizationId = 2 + folders = grafanaConf.GetMonitoredFolders() + assert.False(t, slices.Contains(folders, "SpecialFolder")) + assert.True(t, slices.Contains(folders, "Folder2")) + grafanaConf.OrganizationId = 0 + grafanaConf.MonitoredFoldersOverride = nil + folders = grafanaConf.GetMonitoredFolders() + assert.False(t, slices.Contains(folders, "SpecialFolder")) + assert.True(t, slices.Contains(folders, "Folder2")) + +} + // Ensures that if the config is on a completely different path, the searchPath is updated accordingly func TestSetupDifferentPath(t *testing.T) { cfgFile := DuplicateConfig(t) diff --git a/internal/config/types.go b/internal/config/types.go index c2b47eeb..3853efd1 100644 --- a/internal/config/types.go +++ b/internal/config/types.go @@ -25,22 +25,28 @@ type AppConfig struct { // GrafanaConfig model wraps auth and watched list for grafana type GrafanaConfig struct { - Storage string `mapstructure:"storage" yaml:"storage"` - adminEnabled bool `mapstructure:"-" yaml:"-"` - EnterpriseSupport bool `mapstructure:"enterprise_support" yaml:"enterprise_support"` - URL string `mapstructure:"url" yaml:"url"` - APIToken string `mapstructure:"token" yaml:"token"` - UserName string `mapstructure:"user_name" yaml:"user_name"` - Password string `mapstructure:"password" yaml:"password"` - OrganizationId int64 `mapstructure:"organization_id" yaml:"organization_id"` - MonitoredFolders []string `mapstructure:"watched" yaml:"watched"` - DataSourceSettings *ConnectionSettings `mapstructure:"connections" yaml:"connections"` + Storage string `mapstructure:"storage" yaml:"storage"` + adminEnabled bool `mapstructure:"-" yaml:"-"` + EnterpriseSupport bool `mapstructure:"enterprise_support" yaml:"enterprise_support"` + URL string `mapstructure:"url" yaml:"url"` + APIToken string `mapstructure:"token" yaml:"token"` + UserName string `mapstructure:"user_name" yaml:"user_name"` + Password string `mapstructure:"password" yaml:"password"` + OrganizationId int64 `mapstructure:"organization_id" yaml:"organization_id"` + MonitoredFoldersOverride []MonitoredOrgFolders `mapstructure:"watched_folders_override" yaml:"watched_folders_override"` + MonitoredFolders []string `mapstructure:"watched" yaml:"watched"` + DataSourceSettings *ConnectionSettings `mapstructure:"connections" yaml:"connections"` //Datasources are deprecated, please use Connections LegacyConnectionSettings map[string]interface{} `mapstructure:"datasources" yaml:"datasources"` FilterOverrides *FilterOverrides `mapstructure:"filter_override" yaml:"filter_override"` OutputPath string `mapstructure:"output_path" yaml:"output_path"` } +type MonitoredOrgFolders struct { + OrganizationId int64 `json:"organization_id" yaml:"organization_id"` + Folders []string `json:"folders" yaml:"folders"` +} + // GetOrganizationId returns the id of the organization (defaults to 1 if unset) func (s *GrafanaConfig) GetOrganizationId() int64 { if s.OrganizationId > 1 { From 1d00921eaf81fa20cf2b41a753d32f5603de5a64 Mon Sep 17 00:00:00 2001 From: Samir Faci Date: Wed, 25 Oct 2023 13:44:56 -0700 Subject: [PATCH 2/6] Added a cleanup after test functionality. --- test/cloud_integration_test.go | 6 ++++-- test/common_test.go | 12 +++++++++--- test/connections_integration_test.go | 6 ++++-- test/dashboard_integration_test.go | 9 ++++++--- test/folder_integration_test.go | 3 ++- test/libraryelements_integration_test.go | 3 ++- test/organizations_integration_test.go | 6 ++++-- test/team_integration_test.go | 3 ++- test/users_integration_test.go | 3 ++- .../content/en/docs/releases/current_release.md | 16 +++++++++++++++- 10 files changed, 50 insertions(+), 17 deletions(-) diff --git a/test/cloud_integration_test.go b/test/cloud_integration_test.go index 973e43a0..f76bf8fd 100644 --- a/test/cloud_integration_test.go +++ b/test/cloud_integration_test.go @@ -14,7 +14,8 @@ func TestCloudDataSourceCRUD(t *testing.T) { t.Skip("skipping integration test") } - apiClient, _ := initTest(t, nil) + apiClient, _, cleanup := initTest(t, nil) + defer cleanup() //Wipe all data from grafana dsFilter := service.NewConnectionFilter("") @@ -50,7 +51,8 @@ func TestDashboardCloudCRUD(t *testing.T) { err := os.Setenv("GDG_CONTEXT_NAME", "testing") assert.Nil(t, err, "Failed to set context name via env to testing") - apiClient, _ := initTest(t, nil) + apiClient, _, cleanup := initTest(t, nil) + defer cleanup() //Wipe all data from grafana dashFilter := service.NewDashboardFilter("", "", "") diff --git a/test/common_test.go b/test/common_test.go index 8c032891..cacb4591 100644 --- a/test/common_test.go +++ b/test/common_test.go @@ -116,11 +116,12 @@ func TestMain(m *testing.M) { os.Exit(exitVal) } -func initTest(t *testing.T, cfgName *string) (service.GrafanaService, *viper.Viper) { +func initTest(t *testing.T, cfgName *string) (service.GrafanaService, *viper.Viper, func() error) { apiClient, v := createSimpleClient(t, cfgName) + noOp := func() error { return nil } if os.Getenv("TEST_TOKEN_CONFIG") != "1" { - return apiClient, v + return apiClient, v, noOp } testData, _ := os.ReadFile(v.ConfigFileUsed()) @@ -150,7 +151,12 @@ func initTest(t *testing.T, cfgName *string) (service.GrafanaService, *viper.Vip err = os.WriteFile(newCfg, updatedCfg, 0644) assert.Nil(t, err) - return createSimpleClient(t, &newCfg) + cleanUp := func() error { + return os.Remove(newCfg) + } + + apiClient, v = createSimpleClient(t, &newCfg) + return apiClient, v, cleanUp } diff --git a/test/connections_integration_test.go b/test/connections_integration_test.go index f1ff850f..18307413 100644 --- a/test/connections_integration_test.go +++ b/test/connections_integration_test.go @@ -15,7 +15,8 @@ func TestConnectionsCRUD(t *testing.T) { t.Skip("skipping integration test") } - apiClient, _ := initTest(t, nil) + apiClient, _, cleanup := initTest(t, nil) + defer cleanup() filtersEntity := service.NewConnectionFilter("") log.Info("Exporting all connections") apiClient.UploadConnections(filtersEntity) @@ -48,7 +49,8 @@ func TestConnectionFilter(t *testing.T) { if testing.Short() { t.Skip("skipping integration test") } - initTest(t, nil) + _, _, clean := initTest(t, nil) + defer clean() testingContext := config.Config().GetAppConfig().GetContexts()["testing"] testingContext.GetDataSourceSettings().FilterRules = []config.MatchingRule{ diff --git a/test/dashboard_integration_test.go b/test/dashboard_integration_test.go index 3e8b6150..1010d5c5 100644 --- a/test/dashboard_integration_test.go +++ b/test/dashboard_integration_test.go @@ -21,7 +21,8 @@ func TestDashboardCRUD(t *testing.T) { if testing.Short() { t.Skip("skipping integration test") } - apiClient, _ := initTest(t, nil) + apiClient, _, cleanup := initTest(t, nil) + defer cleanup() filtersEntity := service.NewDashboardFilter("", "", "") log.Info("Exporting all dashboards") apiClient.UploadDashboards(filtersEntity) @@ -64,7 +65,8 @@ func TestDashboardTagsFilter(t *testing.T) { if testing.Short() { t.Skip("skipping integration test") } - apiClient, _ := initTest(t, nil) + apiClient, _, cleanup := initTest(t, nil) + defer cleanup() emptyFilter := filters.NewBaseFilter() filtersEntity := service.NewDashboardFilter("", "", "") @@ -101,7 +103,8 @@ func TestWildcardFilter(t *testing.T) { } // Setup Filters - apiClient, _ := initTest(t, nil) + apiClient, _, cleanup := initTest(t, nil) + defer cleanup() emptyFilter := filters.NewBaseFilter() filtersEntity := service.NewDashboardFilter("", "", "") diff --git a/test/folder_integration_test.go b/test/folder_integration_test.go index f6c8d3f0..59a37410 100644 --- a/test/folder_integration_test.go +++ b/test/folder_integration_test.go @@ -11,7 +11,8 @@ func TestFolderCRUD(t *testing.T) { if testing.Short() { t.Skip("skipping integration test") } - apiClient, _ := initTest(t, nil) + apiClient, _, cleanup := initTest(t, nil) + defer cleanup() log.Info("Exporting all folders") apiClient.UploadFolders(nil) log.Info("Listing all Folders") diff --git a/test/libraryelements_integration_test.go b/test/libraryelements_integration_test.go index 713cfa1d..24f8107d 100644 --- a/test/libraryelements_integration_test.go +++ b/test/libraryelements_integration_test.go @@ -14,7 +14,8 @@ func TestLibraryElementsCRUD(t *testing.T) { t.Skip("skipping integration test") } - apiClient, _ := initTest(t, nil) + apiClient, _, cleanup := initTest(t, nil) + defer cleanup() apiClient.DeleteAllDashboards(service.NewDashboardFilter("", "", "")) filtersEntity := service.NewDashboardFilter("", "", "") log.Info("Exporting all Library Elements") diff --git a/test/organizations_integration_test.go b/test/organizations_integration_test.go index c6839cea..cc4597f3 100644 --- a/test/organizations_integration_test.go +++ b/test/organizations_integration_test.go @@ -18,7 +18,8 @@ func TestOrgsCrud(t *testing.T) { if os.Getenv("TEST_TOKEN_CONFIG") == "1" { t.Skip("Skipping Token configuration, Organization CRUD requires Basic Auth") } - apiClient, _ := initTest(t, nil) + apiClient, _, cleanup := initTest(t, nil) + defer cleanup() orgs := apiClient.ListOrganizations() assert.Equal(t, len(orgs), 1) mainOrg := orgs[0] @@ -38,7 +39,8 @@ func TestOrgUserMembership(t *testing.T) { if os.Getenv("TEST_TOKEN_CONFIG") == "1" { t.Skip("Skipping Token configuration, Organization CRUD requires Basic Auth") } - apiClient, _ := initTest(t, nil) + apiClient, _, cleanup := initTest(t, nil) + defer cleanup() //Create Orgs in case they aren't already present. apiClient.UploadOrganizations() orgs := apiClient.ListOrganizations() diff --git a/test/team_integration_test.go b/test/team_integration_test.go index 833f4884..c12504cc 100644 --- a/test/team_integration_test.go +++ b/test/team_integration_test.go @@ -19,7 +19,8 @@ func TestTeamCRUD(t *testing.T) { t.Skip("Skipping Token configuration, Team and User CRUD requires Basic Auth") } filter := service.NewTeamFilter("") - apiClient, _ := initTest(t, nil) + apiClient, _, cleanup := initTest(t, nil) + defer cleanup() log.Info("Exporting current user list") apiClient.UploadUsers(service.NewUserFilter("")) users := apiClient.ListUsers(service.NewUserFilter("")) diff --git a/test/users_integration_test.go b/test/users_integration_test.go index b6784984..2d4dbde8 100644 --- a/test/users_integration_test.go +++ b/test/users_integration_test.go @@ -15,7 +15,8 @@ func TestUsers(t *testing.T) { if os.Getenv("TEST_TOKEN_CONFIG") == "1" { t.Skip("Skipping Token configuration, Team and User CRUD requires Basic Auth") } - apiClient, _ := initTest(t, nil) + apiClient, _, cleanup := initTest(t, nil) + defer cleanup() apiClient.DeleteAllUsers(service.NewUserFilter("")) //clear any previous state users := apiClient.ListUsers(service.NewUserFilter("")) assert.Equal(t, len(users), 1) diff --git a/website/content/en/docs/releases/current_release.md b/website/content/en/docs/releases/current_release.md index f6a47ca4..48434908 100644 --- a/website/content/en/docs/releases/current_release.md +++ b/website/content/en/docs/releases/current_release.md @@ -10,7 +10,21 @@ toc: true --- ## Release Notes for v0.5.1 - **Release Date: TBD 07/13/2023** +### Changes + - TechDebt: Rewriting the CLI flag parsing to allow for easier testing patterns. Should mostly be transparent to the user. + - OrgWatchedFolders added a way to override watched folders for a given organization + +### Bug Fixes + - Tiny patch to fix website documentation navigatioin + - [#205]((https://github.com/esnet/gdg/issues/205) fixes invalid cross-link device when symlink exists to /tmp filesystem. + - [#206]((https://github.com/esnet/gdg/issues/206) fixed behavior issue + +### Developer Changes + - Replaced Makefile with Taskfiles. + - Added dockertest functionality. Allows for a consistent testing pattern on dev and CI. + - postcss security bug. + - Added a new integration pattern to allow all tests to be executed with tokens and basicauth to ensure behavior is consistent when expected + From c76c105c1cbcaf881eb56d6904a84adf2f1753cf Mon Sep 17 00:00:00 2001 From: Samir Faci Date: Wed, 25 Oct 2023 14:34:45 -0700 Subject: [PATCH 3/6] BUG: Fixing CLI Parser small bug. --- cmd/tools/auth_service_accounts.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/cmd/tools/auth_service_accounts.go b/cmd/tools/auth_service_accounts.go index dcad55bb..fec80601 100644 --- a/cmd/tools/auth_service_accounts.go +++ b/cmd/tools/auth_service_accounts.go @@ -42,7 +42,7 @@ func newListServiceAccountCmd() simplecobra.Commander { NameP: "list", Short: description, Long: description, - CommandsList: []simplecobra.Commander{newTokensCmd()}, + CommandsList: []simplecobra.Commander{}, RunFunc: func(ctx context.Context, cd *simplecobra.Commandeer, rootCmd *support.RootCommand, args []string) error { rootCmd.TableObj.AppendHeader(table.Row{"id", "service name", "role", "tokens", "token id", "token name", "expiration"}) apiKeys := rootCmd.GrafanaSvc().ListServiceAccounts() @@ -83,7 +83,7 @@ func newDeleteServiceAccountTokensCmd() simplecobra.Commander { NameP: "clearTokens", Short: description, Long: description, - CommandsList: []simplecobra.Commander{newTokensCmd()}, + CommandsList: []simplecobra.Commander{}, RunFunc: func(ctx context.Context, cd *simplecobra.Commandeer, rootCmd *support.RootCommand, args []string) error { if len(args) < 1 { return errors.New("requires a service account ID to be specified") @@ -116,7 +116,7 @@ func newDeleteServiceAccountCmd() simplecobra.Commander { NameP: "clear", Short: description, Long: description, - CommandsList: []simplecobra.Commander{newTokensCmd()}, + CommandsList: []simplecobra.Commander{}, RunFunc: func(ctx context.Context, cd *simplecobra.Commandeer, rootCmd *support.RootCommand, args []string) error { savedFiles := rootCmd.GrafanaSvc().DeleteAllServiceAccounts() log.Infof("Delete Service Accounts for context: '%s'", config.Config().AppConfig.GetContext()) @@ -140,7 +140,7 @@ func newServiceAccount() simplecobra.Commander { NameP: "newService", Short: description, Long: description, - CommandsList: []simplecobra.Commander{newTokensCmd()}, + CommandsList: []simplecobra.Commander{}, RunFunc: func(ctx context.Context, cd *simplecobra.Commandeer, rootCmd *support.RootCommand, args []string) error { if len(args) < 2 { return errors.New("requires a key name and a role('admin','viewer','editor') [ttl optional] ") From 763be1c7ea920d892e461869d9c09f282605c40c Mon Sep 17 00:00:00 2001 From: Samir Faci Date: Wed, 25 Oct 2023 14:53:06 -0700 Subject: [PATCH 4/6] Adding CLI test for devel srvinfo --- Taskfile.yml | 6 ++- cmd/{backup => test}/conections_test.go | 5 +- cmd/test/devel_test.go | 50 +++++++++++++++++++ cmd/test/support.go | 66 +++++++++++++++++++++++++ cmd/{ => test}/version_test.go | 34 ++++--------- cmd/test_tools/support.go | 21 -------- 6 files changed, 131 insertions(+), 51 deletions(-) rename cmd/{backup => test}/conections_test.go (92%) create mode 100644 cmd/test/devel_test.go create mode 100644 cmd/test/support.go rename cmd/{ => test}/version_test.go (60%) delete mode 100644 cmd/test_tools/support.go diff --git a/Taskfile.yml b/Taskfile.yml index ea741a24..62a20701 100644 --- a/Taskfile.yml +++ b/Taskfile.yml @@ -107,14 +107,16 @@ tasks: test: desc: "test check" cmds: - - go test -v ./... -cover + - go test -v -coverpkg=./... -covermode=atomic -coverprofile=coverage.out ./... + - go tool cover -html=coverage.out env: GRAFANA_INTEGRATION: "1" TEST_TOKEN_CONFIG: "0" test_tokens: desc: "test Token Based Only" cmds: - - go test -v ./... -cover + - go test -v -coverpkg=./... -covermode=atomic -coverprofile=coverage.out ./... + - go tool cover -html=coverage.out env: GRAFANA_INTEGRATION: "1" TEST_TOKEN_CONFIG: "1" diff --git a/cmd/backup/conections_test.go b/cmd/test/conections_test.go similarity index 92% rename from cmd/backup/conections_test.go rename to cmd/test/conections_test.go index 653d4271..a12ddf00 100644 --- a/cmd/backup/conections_test.go +++ b/cmd/test/conections_test.go @@ -1,9 +1,8 @@ -package backup_test +package test import ( "github.com/esnet/gdg/cmd" "github.com/esnet/gdg/cmd/support" - "github.com/esnet/gdg/cmd/test_tools" "github.com/esnet/gdg/internal/service" "github.com/esnet/gdg/internal/service/mocks" "github.com/esnet/grafana-swagger-api-golang/goclient/models" @@ -38,7 +37,7 @@ func TestConnectionCommand(t *testing.T) { response.GrafanaSvc = getMockSvc } } - r, w, cleanup := test_tools.InterceptStdout() + r, w, cleanup := InterceptStdout() data, err := os.ReadFile("../../config/testing.yml") assert.Nil(t, err) diff --git a/cmd/test/devel_test.go b/cmd/test/devel_test.go new file mode 100644 index 00000000..50d930ac --- /dev/null +++ b/cmd/test/devel_test.go @@ -0,0 +1,50 @@ +package test + +import ( + "github.com/esnet/gdg/cmd" + "github.com/esnet/gdg/cmd/support" + "github.com/esnet/gdg/internal/service/mocks" + "github.com/stretchr/testify/assert" + "strings" + "testing" +) + +func TestDevelSrvInfo(t *testing.T) { + var execMe = func(mock *mocks.GrafanaService, data []byte, optionMockSvc func() support.RootOption) error { + expected := make(map[string]interface{}) + expected["Database"] = "db" + expected["Commit"] = "commit" + expected["Version"] = "version" + + mock.On("GetServerInfo").Return(expected) + err := cmd.Execute(string(data), []string{"tools", "devel", "srvinfo"}, optionMockSvc()) + return err + } + outStr, closeReader := setupAndExecuteMockingServices(t, execMe) + defer closeReader() + + assert.True(t, strings.Contains(outStr, "Version:")) + assert.True(t, strings.Contains(outStr, "Database:")) + assert.True(t, strings.Contains(outStr, "Commit:")) +} + +func TestDevelSrvCompletion(t *testing.T) { + fn := func(args []string) func(mock *mocks.GrafanaService, data []byte, optionMockSvc func() support.RootOption) error { + return func(mock *mocks.GrafanaService, data []byte, optionMockSvc func() support.RootOption) error { + err := cmd.Execute(string(data), args, optionMockSvc()) + return err + } + } + + outStr, closeReader := setupAndExecuteMockingServices(t, fn([]string{"tools", "devel", "completion", "fish"})) + assert.True(t, strings.Contains(outStr, "fish")) + assert.True(t, strings.Contains(outStr, "__completion_prepare_completions")) + closeReader() + outStr, closeReader = setupAndExecuteMockingServices(t, fn([]string{"tools", "devel", "completion", "bash"})) + assert.True(t, strings.Contains(outStr, "bash")) + assert.True(t, strings.Contains(outStr, "flag_parsing_disabled")) + closeReader() + outStr, closeReader = setupAndExecuteMockingServices(t, fn([]string{"tools", "devel", "completion", "zsh"})) + assert.True(t, strings.Contains(outStr, "shellCompDirectiveKeepOrder")) + closeReader() +} diff --git a/cmd/test/support.go b/cmd/test/support.go new file mode 100644 index 00000000..508575f2 --- /dev/null +++ b/cmd/test/support.go @@ -0,0 +1,66 @@ +package test + +import ( + "github.com/esnet/gdg/cmd/support" + "github.com/esnet/gdg/internal/service" + "github.com/esnet/gdg/internal/service/mocks" + log "github.com/sirupsen/logrus" + "github.com/stretchr/testify/assert" + "io" + "os" + "testing" +) + +// setupAndExecuteMockingServices will create a mock for varous required entities allowing to test the CLI flag parsing +// process: function that setups mocks and invokes the Execute command +func setupAndExecuteMockingServices(t *testing.T, process func(mock *mocks.GrafanaService, data []byte, optionMockSvc func() support.RootOption) error) (string, func()) { + testSvc := new(mocks.GrafanaService) + getMockSvc := func() service.GrafanaService { + return testSvc + } + + optionMockSvc := func() support.RootOption { + return func(response *support.RootCommand) { + response.GrafanaSvc = getMockSvc + } + } + + r, w, cleanup := InterceptStdout() + data, err := os.ReadFile("../../config/testing.yml") + assert.Nil(t, err) + + err = process(testSvc, data, optionMockSvc) + assert.Nil(t, err) + defer cleanup() + err = w.Close() + if err != nil { + log.Warn("unable to close write stream") + } + clean := func() { + defer r.Close() + } + out, _ := io.ReadAll(r) + outStr := string(out) + return outStr, clean + +} + +// InterceptStdout is a test helper function that will redirect all stdout in and out to a different file stream. +// It returns the stdout, stderr, and a function to be invoked to close the streams. +func InterceptStdout() (*os.File, *os.File, func()) { + backupStd := os.Stdout + backupErr := os.Stderr + r, w, _ := os.Pipe() + //Restore streams + log.SetOutput(w) + cleanup := func() { + os.Stdout = backupStd + os.Stderr = backupErr + log.SetOutput(os.Stdout) + } + os.Stdout = w + os.Stderr = w + + return r, w, cleanup + +} diff --git a/cmd/version_test.go b/cmd/test/version_test.go similarity index 60% rename from cmd/version_test.go rename to cmd/test/version_test.go index 35c38f33..54e82d64 100644 --- a/cmd/version_test.go +++ b/cmd/test/version_test.go @@ -1,42 +1,26 @@ -package cmd +package test import ( "fmt" + "github.com/esnet/gdg/cmd" "github.com/esnet/gdg/cmd/support" - "github.com/esnet/gdg/cmd/test_tools" "github.com/esnet/gdg/internal/service" "github.com/esnet/gdg/internal/service/mocks" "github.com/esnet/gdg/internal/version" "github.com/stretchr/testify/assert" - "io" "os" "strings" "testing" ) func TestVersionCommand(t *testing.T) { - testSvc := new(mocks.GrafanaService) - getMockSvc := func() service.GrafanaService { - return testSvc + var execMe = func(mock *mocks.GrafanaService, data []byte, optionMockSvc func() support.RootOption) error { + err := cmd.Execute(string(data), []string{"version"}, optionMockSvc()) + return err } + outStr, closeReader := setupAndExecuteMockingServices(t, execMe) + defer closeReader() - optionMockSvc := func() support.RootOption { - return func(response *support.RootCommand) { - response.GrafanaSvc = getMockSvc - } - } - path, _ := os.Getwd() - fmt.Println(path) - r, w, cleanup := test_tools.InterceptStdout() - data, err := os.ReadFile("../config/testing.yml") - assert.Nil(t, err) - - err = Execute(string(data), []string{"version"}, optionMockSvc()) - assert.Nil(t, err) - defer cleanup() - w.Close() - out, _ := io.ReadAll(r) - outStr := string(out) assert.True(t, strings.Contains(outStr, "Build Date:")) assert.True(t, strings.Contains(outStr, "Git Commit:")) assert.True(t, strings.Contains(outStr, "Version:")) @@ -59,10 +43,10 @@ func TestVersionErrCommand(t *testing.T) { } path, _ := os.Getwd() fmt.Println(path) - data, err := os.ReadFile("../config/testing.yml") + data, err := os.ReadFile("../../config/testing.yml") assert.Nil(t, err) - err = Execute(string(data), []string{"dumb", "dumb"}, optionMockSvc()) + err = cmd.Execute(string(data), []string{"dumb", "dumb"}, optionMockSvc()) assert.NotNil(t, err) assert.Equal(t, err.Error(), `command error: unknown command "dumb" for "gdg"`) } diff --git a/cmd/test_tools/support.go b/cmd/test_tools/support.go deleted file mode 100644 index 5c23ecba..00000000 --- a/cmd/test_tools/support.go +++ /dev/null @@ -1,21 +0,0 @@ -package test_tools - -import "os" - -// InterceptStdout is a test helper function that will redirect all stdout in and out to a different file stream. -// It returns the stdout, stderr, and a function to be invoked to close the streams. -func InterceptStdout() (*os.File, *os.File, func()) { - backupStd := os.Stdout - backupErr := os.Stderr - r, w, _ := os.Pipe() - //Restore streams - cleanup := func() { - os.Stdout = backupStd - os.Stderr = backupErr - } - os.Stdout = w - os.Stderr = w - - return r, w, cleanup - -} From a5765f3d5c2b02059d32b9896ccd279b2f77b090 Mon Sep 17 00:00:00 2001 From: Samir Faci Date: Fri, 27 Oct 2023 13:57:57 -0700 Subject: [PATCH 5/6] Updating go version to 1.21.3 to address secrutiy vulnerability --- .github/workflows/code_scanners.yml | 2 +- .github/workflows/go.yml | 6 +++--- docker/Dockerfile | 4 ++-- go.mod | 4 ++-- go.sum | 2 ++ 5 files changed, 10 insertions(+), 8 deletions(-) diff --git a/.github/workflows/code_scanners.yml b/.github/workflows/code_scanners.yml index 6bb556c6..6c2ad20a 100644 --- a/.github/workflows/code_scanners.yml +++ b/.github/workflows/code_scanners.yml @@ -29,7 +29,7 @@ jobs: uses: actions/checkout@v3 - uses: actions/setup-go@v4 with: - go-version: "1.21.2" + go-version: "1.21.3" cache: false - name: Install Task run: sh -c "$(curl --location https://taskfile.dev/install.sh)" -- -d diff --git a/.github/workflows/go.yml b/.github/workflows/go.yml index 4aaf8bff..6934eb76 100644 --- a/.github/workflows/go.yml +++ b/.github/workflows/go.yml @@ -11,7 +11,7 @@ jobs: test: strategy: matrix: - go: [ {version: 1.21.0, token: 1}, {version: 1.21.0, token: 0}] + go: [ {version: 1.21.3, token: 1}, {version: 1.21.3, token: 0}] grafana: [ 10.1.4 ] env: @@ -40,11 +40,11 @@ jobs: run: | echo "token IS $TEST_TOKEN_CONFIG" - name: Calc coverage - if: "${{ matrix.go.version == '1.21.0' && matrix.grafana == '10.1.4' && matrix.go.token == '0' }}" + if: "${{ matrix.go.version == '1.21.3' && matrix.grafana == '10.1.4' && matrix.go.token == '0' }}" run: | go test -v -covermode=atomic -coverprofile=coverage.out ./... - name: Convert coverage.out to coverage.lcov - if: "${{ matrix.go.version == '1.21.0' && matrix.grafana == '10.1.4' && matrix.go.token == '0' }}" + if: "${{ matrix.go.version == '1.21.3' && matrix.grafana == '10.1.4' && matrix.go.token == '0' }}" uses: jandelgado/gcov2lcov-action@v1.0.9 - name: Test if: "${{ matrix.go.token == '1' }}" diff --git a/docker/Dockerfile b/docker/Dockerfile index ab00cc9c..0da58c3b 100644 --- a/docker/Dockerfile +++ b/docker/Dockerfile @@ -1,5 +1,5 @@ # Build Stage -FROM golang:1.21.2 AS build-stage +FROM golang:1.21.3 AS build-stage LABEL app="build-gdg" LABEL REPO="https://github.com/esnet/gdg" @@ -15,7 +15,7 @@ WORKDIR /go/src/github.com/esnet/gdg RUN make build-alpine # Final Stage -FROM golang:1.21.2 +FROM golang:1.21.3 ARG GIT_COMMIT ARG VERSION diff --git a/go.mod b/go.mod index 5aaad4b8..3687df8d 100644 --- a/go.mod +++ b/go.mod @@ -1,6 +1,6 @@ module github.com/esnet/gdg -go 1.21.2 +go 1.21.3 require ( github.com/AlecAivazis/survey/v2 v2.3.7 @@ -147,7 +147,7 @@ require ( go.uber.org/multierr v1.11.0 // indirect golang.org/x/crypto v0.14.0 // indirect golang.org/x/mod v0.13.0 // indirect - golang.org/x/net v0.16.0 // indirect + golang.org/x/net v0.17.0 // indirect golang.org/x/oauth2 v0.13.0 // indirect golang.org/x/sync v0.4.0 // indirect golang.org/x/tools v0.14.0 // indirect diff --git a/go.sum b/go.sum index 097f769b..84afe4ec 100644 --- a/go.sum +++ b/go.sum @@ -664,6 +664,8 @@ golang.org/x/net v0.0.0-20220722155237-a158d28d115b/go.mod h1:XRhObCWvk6IyKnWLug golang.org/x/net v0.1.0/go.mod h1:Cx3nUiGt4eDBEyega/BKRp+/AlGL8hYe7U9odMt2Cco= golang.org/x/net v0.16.0 h1:7eBu7KsSvFDtSXUIDbh3aqlK4DPsZ1rByC8PFfBThos= golang.org/x/net v0.16.0/go.mod h1:NxSsAGuq816PNPmqtQdLE42eU2Fs7NoRIZrHJAlaCOE= +golang.org/x/net v0.17.0 h1:pVaXccu2ozPjCXewfr1S7xza/zcXTity9cCdXQYSjIM= +golang.org/x/net v0.17.0/go.mod h1:NxSsAGuq816PNPmqtQdLE42eU2Fs7NoRIZrHJAlaCOE= golang.org/x/oauth2 v0.0.0-20180821212333-d2e6202438be/go.mod h1:N/0e6XlmueqKjAGxoOufVs8QHGRruUQn6yWY3a++T0U= golang.org/x/oauth2 v0.0.0-20190226205417-e64efc72b421/go.mod h1:gOpvHmFTYa4IltrdGE7lF6nIHvwfUNPOp7c8zoXwtLw= golang.org/x/oauth2 v0.0.0-20190604053449-0f29369cfe45/go.mod h1:gOpvHmFTYa4IltrdGE7lF6nIHvwfUNPOp7c8zoXwtLw= From ebd943d8dfa12da2a697f8bcd69a6945d3421949 Mon Sep 17 00:00:00 2001 From: Samir Faci Date: Tue, 31 Oct 2023 13:49:00 -0400 Subject: [PATCH 6/6] Addressing code review comments --- internal/config/config_test.go | 3 --- 1 file changed, 3 deletions(-) diff --git a/internal/config/config_test.go b/internal/config/config_test.go index 5b1ae019..10b98270 100644 --- a/internal/config/config_test.go +++ b/internal/config/config_test.go @@ -50,7 +50,6 @@ func TestSetup(t *testing.T) { log.Info(conf.ConfigFileUsed()) confobj := config.Config().GetAppConfig() - _ = confobj log.Infof(confobj.ContextName) assert.NotNil(t, conf) context := conf.GetString("context_name") @@ -61,7 +60,6 @@ func TestSetup(t *testing.T) { } func TestWatchedFoldersConfig(t *testing.T) { - os.Setenv("GDG_CONTEXT_NAME", "qa") //clear all ENV values for _, key := range os.Environ() { if strings.Contains(key, "GDG_") { @@ -75,7 +73,6 @@ func TestWatchedFoldersConfig(t *testing.T) { log.Info(conf.ConfigFileUsed()) confobj := config.Config().GetAppConfig() - _ = confobj log.Infof(confobj.ContextName) assert.NotNil(t, conf) context := conf.GetString("context_name")