fix: log errors for incorrect environment inputs (#13121)

Invalid MINIO_ARGS, MINIO_ENDPOINTS would be silently
ignored when using remoteEnv style, make sure to log
errors to indicate invalid configuration.
This commit is contained in:
Harshavardhana 2021-09-01 11:34:07 -07:00 committed by GitHub
parent 03b7bebc96
commit c89aee37b9
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
8 changed files with 71 additions and 84 deletions

View File

@ -117,7 +117,7 @@ func (a adminAPIHandlers) SetConfigKVHandler(w http.ResponseWriter, r *http.Requ
return
}
if err = validateConfig(cfg, objectAPI.SetDriveCounts()); err != nil {
if err = validateConfig(cfg); err != nil {
writeCustomErrorResponseJSON(ctx, w, errorCodes.ToAPIErr(ErrAdminConfigBadJSON), err.Error(), r.URL)
return
}
@ -248,7 +248,7 @@ func (a adminAPIHandlers) RestoreConfigHistoryKVHandler(w http.ResponseWriter, r
return
}
if err = validateConfig(cfg, objectAPI.SetDriveCounts()); err != nil {
if err = validateConfig(cfg); err != nil {
writeCustomErrorResponseJSON(ctx, w, errorCodes.ToAPIErr(ErrAdminConfigBadJSON), err.Error(), r.URL)
return
}
@ -360,7 +360,7 @@ func (a adminAPIHandlers) SetConfigHandler(w http.ResponseWriter, r *http.Reques
return
}
if err = validateConfig(cfg, objectAPI.SetDriveCounts()); err != nil {
if err = validateConfig(cfg); err != nil {
writeCustomErrorResponseJSON(ctx, w, errorCodes.ToAPIErr(ErrAdminConfigBadJSON), err.Error(), r.URL)
return
}

View File

@ -227,7 +227,9 @@ var (
globalServerConfigMu sync.RWMutex
)
func validateConfig(s config.Config, setDriveCounts []int) error {
func validateConfig(s config.Config) error {
objAPI := newObjectLayerFn()
// We must have a global lock for this so nobody else modifies env while we do.
defer env.LockSetEnv()()
@ -250,7 +252,10 @@ func validateConfig(s config.Config, setDriveCounts []int) error {
}
if globalIsErasure {
for _, setDriveCount := range setDriveCounts {
if objAPI == nil {
return errServerNotInitialized
}
for _, setDriveCount := range objAPI.SetDriveCounts() {
if _, err := storageclass.LookupConfig(s[config.StorageClassSubSys][config.Default], setDriveCount); err != nil {
return err
}
@ -265,7 +270,7 @@ func validateConfig(s config.Config, setDriveCounts []int) error {
if err != nil {
return err
}
objAPI := newObjectLayerFn()
if objAPI != nil {
if compCfg.Enabled && !objAPI.IsCompressionSupported() {
return fmt.Errorf("Backend does not support compression")
@ -325,7 +330,7 @@ func validateConfig(s config.Config, setDriveCounts []int) error {
return notify.TestNotificationTargets(GlobalContext, s, NewGatewayHTTPTransport(), globalNotificationSys.ConfiguredTargetIDs())
}
func lookupConfigs(s config.Config, setDriveCounts []int) {
func lookupConfigs(s config.Config, objAPI ObjectLayer) {
ctx := GlobalContext
var err error
@ -337,7 +342,15 @@ func lookupConfigs(s config.Config, setDriveCounts []int) {
}
}
if dnsURL, dnsUser, dnsPass, ok := env.LookupEnv(config.EnvDNSWebhook); ok {
dnsURL, dnsUser, dnsPass, err := env.LookupEnv(config.EnvDNSWebhook)
if err != nil {
if globalIsGateway {
logger.FatalIf(err, "Unable to initialize remote webhook DNS config")
} else {
logger.LogIf(ctx, fmt.Errorf("Unable to initialize remote webhook DNS config %w", err))
}
}
if err == nil && dnsURL != "" {
globalDNSConfig, err = dns.NewOperatorDNS(dnsURL,
dns.Authentication(dnsUser, dnsPass),
dns.RootCAs(globalRootCAs))
@ -417,7 +430,8 @@ func lookupConfigs(s config.Config, setDriveCounts []int) {
getRemoteInstanceTransport = newGatewayHTTPTransport(apiConfig.RemoteTransportDeadline)
})
if globalIsErasure {
if globalIsErasure && objAPI != nil {
setDriveCounts := objAPI.SetDriveCounts()
for i, setDriveCount := range setDriveCounts {
sc, err := storageclass.LookupConfig(s[config.StorageClassSubSys][config.Default], setDriveCount)
if err != nil {
@ -529,16 +543,18 @@ func lookupConfigs(s config.Config, setDriveCounts []int) {
}
// Apply dynamic config values
logger.LogIf(ctx, applyDynamicConfig(ctx, newObjectLayerFn(), s))
if err := applyDynamicConfig(ctx, objAPI, s); err != nil {
if globalIsGateway {
logger.FatalIf(err, "Unable to initialize dynamic configuration")
} else {
logger.LogIf(ctx, err)
}
}
}
// applyDynamicConfig will apply dynamic config values.
// Dynamic systems should be in config.SubSystemsDynamic as well.
func applyDynamicConfig(ctx context.Context, objAPI ObjectLayer, s config.Config) error {
if objAPI == nil {
return nil
}
// Read all dynamic configs.
// API
apiConfig, err := api.LookupConfig(s[config.APISubSys][config.Default])
@ -553,8 +569,10 @@ func applyDynamicConfig(ctx context.Context, objAPI ObjectLayer, s config.Config
}
// Validate if the object layer supports compression.
if cmpCfg.Enabled && !objAPI.IsCompressionSupported() {
return fmt.Errorf("Backend does not support compression")
if objAPI != nil {
if cmpCfg.Enabled && !objAPI.IsCompressionSupported() {
return fmt.Errorf("Backend does not support compression")
}
}
// Heal
@ -571,7 +589,11 @@ func applyDynamicConfig(ctx context.Context, objAPI ObjectLayer, s config.Config
// Apply configurations.
// We should not fail after this.
globalAPIConfig.init(apiConfig, objAPI.SetDriveCounts())
var setDriveCounts []int
if objAPI != nil {
setDriveCounts = objAPI.SetDriveCounts()
}
globalAPIConfig.init(apiConfig, setDriveCounts)
globalCompressConfigMu.Lock()
globalCompressConfig = cmpCfg
@ -702,7 +724,7 @@ func loadConfig(objAPI ObjectLayer) error {
}
// Override any values from ENVs.
lookupConfigs(srvCfg, objAPI.SetDriveCounts())
lookupConfigs(srvCfg, objAPI)
// hold the mutex lock before a new config is assigned.
globalServerConfigMu.Lock()

View File

@ -155,7 +155,7 @@ func readServerConfig(ctx context.Context, objAPI ObjectLayer) (config.Config, e
data, err := readConfig(ctx, objAPI, configFile)
if err != nil {
if errors.Is(err, errConfigNotFound) {
lookupConfigs(srvCfg, objAPI.SetDriveCounts())
lookupConfigs(srvCfg, objAPI)
return srvCfg, nil
}
return nil, err
@ -166,7 +166,7 @@ func readServerConfig(ctx context.Context, objAPI ObjectLayer) (config.Config, e
minioMetaBucket: path.Join(minioMetaBucket, configFile),
})
if err != nil {
lookupConfigs(srvCfg, objAPI.SetDriveCounts())
lookupConfigs(srvCfg, objAPI)
return nil, err
}
}

View File

@ -218,8 +218,6 @@ func StartGateway(ctx *cli.Context, gw Gateway) {
// Set when gateway is enabled
globalIsGateway = true
enableConfigOps := false
// TODO: We need to move this code with globalConfigSys.Init()
// for now keep it here such that "s3" gateway layer initializes
// itself properly when KMS is set.
@ -245,7 +243,7 @@ func StartGateway(ctx *cli.Context, gw Gateway) {
// Enable IAM admin APIs if etcd is enabled, if not just enable basic
// operations such as profiling, server info etc.
registerAdminRouter(router, enableConfigOps)
registerAdminRouter(router, false)
// Add healthcheck router
registerHealthCheckRouter(router)

View File

@ -105,10 +105,18 @@ EXAMPLES:
}
func serverCmdArgs(ctx *cli.Context) []string {
v := env.Get(config.EnvArgs, "")
v, _, _, err := env.LookupEnv(config.EnvArgs)
if err != nil {
logger.FatalIf(err, "Unable to validate passed arguments in %s:%s",
config.EnvArgs, os.Getenv(config.EnvArgs))
}
if v == "" {
// Fall back to older ENV MINIO_ENDPOINTS
v = env.Get(config.EnvEndpoints, "")
// Fall back to older environment value MINIO_ENDPOINTS
v, _, _, err = env.LookupEnv(config.EnvEndpoints)
if err != nil {
logger.FatalIf(err, "Unable to validate passed arguments in %s:%s",
config.EnvEndpoints, os.Getenv(config.EnvEndpoints))
}
}
if v == "" {
if !ctx.Args().Present() || ctx.Args().First() == "help" {

View File

@ -26,7 +26,6 @@ import (
"testing"
"github.com/gorilla/mux"
"github.com/minio/minio/internal/config"
xnet "github.com/minio/pkg/net"
)
@ -416,7 +415,7 @@ func testStorageAPIRenameFile(t *testing.T, storage StorageAPI) {
}
}
func newStorageRESTHTTPServerClient(t *testing.T) (*httptest.Server, *storageRESTClient, config.Config, string) {
func newStorageRESTHTTPServerClient(t *testing.T) (*httptest.Server, *storageRESTClient, string) {
prevHost, prevPort := globalMinioHost, globalMinioPort
defer func() {
globalMinioHost, globalMinioPort = prevHost, prevPort
@ -451,142 +450,102 @@ func newStorageRESTHTTPServerClient(t *testing.T) (*httptest.Server, *storageRES
Endpoints: Endpoints{endpoint},
}})
prevGlobalServerConfig := globalServerConfig
globalServerConfig = newServerConfig()
lookupConfigs(globalServerConfig, nil)
restClient := newStorageRESTClient(endpoint, false)
return httpServer, restClient, prevGlobalServerConfig, endpointPath
return httpServer, restClient, endpointPath
}
func TestStorageRESTClientDiskInfo(t *testing.T) {
httpServer, restClient, prevGlobalServerConfig, endpointPath := newStorageRESTHTTPServerClient(t)
httpServer, restClient, endpointPath := newStorageRESTHTTPServerClient(t)
defer httpServer.Close()
defer func() {
globalServerConfig = prevGlobalServerConfig
}()
defer os.RemoveAll(endpointPath)
testStorageAPIDiskInfo(t, restClient)
}
func TestStorageRESTClientMakeVol(t *testing.T) {
httpServer, restClient, prevGlobalServerConfig, endpointPath := newStorageRESTHTTPServerClient(t)
httpServer, restClient, endpointPath := newStorageRESTHTTPServerClient(t)
defer httpServer.Close()
defer func() {
globalServerConfig = prevGlobalServerConfig
}()
defer os.RemoveAll(endpointPath)
testStorageAPIMakeVol(t, restClient)
}
func TestStorageRESTClientListVols(t *testing.T) {
httpServer, restClient, prevGlobalServerConfig, endpointPath := newStorageRESTHTTPServerClient(t)
httpServer, restClient, endpointPath := newStorageRESTHTTPServerClient(t)
defer httpServer.Close()
defer func() {
globalServerConfig = prevGlobalServerConfig
}()
defer os.RemoveAll(endpointPath)
testStorageAPIListVols(t, restClient)
}
func TestStorageRESTClientStatVol(t *testing.T) {
httpServer, restClient, prevGlobalServerConfig, endpointPath := newStorageRESTHTTPServerClient(t)
httpServer, restClient, endpointPath := newStorageRESTHTTPServerClient(t)
defer httpServer.Close()
defer func() {
globalServerConfig = prevGlobalServerConfig
}()
defer os.RemoveAll(endpointPath)
testStorageAPIStatVol(t, restClient)
}
func TestStorageRESTClientDeleteVol(t *testing.T) {
httpServer, restClient, prevGlobalServerConfig, endpointPath := newStorageRESTHTTPServerClient(t)
httpServer, restClient, endpointPath := newStorageRESTHTTPServerClient(t)
defer httpServer.Close()
defer func() {
globalServerConfig = prevGlobalServerConfig
}()
defer os.RemoveAll(endpointPath)
testStorageAPIDeleteVol(t, restClient)
}
func TestStorageRESTClientStatInfoFile(t *testing.T) {
httpServer, restClient, prevGlobalServerConfig, endpointPath := newStorageRESTHTTPServerClient(t)
httpServer, restClient, endpointPath := newStorageRESTHTTPServerClient(t)
defer httpServer.Close()
defer func() {
globalServerConfig = prevGlobalServerConfig
}()
defer os.RemoveAll(endpointPath)
testStorageAPIStatInfoFile(t, restClient)
}
func TestStorageRESTClientListDir(t *testing.T) {
httpServer, restClient, prevGlobalServerConfig, endpointPath := newStorageRESTHTTPServerClient(t)
httpServer, restClient, endpointPath := newStorageRESTHTTPServerClient(t)
defer httpServer.Close()
defer func() {
globalServerConfig = prevGlobalServerConfig
}()
defer os.RemoveAll(endpointPath)
testStorageAPIListDir(t, restClient)
}
func TestStorageRESTClientReadAll(t *testing.T) {
httpServer, restClient, prevGlobalServerConfig, endpointPath := newStorageRESTHTTPServerClient(t)
httpServer, restClient, endpointPath := newStorageRESTHTTPServerClient(t)
defer httpServer.Close()
defer func() {
globalServerConfig = prevGlobalServerConfig
}()
defer os.RemoveAll(endpointPath)
testStorageAPIReadAll(t, restClient)
}
func TestStorageRESTClientReadFile(t *testing.T) {
httpServer, restClient, prevGlobalServerConfig, endpointPath := newStorageRESTHTTPServerClient(t)
httpServer, restClient, endpointPath := newStorageRESTHTTPServerClient(t)
defer httpServer.Close()
defer func() {
globalServerConfig = prevGlobalServerConfig
}()
defer os.RemoveAll(endpointPath)
testStorageAPIReadFile(t, restClient)
}
func TestStorageRESTClientAppendFile(t *testing.T) {
httpServer, restClient, prevGlobalServerConfig, endpointPath := newStorageRESTHTTPServerClient(t)
httpServer, restClient, endpointPath := newStorageRESTHTTPServerClient(t)
defer httpServer.Close()
defer func() {
globalServerConfig = prevGlobalServerConfig
}()
defer os.RemoveAll(endpointPath)
testStorageAPIAppendFile(t, restClient)
}
func TestStorageRESTClientDeleteFile(t *testing.T) {
httpServer, restClient, prevGlobalServerConfig, endpointPath := newStorageRESTHTTPServerClient(t)
httpServer, restClient, endpointPath := newStorageRESTHTTPServerClient(t)
defer httpServer.Close()
defer func() {
globalServerConfig = prevGlobalServerConfig
}()
defer os.RemoveAll(endpointPath)
testStorageAPIDeleteFile(t, restClient)
}
func TestStorageRESTClientRenameFile(t *testing.T) {
httpServer, restClient, prevGlobalServerConfig, endpointPath := newStorageRESTHTTPServerClient(t)
httpServer, restClient, endpointPath := newStorageRESTHTTPServerClient(t)
defer httpServer.Close()
defer func() {
globalServerConfig = prevGlobalServerConfig
}()
defer os.RemoveAll(endpointPath)
testStorageAPIRenameFile(t, restClient)

2
go.mod
View File

@ -48,7 +48,7 @@ require (
github.com/minio/madmin-go v1.1.0
github.com/minio/minio-go/v7 v7.0.13-0.20210823191913-cee488b95ff2
github.com/minio/parquet-go v1.0.0
github.com/minio/pkg v1.0.11
github.com/minio/pkg v1.1.2
github.com/minio/selfupdate v0.3.1
github.com/minio/sha256-simd v1.0.0
github.com/minio/simdjson-go v0.2.1

4
go.sum
View File

@ -1047,8 +1047,8 @@ github.com/minio/parquet-go v1.0.0/go.mod h1:aQlkSOfOq2AtQKkuou3mosNVMwNokd+faTa
github.com/minio/pkg v1.0.3/go.mod h1:obU54TZ9QlMv0TRaDgQ/JTzf11ZSXxnSfLrm4tMtBP8=
github.com/minio/pkg v1.0.4/go.mod h1:obU54TZ9QlMv0TRaDgQ/JTzf11ZSXxnSfLrm4tMtBP8=
github.com/minio/pkg v1.0.8/go.mod h1:32x/3OmGB0EOi1N+3ggnp+B5VFkSBBB9svPMVfpnf14=
github.com/minio/pkg v1.0.11 h1:gfpkP7SiznM7EyyHIfQ7lu98Ae4hV4Z+8YsoFQbH7PY=
github.com/minio/pkg v1.0.11/go.mod h1:32x/3OmGB0EOi1N+3ggnp+B5VFkSBBB9svPMVfpnf14=
github.com/minio/pkg v1.1.2 h1:A7zSlVIQCf71DPYM1kqBpybq6fmsRY3RuTMD0ZNcqZA=
github.com/minio/pkg v1.1.2/go.mod h1:32x/3OmGB0EOi1N+3ggnp+B5VFkSBBB9svPMVfpnf14=
github.com/minio/selfupdate v0.3.1 h1:BWEFSNnrZVMUWXbXIgLDNDjbejkmpAmZvy/nCz1HlEs=
github.com/minio/selfupdate v0.3.1/go.mod h1:b8ThJzzH7u2MkF6PcIra7KaXO9Khf6alWPvMSyTDCFM=
github.com/minio/sha256-simd v0.1.1/go.mod h1:B5e1o+1/KgNmWrSQK08Y6Z1Vb5pwIktudl0J58iy0KM=