From 75586dcbf35ca0b8fd61f46e3a9a694a1dbc3db0 Mon Sep 17 00:00:00 2001 From: acabarbaye Date: Mon, 18 Nov 2024 17:31:01 +0000 Subject: [PATCH 1/2] :sparkles: `[config]` Added a way to bind mutiple flags to a same environment input: `BindFlagsToEnv` --- .secrets.baseline | 14 +-- changes/20241118165828.feature | 1 + changes/20241118171703.feature | 1 + changes/20241118172847.bugfix | 1 + utils/collection/search.go | 15 ++- utils/collection/search_test.go | 13 +++ utils/config/service_configuration.go | 103 +++++++++++++++++++ utils/config/service_configuration_test.go | 114 ++++++++++++++++++++- utils/git/git.go | 6 +- utils/git/git_test.go | 10 +- utils/go.mod | 2 +- utils/go.sum | 4 +- 12 files changed, 261 insertions(+), 23 deletions(-) create mode 100644 changes/20241118165828.feature create mode 100644 changes/20241118171703.feature create mode 100644 changes/20241118172847.bugfix diff --git a/.secrets.baseline b/.secrets.baseline index ede9692d9c..16074ad873 100644 --- a/.secrets.baseline +++ b/.secrets.baseline @@ -66,10 +66,6 @@ { "path": "detect_secrets.filters.allowlist.is_line_allowlisted" }, - { - "path": "detect_secrets.filters.common.is_baseline_file", - "filename": ".secrets.baseline" - }, { "path": "detect_secrets.filters.common.is_ignored_due_to_verification_policies", "min_level": 2 @@ -126,28 +122,28 @@ "filename": "utils/config/service_configuration_test.go", "hashed_secret": "ddcec2f503a5d58f432a0beee3fb9544fa581f54", "is_verified": false, - "line_number": 32 + "line_number": 33 }, { "type": "Secret Keyword", "filename": "utils/config/service_configuration_test.go", "hashed_secret": "7ca1cc114e7e5f955880bb96a5bf391b4dc20ab6", "is_verified": false, - "line_number": 369 + "line_number": 481 }, { "type": "Secret Keyword", "filename": "utils/config/service_configuration_test.go", "hashed_secret": "11519c144be4850d95b34220a40030cbd5a36b57", "is_verified": false, - "line_number": 464 + "line_number": 576 }, { "type": "Secret Keyword", "filename": "utils/config/service_configuration_test.go", "hashed_secret": "15fae91d8fa7f2c531c1cf3ddc745e1f4473c02d", "is_verified": false, - "line_number": 471 + "line_number": 583 } ], "utils/filesystem/filehash_test.go": [ @@ -268,5 +264,5 @@ } ] }, - "generated_at": "2024-08-14T13:57:27Z" + "generated_at": "2024-11-18T17:28:14Z" } diff --git a/changes/20241118165828.feature b/changes/20241118165828.feature new file mode 100644 index 0000000000..a8014db9ed --- /dev/null +++ b/changes/20241118165828.feature @@ -0,0 +1 @@ +:sparkles: `[collection]` Added a way to determine unique values in a slice: `UniqueEntries` diff --git a/changes/20241118171703.feature b/changes/20241118171703.feature new file mode 100644 index 0000000000..fdcee82ebc --- /dev/null +++ b/changes/20241118171703.feature @@ -0,0 +1 @@ +:sparkles: `[config]` Added a way to bind mutiple flag to a same environment input: `BindFlagsToEnv` diff --git a/changes/20241118172847.bugfix b/changes/20241118172847.bugfix new file mode 100644 index 0000000000..5497cd58f0 --- /dev/null +++ b/changes/20241118172847.bugfix @@ -0,0 +1 @@ +:arrow_up: Upgrade dependency `https://github.com/deckarep/golang-set` diff --git a/utils/collection/search.go b/utils/collection/search.go index a86e1c84ee..e47c33404b 100644 --- a/utils/collection/search.go +++ b/utils/collection/search.go @@ -4,7 +4,11 @@ */ package collection -import "strings" +import ( + "strings" + + mapset "github.com/deckarep/golang-set/v2" +) // Find looks for an element in a slice. If found it will // return its index and true; otherwise it will return -1 and false. @@ -16,7 +20,7 @@ func Find(slice *[]string, val string) (int, bool) { } // FindInSlice finds if any values val are present in the slice and if so returns the first index. -// if strict, it check of an exact match; otherwise discard whitespaces and case. +// if strict, it checks for an exact match; otherwise it discards whitespaces and case. func FindInSlice(strict bool, slice []string, val ...string) (int, bool) { if len(val) == 0 || len(slice) == 0 { return -1, false @@ -46,6 +50,13 @@ func FindInSlice(strict bool, slice []string, val ...string) (int, bool) { return -1, false } +// UniqueEntries returns all the unique values contained in a slice. +func UniqueEntries[T comparable](slice []T) []T { + subSet := mapset.NewSet[T]() + _ = subSet.Append(slice...) + return subSet.ToSlice() +} + // Any returns true if there is at least one element of the slice which is true. func Any(slice []bool) bool { for i := range slice { diff --git a/utils/collection/search_test.go b/utils/collection/search_test.go index 42f209054b..7cc0b6984e 100644 --- a/utils/collection/search_test.go +++ b/utils/collection/search_test.go @@ -90,3 +90,16 @@ func TestAllNotEmpty(t *testing.T) { assert.False(t, AllNotEmpty(false, []string{faker.Username(), "", faker.Name(), "", faker.Sentence()})) assert.True(t, AllNotEmpty(false, []string{faker.Username(), faker.Name(), faker.Sentence()})) } + +func TestUniqueEntries(t *testing.T) { + assert.Len(t, UniqueEntries([]string{faker.Username(), faker.Name(), faker.Sentence(), faker.Name()}), 4) + values := UniqueEntries([]string{"test1", "test12", "test1", "test1", "test12", "test12"}) + assert.Len(t, values, 2) + _, found := FindInSlice(true, values, "test1") + assert.True(t, found) + _, found = FindInSlice(true, values, "test12") + assert.True(t, found) + + intValues := UniqueEntries([]int{1, 2, 3, 4, 1, 2, 3, 4, 1, 2, 3, 4, 1, 2, 3, 4}) + assert.Len(t, intValues, 4) +} diff --git a/utils/config/service_configuration.go b/utils/config/service_configuration.go index 2e3d3e887d..905474ab03 100644 --- a/utils/config/service_configuration.go +++ b/utils/config/service_configuration.go @@ -14,6 +14,7 @@ import ( "github.com/spf13/pflag" "github.com/spf13/viper" + "github.com/ARM-software/golang-utils/utils/collection" "github.com/ARM-software/golang-utils/utils/commonerrors" "github.com/ARM-software/golang-utils/utils/reflection" ) @@ -136,6 +137,108 @@ func BindFlagToEnv(viperSession *viper.Viper, envVarPrefix string, envVar string return } +// BindFlagsToEnv binds a set of pflags to an environment variable. +// Envvar is the environment variable string with or without the prefix envVarPrefix +// It is similar to BindFlagToEnv but can be applied to multiple flags. +// Note: all the flags will have to be of the same type. If more than one flags is changed, the system will pick one at random. +func BindFlagsToEnv(viperSession *viper.Viper, envVarPrefix string, envVar string, flags ...*pflag.Flag) (err error) { + + setEnvOptions(viperSession, envVarPrefix) + shortKey, cleansedEnvVar := generateEnvVarConfigKeys(envVar, envVarPrefix) + + flagset, err := newMultiFlags(shortKey, flags...) + if err != nil { + return + } + err = viperSession.BindFlagValue(shortKey, flagset) + if err != nil { + return + } + + err = viperSession.BindEnv(shortKey, cleansedEnvVar) + return +} + +func newMultiFlags(name string, flags ...*pflag.Flag) (f viper.FlagValue, err error) { + if name == "" { + err = fmt.Errorf("%w: flag set must be associated with a name", commonerrors.ErrUndefined) + return + } + if flags == nil || len(flags) == 0 { + err = fmt.Errorf("%w: flags must be specified", commonerrors.ErrUndefined) + return + } + var fTypes []string + for i := range flags { + if flags[i] != nil { + fTypes = append(fTypes, flags[i].Value.Type()) + } + } + fTypes = collection.UniqueEntries(fTypes) + if len(fTypes) != 1 { + err = fmt.Errorf("%w: flags in a set can only be of the same type: %v", commonerrors.ErrInvalid, fTypes) + return + } + + f = &multiFlags{ + commonName: name, + flags: flags, + } + return +} + +type multiFlags struct { + commonName string + flags []*pflag.Flag +} + +func (m *multiFlags) HasChanged() bool { + for i := range m.flags { + flag := m.flags[i] + if flag != nil && flag.Changed { + return true + } + } + return false +} + +func (m *multiFlags) Name() string { + return m.commonName +} + +func (m *multiFlags) ValueString() string { + var values []string + var firstValue string + for i := range m.flags { + flag := m.flags[i] + if flag != nil { + firstValue = flag.Value.String() + if flag.Changed { + values = append(values, flag.Value.String()) + } + } + } + values = collection.UniqueEntries(values) + if len(values) >= 1 { + return values[0] + } else { + return firstValue + } +} + +func (m *multiFlags) ValueType() string { + for i := range m.flags { + flag := m.flags[i] + if flag != nil { + vType := flag.Value.Type() + if vType != "" { + return vType + } + } + } + return "" +} + func generateEnvVarConfigKeys(envVar, envVarPrefix string) (shortKey string, cleansedEnvVar string) { envVarLower := strings.ToLower(envVar) envVarPrefixLower := strings.ToLower(envVarPrefix) diff --git a/utils/config/service_configuration_test.go b/utils/config/service_configuration_test.go index 064a87ac67..6f74a6d151 100644 --- a/utils/config/service_configuration_test.go +++ b/utils/config/service_configuration_test.go @@ -21,6 +21,7 @@ import ( "github.com/stretchr/testify/require" "github.com/ARM-software/golang-utils/utils/commonerrors" + "github.com/ARM-software/golang-utils/utils/commonerrors/errortest" ) var ( @@ -291,6 +292,117 @@ func TestFlagBinding(t *testing.T) { assert.False(t, configTest.TestConfig2.Flag) } +func TestFlagsBinding(t *testing.T) { + os.Clearenv() + configTest := &ConfigurationTest{} + defaults := DefaultConfiguration() + session := viper.New() + var err error + flagSet := pflag.FlagSet{} + prefix := "test" + flagSet.String("host1", "a host", "dummy host") + flagSet.String("host2", "a host", "dummy host") + flagSet.String("password1", "a password1", "dummy password1") + flagSet.String("password2", "a password2", "dummy password2") + flagSet.String("password3", "a password3", "dummy password3") + flagSet.String("user", "a user", "dummy user") + flagSet.String("user1", "a user", "dummy user 1") + flagSet.String("user2", "a user", "dummy user 2") + flagSet.String("db", "a db", "dummy db") + flagSet.String("db2", "a db", "dummy db") + flagSet.Int("int", 0, "dummy int") + flagSet.Duration("time", time.Second, "dummy time") + flagSet.Bool("flag", false, "dummy flag") + err = BindFlagsToEnv(session, prefix, "TEST_DUMMYCONFIG_DUMMY_HOST", flagSet.Lookup("host2"), flagSet.Lookup("host2")) + require.NoError(t, err) + err = BindFlagsToEnv(session, prefix, "TEST_DUMMY_CONFIG_DUMMY_HOST", flagSet.Lookup("host1"), flagSet.Lookup("host2")) + require.NoError(t, err) + err = BindFlagsToEnv(session, prefix, "DUMMYCONFIG_PASSWORD", flagSet.Lookup("password2"), flagSet.Lookup("password3")) + require.NoError(t, err) + err = BindFlagsToEnv(session, prefix, "DUMMY_CONFIG_PASSWORD", flagSet.Lookup("password1"), flagSet.Lookup("password2")) + require.NoError(t, err) + err = BindFlagsToEnv(session, prefix, "DUMMYCONFIG_USER", flagSet.Lookup("user"), flagSet.Lookup("user1"), flagSet.Lookup("user2")) + require.NoError(t, err) + err = BindFlagsToEnv(session, prefix, "DUMMY_CONFIG_USER", flagSet.Lookup("user1"), flagSet.Lookup("user2")) + require.NoError(t, err) + err = BindFlagsToEnv(session, prefix, "TEST_DUMMYCONFIG_DB", flagSet.Lookup("db")) + require.NoError(t, err) + err = BindFlagsToEnv(session, prefix, "DUMMY_CONFIG_DB", flagSet.Lookup("db2"), flagSet.Lookup("db2"), flagSet.Lookup("db2"), flagSet.Lookup("db2")) + require.NoError(t, err) + err = BindFlagsToEnv(session, prefix, "DUMMY_CONFIG_FLAG", flagSet.Lookup("flag"), flagSet.Lookup("flag"), flagSet.Lookup("flag")) + require.NoError(t, err) + err = BindFlagsToEnv(session, prefix, "DUMMY_INT", flagSet.Lookup("int"), flagSet.Lookup("int"), flagSet.Lookup("int"), flagSet.Lookup("int")) + require.NoError(t, err) + err = BindFlagsToEnv(session, prefix, "DUMMY_Time", flagSet.Lookup("time"), flagSet.Lookup("time"), flagSet.Lookup("time")) + require.NoError(t, err) + err = flagSet.Set("host2", expectedHost) + require.NoError(t, err) + fvalue, err := flagSet.GetString("host2") + require.NoError(t, err) + assert.Equal(t, expectedHost, fvalue) + fvalue, err = flagSet.GetString("host1") + require.NoError(t, err) + assert.NotEqual(t, expectedHost, fvalue) + err = flagSet.Set("password2", expectedPassword) + require.NoError(t, err) + user1V := faker.Name() + err = flagSet.Set("user1", user1V) + require.NoError(t, err) + user2V := faker.Name() + err = flagSet.Set("user2", user2V) + require.NoError(t, err) + assert.NotEqual(t, user1V, user2V) + err = flagSet.Set("db", expectedDB) // Should take precedence over environment + require.NoError(t, err) + aDifferentDB := "another test db" + assert.NotEqual(t, expectedDB, aDifferentDB) + err = os.Setenv("TEST_DUMMY_CONFIG_DB", aDifferentDB) + require.NoError(t, err) + err = os.Setenv("TEST_DUMMYCONFIG_DB", aDifferentDB) + require.NoError(t, err) + err = flagSet.Set("int", fmt.Sprintf("%v", expectedInt)) + require.NoError(t, err) + err = flagSet.Set("time", expectedDuration.String()) + require.NoError(t, err) + err = flagSet.Set("flag", fmt.Sprintf("%v", false)) + require.NoError(t, err) + flag, err := flagSet.GetBool("flag") + require.NoError(t, err) + assert.False(t, flag) + assert.False(t, session.GetBool("dummy.config.flag")) + err = LoadFromViper(session, prefix, configTest, defaults) + require.NoError(t, err) + require.NoError(t, configTest.Validate()) + assert.Equal(t, expectedString, configTest.TestString) + assert.Equal(t, expectedInt, configTest.TestInt) + assert.Equal(t, expectedDuration, configTest.TestTime) + assert.Equal(t, defaults.TestConfig.Port, configTest.TestConfig.Port) + assert.Contains(t, []string{user1V, user2V}, configTest.TestConfig2.User) + assert.Equal(t, expectedHost, configTest.TestConfig.Host) + assert.Equal(t, expectedHost, configTest.TestConfig2.Host) + assert.Equal(t, expectedPassword, configTest.TestConfig.Password) + assert.Equal(t, expectedPassword, configTest.TestConfig2.Password) + assert.Equal(t, expectedDB, configTest.TestConfig.DB) + assert.Equal(t, aDifferentDB, configTest.TestConfig2.DB) + assert.NotEqual(t, expectedDB, configTest.TestConfig2.DB) + assert.True(t, configTest.TestConfig.Flag) + assert.False(t, configTest.TestConfig2.Flag) +} + +func TestFlagsBindingErrors(t *testing.T) { + os.Clearenv() + session := viper.New() + flagSet := pflag.FlagSet{} + prefix := "test" + flagSet.String("db2", "a db", "dummy db") + flagSet.Int("int", 0, "dummy int") + err := BindFlagsToEnv(session, prefix, "TEST_DUMMYCONFIG_DUMMY_HOST") + errortest.AssertError(t, err, commonerrors.ErrUndefined) + err = BindFlagsToEnv(session, prefix, "TEST_DUMMYCONFIG_DUMMY_HOST", flagSet.Lookup("db2"), flagSet.Lookup("int")) + errortest.AssertError(t, err, commonerrors.ErrInvalid) + +} + func TestFlagBindingDefaults(t *testing.T) { os.Clearenv() configTest := &ConfigurationTest{} @@ -545,7 +657,7 @@ func Test_convertViperError(t *testing.T) { for i := range tests { t.Run(fmt.Sprintf("%v", i), func(t *testing.T) { test := tests[i] - require.True(t, commonerrors.Any(convertViperError(test.viperErr), test.expectedError)) + errortest.RequireError(t, convertViperError(test.viperErr), test.expectedError) }) } } diff --git a/utils/git/git.go b/utils/git/git.go index af30292f99..bb84851835 100644 --- a/utils/git/git.go +++ b/utils/git/git.go @@ -7,7 +7,7 @@ import ( "strings" "sync" - mapset "github.com/deckarep/golang-set" + mapset "github.com/deckarep/golang-set/v2" git "github.com/go-git/go-git/v5" "github.com/go-git/go-git/v5/plumbing" "github.com/go-git/go-git/v5/plumbing/object" @@ -44,7 +44,7 @@ type CloneObject struct { repo *git.Repository allEntries chan Entry - seen mapset.Set + seen mapset.Set[plumbing.Hash] totalSize *atomic.Int64 trueSize *atomic.Int64 @@ -255,7 +255,7 @@ func NewCloneObject() *CloneObject { processNonTreeOnly: atomic.NewBool(false), treeSeenIdentifier: atomic.NewString(""), nonTreeOnlyMutex: make(chan int, 1), - seen: mapset.NewSet(), + seen: mapset.NewSet[plumbing.Hash](), } cloneObject.nonTreeOnlyMutex <- 1 return cloneObject diff --git a/utils/git/git_test.go b/utils/git/git_test.go index 6d7d6735d9..dee85dbdb8 100644 --- a/utils/git/git_test.go +++ b/utils/git/git_test.go @@ -9,7 +9,7 @@ import ( "testing" "time" - mapset "github.com/deckarep/golang-set" + mapset "github.com/deckarep/golang-set/v2" "github.com/go-git/go-git/v5" "github.com/go-git/go-git/v5/plumbing" "github.com/go-git/go-git/v5/plumbing/object" @@ -189,7 +189,7 @@ func TestHandleBlobEntry(t *testing.T) { err = c.SetupLimits(limits) require.NoError(t, err) - c.seen = mapset.NewSet() + c.seen = mapset.NewSet[plumbing.Hash]() totalSize = atomic.NewInt64(0) totalFileCount = atomic.NewInt64(0) totalTrueSize = atomic.NewInt64(0) @@ -211,7 +211,7 @@ func TestHandleBlobEntry(t *testing.T) { err = c.SetupLimits(limits) require.NoError(t, err) - c.seen = mapset.NewSet() + c.seen = mapset.NewSet[plumbing.Hash]() totalSize = atomic.NewInt64(0) totalFileCount = atomic.NewInt64(0) totalTrueSize = atomic.NewInt64(0) @@ -233,7 +233,7 @@ func TestHandleBlobEntry(t *testing.T) { err = c.SetupLimits(limits) require.NoError(t, err) - c.seen = mapset.NewSet() + c.seen = mapset.NewSet[plumbing.Hash]() totalSize = atomic.NewInt64(0) totalFileCount = atomic.NewInt64(0) totalTrueSize = atomic.NewInt64(0) @@ -255,7 +255,7 @@ func TestHandleBlobEntry(t *testing.T) { err = c.SetupLimits(limits) require.NoError(t, err) - c.seen = mapset.NewSet() + c.seen = mapset.NewSet[plumbing.Hash]() totalSize = atomic.NewInt64(0) totalFileCount = atomic.NewInt64(0) totalTrueSize = atomic.NewInt64(0) diff --git a/utils/go.mod b/utils/go.mod index 0d7402dbf8..98ed65bf87 100644 --- a/utils/go.mod +++ b/utils/go.mod @@ -7,7 +7,7 @@ require ( github.com/avast/retry-go v3.0.0+incompatible github.com/bmatcuk/doublestar/v3 v3.0.0 github.com/bombsimon/logrusr/v4 v4.1.0 - github.com/deckarep/golang-set v1.8.0 + github.com/deckarep/golang-set/v2 v2.6.0 github.com/djherbis/times v1.6.0 github.com/dolmen-go/contextio v1.0.0 github.com/evanphx/hclogr v0.2.0 diff --git a/utils/go.sum b/utils/go.sum index e3f1844435..3533515d06 100644 --- a/utils/go.sum +++ b/utils/go.sum @@ -33,8 +33,8 @@ github.com/davecgh/go-spew v1.1.0/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSs github.com/davecgh/go-spew v1.1.1/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= github.com/davecgh/go-spew v1.1.2-0.20180830191138-d8f796af33cc h1:U9qPSI2PIWSS1VwoXQT9A3Wy9MM3WgvqSxFWenqJduM= github.com/davecgh/go-spew v1.1.2-0.20180830191138-d8f796af33cc/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= -github.com/deckarep/golang-set v1.8.0 h1:sk9/l/KqpunDwP7pSjUg0keiOOLEnOBHzykLrsPppp4= -github.com/deckarep/golang-set v1.8.0/go.mod h1:5nI87KwE7wgsBU1F4GKAw2Qod7p5kyS383rP6+o6qqo= +github.com/deckarep/golang-set/v2 v2.6.0 h1:XfcQbWM1LlMB8BsJ8N9vW5ehnnPVIw0je80NsVHagjM= +github.com/deckarep/golang-set/v2 v2.6.0/go.mod h1:VAky9rY/yGXJOLEDv3OMci+7wtDpOF4IN+y82NBOac4= github.com/djherbis/times v1.6.0 h1:w2ctJ92J8fBvWPxugmXIv7Nz7Q3iDMKNx9v5ocVH20c= github.com/djherbis/times v1.6.0/go.mod h1:gOHeRAz2h+VJNZ5Gmc/o7iD9k4wW7NMVqieYCY99oc0= github.com/dolmen-go/contextio v1.0.0 h1:bNfCo4gsRIhMeo6Z1ImXzkxZG81B6I5t2fUFJjphdAU= From a5b3fe2c7e83cfaf843590a3db761f3096234760 Mon Sep 17 00:00:00 2001 From: acabarbaye Date: Mon, 18 Nov 2024 17:37:12 +0000 Subject: [PATCH 2/2] linting --- .secrets.baseline | 2 +- utils/config/service_configuration.go | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/.secrets.baseline b/.secrets.baseline index 16074ad873..0b7013a1d7 100644 --- a/.secrets.baseline +++ b/.secrets.baseline @@ -264,5 +264,5 @@ } ] }, - "generated_at": "2024-11-18T17:28:14Z" + "generated_at": "2024-11-18T17:37:06Z" } diff --git a/utils/config/service_configuration.go b/utils/config/service_configuration.go index 905474ab03..3495289a90 100644 --- a/utils/config/service_configuration.go +++ b/utils/config/service_configuration.go @@ -164,7 +164,7 @@ func newMultiFlags(name string, flags ...*pflag.Flag) (f viper.FlagValue, err er err = fmt.Errorf("%w: flag set must be associated with a name", commonerrors.ErrUndefined) return } - if flags == nil || len(flags) == 0 { + if len(flags) == 0 { err = fmt.Errorf("%w: flags must be specified", commonerrors.ErrUndefined) return }