From e12ab486a23b22c841ccaf88ab94550e585b30e5 Mon Sep 17 00:00:00 2001 From: Harshavardhana Date: Thu, 20 Jul 2023 07:52:49 -0700 Subject: [PATCH] avoid using os.Getenv for internal code, use env.Get() instead (#17688) --- .github/workflows/iam-integrations.yaml | 22 +++++++++++----------- cmd/admin-handlers-users_test.go | 10 +++++----- cmd/main.go | 3 ++- cmd/sts-handlers_test.go | 6 +++--- internal/dsync/drwmutex.go | 6 +++--- internal/s3select/select.go | 11 +++++++++-- internal/s3select/select_test.go | 18 +++++++++++++++--- 7 files changed, 48 insertions(+), 28 deletions(-) diff --git a/.github/workflows/iam-integrations.yaml b/.github/workflows/iam-integrations.yaml index 9a075d251..3c108bdcf 100644 --- a/.github/workflows/iam-integrations.yaml +++ b/.github/workflows/iam-integrations.yaml @@ -82,9 +82,9 @@ jobs: check-latest: true - name: Test LDAP/OpenID/Etcd combo env: - LDAP_TEST_SERVER: ${{ matrix.ldap }} - ETCD_SERVER: ${{ matrix.etcd }} - OPENID_TEST_SERVER: ${{ matrix.openid }} + _MINIO_LDAP_TEST_SERVER: ${{ matrix.ldap }} + _MINIO_ETCD_TEST_SERVER: ${{ matrix.etcd }} + _MINIO_OPENID_TEST_SERVER: ${{ matrix.openid }} run: | sudo sysctl net.ipv6.conf.all.disable_ipv6=0 sudo sysctl net.ipv6.conf.default.disable_ipv6=0 @@ -92,20 +92,20 @@ jobs: - name: Test with multiple OpenID providers if: matrix.openid == 'http://127.0.0.1:5556/dex' env: - LDAP_TEST_SERVER: ${{ matrix.ldap }} - ETCD_SERVER: ${{ matrix.etcd }} - OPENID_TEST_SERVER: ${{ matrix.openid }} - OPENID_TEST_SERVER_2: "http://127.0.0.1:5557/dex" + _MINIO_LDAP_TEST_SERVER: ${{ matrix.ldap }} + _MINIO_ETCD_TEST_SERVER: ${{ matrix.etcd }} + _MINIO_OPENID_TEST_SERVER: ${{ matrix.openid }} + _MINIO_OPENID_TEST_SERVER_2: "http://127.0.0.1:5557/dex" run: | sudo sysctl net.ipv6.conf.all.disable_ipv6=0 sudo sysctl net.ipv6.conf.default.disable_ipv6=0 make test-iam - name: Test with Access Management Plugin enabled env: - LDAP_TEST_SERVER: ${{ matrix.ldap }} - ETCD_SERVER: ${{ matrix.etcd }} - OPENID_TEST_SERVER: ${{ matrix.openid }} - POLICY_PLUGIN_ENDPOINT: "http://127.0.0.1:8080" + _MINIO_LDAP_TEST_SERVER: ${{ matrix.ldap }} + _MINIO_ETCD_TEST_SERVER: ${{ matrix.etcd }} + _MINIO_OPENID_TEST_SERVER: ${{ matrix.openid }} + _MINIO_POLICY_PLUGIN_TEST_ENDPOINT: "http://127.0.0.1:8080" run: | sudo sysctl net.ipv6.conf.all.disable_ipv6=0 sudo sysctl net.ipv6.conf.default.disable_ipv6=0 diff --git a/cmd/admin-handlers-users_test.go b/cmd/admin-handlers-users_test.go index 656d5ee66..526ea9e0e 100644 --- a/cmd/admin-handlers-users_test.go +++ b/cmd/admin-handlers-users_test.go @@ -27,7 +27,6 @@ import ( "io" "net/http" "net/url" - "os" "runtime" "strings" "testing" @@ -40,6 +39,7 @@ import ( "github.com/minio/minio-go/v7/pkg/set" "github.com/minio/minio-go/v7/pkg/signer" "github.com/minio/minio/internal/auth" + "github.com/minio/pkg/env" ) const ( @@ -121,7 +121,7 @@ var iamTestSuites = func() []*TestSuiteIAM { }() const ( - EnvTestEtcdBackend = "ETCD_SERVER" + EnvTestEtcdBackend = "_MINIO_ETCD_TEST_SERVER" ) func (s *TestSuiteIAM) setUpEtcd(c *check, etcdServer string) { @@ -144,7 +144,7 @@ func (s *TestSuiteIAM) setUpEtcd(c *check, etcdServer string) { func (s *TestSuiteIAM) SetUpSuite(c *check) { // If etcd backend is specified and etcd server is not present, the test // is skipped. - etcdServer := os.Getenv(EnvTestEtcdBackend) + etcdServer := env.Get(EnvTestEtcdBackend, "") if s.withEtcdBackend && etcdServer == "" { c.Skip("Skipping etcd backend IAM test as no etcd server is configured.") } @@ -983,9 +983,9 @@ func (s *TestSuiteIAM) SetUpAccMgmtPlugin(c *check) { ctx, cancel := context.WithTimeout(context.Background(), testDefaultTimeout) defer cancel() - pluginEndpoint := os.Getenv("POLICY_PLUGIN_ENDPOINT") + pluginEndpoint := env.Get("_MINIO_POLICY_PLUGIN_ENDPOINT", "") if pluginEndpoint == "" { - c.Skip("POLICY_PLUGIN_ENDPOINT not given - skipping.") + c.Skip("_MINIO_POLICY_PLUGIN_ENDPOINT not given - skipping.") } configCmds := []string{ diff --git a/cmd/main.go b/cmd/main.go index c54b1e228..c0a34f1f0 100644 --- a/cmd/main.go +++ b/cmd/main.go @@ -31,6 +31,7 @@ import ( "github.com/minio/minio/internal/color" "github.com/minio/minio/internal/logger" "github.com/minio/pkg/console" + "github.com/minio/pkg/env" "github.com/minio/pkg/trie" "github.com/minio/pkg/words" ) @@ -191,7 +192,7 @@ func Main(args []string) { // Set the minio app name. appName := filepath.Base(args[0]) - if os.Getenv("_MINIO_DEBUG_NO_EXIT") != "" { + if env.Get("_MINIO_DEBUG_NO_EXIT", "") != "" { freeze := func(_ int) { // Infinite blocking op <-make(chan struct{}) diff --git a/cmd/sts-handlers_test.go b/cmd/sts-handlers_test.go index fe1155e57..9bafaceb8 100644 --- a/cmd/sts-handlers_test.go +++ b/cmd/sts-handlers_test.go @@ -660,7 +660,7 @@ func (s *TestSuiteIAM) SetUpLDAP(c *check, serverAddr string) { } const ( - EnvTestLDAPServer = "LDAP_TEST_SERVER" + EnvTestLDAPServer = "_MINIO_LDAP_TEST_SERVER" ) func TestIAMWithLDAPServerSuite(t *testing.T) { @@ -1379,8 +1379,8 @@ var testAppParams = OpenIDClientAppParams{ } const ( - EnvTestOpenIDServer = "OPENID_TEST_SERVER" - EnvTestOpenIDServer2 = "OPENID_TEST_SERVER_2" + EnvTestOpenIDServer = "_MINIO_OPENID_TEST_SERVER" + EnvTestOpenIDServer2 = "_MINIO_OPENID_TEST_SERVER_2" ) // SetUpOpenIDs - sets up one or more OpenID test servers using the test OpenID diff --git a/internal/dsync/drwmutex.go b/internal/dsync/drwmutex.go index 55c4a884b..e8c1575b9 100644 --- a/internal/dsync/drwmutex.go +++ b/internal/dsync/drwmutex.go @@ -21,7 +21,6 @@ import ( "context" "errors" "math/rand" - "os" "sort" "strconv" "sync" @@ -29,6 +28,7 @@ import ( "github.com/minio/minio/internal/mcontext" "github.com/minio/pkg/console" + "github.com/minio/pkg/env" ) // Indicator if logging is enabled. @@ -41,10 +41,10 @@ var lockRetryBackOff func(*rand.Rand, uint) time.Duration func init() { // Check for MINIO_DSYNC_TRACE env variable, if set logging will be enabled for failed REST operations. - dsyncLog = os.Getenv("_MINIO_DSYNC_TRACE") == "1" + dsyncLog = env.Get("_MINIO_DSYNC_TRACE", "0") == "1" lockRetryMinInterval = 250 * time.Millisecond - if lri := os.Getenv("_MINIO_LOCK_RETRY_INTERVAL"); lri != "" { + if lri := env.Get("_MINIO_LOCK_RETRY_INTERVAL", ""); lri != "" { v, err := strconv.Atoi(lri) if err != nil { panic(err) diff --git a/internal/s3select/select.go b/internal/s3select/select.go index f2575e537..e2f207517 100644 --- a/internal/s3select/select.go +++ b/internal/s3select/select.go @@ -26,18 +26,19 @@ import ( "fmt" "io" "net/http" - "os" "strings" "sync" "github.com/klauspost/compress/s2" "github.com/klauspost/compress/zstd" gzip "github.com/klauspost/pgzip" + "github.com/minio/minio/internal/config" "github.com/minio/minio/internal/s3select/csv" "github.com/minio/minio/internal/s3select/json" "github.com/minio/minio/internal/s3select/parquet" "github.com/minio/minio/internal/s3select/simdj" "github.com/minio/minio/internal/s3select/sql" + "github.com/minio/pkg/env" "github.com/minio/simdjson-go" "github.com/pierrec/lz4" ) @@ -73,6 +74,12 @@ const ( maxRecordSize = 1 << 20 // 1 MiB ) +var parquetSupport bool + +func init() { + parquetSupport = env.Get("MINIO_API_SELECT_PARQUET", config.EnableOff) == config.EnableOn +} + var bufPool = sync.Pool{ New: func() interface{} { // make a buffer with a reasonable capacity. @@ -439,7 +446,7 @@ func (s3Select *S3Select) Open(rsc io.ReadSeekCloser) error { return nil case parquetFormat: - if !strings.EqualFold(os.Getenv("MINIO_API_SELECT_PARQUET"), "on") { + if !parquetSupport { return errors.New("parquet format parsing not enabled on server") } if offset != 0 || length != -1 { diff --git a/internal/s3select/select_test.go b/internal/s3select/select_test.go index 9d83ac9bd..f74383995 100644 --- a/internal/s3select/select_test.go +++ b/internal/s3select/select_test.go @@ -1684,7 +1684,11 @@ func TestCSVRanges(t *testing.T) { } func TestParquetInput(t *testing.T) { - t.Setenv("MINIO_API_SELECT_PARQUET", "on") + saved := parquetSupport + defer func() { + parquetSupport = saved + }() + parquetSupport = true testTable := []struct { requestXML []byte @@ -1785,7 +1789,11 @@ func TestParquetInput(t *testing.T) { } func TestParquetInputSchema(t *testing.T) { - t.Setenv("MINIO_API_SELECT_PARQUET", "on") + saved := parquetSupport + defer func() { + parquetSupport = saved + }() + parquetSupport = true testTable := []struct { requestXML []byte @@ -1887,7 +1895,11 @@ func TestParquetInputSchema(t *testing.T) { } func TestParquetInputSchemaCSV(t *testing.T) { - t.Setenv("MINIO_API_SELECT_PARQUET", "on") + saved := parquetSupport + defer func() { + parquetSupport = saved + }() + parquetSupport = true testTable := []struct { requestXML []byte