Disable xattrs by default

Changed --optimizations xattr to be default behavior, thus removing this
flag and adding a new flag to disable this annotation on SOCI index
creation. Additionally, added a TOML variable (force_get_xattrs) to
optionally ignore this annotation at runtime.

Signed-off-by: David Son <davbson@amazon.com>

cmd/soci/commands/create.go

@@ -29,10 +29,11 @@ import (
 )
 
 const (
-	buildToolIdentifier = "AWS SOCI CLI v0.1"
-	spanSizeFlag        = "span-size"
-	minLayerSizeFlag    = "min-layer-size"
-	optimizationFlag    = "optimizations"
+	buildToolIdentifier    = "AWS SOCI CLI v0.1"
+	spanSizeFlag           = "span-size"
+	minLayerSizeFlag       = "min-layer-size"
+	forceXAttrsEnabledFlag = "force-xattrs-enabled"
+	optimizationFlag       = "optimizations"
 )
 
 // CreateCommand creates SOCI index for an image
@@ -55,6 +56,10 @@ var CreateCommand = cli.Command{
 			Usage: "Minimum layer size to build zTOC for. Smaller layers won't have zTOC and not lazy pulled. Default is 10 MiB.",
 			Value: 10 << 20,
 		},
+		cli.BoolFlag{
+			Name:  forceXAttrsEnabledFlag,
+			Usage: "Always get xattrs for every layer.",
+		},
 		cli.StringSliceFlag{
 			Name:  optimizationFlag,
 			Usage: fmt.Sprintf("(Experimental) Enable optional optimizations. Valid values are %v", soci.Optimizations),
@@ -122,6 +127,10 @@ var CreateCommand = cli.Command{
 			soci.WithOptimizations(optimizations),
 		}
 
+		if cliContext.Bool(forceXAttrsEnabledFlag) {
+			builderOpts = append(builderOpts, soci.WithForceGetXAttrs())
+		}
+
 		for _, plat := range ps {
 			builder, err := soci.NewIndexBuilder(cs, blobStore, artifactsDb, append(builderOpts, soci.WithPlatform(plat))...)
 

config/config.toml

@@ -19,10 +19,11 @@ allow_no_verification=true
 # disable_verification=false
 # Causes TestRunWithDefaultConfig to break, but
 # fine to use in /etc/soci-snapshotter-grpc-config.toml
-max_concurrency=0  # Actually zero
+max_concurrency=0
 no_prometheus=false
 mount_timeout_sec=0
 fuse_metrics_emit_wait_duration_sec=0
+force_get_xattrs=false
 
 ## config/config.go Config
 

config/fs.go

@@ -56,6 +56,7 @@ type FSConfig struct {
 	NoPrometheus                   bool   `toml:"no_prometheus"`
 	MountTimeoutSec                int64  `toml:"mount_timeout_sec"`
 	FuseMetricsEmitWaitDurationSec int64  `toml:"fuse_metrics_emit_wait_duration_sec"`
+	ForceGetXAttrs                 bool   `toml:"force_get_xattrs"`
 
 	RetryableHTTPClientConfig `toml:"http"`
 	BlobConfig                `toml:"blob"`

fs/layer/layer.go

@@ -345,9 +345,12 @@ func (r *Resolver) Resolve(ctx context.Context, hosts []docker.RegistryHost, ref
 	if err != nil {
 		return nil, fmt.Errorf("failed to read layer: %w", err)
 	}
-	disableXattrs := getDisableXAttrAnnotation(sociDesc)
+	disableXAttrs := getDisableXAttrAnnotation(sociDesc)
+	if r.config.ForceGetXAttrs {
+		disableXAttrs = false
+	}
 	// Combine layer information together and cache it.
-	l := newLayer(r, desc, blobR, vr, bgLayerResolver, opCounter, disableXattrs)
+	l := newLayer(r, desc, blobR, vr, bgLayerResolver, opCounter, disableXAttrs)
 	r.layerCacheMu.Lock()
 	cachedL, done2, added := r.layerCache.Add(name, l)
 	r.layerCacheMu.Unlock()

soci/soci_index.go

@@ -216,8 +216,15 @@ type buildConfig struct {
 	artifactsDb         *ArtifactsDb
 	platform            ocispec.Platform
 	optimizations       []Optimization
+	forceGetXAttrs      bool
 }
 
+// Optimization represents an optional optimization to be applied when building the SOCI index
+type Optimization string
+
+// Uncomment the below block when adding optimizations.
+// This is commented out for now to pass CI lint checker.
+/*
 func (b *buildConfig) hasOptimization(o Optimization) bool {
 	for _, optimization := range b.optimizations {
 		if o == optimization {
@@ -227,17 +234,16 @@ func (b *buildConfig) hasOptimization(o Optimization) bool {
 	return false
 }
 
-// Optimization represents an optional optimization to be applied when building the SOCI index
-type Optimization string
-
 const (
-	// XAttrOptimization optimizes xattrs by disabling them for layers where there are no xattrs or opaque directories
-	XAttrOptimization Optimization = "xattr"
+	OptimizationName = "optname"
 	// Be sure to add any new optimizations to `Optimizations` below
 )
 
 // Optimizations contains the list of all known optimizations
-var Optimizations = []Optimization{XAttrOptimization}
+var Optimizations = []Optimization{OptimizationName}
+*/
+
+var Optimizations = []Optimization{}
 
 // ParseOptimization parses a string into a known optimization.
 // If the string does not match a known optimization, an error is returned.
@@ -277,6 +283,14 @@ func WithOptimizations(optimizations []Optimization) BuildOption {
 	}
 }
 
+// WithForceXAttrsEnabledFlag forces GetXAttrs calls on every layer
+func WithForceGetXAttrs() BuildOption {
+	return func(c *buildConfig) error {
+		c.forceGetXAttrs = true
+		return nil
+	}
+}
+
 // WithBuildToolIdentifier specifies the build tool annotation value.
 func WithBuildToolIdentifier(tool string) BuildOption {
 	return func(c *buildConfig) error {
@@ -511,6 +525,9 @@ func (b *IndexBuilder) buildSociLayer(ctx context.Context, desc ocispec.Descript
 		IndexAnnotationImageLayerDigest:    desc.Digest.String(),
 	}
 	b.maybeAddDisableXattrAnnotation(&ztocDesc, toc)
+	if desc.Annotations[IndexAnnotationDisableXAttrs] == "true" {
+		log.G(ctx).WithField("layer", ztocDesc.Digest.String()).Debug("xattrs disabled")
+	}
 	return &ztocDesc, err
 }
 
@@ -648,7 +665,7 @@ func WriteSociIndex(ctx context.Context, indexWithMetadata *IndexWithMetadata, c
 }
 
 func (b *IndexBuilder) maybeAddDisableXattrAnnotation(ztocDesc *ocispec.Descriptor, ztoc *ztoc.Ztoc) {
-	if b.config.hasOptimization(XAttrOptimization) && shouldDisableXattrs(ztoc) {
+	if !b.config.forceGetXAttrs && shouldDisableXattrs(ztoc) {
 		if ztocDesc.Annotations == nil {
 			ztocDesc.Annotations = make(map[string]string, 1)
 		}

soci/soci_index_test.go

@@ -235,6 +235,7 @@ func TestDisableXattrs(t *testing.T) {
 		name                string
 		metadata            []ztoc.FileMetadata
 		shouldDisableXattrs bool
+		forceGetXAttrs      bool
 	}{
 		{
 			name: "ztoc with xattrs should not have xattrs disabled",
@@ -247,6 +248,7 @@ func TestDisableXattrs(t *testing.T) {
 				},
 			},
 			shouldDisableXattrs: false,
+			forceGetXAttrs:      false,
 		},
 		{
 			name: "ztoc with opaque dirs should not have xattrs disabled",
@@ -259,6 +261,7 @@ func TestDisableXattrs(t *testing.T) {
 				},
 			},
 			shouldDisableXattrs: false,
+			forceGetXAttrs:      false,
 		},
 		{
 			name: "ztoc with no xattrs or opaque dirs should have xattrs disabled",
@@ -271,6 +274,20 @@ func TestDisableXattrs(t *testing.T) {
 				},
 			},
 			shouldDisableXattrs: true,
+			forceGetXAttrs:      false,
+		},
+		{
+			name: "ztoc with no xattrs and forced GetXAttrs should have xattrs enabled",
+			metadata: []ztoc.FileMetadata{
+				{
+					Name: "dir/",
+				},
+				{
+					Name: "dir/file",
+				},
+			},
+			shouldDisableXattrs: false,
+			forceGetXAttrs:      true,
 		},
 	}
 	for _, tc := range testcases {
@@ -293,7 +310,11 @@ func TestDisableXattrs(t *testing.T) {
 			if err != nil {
 				t.Fatalf("can't create a test db")
 			}
-			builder, _ := NewIndexBuilder(cs, blobStore, artifactsDb, WithOptimizations([]Optimization{XAttrOptimization}))
+			opts := []BuildOption{}
+			if tc.forceGetXAttrs {
+				opts = append(opts, WithForceGetXAttrs())
+			}
+			builder, _ := NewIndexBuilder(cs, blobStore, artifactsDb, opts...)
 			builder.maybeAddDisableXattrAnnotation(&desc, &ztoc)
 			disableXattrs := desc.Annotations[IndexAnnotationDisableXAttrs] == "true"
 			if disableXattrs != tc.shouldDisableXattrs {