diff --git a/.github/header-checker-lint.yml b/.github/header-checker-lint.yml index 39c1acdd6d..2b85216072 100644 --- a/.github/header-checker-lint.yml +++ b/.github/header-checker-lint.yml @@ -24,7 +24,6 @@ sourceFileExtensions: - 'go' - 'Makefile' - 'json' - - 'yaml' - 'yml' - 'txt' - 'py' @@ -40,3 +39,4 @@ ignoreFiles: - 'perfmetrics/scripts/ls_metrics/directory_pb2.py' - '*.json' - '.github/**' + - '*.yaml' diff --git a/cfg/config.go b/cfg/config.go index 0a06d9d0ec..8b1956270c 100644 --- a/cfg/config.go +++ b/cfg/config.go @@ -345,7 +345,7 @@ func BindFlags(v *viper.Viper, flagSet *pflag.FlagSet) error { return err } - flagSet.BoolP("enable-hns", "", false, "Enables support for HNS buckets") + flagSet.BoolP("enable-hns", "", true, "Enables support for HNS buckets") if err := v.BindPFlag("enable-hns", flagSet.Lookup("enable-hns")); err != nil { return err diff --git a/cfg/rationalize.go b/cfg/rationalize.go index 3cd096975c..128e89b42c 100644 --- a/cfg/rationalize.go +++ b/cfg/rationalize.go @@ -27,11 +27,6 @@ func decodeURL(u string) (string, error) { // Rationalize updates the config fields based on the values of other fields. func Rationalize(c *Config) error { - // The EnableEmptyManagedFolders flag must be set to true to enforce folder prefixes for Hierarchical buckets. - if c.EnableHns { - c.List.EnableEmptyManagedFolders = true - } - var err error if c.GcsConnection.CustomEndpoint, err = decodeURL(c.GcsConnection.CustomEndpoint); err != nil { return err diff --git a/cfg/rationalize_test.go b/cfg/rationalize_test.go index 8fb34dbc71..0ffbb589ac 100644 --- a/cfg/rationalize_test.go +++ b/cfg/rationalize_test.go @@ -20,54 +20,6 @@ import ( "github.com/stretchr/testify/assert" ) -func TestRationalizeEnableEmptyManagedFolders(t *testing.T) { - testcases := []struct { - name string - enableHns bool - enableEmptyManagedFolders bool - expectedEnableEmptyManagedFolders bool - }{ - { - name: "both enable-hns and enable-empty-managed-folders set to true", - enableHns: true, - enableEmptyManagedFolders: true, - expectedEnableEmptyManagedFolders: true, - }, - { - name: "enable-hns set to true and enable-empty-managed-folders set to false", - enableHns: true, - enableEmptyManagedFolders: false, - expectedEnableEmptyManagedFolders: true, - }, - { - name: "enable-hns set to false and enable-empty-managed-folders set to true", - enableHns: false, - enableEmptyManagedFolders: true, - expectedEnableEmptyManagedFolders: true, - }, - { - name: "both enable-hns and enable-empty-managed-folders set to false", - enableHns: false, - enableEmptyManagedFolders: false, - expectedEnableEmptyManagedFolders: false, - }, - } - for _, tc := range testcases { - t.Run(tc.name, func(t *testing.T) { - c := Config{ - EnableHns: tc.enableHns, - List: ListConfig{EnableEmptyManagedFolders: tc.enableEmptyManagedFolders}, - } - - err := Rationalize(&c) - - if assert.NoError(t, err) { - assert.Equal(t, tc.expectedEnableEmptyManagedFolders, c.List.EnableEmptyManagedFolders) - } - }) - } -} - func TestRationalizeCustomEndpointSuccessful(t *testing.T) { testCases := []struct { name string diff --git a/cmd/config_rationalization_test.go b/cmd/config_rationalization_test.go deleted file mode 100644 index 7984ad5c35..0000000000 --- a/cmd/config_rationalization_test.go +++ /dev/null @@ -1,59 +0,0 @@ -// Copyright 2024 Google LLC -// -// 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 ( - "testing" - - "github.com/stretchr/testify/assert" -) - -func TestEnableEmptyManagedFoldersResolution(t *testing.T) { - testcases := []struct { - name string - args []string - expected bool - }{ - { - name: "enable-hns set to true", - args: []string{"--enable-hns"}, - expected: true, - }, - { - name: "enable-hns set to true and enable-empty-managed-folders set to false", - args: []string{"--enable-hns", "--enable-empty-managed-folders=false"}, - expected: true, - }, - { - name: "enable-hns set to false and enable-empty-managed-folders set to true", - args: []string{"--enable-hns=false", "--enable-empty-managed-folders=true"}, - expected: true, - }, - { - name: "both enable-hns and enable-empty-managed-folders set to false", - args: []string{"--enable-hns=false", "--enable-empty-managed-folders=false"}, - expected: false, - }, - } - for _, tc := range testcases { - t.Run(tc.name, func(t *testing.T) { - c, err := getConfigObject(t, tc.args) - - if assert.NoError(t, err) { - assert.Equal(t, tc.expected, c.List.EnableEmptyManagedFolders) - } - }) - } -} diff --git a/cmd/config_validation_test.go b/cmd/config_validation_test.go index 3f51a7b5db..fa527b1ac4 100644 --- a/cmd/config_validation_test.go +++ b/cmd/config_validation_test.go @@ -550,14 +550,14 @@ func TestValidateConfigFile_EnableHNSConfigSuccessful(t *testing.T) { name: "empty_config_file", configFile: "testdata/empty_file.yaml", expectedConfig: &cfg.Config{ - EnableHns: false, + EnableHns: true, }, }, { name: "valid_config_file", configFile: "testdata/valid_config.yaml", expectedConfig: &cfg.Config{ - EnableHns: true, + EnableHns: false, }, }, } diff --git a/cmd/legacy_param_mapper_test.go b/cmd/legacy_param_mapper_test.go index 363a85cad8..02f8b3ba9f 100644 --- a/cmd/legacy_param_mapper_test.go +++ b/cmd/legacy_param_mapper_test.go @@ -644,58 +644,6 @@ func TestLogSeverityRationalization(t *testing.T) { } } -func TestEnableEmptyManagedFoldersRationalization(t *testing.T) { - testcases := []struct { - name string - enableHns bool - enableEmptyManagedFolders bool - expectedEnableEmptyManagedFolders bool - }{ - { - name: "both enable-hns and enable-empty-managed-folders set to true", - enableHns: true, - enableEmptyManagedFolders: true, - expectedEnableEmptyManagedFolders: true, - }, - { - name: "enable-hns set to true and enable-empty-managed-folders set to false", - enableHns: true, - enableEmptyManagedFolders: false, - expectedEnableEmptyManagedFolders: true, - }, - { - name: "enable-hns set to false and enable-empty-managed-folders set to true", - enableHns: false, - enableEmptyManagedFolders: true, - expectedEnableEmptyManagedFolders: true, - }, - { - name: "both enable-hns and enable-empty-managed-folders set to false", - enableHns: false, - enableEmptyManagedFolders: false, - expectedEnableEmptyManagedFolders: false, - }, - } - for _, tc := range testcases { - t.Run(tc.name, func(t *testing.T) { - flags := &flagStorage{ - ClientProtocol: mountpkg.ClientProtocol("http1"), - SequentialReadSizeMb: 200, - ExperimentalMetadataPrefetchOnMount: "disabled", - } - c := config.NewMountConfig() - c.EnableHNS = tc.enableHns - c.ListConfig.EnableEmptyManagedFolders = tc.enableEmptyManagedFolders - - resolvedConfig, err := PopulateNewConfigFromLegacyFlagsAndConfig(&mockCLIContext{}, flags, c) - - if assert.NoError(t, err) { - assert.Equal(t, tc.expectedEnableEmptyManagedFolders, resolvedConfig.List.EnableEmptyManagedFolders) - } - }) - } -} - func TestPopulateConfigFromLegacyFlags_MountOption(t *testing.T) { flags := &flagStorage{ MountOptions: []string{"rw,nodev", "user=jacobsa,noauto"}, diff --git a/cmd/root_test.go b/cmd/root_test.go index 9b61a490c9..a06a913ac6 100644 --- a/cmd/root_test.go +++ b/cmd/root_test.go @@ -735,13 +735,13 @@ func TestArgsParsing_EnableHNSFlags(t *testing.T) { }{ { name: "normal", - args: []string{"gcsfuse", "--enable-hns", "abc", "pqr"}, - expectedEnableHNS: true, + args: []string{"gcsfuse", "--enable-hns=false", "abc", "pqr"}, + expectedEnableHNS: false, }, { name: "default", args: []string{"gcsfuse", "abc", "pqr"}, - expectedEnableHNS: false, + expectedEnableHNS: true, }, } diff --git a/cmd/testdata/valid_config.yaml b/cmd/testdata/valid_config.yaml index d97beb3ebf..43e8f88fdd 100644 --- a/cmd/testdata/valid_config.yaml +++ b/cmd/testdata/valid_config.yaml @@ -40,4 +40,4 @@ file-system: temp-dir: ~/temp list: enable-empty-managed-folders: true -enable-hns: true +enable-hns: false diff --git a/internal/config/mount_config.go b/internal/config/mount_config.go index a2dbd49820..08db78106c 100644 --- a/internal/config/mount_config.go +++ b/internal/config/mount_config.go @@ -43,7 +43,7 @@ const ( DefaultEnableEmptyManagedFoldersListing = false DefaultGrpcConnPoolSize = 1 DefaultAnonymousAccess = false - DefaultEnableHNS = false + DefaultEnableHNS = true DefaultIgnoreInterrupts = true DefaultPrometheusPort = 0 diff --git a/internal/config/yaml_parser_test.go b/internal/config/yaml_parser_test.go index 0a62105d98..501d9f26a0 100644 --- a/internal/config/yaml_parser_test.go +++ b/internal/config/yaml_parser_test.go @@ -51,7 +51,7 @@ func validateDefaultConfig(t *testing.T, mountConfig *MountConfig) { assert.Equal(t, int64(4*1024*1024), mountConfig.FileCacheConfig.WriteBufferSize) assert.Equal(t, 1, mountConfig.GCSConnection.GRPCConnPoolSize) assert.False(t, mountConfig.GCSAuth.AnonymousAccess) - assert.False(t, bool(mountConfig.EnableHNS)) + assert.True(t, bool(mountConfig.EnableHNS)) assert.True(t, mountConfig.FileSystemConfig.IgnoreInterrupts) assert.False(t, mountConfig.FileSystemConfig.DisableParallelDirops) assert.Equal(t, DefaultKernelListCacheTtlSeconds, mountConfig.KernelListCacheTtlSeconds) diff --git a/internal/fs/inode/dir.go b/internal/fs/inode/dir.go index 9423672678..9a26b4404c 100644 --- a/internal/fs/inode/dir.go +++ b/internal/fs/inode/dir.go @@ -182,9 +182,9 @@ type dirInode struct { // Constant data ///////////////////////// - id fuseops.InodeID - implicitDirs bool - enableManagedFoldersListing bool + id fuseops.InodeID + implicitDirs bool + includeFoldersAsPrefixes bool enableNonexistentTypeCache bool @@ -247,7 +247,7 @@ func NewDirInode( name Name, attrs fuseops.InodeAttributes, implicitDirs bool, - enableManagedFoldersListing bool, + includeFoldersAsPrefixes bool, enableNonexistentTypeCache bool, typeCacheTTL time.Duration, bucket *gcsx.SyncerBucket, @@ -262,18 +262,18 @@ func NewDirInode( } typed := &dirInode{ - bucket: bucket, - mtimeClock: mtimeClock, - cacheClock: cacheClock, - id: id, - implicitDirs: implicitDirs, - enableManagedFoldersListing: enableManagedFoldersListing, - enableNonexistentTypeCache: enableNonexistentTypeCache, - name: name, - attrs: attrs, - cache: metadata.NewTypeCache(typeCacheMaxSizeMB, typeCacheTTL), - isHNSEnabled: isHNSEnabled, - unlinked: false, + bucket: bucket, + mtimeClock: mtimeClock, + cacheClock: cacheClock, + id: id, + implicitDirs: implicitDirs, + includeFoldersAsPrefixes: includeFoldersAsPrefixes, + enableNonexistentTypeCache: enableNonexistentTypeCache, + name: name, + attrs: attrs, + cache: metadata.NewTypeCache(typeCacheMaxSizeMB, typeCacheTTL), + isHNSEnabled: isHNSEnabled, + unlinked: false, } typed.lc.Init(id) @@ -654,6 +654,9 @@ func (d *dirInode) ReadDescendants(ctx context.Context, limit int) (map[Name]*Co func (d *dirInode) readObjects( ctx context.Context, tok string) (cores map[Name]*Core, newTok string, err error) { + if d.isBucketHierarchical() { + d.includeFoldersAsPrefixes = true + } // Ask the bucket to list some objects. req := &gcs.ListObjectsRequest{ Delimiter: "/", @@ -664,7 +667,7 @@ func (d *dirInode) readObjects( // Setting Projection param to noAcl since fetching owner and acls are not // required. ProjectionVal: gcs.NoAcl, - IncludeFoldersAsPrefixes: d.enableManagedFoldersListing, + IncludeFoldersAsPrefixes: d.includeFoldersAsPrefixes, } listing, err := d.bucket.ListObjects(ctx, req) diff --git a/internal/fs/inode/dir_test.go b/internal/fs/inode/dir_test.go index 1da4b06b0b..3f403b4a6e 100644 --- a/internal/fs/inode/dir_test.go +++ b/internal/fs/inode/dir_test.go @@ -92,7 +92,7 @@ func (p DirentSlice) Less(i, j int) bool { return p[i].Name < p[j].Name } func (p DirentSlice) Swap(i, j int) { p[i], p[j] = p[j], p[i] } // NOTE: A limitation in the fake bucket's API prevents the direct creation of managed folders. -// This poses a challenge for writing unit tests for enableManagedFoldersListing. +// This poses a challenge for writing unit tests for includeFoldersAsPrefixes. func (t *DirTest) resetInode(implicitDirs, enableNonexistentTypeCache, enableManagedFoldersListing bool) { t.resetInodeWithTypeCacheConfigs(implicitDirs, enableNonexistentTypeCache, enableManagedFoldersListing, config.DefaultTypeCacheMaxSizeMB, typeCacheTTL) diff --git a/internal/fs/inode/explicit_dir.go b/internal/fs/inode/explicit_dir.go index 265b8064f2..36e3c609d6 100644 --- a/internal/fs/inode/explicit_dir.go +++ b/internal/fs/inode/explicit_dir.go @@ -38,7 +38,7 @@ func NewExplicitDirInode( m *gcs.MinObject, attrs fuseops.InodeAttributes, implicitDirs bool, - enableManagedFoldersListing bool, + includeFoldersAsPrefixes bool, enableNonexistentTypeCache bool, typeCacheTTL time.Duration, bucket *gcsx.SyncerBucket, @@ -51,7 +51,7 @@ func NewExplicitDirInode( name, attrs, implicitDirs, - enableManagedFoldersListing, + includeFoldersAsPrefixes, enableNonexistentTypeCache, typeCacheTTL, bucket, diff --git a/tools/config-gen/params.yaml b/tools/config-gen/params.yaml index d4c61978d0..4c12b9f07b 100644 --- a/tools/config-gen/params.yaml +++ b/tools/config-gen/params.yaml @@ -61,7 +61,7 @@ flag-name: "enable-hns" type: "bool" usage: "Enables support for HNS buckets" - default: false + default: true - config-path: "file-cache.cache-file-for-range-read" flag-name: "cache-file-for-range-read"