Merge config-cli changes to master

cmd/flags.go

@@ -485,16 +485,6 @@ func resolvePathForTheFlagInContext(flagKey string, c *cli.Context) (err error)
 // GCSFUSE_PARENT_PROCESS_DIR. Child process is spawned when --foreground flag
 // is disabled.
 func resolvePathForTheFlagsInContext(c *cli.Context) (err error) {
-	err = resolvePathForTheFlagInContext("log-file", c)
-	if err != nil {
-		return fmt.Errorf("resolving for log-file: %w", err)
-	}
-
-	err = resolvePathForTheFlagInContext("key-file", c)
-	if err != nil {
-		return fmt.Errorf("resolving for key-file: %w", err)
-	}
-
 	err = resolvePathForTheFlagInContext("config-file", c)
 	if err != nil {
 		return fmt.Errorf("resolving for config-file: %w", err)
@@ -505,14 +495,9 @@ func resolvePathForTheFlagsInContext(c *cli.Context) (err error) {
 
 // resolveConfigFilePaths resolves the config file paths specified in the config file.
 func resolveConfigFilePaths(mountConfig *config.MountConfig) (err error) {
-	mountConfig.LogConfig.FilePath, err = resolveFilePath(mountConfig.LogConfig.FilePath, "logging: file")
-	if err != nil {
-		return
-	}
-
 	// Resolve cache-dir path
 	resolvedPath, err := resolveFilePath(string(mountConfig.CacheDir), "cache-dir")
-	mountConfig.CacheDir = config.CacheDir(resolvedPath)
+	mountConfig.CacheDir = resolvedPath
 	if err != nil {
 		return
 	}

cmd/flags_test.go

@@ -274,23 +274,18 @@ func (t *FlagsTest) TestResolvePathForTheFlagInContext() {
 	currentWorkingDir, err := os.Getwd()
 	assert.Equal(t.T(), nil, err)
 	app.Action = func(appCtx *cli.Context) {
-		err = resolvePathForTheFlagInContext("log-file", appCtx)
-		assert.Equal(t.T(), nil, err)
 		err = resolvePathForTheFlagInContext("key-file", appCtx)
 		assert.Equal(t.T(), nil, err)
 		err = resolvePathForTheFlagInContext("config-file", appCtx)
 		assert.Equal(t.T(), nil, err)
 
-		assert.Equal(t.T(), filepath.Join(currentWorkingDir, "test.txt"),
-			appCtx.String("log-file"))
 		assert.Equal(t.T(), filepath.Join(currentWorkingDir, "test.txt"),
 			appCtx.String("key-file"))
 		assert.Equal(t.T(), filepath.Join(currentWorkingDir, "config.yaml"),
 			appCtx.String("config-file"))
 	}
 	// Simulate argv.
-	fullArgs := []string{"some_app", "--log-file=test.txt",
-		"--key-file=test.txt", "--config-file=config.yaml"}
+	fullArgs := []string{"some_app", "--key-file=test.txt", "--config-file=config.yaml"}
 
 	err = app.Run(fullArgs)
 
@@ -304,16 +299,11 @@ func (t *FlagsTest) TestResolvePathForTheFlagsInContext() {
 	app.Action = func(appCtx *cli.Context) {
 		resolvePathForTheFlagsInContext(appCtx)
 
-		assert.Equal(t.T(), filepath.Join(currentWorkingDir, "test.txt"),
-			appCtx.String("log-file"))
-		assert.Equal(t.T(), filepath.Join(currentWorkingDir, "test.txt"),
-			appCtx.String("key-file"))
 		assert.Equal(t.T(), filepath.Join(currentWorkingDir, "config.yaml"),
 			appCtx.String("config-file"))
 	}
 	// Simulate argv.
-	fullArgs := []string{"some_app", "--log-file=test.txt",
-		"--key-file=test.txt", "--config-file=config.yaml"}
+	fullArgs := []string{"some_app", "--config-file=config.yaml"}
 
 	err = app.Run(fullArgs)
 
@@ -421,17 +411,13 @@ func (t *FlagsTest) TestValidateFlagsForUnsupportedExperimentalMetadataPrefetchO
 
 func (t *FlagsTest) Test_resolveConfigFilePaths() {
 	mountConfig := &config.MountConfig{}
-	mountConfig.LogConfig = config.LogConfig{
-		FilePath: "~/test.txt",
-	}
 	mountConfig.CacheDir = "~/cache-dir"
 
 	err := resolveConfigFilePaths(mountConfig)
 
 	assert.Equal(t.T(), nil, err)
 	homeDir, err := os.UserHomeDir()
 	assert.Equal(t.T(), nil, err)
-	assert.Equal(t.T(), filepath.Join(homeDir, "test.txt"), mountConfig.LogConfig.FilePath)
 	assert.EqualValues(t.T(), filepath.Join(homeDir, "cache-dir"), mountConfig.CacheDir)
 }
 
@@ -441,7 +427,6 @@ func (t *FlagsTest) Test_resolveConfigFilePaths_WithoutSettingPaths() {
 	err := resolveConfigFilePaths(mountConfig)
 
 	assert.Equal(t.T(), nil, err)
-	assert.Equal(t.T(), "", mountConfig.LogConfig.FilePath)
 	assert.EqualValues(t.T(), "", mountConfig.CacheDir)
 }
 

cmd/legacy_main.go

@@ -28,6 +28,7 @@ import (
 	"path/filepath"
 	"strings"
 
+	"github.com/googlecloudplatform/gcsfuse/v2/cfg"
 	"github.com/googlecloudplatform/gcsfuse/v2/internal/canned"
 	"github.com/googlecloudplatform/gcsfuse/v2/internal/config"
 	"github.com/googlecloudplatform/gcsfuse/v2/internal/locker"
@@ -100,7 +101,7 @@ func getConfigForUserAgent(mountConfig *config.MountConfig) string {
 	}
 	return fmt.Sprintf("%s:%s", isFileCacheEnabled, isFileCacheForRangeReadEnabled)
 }
-func createStorageHandle(flags *flagStorage, mountConfig *config.MountConfig, userAgent string) (storageHandle storage.StorageHandle, err error) {
+func createStorageHandle(newConfig *cfg.Config, flags *flagStorage, mountConfig *config.MountConfig, userAgent string) (storageHandle storage.StorageHandle, err error) {
 	storageClientConfig := storageutil.StorageClientConfig{
 		ClientProtocol:             flags.ClientProtocol,
 		MaxConnsPerHost:            flags.MaxConnsPerHost,
@@ -110,7 +111,7 @@ func createStorageHandle(flags *flagStorage, mountConfig *config.MountConfig, us
 		RetryMultiplier:            flags.RetryMultiplier,
 		UserAgent:                  userAgent,
 		CustomEndpoint:             flags.CustomEndpoint,
-		KeyFile:                    flags.KeyFile,
+		KeyFile:                    string(newConfig.GcsAuth.KeyFile),
 		AnonymousAccess:            mountConfig.GCSAuth.AnonymousAccess,
 		TokenUrl:                   flags.TokenUrl,
 		ReuseTokenFromUrl:          flags.ReuseTokenFromUrl,
@@ -132,7 +133,8 @@ func mountWithArgs(
 	bucketName string,
 	mountPoint string,
 	flags *flagStorage,
-	mountConfig *config.MountConfig) (mfs *fuse.MountedFileSystem, err error) {
+	mountConfig *config.MountConfig,
+	newConfig *cfg.Config) (mfs *fuse.MountedFileSystem, err error) {
 	// Enable invariant checking if requested.
 	if flags.DebugInvariants {
 		locker.EnableInvariantsCheck()
@@ -149,7 +151,7 @@ func mountWithArgs(
 	if bucketName != canned.FakeBucketName {
 		userAgent := getUserAgent(flags.AppName, getConfigForUserAgent(mountConfig))
 		logger.Info("Creating Storage handle...")
-		storageHandle, err = createStorageHandle(flags, mountConfig, userAgent)
+		storageHandle, err = createStorageHandle(newConfig, flags, mountConfig, userAgent)
 		if err != nil {
 			err = fmt.Errorf("Failed to create storage handle using createStorageHandle: %w", err)
 			return
@@ -251,7 +253,12 @@ func runCLIApp(c *cli.Context) (err error) {
 		return fmt.Errorf("parsing config file failed: %w", err)
 	}
 
-	config.OverrideWithLoggingFlags(mountConfig, flags.LogFile, flags.LogFormat,
+	newConfig, err := PopulateNewConfigFromLegacyFlagsAndConfig(c, flags, mountConfig)
+	if err != nil {
+		return fmt.Errorf("error resolving flags and configs: %w", err)
+	}
+
+	config.OverrideWithLoggingFlags(mountConfig, flags.LogFormat,
 		flags.DebugFuse, flags.DebugGCS, flags.DebugMutex)
 	config.OverrideWithIgnoreInterruptsFlag(c, mountConfig, flags.IgnoreInterrupts)
 	config.OverrideWithAnonymousAccessFlag(c, mountConfig, flags.AnonymousAccess)
@@ -269,7 +276,7 @@ func runCLIApp(c *cli.Context) (err error) {
 	}
 
 	if flags.Foreground {
-		err = logger.InitLogFile(mountConfig.LogConfig)
+		err = logger.InitLogFile(mountConfig.LogConfig, newConfig.Logging)
 		if err != nil {
 			return fmt.Errorf("init log file: %w", err)
 		}
@@ -289,7 +296,7 @@ func runCLIApp(c *cli.Context) (err error) {
 	// Do not log these in stdout in case of daemonized run
 	// if these are already being logged into a log-file, otherwise
 	// there will be duplicate logs for these in both places (stdout and log-file).
-	if flags.Foreground || mountConfig.LogConfig.FilePath == "" {
+	if flags.Foreground || newConfig.Logging.FilePath == "" {
 		flagsStringified, err := util.Stringify(*flags)
 		if err != nil {
 			logger.Warnf("failed to stringify cli flags: %v", err)
@@ -404,7 +411,7 @@ func runCLIApp(c *cli.Context) (err error) {
 	// daemonize gives us and telling it about the outcome.
 	var mfs *fuse.MountedFileSystem
 	{
-		mfs, err = mountWithArgs(bucketName, mountPoint, flags, mountConfig)
+		mfs, err = mountWithArgs(bucketName, mountPoint, flags, mountConfig, newConfig)
 
 		// This utility is to absorb the error
 		// returned by daemonize.SignalOutcome calls by simply

cmd/legacy_main_test.go

@@ -20,6 +20,7 @@ import (
 	"strings"
 	"testing"
 
+	"github.com/googlecloudplatform/gcsfuse/v2/cfg"
 	"github.com/googlecloudplatform/gcsfuse/v2/internal/config"
 	"github.com/googlecloudplatform/gcsfuse/v2/internal/util"
 
@@ -49,12 +50,12 @@ func (t *MainTest) TestCreateStorageHandle() {
 		MaxRetrySleep:       7,
 		RetryMultiplier:     2,
 		AppName:             "app",
-		KeyFile:             "testdata/test_creds.json",
 	}
 	mountConfig := &config.MountConfig{}
+	newConfig := &cfg.Config{GcsAuth: cfg.GcsAuthConfig{KeyFile: "testdata/test_creds.json"}}
 
 	userAgent := "AppName"
-	storageHandle, err := createStorageHandle(flags, mountConfig, userAgent)
+	storageHandle, err := createStorageHandle(newConfig, flags, mountConfig, userAgent)
 
 	assert.Equal(t.T(), nil, err)
 	assert.NotEqual(t.T(), nil, storageHandle)
@@ -69,14 +70,14 @@ func (t *MainTest) TestCreateStorageHandle_WithClientProtocolAsGRPC() {
 		MaxRetrySleep:       7,
 		RetryMultiplier:     2,
 		AppName:             "app",
-		KeyFile:             "testdata/test_creds.json",
 	}
 	mountConfig := &config.MountConfig{
 		GCSConnection: config.GCSConnection{GRPCConnPoolSize: 1},
 	}
+	newConfig := &cfg.Config{GcsAuth: cfg.GcsAuthConfig{KeyFile: "testdata/test_creds.json"}}
 
 	userAgent := "AppName"
-	storageHandle, err := createStorageHandle(flags, mountConfig, userAgent)
+	storageHandle, err := createStorageHandle(newConfig, flags, mountConfig, userAgent)
 
 	assert.Equal(t.T(), nil, err)
 	assert.NotEqual(t.T(), nil, storageHandle)

cmd/legacy_param_mapper.go

@@ -0,0 +1,155 @@
+// Copyright 2024 Google Inc. All Rights Reserved.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package cmd
+
+import (
+	"fmt"
+	"reflect"
+
+	"github.com/googlecloudplatform/gcsfuse/v2/cfg"
+	"github.com/googlecloudplatform/gcsfuse/v2/internal/config"
+	"github.com/mitchellh/mapstructure"
+)
+
+// cliContext is abstraction over the IsSet() method of cli.Context, specially
+// added to keep OverrideWithIgnoreInterruptsFlag method's unit test simple.
+type cliContext interface {
+	IsSet(string) bool
+}
+
+// PopulateNewConfigFromLegacyFlagsAndConfig takes cliContext, legacy flags and legacy MountConfig and resolves it into new cfg.Config Object.
+func PopulateNewConfigFromLegacyFlagsAndConfig(c cliContext, flags *flagStorage, legacyConfig *config.MountConfig) (*cfg.Config, error) {
+	if flags == nil || legacyConfig == nil {
+		return nil, fmt.Errorf("PopulateNewConfigFromLegacyFlagsAndConfig: unexpected nil flags or mount config")
+	}
+
+	resolvedConfig := &cfg.Config{}
+
+	structuredFlags := &map[string]interface{}{
+		"app-name": flags.AppName,
+		"debug": &map[string]interface{}{
+			"exit-on-invariant-violation": flags.DebugInvariants,
+			"gcs":                         flags.DebugGCS,
+			"log-mutex":                   flags.DebugMutex,
+		},
+		"file-system": map[string]interface{}{
+			"dir-mode":  flags.DirMode,
+			"file-mode": flags.FileMode,
+			// Todo: "fuse-options":      nil,
+			"gid":               flags.Gid,
+			"ignore-interrupts": flags.IgnoreInterrupts,
+			"rename-dir-limit":  flags.RenameDirLimit,
+			"temp-dir":          flags.TempDir,
+			"uid":               flags.Uid,
+		},
+		"foreground": flags.Foreground,
+		"gcs-auth": map[string]interface{}{
+			"anonymous-access":     flags.AnonymousAccess,
+			"key-file":             flags.KeyFile,
+			"reuse-token-from-url": flags.ReuseTokenFromUrl,
+			"token-url":            flags.TokenUrl,
+		},
+		"gcs-connection": map[string]interface{}{
+			"billing-project":               flags.BillingProject,
+			"client-protocol":               string(flags.ClientProtocol),
+			"custom-endpoint":               flags.CustomEndpoint,
+			"experimental-enable-json-read": flags.ExperimentalEnableJsonRead,
+			"http-client-timeout":           flags.HttpClientTimeout,
+			"limit-bytes-per-sec":           flags.EgressBandwidthLimitBytesPerSecond,
+			"limit-ops-per-sec":             flags.OpRateLimitHz,
+			"max-conns-per-host":            flags.MaxConnsPerHost,
+			"max-idle-conns-per-host":       flags.MaxIdleConnsPerHost,
+			"sequential-read-size-mb":       flags.SequentialReadSizeMb,
+		},
+		"gcs-retries": map[string]interface{}{
+			"max-retry-sleep": flags.MaxRetrySleep,
+			"multiplier":      flags.RetryMultiplier,
+		},
+		"implicit-dirs": flags.ImplicitDirs,
+		"list": map[string]interface{}{
+			"kernel-list-cache-ttl-secs": flags.KernelListCacheTtlSeconds,
+		},
+		"logging": map[string]interface{}{
+			"file-path": flags.LogFile,
+			"format":    flags.LogFormat,
+		},
+		"metadata-cache": map[string]interface{}{
+			"deprecated-stat-cache-capacity":          flags.StatCacheCapacity,
+			"deprecated-stat-cache-ttl":               flags.StatCacheTTL,
+			"deprecated-type-cache-ttl":               flags.TypeCacheTTL,
+			"enable-nonexistent-type-cache":           flags.EnableNonexistentTypeCache,
+			"experimental-metadata-prefetch-on-mount": flags.ExperimentalMetadataPrefetchOnMount,
+		},
+		"metrics": map[string]interface{}{
+			"stackdriver-export-interval": flags.StackdriverExportInterval,
+		},
+		"monitoring": map[string]interface{}{
+			"experimental-opentelemetry-collector-address": flags.OtelCollectorAddress,
+		},
+		"only-dir": flags.OnlyDir,
+	}
+
+	// Use decoder to convert flagStorage to cfg.Config.
+	decoderConfig := &mapstructure.DecoderConfig{
+		DecodeHook: cfg.DecodeHook(),
+		Result:     resolvedConfig,
+		TagName:    "yaml",
+	}
+	decoder, err := mapstructure.NewDecoder(decoderConfig)
+	if err != nil {
+		return nil, fmt.Errorf("mapstructure.NewDecoder: %w", err)
+	}
+	// Decoding flags.
+	if err = decoder.Decode(structuredFlags); err != nil {
+		return nil, fmt.Errorf("decoder.Decode(structuredFlags): %w", err)
+	}
+
+	// If config file does not have any values, no need to decode it.
+	if reflect.ValueOf(*legacyConfig).IsZero() {
+		return resolvedConfig, nil
+	}
+
+	// Save overlapping flags in a map to override the config value later.
+	var (
+		logFile                = resolvedConfig.Logging.FilePath
+		logFormat              = resolvedConfig.Logging.Format
+		ignoreInterrupts       = resolvedConfig.FileSystem.IgnoreInterrupts
+		anonymousAccess        = resolvedConfig.GcsAuth.AnonymousAccess
+		kernelListCacheTTLSecs = resolvedConfig.List.KernelListCacheTtlSecs
+	)
+
+	// Decoding config to the same config structure (resolvedConfig).
+	if err = decoder.Decode(legacyConfig); err != nil {
+		return nil, fmt.Errorf("decoder.Decode(config): %w", err)
+	}
+
+	// Override/Give priority to flags in case of overlap in flags and config.
+	overrideWithFlag(c, "log-file", &resolvedConfig.Logging.FilePath, logFile)
+	overrideWithFlag(c, "log-format", &resolvedConfig.Logging.Format, logFormat)
+	overrideWithFlag(c, "ignore-interrupts", &resolvedConfig.FileSystem.IgnoreInterrupts, ignoreInterrupts)
+	overrideWithFlag(c, "anonymous-access", &resolvedConfig.GcsAuth.AnonymousAccess, anonymousAccess)
+	overrideWithFlag(c, "kernel-list-cache-ttl-secs", &resolvedConfig.List.KernelListCacheTtlSecs, kernelListCacheTTLSecs)
+
+	return resolvedConfig, nil
+}
+
+// overrideWithFlag function overrides the toUpdate value with updateValue if
+// the flag is set in cliCOntext.
+func overrideWithFlag[T any](c cliContext, flag string, toUpdate *T, updateValue T) {
+	if !c.IsSet(flag) {
+		return
+	}
+	*toUpdate = updateValue
+}