Overriding authConfig flag (#1878)

* adding disable auth flag

* rebasing on top of master and fix lint tests

* fix lint tests

* review comments

* Adding unit test

* removing transoport

* adding user agent option

* changing flag name

* removing unnecessary changes

* Adding comment

* removed user-agent

* overriding flag

* overriding config flag

* rebasing

* nit

* lint fix

* lint fix

* small fix

flags.go

@@ -158,6 +158,11 @@ func newApp() (app *cli.App) {
 					"GCSFuse uses the global GCS JSON API endpoint, https://storage.googleapis.com/storage/v1.",
 			},
 
+			cli.BoolFlag{
+				Name:  config.AnonymousAccess,
+				Usage: "Authentication is enabled by default. This flag will disable authentication",
+			},
+
 			cli.StringFlag{
 				Name:  "billing-project",
 				Value: "",
@@ -385,6 +390,7 @@ type flagStorage struct {
 	EgressBandwidthLimitBytesPerSecond float64
 	OpRateLimitHz                      float64
 	SequentialReadSizeMb               int32
+	AnonymousAccess                    bool
 
 	// Tuning
 	MaxRetrySleep              time.Duration
@@ -516,6 +522,7 @@ func populateFlags(c *cli.Context) (flags *flagStorage, err error) {
 
 		// GCS,
 		CustomEndpoint:                     customEndpoint,
+		AnonymousAccess:                    c.Bool("anonymous-access"),
 		BillingProject:                     c.String("billing-project"),
 		KeyFile:                            c.String("key-file"),
 		TokenUrl:                           c.String("token-url"),

flags_test.go

@@ -83,6 +83,7 @@ func (t *FlagsTest) Defaults() {
 	ExpectEq(-1, f.OpRateLimitHz)
 	ExpectTrue(f.ReuseTokenFromUrl)
 	ExpectEq(nil, f.CustomEndpoint)
+	ExpectFalse(f.AnonymousAccess)
 
 	// Tuning
 	ExpectEq(mount.DefaultStatCacheCapacity, f.StatCacheCapacity)
@@ -115,6 +116,7 @@ func (t *FlagsTest) Bools() {
 		"enable-nonexistent-type-cache",
 		"experimental-enable-json-read",
 		"ignore-interrupts",
+		"anonymous-access",
 	}
 
 	var args []string
@@ -137,6 +139,7 @@ func (t *FlagsTest) Bools() {
 	ExpectTrue(f.EnableNonexistentTypeCache)
 	ExpectTrue(f.ExperimentalEnableJsonRead)
 	ExpectTrue(f.IgnoreInterrupts)
+	ExpectTrue(f.AnonymousAccess)
 
 	// --foo=false form
 	args = nil

internal/config/config_util.go

@@ -15,6 +15,7 @@
 package config
 
 const IgnoreInterruptsFlagName = "ignore-interrupts"
+const AnonymousAccess = "anonymous-access"
 
 // OverrideWithLoggingFlags overwrites the configs with the flag values if the
 // config values are empty.
@@ -50,6 +51,15 @@ func OverrideWithIgnoreInterruptsFlag(c cliContext, mountConfig *MountConfig, ig
 	}
 }
 
+// OverrideWithAnonymousAccessFlag overwrites the anonymous-access config with
+// the anonymous-access flag value if the flag is set.
+func OverrideWithAnonymousAccessFlag(c cliContext, mountConfig *MountConfig, anonymousAccess bool) {
+	// If the  anonymous-access flag is set, give it priority over the value in config file.
+	if c.IsSet(AnonymousAccess) {
+		mountConfig.AuthConfig.AnonymousAccess = anonymousAccess
+	}
+}
+
 func IsFileCacheEnabled(mountConfig *MountConfig) bool {
 	return mountConfig.FileCacheConfig.MaxSizeMB != 0 && string(mountConfig.CacheDir) != ""
 }

internal/config/config_util_test.go

@@ -33,6 +33,7 @@ type flags struct {
 	DebugGCS         bool
 	DebugMutex       bool
 	IgnoreInterrupts bool
+	AnonymousAccess  bool
 }
 type ConfigTest struct {
 }
@@ -154,3 +155,31 @@ func TestOverrideWithIgnoreInterruptsFlag(t *testing.T) {
 		})
 	}
 }
+
+func TestOverrideWithAnonymousAccessFlag(t *testing.T) {
+	var overrideWithAnonymousAccessFlagTests = []struct {
+		testName                   string
+		anonymousAccessConfigValue bool
+		isFlagSet                  bool
+		anonymousAccessFlagValue   bool
+		expectedAnonymousAccess    bool
+	}{
+		{"anonymous-access config true and flag not set", true, false, false, true},
+		{"anonymous-access config false and flag not set", false, false, false, false},
+		{"anonymous-access config false and anonymous-access flag false", false, true, false, false},
+		{"anonymous-access config false and anonymous-access flag true", false, true, true, true},
+		{"anonymous-access config true and anonymous-access flag false", true, true, false, false},
+		{"anonymous-access config true and anonymous-access flag true", true, true, true, true},
+	}
+
+	for _, tt := range overrideWithAnonymousAccessFlagTests {
+		t.Run(tt.testName, func(t *testing.T) {
+			testContext := &TestCliContext{isSet: tt.isFlagSet}
+			mountConfig := &MountConfig{AuthConfig: AuthConfig{AnonymousAccess: tt.anonymousAccessConfigValue}}
+
+			OverrideWithAnonymousAccessFlag(testContext, mountConfig, tt.anonymousAccessFlagValue)
+
+			AssertEq(tt.expectedAnonymousAccess, mountConfig.AuthConfig.AnonymousAccess)
+		})
+	}
+}

internal/config/testdata/auth_config/invalid_anonymous_access.yaml

@@ -0,0 +1,2 @@
+auth-config:
+  anonymous-access: abc

internal/config/testdata/auth_config/unset_anonymous_access.yaml

@@ -0,0 +1,2 @@
+write:
+  create-empty-file: true

internal/config/yaml_parser_test.go

@@ -251,3 +251,18 @@ func (t *YamlParserTest) TestReadConfigFile_FileSystemConfig_UnsetIgnoreInterrup
 	AssertNe(nil, mountConfig)
 	AssertEq(false, mountConfig.FileSystemConfig.IgnoreInterrupts)
 }
+
+func (t *YamlParserTest) TestReadConfigFile_FileSystemConfig_InvalidAnonymousAccessValue() {
+	_, err := ParseConfigFile("testdata/auth_config/invalid_anonymous_access.yaml")
+
+	AssertNe(nil, err)
+	AssertTrue(strings.Contains(err.Error(), "error parsing config file: yaml: unmarshal errors:\n  line 2: cannot unmarshal !!str `abc` into bool"))
+}
+
+func (t *YamlParserTest) TestReadConfigFile_FileSystemConfig_UnsetAnonymousAccessValue() {
+	mountConfig, err := ParseConfigFile("testdata/auth_config/unset_anonymous_access.yaml")
+
+	AssertEq(nil, err)
+	AssertNe(nil, mountConfig)
+	AssertEq(false, mountConfig.AuthConfig.AnonymousAccess)
+}

internal/storage/storageutil/client.go

@@ -78,9 +78,12 @@ func CreateHttpClient(storageClientConfig *StorageClientConfig) (httpClient *htt
 	}
 
 	if storageClientConfig.AnonymousAccess {
-		// UserAgent will not be added if authentication is disabled. Bypassing authentication prevents the creation of an HTTP transport because it requires a token source.
+		// UserAgent will not be added if authentication is disabled.
+		// Bypassing authentication prevents the creation of an HTTP transport
+		// because it requires a token source.
 		// Setting a dummy token would conflict with the "WithoutAuthentication" option.
-		// While the "WithUserAgent" option could set a custom User-Agent, it's incompatible with the "WithHTTPClient" option, preventing the direct injection of a user agent
+		// While the "WithUserAgent" option could set a custom User-Agent, it's incompatible
+		// with the "WithHTTPClient" option, preventing the direct injection of a user agent
 		// when authentication is skipped.
 		httpClient = &http.Client{
 			Timeout: storageClientConfig.HttpClientTimeout,

main.go

@@ -225,6 +225,7 @@ func runCLIApp(c *cli.Context) (err error) {
 	config.OverrideWithLoggingFlags(mountConfig, flags.LogFile, flags.LogFormat,
 		flags.DebugFuse, flags.DebugGCS, flags.DebugMutex)
 	config.OverrideWithIgnoreInterruptsFlag(c, mountConfig, flags.IgnoreInterrupts)
+	config.OverrideWithAnonymousAccessFlag(c, mountConfig, flags.AnonymousAccess)
 
 	// Ideally this call to SetLogFormat (which internally creates a new defaultLogger)
 	// should be set as an else to the 'if flags.Foreground' check below, but currently

main_test.go

@@ -207,6 +207,7 @@ func (t *MainTest) TestStringifyShouldReturnAllFlagsPassedInFlagStorageAsMarshal
 	actual, err := util.Stringify(flags)
 	AssertEq(nil, err)
 
-	expected := "{\"AppName\":\"\",\"Foreground\":false,\"ConfigFile\":\"\",\"MountOptions\":{\"1\":\"one\",\"2\":\"two\",\"3\":\"three\"},\"DirMode\":0,\"FileMode\":0,\"Uid\":0,\"Gid\":0,\"ImplicitDirs\":false,\"OnlyDir\":\"\",\"RenameDirLimit\":0,\"IgnoreInterrupts\":false,\"CustomEndpoint\":null,\"BillingProject\":\"\",\"KeyFile\":\"\",\"TokenUrl\":\"\",\"ReuseTokenFromUrl\":false,\"EgressBandwidthLimitBytesPerSecond\":0,\"OpRateLimitHz\":0,\"SequentialReadSizeMb\":10,\"MaxRetrySleep\":0,\"StatCacheCapacity\":0,\"StatCacheTTL\":0,\"TypeCacheTTL\":0,\"HttpClientTimeout\":0,\"MaxRetryDuration\":0,\"RetryMultiplier\":0,\"LocalFileCache\":false,\"TempDir\":\"\",\"ClientProtocol\":\"http4\",\"MaxConnsPerHost\":0,\"MaxIdleConnsPerHost\":0,\"EnableNonexistentTypeCache\":false,\"StackdriverExportInterval\":0,\"OtelCollectorAddress\":\"\",\"LogFile\":\"\",\"LogFormat\":\"\",\"ExperimentalEnableJsonRead\":false,\"DebugFuseErrors\":false,\"DebugFuse\":false,\"DebugFS\":false,\"DebugGCS\":false,\"DebugHTTP\":false,\"DebugInvariants\":false,\"DebugMutex\":false}"
+	expected := "{\"AppName\":\"\",\"Foreground\":false,\"ConfigFile\":\"\",\"MountOptions\":{\"1\":\"one\",\"2\":\"two\",\"3\":\"three\"},\"DirMode\":0,\"FileMode\":0,\"Uid\":0,\"Gid\":0,\"ImplicitDirs\":false,\"OnlyDir\":\"\",\"RenameDirLimit\":0,\"IgnoreInterrupts\":false,\"CustomEndpoint\":null,\"BillingProject\":\"\",\"KeyFile\":\"\",\"TokenUrl\":\"\",\"ReuseTokenFromUrl\":false,\"EgressBandwidthLimitBytesPerSecond\":0,\"OpRateLimitHz\":0,\"SequentialReadSizeMb\":10,\"AnonymousAccess\":false,\"MaxRetrySleep\":0,\"StatCacheCapacity\":0,\"StatCacheTTL\":0,\"TypeCacheTTL\":0,\"HttpClientTimeout\":0,\"MaxRetryDuration\":0,\"RetryMultiplier\":0,\"LocalFileCache\":false,\"TempDir\":\"\",\"ClientProtocol\":\"http4\",\"MaxConnsPerHost\":0,\"MaxIdleConnsPerHost\":0,\"EnableNonexistentTypeCache\":false,\"StackdriverExportInterval\":0,\"OtelCollectorAddress\":\"\",\"LogFile\":\"\",\"LogFormat\":\"\",\"ExperimentalEnableJsonRead\":false,\"DebugFuseErrors\":false,\"DebugFuse\":false,\"DebugFS\":false,\"DebugGCS\":false,\"DebugHTTP\":false,\"DebugInvariants\":false,\"DebugMutex\":false}"
+	AssertEq(expected, actual)
 	AssertEq(expected, actual)
 }