Fix config leaks and deprecate file-based config setters in NAS gateway (#9884)

This PR has the following changes

- Removing duplicate lookupConfigs() calls.
- Deprecate admin config APIs for NAS gateways. This will avoid repeated reloads of the config from the disk.
- WatchConfigNASDisk will be removed
- Migration guide for NAS gateways users to migrate to ENV settings.

NOTE: THIS PR HAS A BREAKING CHANGE

Fixes #9875

Co-authored-by: Harshavardhana <harsha@minio.io>

cmd/config-current.go

@@ -482,12 +482,12 @@ func lookupConfigs(s config.Config) {
 		}
 	}
 
-	globalConfigTargetList, err = notify.GetNotificationTargets(s, GlobalContext.Done(), NewGatewayHTTPTransport())
+	globalConfigTargetList, err = notify.GetNotificationTargets(s, GlobalContext.Done(), NewGatewayHTTPTransport(), false)
 	if err != nil {
 		logger.LogIf(ctx, fmt.Errorf("Unable to initialize notification target(s): %w", err))
 	}
 
-	globalEnvTargetList, err = notify.GetNotificationTargets(newServerConfig(), GlobalContext.Done(), NewGatewayHTTPTransport())
+	globalEnvTargetList, err = notify.GetNotificationTargets(newServerConfig(), GlobalContext.Done(), NewGatewayHTTPTransport(), true)
 	if err != nil {
 		logger.LogIf(ctx, fmt.Errorf("Unable to initialize notification target(s): %w", err))
 	}
@@ -579,9 +579,6 @@ func newSrvConfig(objAPI ObjectLayer) error {
 	// Initialize server config.
 	srvCfg := newServerConfig()
 
-	// Override any values from ENVs.
-	lookupConfigs(srvCfg)
-
 	// hold the mutex lock before a new config is assigned.
 	globalServerConfigMu.Lock()
 	globalServerConfig = srvCfg

cmd/config-encrypted.go

@@ -31,10 +31,7 @@ import (
 	"github.com/minio/minio/pkg/madmin"
 )
 
-func handleEncryptedConfigBackend(objAPI ObjectLayer, server bool) error {
+func handleEncryptedConfigBackend(objAPI ObjectLayer) error {
-	if !server {
-		return nil
-	}
 
 	encrypted, err := checkBackendEncrypted(objAPI)
 	if err != nil {

cmd/config.go

@@ -23,7 +23,6 @@ import (
 	"path"
 	"sort"
 	"strings"
-	"time"
 
 	jsoniter "github.com/json-iterator/go"
 	"github.com/minio/minio/cmd/config"
@@ -184,23 +183,6 @@ func (sys *ConfigSys) Load(objAPI ObjectLayer) error {
 	return sys.Init(objAPI)
 }
 
-// WatchConfigNASDisk - watches nas disk on periodic basis.
-func (sys *ConfigSys) WatchConfigNASDisk(ctx context.Context, objAPI ObjectLayer) {
-	configInterval := globalRefreshIAMInterval
-	watchDisk := func() {
-		for {
-			select {
-			case <-ctx.Done():
-				return
-			case <-time.After(configInterval):
-				loadConfig(objAPI)
-			}
-		}
-	}
-	// Refresh configSys in background for NAS gateway.
-	go watchDisk()
-}
-
 // Init - initializes config system from config.json.
 func (sys *ConfigSys) Init(objAPI ObjectLayer) error {
 	if objAPI == nil {

cmd/config/notify/parse.go

@@ -60,8 +60,7 @@ func TestNotificationTargets(cfg config.Config, doneCh <-chan struct{}, transpor
 
 // GetNotificationTargets registers and initializes all notification
 // targets, returns error if any.
-func GetNotificationTargets(cfg config.Config, doneCh <-chan struct{}, transport *http.Transport) (*event.TargetList, error) {
+func GetNotificationTargets(cfg config.Config, doneCh <-chan struct{}, transport *http.Transport, test bool) (*event.TargetList, error) {
-	test := false
 	returnOnTargetError := false
 	return RegisterNotificationTargets(cfg, doneCh, transport, nil, test, returnOnTargetError)
 }
@@ -72,7 +71,6 @@ func GetNotificationTargets(cfg config.Config, doneCh <-chan struct{}, transport
 // * Add newly added target configuration to serverConfig.Notify.<TARGET_NAME>.
 // * Handle the configuration in this function to create/add into TargetList.
 func RegisterNotificationTargets(cfg config.Config, doneCh <-chan struct{}, transport *http.Transport, targetIDs []event.TargetID, test bool, returnOnTargetError bool) (*event.TargetList, error) {
-
 	targetList, err := FetchRegisteredTargets(cfg, doneCh, transport, test, returnOnTargetError)
 	if err != nil {
 		return targetList, err

cmd/gateway-main.go

@@ -152,7 +152,7 @@ func StartGateway(ctx *cli.Context, gw Gateway) {
 	// Set when gateway is enabled
 	globalIsGateway = true
 
-	enableConfigOps := gatewayName == "nas"
+	enableConfigOps := false
 
 	// TODO: We need to move this code with globalConfigSys.Init()
 	// for now keep it here such that "s3" gateway layer initializes
@@ -242,29 +242,15 @@ func StartGateway(ctx *cli.Context, gw Gateway) {
 	globalObjectAPI = newObject
 	globalObjLayerMutex.Unlock()
 
-	// Migrate all backend configs to encrypted backend, also handles rotation as well.
-	// For "nas" gateway we need to specially handle the backend migration as well.
-	// Internally code handles migrating etcd if enabled automatically.
-	logger.FatalIf(handleEncryptedConfigBackend(newObject, enableConfigOps),
-		"Unable to handle encrypted backend for config, iam and policies")
-
 	// Calls all New() for all sub-systems.
 	newAllSubsystems()
 
-	// ****  WARNING ****
+	if gatewayName == "nas" {
-	// Migrating to encrypted backend should happen before initialization of any
-	// sub-systems, make sure that we do not move the above codeblock elsewhere.
-	if enableConfigOps {
-		logger.FatalIf(globalConfigSys.Init(newObject), "Unable to initialize config system")
 		buckets, err := newObject.ListBuckets(GlobalContext)
 		if err != nil {
 			logger.Fatal(err, "Unable to list buckets")
 		}
-
 		logger.FatalIf(globalNotificationSys.Init(buckets, newObject), "Unable to initialize notification system")
-		// Start watching disk for reloading config, this
-		// is only enabled for "NAS" gateway.
-		globalConfigSys.WatchConfigNASDisk(GlobalContext, newObject)
 	}
 
 	if globalEtcdClient != nil {

cmd/server-main.go

@@ -235,7 +235,7 @@ func initSafeMode(ctx context.Context, newObject ObjectLayer) (err error) {
 		// Migrate all backend configs to encrypted backend configs, optionally
 		// handles rotating keys for encryption, if there is any retriable failure
 		// that shall be retried if there is an error.
-		if err = handleEncryptedConfigBackend(newObject, true); err == nil {
+		if err = handleEncryptedConfigBackend(newObject); err == nil {
 			// Upon success migrating the config, initialize all sub-systems
 			// if all sub-systems initialized successfully return right away
 			if err = initAllSubsystems(retryCtx, newObject); err == nil {

docs/gateway/nas.md

@@ -49,6 +49,36 @@ mc ls mynas
 [2017-02-26 22:10:11 PST]     0B test-bucket1/
 ```
 
+## Breaking changes
+
+There will be a breaking change after the release version 'RELEASE.2020-06-22T03-12-50Z'.
+
+### The file-based config settings are deprecated in NAS
+
+The support for admin config APIs will be removed. This will include getters and setters like `mc admin config get` and `mc admin config`  and any other `mc admin config` options. The reason for this change is to avoid un-necessary reloads of the config from the disk. And to comply with the Environment variable based settings like other gateways.
+
+### Migration guide
+
+The users who have been using the older config approach should migrate to ENV settings by setting environment variables accordingly.
+
+For example,
+
+Consider the following webhook target config.
+
+```
+notify_webhook:1 endpoint=http://localhost:8080/ auth_token= queue_limit=0 queue_dir=/tmp/webhk client_cert= client_key=
+```
+
+The corresponding environment variable setting can be
+
+```
+export MINIO_NOTIFY_WEBHOOK_ENABLE_1=on
+export MINIO_NOTIFY_WEBHOOK_ENDPOINT_1=http://localhost:8080/
+export MINIO_NOTIFY_WEBHOOK_QUEUE_DIR_1=/tmp/webhk
+```
+
+> NOTE: Please check the docs for the corresponding ENV setting. Alternatively, We can obtain other ENVs in the form `mc admin config set alias/ <sub-sys> --env`
+
 ## Explore Further
 - [`mc` command-line interface](https://docs.min.io/docs/minio-client-quickstart-guide)
 - [`aws` command-line interface](https://docs.min.io/docs/aws-cli-with-minio)