From 4f9c79c0230e93bc715f8d9facc0b96c0d51a399 Mon Sep 17 00:00:00 2001 From: Paul Maidment Date: Tue, 26 Nov 2024 13:57:28 +0200 Subject: [PATCH] OCPBUGS-44849: Skip manifest metadata operations if xattr support not present. Some customers are expereincing breaking issues when their OS does not support xattr, most notably NFS + Xattr This causes installations not to proceed and needs to be addressed urgently. As an intermim measure, it has been decided that we will skip manifest metadata operations and warn the user if we find that xattr is unsupported for the file path. --- cmd/main.go | 17 +- pkg/s3wrapper/composite_xattr_client.go | 122 ++++ pkg/s3wrapper/composite_xattr_client_test.go | 150 +++++ pkg/s3wrapper/filesystem.go | 47 +- pkg/s3wrapper/filesystem_test.go | 563 +++++++++--------- pkg/s3wrapper/filesystem_xattr_client.go | 117 ++++ pkg/s3wrapper/mock_xattr_client.go | 148 +++++ pkg/s3wrapper/xattr_client.go | 164 +++++ .../api/common/common_types.go | 1 - 9 files changed, 1036 insertions(+), 293 deletions(-) create mode 100644 pkg/s3wrapper/composite_xattr_client.go create mode 100644 pkg/s3wrapper/composite_xattr_client_test.go create mode 100644 pkg/s3wrapper/filesystem_xattr_client.go create mode 100644 pkg/s3wrapper/mock_xattr_client.go create mode 100644 pkg/s3wrapper/xattr_client.go diff --git a/cmd/main.go b/cmd/main.go index 6ce8cd92760..63ab38bac3c 100644 --- a/cmd/main.go +++ b/cmd/main.go @@ -9,6 +9,7 @@ import ( _ "net/http/pprof" "os" "os/signal" + "path/filepath" "strings" "syscall" "time" @@ -325,8 +326,16 @@ func main() { failOnError(err, "failed to create ignition builder") installConfigBuilder := installcfg.NewInstallConfigBuilder(log.WithField("pkg", "installcfg"), mirrorRegistriesBuilder, providerRegistry) + compositeXattrClient, err := s3wrapper.NewCompositeXattrClient( + log, + s3wrapper.NewOSxAttrClient(log, filepath.Join(Options.WorkDir, Options.S3Config.S3Bucket)), + s3wrapper.NewFilesystemBasedXattrClient(log, filepath.Join(Options.WorkDir, Options.S3Config.S3Bucket)), + Options.WorkDir, + ) + failOnError(err, "failed to initialize xattr handling") + var objectHandler = createStorageClient(Options.DeployTarget, Options.Storage, &Options.S3Config, - Options.WorkDir, log, metricsManager, Options.FileSystemUsageThreshold) + Options.WorkDir, log, metricsManager, Options.FileSystemUsageThreshold, compositeXattrClient) createS3Bucket(objectHandler, log) manifestsApi := manifests.NewManifestsAPI(db, log.WithField("pkg", "manifests"), objectHandler, usageManager) @@ -737,7 +746,7 @@ func createS3Bucket(objectHandler s3wrapper.API, log logrus.FieldLogger) { } func createStorageClient(deployTarget string, storage string, s3cfg *s3wrapper.Config, fsWorkDir string, - log logrus.FieldLogger, metricsAPI metrics.API, fsThreshold int) s3wrapper.API { + log logrus.FieldLogger, metricsAPI metrics.API, fsThreshold int, xattrClient s3wrapper.XattrClient) s3wrapper.API { var storageClient s3wrapper.API = nil if storage != "" { switch storage { @@ -746,7 +755,7 @@ func createStorageClient(deployTarget string, storage string, s3cfg *s3wrapper.C log.Fatal("failed to create S3 client") } case storage_filesystem: - storageClient = s3wrapper.NewFSClient(fsWorkDir, log, metricsAPI, fsThreshold) + storageClient = s3wrapper.NewFSClient(fsWorkDir, log, metricsAPI, fsThreshold, xattrClient) default: log.Fatalf("unsupported storage client: %s", storage) } @@ -758,7 +767,7 @@ func createStorageClient(deployTarget string, storage string, s3cfg *s3wrapper.C log.Fatal("failed to create S3 client") } case deployment_type_onprem, deployment_type_ocp: - storageClient = s3wrapper.NewFSClient(fsWorkDir, log, metricsAPI, fsThreshold) + storageClient = s3wrapper.NewFSClient(fsWorkDir, log, metricsAPI, fsThreshold, xattrClient) default: log.Fatalf("unsupported deploy target %s", deployTarget) } diff --git a/pkg/s3wrapper/composite_xattr_client.go b/pkg/s3wrapper/composite_xattr_client.go new file mode 100644 index 00000000000..23fb2daa41c --- /dev/null +++ b/pkg/s3wrapper/composite_xattr_client.go @@ -0,0 +1,122 @@ +package s3wrapper + +import ( + "slices" + + "github.com/sirupsen/logrus" +) + +func NewCompositeXattrClient( + log logrus.FieldLogger, + oSxattrClient XattrClient, + filesystemBasedXattrClient XattrClient, + rootDirectory string, + ) (*CompositeXattrClient, error) { + oSxattrClientSupported, err := oSxattrClient.IsSupported() + if err != nil { + return nil, err + } + filesystemBasedXattrClientSupported, err := filesystemBasedXattrClient.IsSupported() + if err != nil { + return nil, err + } + return &CompositeXattrClient{ + rootDirectory: rootDirectory, + oSxattrClient: oSxattrClient, + filesystemBasedXattrClient: filesystemBasedXattrClient, + oSxattrClientSupported: oSxattrClientSupported, + filesystemBasedXattrClientSupported: filesystemBasedXattrClientSupported, + }, nil +} + +type CompositeXattrClient struct { + rootDirectory string + oSxattrClient XattrClient + filesystemBasedXattrClient XattrClient + oSxattrClientSupported bool + filesystemBasedXattrClientSupported bool +} + +func (c *CompositeXattrClient) IsSupported() (bool, error) { + return true, nil +} + +func (c *CompositeXattrClient) SetRootDirectory(path string) { + c.rootDirectory = path +} +func (c *CompositeXattrClient) GetRootDirectory() string { + return c.rootDirectory +} + +func (c *CompositeXattrClient) Set(path, key string, value string) error { + // If the native xattr writes are supported then use those + // otherwise fall back to filesystem based writes. + if c.oSxattrClientSupported { + return c.oSxattrClient.Set(path, key, value) + } + return c.filesystemBasedXattrClient.Set(path, key, value) +} + +func (c *CompositeXattrClient) Get(path, key string) (string, bool, error) { + // Search for the record first in the oSxattrClient + // if not found then look in the filesystemBasedXattrClient. + var err error + var ok bool + var result string + if c.oSxattrClientSupported { + result, ok, err = c.oSxattrClient.Get(path, key) + if err != nil { + return "", ok, err + } + if ok { + return result, ok, nil + } + } + return c.filesystemBasedXattrClient.Get(path, key) +} + +func (c *CompositeXattrClient) List(path string) ([]string, error) { + // Produce a list that is the union of keys from both clients + // respect that the oSxattrClient takes priority when available. + var primaryList []string + var tertiarayList []string + var err error + if c.oSxattrClientSupported { + primaryList, err = c.oSxattrClient.List(path) + if err != nil { + return nil, err + } + } + tertiarayList, err = c.filesystemBasedXattrClient.List(path) + if err != nil { + return nil, err + } + for _, tertiarayListItem := range tertiarayList { + if !slices.Contains(primaryList, tertiarayListItem) { + primaryList = append(primaryList, tertiarayListItem) + } + } + return primaryList, nil +} + +func (c *CompositeXattrClient) Remove(path string, key string) error { + // When asked to remove an item, we will remove using both clients if possible. + if c.oSxattrClientSupported { + err := c.oSxattrClient.Remove(path, key) + if err != nil { + return err + } + } + return c.filesystemBasedXattrClient.Remove(path, key) +} + +func (c *CompositeXattrClient) RemoveAll(path string) error { + // When asked to remove all items for a pth, we will remove using both clients if possible. + if c.oSxattrClientSupported { + err := c.oSxattrClient.RemoveAll(path) + if err != nil { + return err + } + } + return c.filesystemBasedXattrClient.RemoveAll(path) +} diff --git a/pkg/s3wrapper/composite_xattr_client_test.go b/pkg/s3wrapper/composite_xattr_client_test.go new file mode 100644 index 00000000000..21343dc12c4 --- /dev/null +++ b/pkg/s3wrapper/composite_xattr_client_test.go @@ -0,0 +1,150 @@ +package s3wrapper + +import ( + "fmt" + "os" + "path/filepath" + + "github.com/golang/mock/gomock" + "github.com/onsi/ginkgo" + . "github.com/onsi/ginkgo" + . "github.com/onsi/gomega" + "github.com/sirupsen/logrus" +) + +var _ = Describe("Composite Xattr Client", func() { + + var ( + log *logrus.Logger + ctrl *gomock.Controller + baseDir string + osXattrClient *MockXattrClient + fileSystemBasedXattrClient XattrClient + file1path string + file2path string + ) + + BeforeEach(func() { + var err error + log = logrus.New() + log.SetOutput(ginkgo.GinkgoWriter) + baseDir, err = os.MkdirTemp("", "test") + Expect(err).ToNot(HaveOccurred()) + ctrl = gomock.NewController(GinkgoT()) + fileSystemBasedXattrClient = NewFilesystemBasedXattrClient(log, baseDir) + err = os.MkdirAll(filepath.Join(baseDir, "manifests", "openshift"), 0o700) + Expect(err).ToNot(HaveOccurred()) + err = os.MkdirAll(filepath.Join(baseDir, "manifests", "manifests"), 0o700) + Expect(err).ToNot(HaveOccurred()) + file1path = filepath.Join(baseDir, "manifests", "openshift", "file1.yaml") + file2path = filepath.Join(baseDir, "manifests", "manifests", "file2.yaml") + err = os.WriteFile(file1path, []byte{}, 0o600) + Expect(err).ToNot(HaveOccurred()) + err = os.WriteFile(file2path, []byte{}, 0o600) + Expect(err).ToNot(HaveOccurred()) + }) + + getExpectedMetadataPath := func(filePath string, attributeName string) string { + relativePath := filePath[len(baseDir):] + return fmt.Sprintf("%s%s%s%s", filepath.Join(baseDir, filesystemXattrMetaDataDirectoryName, relativePath), delimiter, "user.", attributeName) + } + + assertFileMetadataCorrect := func(filepath string, attributeName string, expectedValue string) { + fileMetadataItemPath := getExpectedMetadataPath(filepath, attributeName) + data, err := os.ReadFile(fileMetadataItemPath) + Expect(err).ToNot(HaveOccurred()) + Expect(string(data)).To(Equal(expectedValue)) + } + + assertFileSystemBasedAttributeWrite := func(path string, attribute string, value string, compositeXattrClient XattrClient) { + err := compositeXattrClient.Set(path, attribute, value) + Expect(err).ToNot(HaveOccurred()) + assertFileMetadataCorrect(path, attribute, value) + } + + It("Upgrading to xattr supported natively after unsupported", func() { + var ( + compositeXattrClient XattrClient + ) + + By("Native xattr is unsupported", func() { + osXattrClient = NewMockXattrClient(ctrl) + osXattrClient.EXPECT().IsSupported().Return(false, nil).AnyTimes() + }) + By("Create composite xattr client", func() { + var err error + compositeXattrClient, err = NewCompositeXattrClient(log, osXattrClient, fileSystemBasedXattrClient, baseDir) + Expect(err).ToNot(HaveOccurred()) + }) + By("Filesystem xattr client should be used for writes", func() { + assertFileSystemBasedAttributeWrite(file1path, "arbitrary-attribute", "some-arbitrary-value", compositeXattrClient) + assertFileSystemBasedAttributeWrite(file1path, "another-arbitrary-attribute", "another-arbitrary-value", compositeXattrClient) + assertFileSystemBasedAttributeWrite(file2path, "arbitrary-attribute-a", "some-arbitrary-value-a", compositeXattrClient) + assertFileSystemBasedAttributeWrite(file2path, "arbitrary-attribute-b", "some-arbitrary-value-b", compositeXattrClient) + }) + By("Native xattr is now supported - simulated upgrade", func() { + var err error + compositeXattrClient, err = NewCompositeXattrClient(log, NewOSxAttrClient(log, baseDir), fileSystemBasedXattrClient, baseDir) + Expect(err).ToNot(HaveOccurred()) + }) + By("Composite client should return previously stored keys from filesystem", func() { + value, ok, err := compositeXattrClient.Get(file1path, "arbitrary-attribute") + Expect(err).ToNot(HaveOccurred()) + Expect(ok).To(BeTrue()) + Expect(value).To(Equal("some-arbitrary-value")) + }) + By("Composite client should write to os native xattr, leaving filesystem based xattr intact", func() { + err := compositeXattrClient.Set(file1path, "arbitrary-attribute", "a-new-value") + Expect(err).ToNot(HaveOccurred()) + assertFileMetadataCorrect(file1path, "arbitrary-attribute", "some-arbitrary-value") + }) + By("should fetch newly written value from the composite client", func() { + value, ok, err := compositeXattrClient.Get(file1path, "arbitrary-attribute") + Expect(err).ToNot(HaveOccurred()) + Expect(ok).To(BeTrue()) + Expect(value).To(Equal("a-new-value")) + }) + By("list from composite xattr client should merge keys", func() { + items, err := compositeXattrClient.List(file1path) + Expect(err).ToNot(HaveOccurred()) + Expect(items).To(ContainElement("arbitrary-attribute")) + Expect(items).To(ContainElement("another-arbitrary-attribute")) + }) + By("when key that only exists in filesystem is removed - it should be removed", func() { + err := compositeXattrClient.Remove(file1path, "another-arbitrary-attribute") + Expect(err).ToNot(HaveOccurred()) + items, err := compositeXattrClient.List(file1path) + Expect(err).ToNot(HaveOccurred()) + Expect(items).To(ContainElement("arbitrary-attribute")) + Expect(items).ToNot(ContainElement("another-arbitrary-attribute")) + }) + By("when key is removed, should be removed from os xattr and filesystem xattr", func() { + err := compositeXattrClient.Remove(file1path, "arbitrary-attribute") + Expect(err).ToNot(HaveOccurred()) + items, err := compositeXattrClient.List(file1path) + Expect(err).ToNot(HaveOccurred()) + Expect(items).ToNot(ContainElement("arbitrary-attribute")) + }) + By("RemoveAll should remove all keys", func() { + err := compositeXattrClient.Set(file2path, "additional-os-xattr", "some-value") + Expect(err).ToNot(HaveOccurred()) + items, err := compositeXattrClient.List(file2path) + Expect(err).ToNot(HaveOccurred()) + Expect(items).To(ContainElement("arbitrary-attribute-a")) + Expect(items).To(ContainElement("arbitrary-attribute-a")) + Expect(items).To(ContainElement("additional-os-xattr")) + err = compositeXattrClient.RemoveAll(file2path) + Expect(err).ToNot(HaveOccurred()) + items, err = compositeXattrClient.List(file2path) + Expect(err).ToNot(HaveOccurred()) + Expect(len(items)).To(Equal(0)) + }) + }) + + AfterEach(func() { + ctrl.Finish() + err := os.RemoveAll(baseDir) + Expect(err).ToNot(HaveOccurred()) + }) + +}) diff --git a/pkg/s3wrapper/filesystem.go b/pkg/s3wrapper/filesystem.go index d5986278588..4e4871faff3 100644 --- a/pkg/s3wrapper/filesystem.go +++ b/pkg/s3wrapper/filesystem.go @@ -9,6 +9,7 @@ import ( "path" "path/filepath" "strings" + "syscall" "time" "github.com/alecthomas/units" @@ -18,25 +19,25 @@ import ( "github.com/openshift/assisted-service/internal/metrics" logutil "github.com/openshift/assisted-service/pkg/log" "github.com/pkg/errors" - "github.com/pkg/xattr" "github.com/sirupsen/logrus" - syscall "golang.org/x/sys/unix" ) type FSClient struct { - log logrus.FieldLogger - basedir string + log logrus.FieldLogger + basedir string + xattrClient XattrClient } var _ API = &FSClient{} -func NewFSClient(basedir string, logger logrus.FieldLogger, metricsAPI metrics.API, fsThreshold int) *FSClientDecorator { +func NewFSClient(basedir string, logger logrus.FieldLogger, metricsAPI metrics.API, fsThreshold int, xattrClient XattrClient) *FSClientDecorator { return &FSClientDecorator{ log: logger, metricsAPI: metricsAPI, fsClient: FSClient{ - log: logger, - basedir: basedir, + log: logger, + basedir: basedir, + xattrClient: xattrClient, }, fsUsageThreshold: fsThreshold, timeFSUsageLog: time.Now().Add(-1 * time.Hour), @@ -53,11 +54,9 @@ func (f *FSClient) CreateBucket() error { return nil } -const xattrUserAttributePrefix = "user." - func (f *FSClient) writeFileMetadata(filePath string, metadata map[string]string) error { for attributeName, attributeValue := range metadata { - err := xattr.Set(filePath, strings.ToLower(fmt.Sprintf("%s%s", xattrUserAttributePrefix, attributeName)), []byte(attributeValue)) + err := f.xattrClient.Set(filePath, strings.ToLower(attributeName), attributeValue) if err != nil { return errors.Wrapf(err, "unable to store metadata key %s = %s", attributeName, attributeValue) } @@ -67,20 +66,16 @@ func (f *FSClient) writeFileMetadata(filePath string, metadata map[string]string func (f *FSClient) getFileMetadata(filePath string) (map[string]string, error) { attributes := make(map[string]string) - attributeNames, err := xattr.List(filePath) + attributeNames, err := f.xattrClient.List(filePath) if err != nil { return nil, errors.Wrap(err, "Unable to obtain extended file attributes while retrieving file metadata") } for _, attributeName := range attributeNames { - if !strings.HasPrefix(attributeName, xattrUserAttributePrefix) { - continue - } - attributeByteValue, err := xattr.Get(filePath, attributeName) + attributeValue, _, err := f.xattrClient.Get(filePath, attributeName) if err != nil { - return nil, errors.Wrap(err, "Unable to obtain extended file attributes while retrieving file metadata") + return nil, errors.Wrapf(err, "Unable to obtain extended file attribute %s while retrieving file metadata", attributeName) } - attributeValue := string(attributeByteValue) - attributes[strings.TrimPrefix(attributeName, xattrUserAttributePrefix)] = attributeValue + attributes[attributeName] = attributeValue } return attributes, nil } @@ -120,7 +115,7 @@ func (f *FSClient) upload(ctx context.Context, data []byte, objectName string, m log.Error(err) return err } - if err := f.writeFileMetadata(t.Name(), metadata); err != nil { + if err := f.writeFileMetadata(filePath, metadata); err != nil { err = errors.Wrapf(err, "Unable to write file metadata for file %s", filePath) log.Error(err) return err @@ -207,7 +202,7 @@ func (f *FSClient) uploadStream(ctx context.Context, reader io.Reader, objectNam break } } - if err := f.writeFileMetadata(t.Name(), metadata); err != nil { + if err := f.writeFileMetadata(filePath, metadata); err != nil { err = errors.Wrapf(err, "Unable to write file metadata for file %s", filePath) log.Error(err) return err @@ -263,7 +258,11 @@ func (f *FSClient) DoesObjectExist(ctx context.Context, objectName string) (bool func (f *FSClient) DeleteObject(ctx context.Context, objectName string) (bool, error) { log := logutil.FromContext(ctx, f.log) filePath := filepath.Join(f.basedir, objectName) - err := os.Remove(filePath) + err := f.xattrClient.RemoveAll(filePath) + if err != nil { + return false, err + } + err = os.Remove(filePath) if err != nil { if os.IsNotExist(err) { return false, nil @@ -326,7 +325,11 @@ func (f *FSClient) handleFile(ctx context.Context, log logrus.FieldLogger, fileP if now.Before(fileInfo.ModTime().Add(deleteTime)) { return } - err := os.Remove(filePath) + err := f.xattrClient.RemoveAll(filePath) + if err != nil { + log.WithError(err).Errorf("Failed to remove xattr data for file %s", filePath) + } + err = os.Remove(filePath) if err != nil { if !os.IsNotExist(err) { log.WithError(err).Errorf("Failed to delete file %s", filePath) diff --git a/pkg/s3wrapper/filesystem_test.go b/pkg/s3wrapper/filesystem_test.go index 32bbd7a19d3..4165d224f63 100644 --- a/pkg/s3wrapper/filesystem_test.go +++ b/pkg/s3wrapper/filesystem_test.go @@ -19,6 +19,7 @@ import ( ) var _ = Describe("s3filesystem", func() { + var ( ctx = context.Background() log = logrus.New() @@ -32,313 +33,343 @@ var _ = Describe("s3filesystem", func() { objKey = "discovery-image-d183c403-d27b-42e1-b0a4-1274ea1a5d77.iso" objKey2 = "discovery-image-f318a87b-ba57-4c7e-ae5f-ee562a6d1e8c.iso" ) - BeforeEach(func() { - log.SetOutput(io.Discard) - var err error - baseDir, err = os.MkdirTemp("", "test") - Expect(err).Should(BeNil()) - ctrl = gomock.NewController(GinkgoT()) - mockMetricsAPI = metrics.NewMockAPI(ctrl) - client = &FSClient{basedir: baseDir, log: log} - deleteTime, _ = time.ParseDuration("60m") - now, _ = time.Parse(time.RFC3339, "2020-01-01T10:00:00+00:00") - }) - AfterEach(func() { - err := os.RemoveAll(baseDir) - Expect(err).Should(BeNil()) - }) - It("upload_download", func() { - mockMetricsAPI.EXPECT().FileSystemUsage(gomock.Any()).Times(1) - expLen := len(dataStr) - err := client.Upload(ctx, []byte(dataStr), objKey) - Expect(err).Should(BeNil()) + // Run the test suite with each of the xattr clients... - size, err := client.GetObjectSizeBytes(ctx, objKey) - Expect(err).Should(BeNil()) - Expect(size).To(Equal(int64(expLen))) - - buf := make([]byte, expLen+5) // Buf with some extra room - reader, downloadLength, err := client.Download(ctx, objKey) - Expect(err).Should(BeNil()) - length, err := reader.Read(buf) - Expect(err).Should(BeNil()) - Expect(length).To(Equal(expLen)) - Expect(downloadLength).To(Equal(int64(expLen))) - }) - It("uploadfile_download", func() { - mockMetricsAPI.EXPECT().FileSystemUsage(gomock.Any()).Times(1) - expLen := len(dataStr) - filePath, _ := createFileObject(client.basedir, objKey, now, nil) - err := client.UploadFile(ctx, filePath, objKey) - Expect(err).Should(BeNil()) - - size, err := client.GetObjectSizeBytes(ctx, objKey) - Expect(err).Should(BeNil()) - Expect(size).To(Equal(int64(expLen))) - - buf := make([]byte, expLen+5) // Buf with some extra room - reader, downloadLength, err := client.Download(ctx, objKey) - Expect(err).Should(BeNil()) - length, err := reader.Read(buf) - Expect(err).Should(BeNil()) - Expect(length).To(Equal(expLen)) - Expect(downloadLength).To(Equal(int64(expLen))) - }) - It("uploadstream_download", func() { - mockMetricsAPI.EXPECT().FileSystemUsage(gomock.Any()).Times(1) - expLen := len(dataStr) - filePath, _ := createFileObject(client.basedir, "foo", now, nil) - fileReader, err := os.Open(filePath) - Expect(err).Should(BeNil()) - err = client.UploadStream(ctx, fileReader, objKey) - Expect(err).Should(BeNil()) - fileReader.Close() - - size, err := client.GetObjectSizeBytes(ctx, objKey) - Expect(err).Should(BeNil()) - Expect(size).To(Equal(int64(expLen))) - - buf := make([]byte, expLen+5) // Buf with some extra room - reader, downloadLength, err := client.Download(ctx, objKey) - Expect(err).Should(BeNil()) - length, err := reader.Read(buf) - Expect(err).Should(BeNil()) - Expect(length).To(Equal(expLen)) - Expect(downloadLength).To(Equal(int64(expLen))) - }) - It("doesobjectexist_delete", func() { - mockMetricsAPI.EXPECT().FileSystemUsage(gomock.Any()).Times(2) - err := client.Upload(ctx, []byte(dataStr), objKey) - Expect(err).Should(BeNil()) - - exists, err := client.DoesObjectExist(ctx, objKey) - Expect(err).Should(BeNil()) - Expect(exists).To(Equal(true)) - - existed, err := client.DeleteObject(ctx, objKey) - Expect(existed).To(Equal(true)) - Expect(err).Should(BeNil()) - - exists, err = client.DoesObjectExist(ctx, objKey) - Expect(err).Should(BeNil()) - Expect(exists).To(Equal(false)) - }) - It("expiration", func() { - imgCreatedAt, _ := time.Parse(time.RFC3339, "2020-01-01T09:30:00+00:00") // Long ago - createFileObject(client.basedir, objKey, imgCreatedAt, nil) - createFileObject(client.basedir, objKey2, imgCreatedAt, nil) - - called := 0 - mockMetricsAPI.EXPECT().FileSystemUsage(gomock.Any()).Times(2) - client.ExpireObjects(ctx, "discovery-image-", deleteTime, func(ctx context.Context, log logrus.FieldLogger, objectName string) { called = called + 1 }) - Expect(called).To(Equal(2)) - - exists, err := client.DoesObjectExist(ctx, objKey) - Expect(err).Should(BeNil()) - Expect(exists).To(Equal(false)) + type test struct { + xattrClient XattrClient + testXattrWriter xattrwriter + } - exists, err = client.DoesObjectExist(ctx, objKey2) - Expect(err).Should(BeNil()) - Expect(exists).To(Equal(false)) - }) - It("expire_not_expired_image", func() { - imgCreatedAt, _ := time.Parse(time.RFC3339, "2020-01-01T09:30:00+00:00") // 30 minutes ago - filePath, info := createFileObject(client.basedir, objKey, imgCreatedAt, nil) - called := false - mockMetricsAPI.EXPECT().FileSystemUsage(gomock.Any()).Times(1) - client.handleFile(ctx, log, filePath, info, now, deleteTime, func(ctx context.Context, log logrus.FieldLogger, objectName string) { called = true }) - Expect(called).To(Equal(false)) - }) - It("expire_expired_image", func() { - imgCreatedAt, _ := time.Parse(time.RFC3339, "2020-01-01T08:00:00+00:00") // Two hours ago - filePath, info := createFileObject(client.basedir, objKey, imgCreatedAt, nil) - called := false - mockMetricsAPI.EXPECT().FileSystemUsage(gomock.Any()).Times(1) - client.handleFile(ctx, log, filePath, info, now, deleteTime, func(ctx context.Context, log logrus.FieldLogger, objectName string) { called = true }) - Expect(called).To(Equal(true)) - }) - It("expire_delete_error", func() { - imgCreatedAt, _ := time.Parse(time.RFC3339, "2020-01-01T08:00:00+00:00") // Two hours ago - filePath, info := createFileObject(client.basedir, objKey, imgCreatedAt, nil) - os.Remove(filePath) - called := false - client.handleFile(ctx, log, filePath, info, now, deleteTime, func(ctx context.Context, log logrus.FieldLogger, objectName string) { called = true }) - Expect(called).To(Equal(false)) - }) + testCases := []test{ + {xattrClient: &OSxattrClient{log: log},testXattrWriter: setxattrnative}, + {xattrClient: &FilesystemBasedXattrClient{log: log},testXattrWriter: setxattrfilebased}, + } - It("ListObjectByPrefix lists the correct object without a leading slash", func() { - _, _ = createFileObject(client.basedir, "dir/other/file", now, nil) - _, _ = createFileObject(client.basedir, "dir/other/file2", now, nil) - _, _ = createFileObject(client.basedir, "dir/file", now, nil) - _, _ = createFileObject(client.basedir, "dir2/file", now, nil) - - var paths []string - var err error - containsObj := func(obj string) bool { - for _, o := range paths { - if obj == o { - return true - } - } - return false - } - - paths, err = client.ListObjectsByPrefix(ctx, "dir/other") - Expect(err).NotTo(HaveOccurred()) - Expect(len(paths)).To(Equal(2)) - Expect(containsObj("dir/other/file")).To(BeTrue(), "file list %v does not contain \"dir/other/file\"", paths) - Expect(containsObj("dir/other/file2")).To(BeTrue(), "file list %v does not contain \"dir/other/file2\"", paths) - - paths, err = client.ListObjectsByPrefix(ctx, "dir2") - Expect(err).NotTo(HaveOccurred()) - Expect(len(paths)).To(Equal(1)) - Expect(paths[0]).To(Equal("dir2/file")) - - paths, err = client.ListObjectsByPrefix(ctx, "") - Expect(err).NotTo(HaveOccurred()) - Expect(len(paths)).To(Equal(4)) - - Expect(containsObj("dir/other/file")).To(BeTrue(), "file list %v does not contain \"dir/other/file\"", paths) - Expect(containsObj("dir/other/file2")).To(BeTrue(), "file list %v does not contain \"dir/other/file2\"", paths) - Expect(containsObj("dir/file")).To(BeTrue(), "file list %v does not contain \"dir/file\"", paths) - Expect(containsObj("dir2/file")).To(BeTrue(), "file list %v does not contain \"dir2/file\"", paths) - }) + for _, testCase := range testCases { + BeforeEach(func() { + log.SetOutput(io.Discard) + var err error + baseDir, err = os.MkdirTemp("", "test") + Expect(err).Should(BeNil()) + ctrl = gomock.NewController(GinkgoT()) + mockMetricsAPI = metrics.NewMockAPI(ctrl) + testCase.xattrClient.SetRootDirectory(baseDir) + client = &FSClient{basedir: baseDir, log: log, xattrClient: testCase.xattrClient} + deleteTime, _ = time.ParseDuration("60m") + now, _ = time.Parse(time.RFC3339, "2020-01-01T10:00:00+00:00") + }) + AfterEach(func() { + err := os.RemoveAll(baseDir) + Expect(err).Should(BeNil()) + }) - Context("Metadata", func() { + It("upload_download", func() { + mockMetricsAPI.EXPECT().FileSystemUsage(gomock.Any()).Times(1) + expLen := len(dataStr) + err := client.Upload(ctx, []byte(dataStr), objKey) + Expect(err).Should(BeNil()) + + size, err := client.GetObjectSizeBytes(ctx, objKey) + Expect(err).Should(BeNil()) + Expect(size).To(Equal(int64(expLen))) + + buf := make([]byte, expLen+5) // Buf with some extra room + reader, downloadLength, err := client.Download(ctx, objKey) + Expect(err).Should(BeNil()) + length, err := reader.Read(buf) + Expect(err).Should(BeNil()) + Expect(length).To(Equal(expLen)) + Expect(downloadLength).To(Equal(int64(expLen))) + }) + It("uploadfile_download", func() { + mockMetricsAPI.EXPECT().FileSystemUsage(gomock.Any()).Times(1) + expLen := len(dataStr) + filePath, _ := createFileObject(client.basedir, objKey, now, nil, testCase.testXattrWriter) + err := client.UploadFile(ctx, filePath, objKey) + Expect(err).Should(BeNil()) + + size, err := client.GetObjectSizeBytes(ctx, objKey) + Expect(err).Should(BeNil()) + Expect(size).To(Equal(int64(expLen))) + + buf := make([]byte, expLen+5) // Buf with some extra room + reader, downloadLength, err := client.Download(ctx, objKey) + Expect(err).Should(BeNil()) + length, err := reader.Read(buf) + Expect(err).Should(BeNil()) + Expect(length).To(Equal(expLen)) + Expect(downloadLength).To(Equal(int64(expLen))) + }) + It("uploadstream_download", func() { + mockMetricsAPI.EXPECT().FileSystemUsage(gomock.Any()).Times(1) + expLen := len(dataStr) + filePath, _ := createFileObject(client.basedir, "foo", now, nil, testCase.testXattrWriter) + fileReader, err := os.Open(filePath) + Expect(err).Should(BeNil()) + err = client.UploadStream(ctx, fileReader, objKey) + Expect(err).Should(BeNil()) + fileReader.Close() + + size, err := client.GetObjectSizeBytes(ctx, objKey) + Expect(err).Should(BeNil()) + Expect(size).To(Equal(int64(expLen))) + + buf := make([]byte, expLen+5) // Buf with some extra room + reader, downloadLength, err := client.Download(ctx, objKey) + Expect(err).Should(BeNil()) + length, err := reader.Read(buf) + Expect(err).Should(BeNil()) + Expect(length).To(Equal(expLen)) + Expect(downloadLength).To(Equal(int64(expLen))) + }) + It("doesobjectexist_delete", func() { + mockMetricsAPI.EXPECT().FileSystemUsage(gomock.Any()).Times(2) + err := client.Upload(ctx, []byte(dataStr), objKey) + Expect(err).Should(BeNil()) + + exists, err := client.DoesObjectExist(ctx, objKey) + Expect(err).Should(BeNil()) + Expect(exists).To(Equal(true)) + + existed, err := client.DeleteObject(ctx, objKey) + Expect(existed).To(Equal(true)) + Expect(err).Should(BeNil()) + + exists, err = client.DoesObjectExist(ctx, objKey) + Expect(err).Should(BeNil()) + Expect(exists).To(Equal(false)) + }) + It("expiration", func() { + imgCreatedAt, _ := time.Parse(time.RFC3339, "2020-01-01T09:30:00+00:00") // Long ago + createFileObject(client.basedir, objKey, imgCreatedAt, nil, testCase.testXattrWriter) + createFileObject(client.basedir, objKey2, imgCreatedAt, nil, testCase.testXattrWriter) + + called := 0 + mockMetricsAPI.EXPECT().FileSystemUsage(gomock.Any()).Times(2) + client.ExpireObjects(ctx, "discovery-image-", deleteTime, func(ctx context.Context, log logrus.FieldLogger, objectName string) { called = called + 1 }) + Expect(called).To(Equal(2)) + + exists, err := client.DoesObjectExist(ctx, objKey) + Expect(err).Should(BeNil()) + Expect(exists).To(Equal(false)) + + exists, err = client.DoesObjectExist(ctx, objKey2) + Expect(err).Should(BeNil()) + Expect(exists).To(Equal(false)) + }) + It("expire_not_expired_image", func() { + imgCreatedAt, _ := time.Parse(time.RFC3339, "2020-01-01T09:30:00+00:00") // 30 minutes ago + filePath, info := createFileObject(client.basedir, objKey, imgCreatedAt, nil, testCase.testXattrWriter) + called := false + mockMetricsAPI.EXPECT().FileSystemUsage(gomock.Any()).Times(1) + client.handleFile(ctx, log, filePath, info, now, deleteTime, func(ctx context.Context, log logrus.FieldLogger, objectName string) { called = true }) + Expect(called).To(Equal(false)) + }) + It("expire_expired_image", func() { + imgCreatedAt, _ := time.Parse(time.RFC3339, "2020-01-01T08:00:00+00:00") // Two hours ago + filePath, info := createFileObject(client.basedir, objKey, imgCreatedAt, nil, testCase.testXattrWriter) + called := false + mockMetricsAPI.EXPECT().FileSystemUsage(gomock.Any()).Times(1) + client.handleFile(ctx, log, filePath, info, now, deleteTime, func(ctx context.Context, log logrus.FieldLogger, objectName string) { called = true }) + Expect(called).To(Equal(true)) + }) + It("expire_delete_error", func() { + imgCreatedAt, _ := time.Parse(time.RFC3339, "2020-01-01T08:00:00+00:00") // Two hours ago + filePath, info := createFileObject(client.basedir, objKey, imgCreatedAt, nil, testCase.testXattrWriter) + os.Remove(filePath) + called := false + client.handleFile(ctx, log, filePath, info, now, deleteTime, func(ctx context.Context, log logrus.FieldLogger, objectName string) { called = true }) + Expect(called).To(Equal(false)) + }) - containsMetadataEntry := func(objects []ObjectInfo, metadata map[string]string) bool { - for _, object := range objects { - if len(object.Metadata) != len(metadata) { - return false - } - for key, value := range object.Metadata { - if val, ok := metadata[key]; ok && val == value { + It("ListObjectByPrefix lists the correct object without a leading slash", func() { + _, _ = createFileObject(client.basedir, "dir/other/file", now, nil, testCase.testXattrWriter) + _, _ = createFileObject(client.basedir, "dir/other/file2", now, nil, testCase.testXattrWriter) + _, _ = createFileObject(client.basedir, "dir/file", now, nil, testCase.testXattrWriter) + _, _ = createFileObject(client.basedir, "dir2/file", now, nil, testCase.testXattrWriter) + + var paths []string + var err error + containsObj := func(obj string) bool { + for _, o := range paths { + if obj == o { return true } } + return false } - return false - } - validateListObjectsByPrefix := func() { - objects, err := client.ListObjectsByPrefixWithMetadata(ctx, "dir/other") + paths, err = client.ListObjectsByPrefix(ctx, "dir/other") Expect(err).NotTo(HaveOccurred()) - Expect(len(objects)).To(Equal(2)) - Expect(containsMetadataEntry(objects, map[string]string{ - "key1": "bar", - "key2": "foo", - })).To(BeTrue()) - Expect(containsMetadataEntry(objects, map[string]string{ - "key3": "bar2", - "key4": "foo2", - })).To(BeTrue()) - Expect(containsMetadataEntry(objects, map[string]string{ - "key5": "fake", - "key6": "fabricated", - })).To(BeFalse()) - Expect(containsMetadataEntry(objects, map[string]string{ - "key3": "bar", - "key5": "foo", - })).To(BeFalse()) - } - - It("ListObjectsByPrefix returns metadata about objects", func() { - createFileObject(client.basedir, fmt.Sprintf("dir/other/%s", uuid.New().String()), now, map[string]string{ - "Key1": "bar", - "key2": "foo", - }) - createFileObject(client.basedir, fmt.Sprintf("dir/other/%s", uuid.New().String()), now, map[string]string{ - "key3": "bar2", - "Key4": "foo2", - }) - validateListObjectsByPrefix() - }) + Expect(len(paths)).To(Equal(2)) + Expect(containsObj("dir/other/file")).To(BeTrue(), "file list %v does not contain \"dir/other/file\"", paths) + Expect(containsObj("dir/other/file2")).To(BeTrue(), "file list %v does not contain \"dir/other/file2\"", paths) - Context("Upload", func() { + paths, err = client.ListObjectsByPrefix(ctx, "dir2") + Expect(err).NotTo(HaveOccurred()) + Expect(len(paths)).To(Equal(1)) + Expect(paths[0]).To(Equal("dir2/file")) - var ( - file1Path string - file2Path string - tempFileDirectory string - ) + paths, err = client.ListObjectsByPrefix(ctx, "") + Expect(err).NotTo(HaveOccurred()) + Expect(len(paths)).To(Equal(4)) - BeforeEach(func() { - var err error - tempFileDirectory, err = os.MkdirTemp(os.TempDir(), fmt.Sprintf("manifest-test-%s", uuid.NewString())) - Expect(err).ToNot(HaveOccurred()) - file1Path, _ = createFileObject(tempFileDirectory, uuid.NewString(), now, nil) - file2Path, _ = createFileObject(tempFileDirectory, uuid.NewString(), now, nil) - }) + Expect(containsObj("dir/other/file")).To(BeTrue(), "file list %v does not contain \"dir/other/file\"", paths) + Expect(containsObj("dir/other/file2")).To(BeTrue(), "file list %v does not contain \"dir/other/file2\"", paths) + Expect(containsObj("dir/file")).To(BeTrue(), "file list %v does not contain \"dir/file\"", paths) + Expect(containsObj("dir2/file")).To(BeTrue(), "file list %v does not contain \"dir2/file\"", paths) + }) - AfterEach(func() { - Expect(os.Remove(file1Path)).To(BeNil()) - Expect(os.Remove(file2Path)).To(BeNil()) - Expect(os.Remove(tempFileDirectory)).To(BeNil()) - }) + Context("Metadata", func() { - It("Upload stores metadata about objects", func() { - dataStr := "hello world" - err := client.UploadWithMetadata(ctx, []byte(dataStr), "dir/other/file1", map[string]string{ - "Key1": "bar", + containsMetadataEntry := func(objects []ObjectInfo, metadata map[string]string) bool { + for _, object := range objects { + if len(object.Metadata) != len(metadata) { + return false + } + for key, value := range object.Metadata { + if val, ok := metadata[key]; ok && val == value { + return true + } + } + } + return false + } + + validateListObjectsByPrefix := func() { + objects, err := client.ListObjectsByPrefixWithMetadata(ctx, "dir/other") + Expect(err).NotTo(HaveOccurred()) + Expect(len(objects)).To(Equal(2)) + Expect(containsMetadataEntry(objects, map[string]string{ + "key1": "bar", "key2": "foo", - }) - Expect(err).Should(BeNil()) - err = client.UploadWithMetadata(ctx, []byte(dataStr), "dir/other/file2", map[string]string{ + })).To(BeTrue()) + Expect(containsMetadataEntry(objects, map[string]string{ "key3": "bar2", - "Key4": "foo2", - }) - Expect(err).Should(BeNil()) - validateListObjectsByPrefix() - }) + "key4": "foo2", + })).To(BeTrue()) + Expect(containsMetadataEntry(objects, map[string]string{ + "key5": "fake", + "key6": "fabricated", + })).To(BeFalse()) + Expect(containsMetadataEntry(objects, map[string]string{ + "key3": "bar", + "key5": "foo", + })).To(BeFalse()) + } - It("UploadFile stores metadata about objects", func() { - Expect(client.UploadFileWithMetadata(ctx, file1Path, "dir/other/file1", map[string]string{ + It("ListObjectsByPrefix returns metadata about objects", func() { + createFileObject(client.basedir, fmt.Sprintf("dir/other/%s", uuid.New().String()), now, map[string]string{ "Key1": "bar", "key2": "foo", - })).To(BeNil()) - Expect(client.UploadFileWithMetadata(ctx, file2Path, "dir/other/file2", map[string]string{ + }, testCase.testXattrWriter) + createFileObject(client.basedir, fmt.Sprintf("dir/other/%s", uuid.New().String()), now, map[string]string{ "key3": "bar2", "Key4": "foo2", - })).To(BeNil()) + }, testCase.testXattrWriter) validateListObjectsByPrefix() }) - It("UploadStream stores metadata about objects", func() { - fileReader, err := os.Open(file1Path) - Expect(err).Should(BeNil()) - err = client.UploadStreamWithMetadata(ctx, fileReader, "dir/other/file1", map[string]string{ - "Key1": "bar", - "key2": "foo", + Context("Upload", func() { + + var ( + file1Path string + file2Path string + tempFileDirectory string + ) + + BeforeEach(func() { + var err error + tempFileDirectory, err = os.MkdirTemp(os.TempDir(), fmt.Sprintf("manifest-test-%s", uuid.NewString())) + Expect(err).ToNot(HaveOccurred()) + file1Path, _ = createFileObject(tempFileDirectory, uuid.NewString(), now, nil, testCase.testXattrWriter) + file2Path, _ = createFileObject(tempFileDirectory, uuid.NewString(), now, nil, testCase.testXattrWriter) }) - Expect(err).Should(BeNil()) - fileReader.Close() - fileReader, err = os.Open(file2Path) - Expect(err).Should(BeNil()) - err = client.UploadStreamWithMetadata(ctx, fileReader, "dir/other/file2", map[string]string{ - "key3": "bar2", - "Key4": "foo2", + AfterEach(func() { + Expect(os.Remove(file1Path)).To(BeNil()) + Expect(os.Remove(file2Path)).To(BeNil()) + Expect(os.Remove(tempFileDirectory)).To(BeNil()) }) - Expect(err).Should(BeNil()) - fileReader.Close() - validateListObjectsByPrefix() + It("Upload stores metadata about objects", func() { + dataStr := "hello world" + err := client.UploadWithMetadata(ctx, []byte(dataStr), "dir/other/file1", map[string]string{ + "Key1": "bar", + "key2": "foo", + }) + Expect(err).Should(BeNil()) + err = client.UploadWithMetadata(ctx, []byte(dataStr), "dir/other/file2", map[string]string{ + "key3": "bar2", + "Key4": "foo2", + }) + Expect(err).Should(BeNil()) + validateListObjectsByPrefix() + }) + + It("UploadFile stores metadata about objects", func() { + Expect(client.UploadFileWithMetadata(ctx, file1Path, "dir/other/file1", map[string]string{ + "Key1": "bar", + "key2": "foo", + })).To(BeNil()) + Expect(client.UploadFileWithMetadata(ctx, file2Path, "dir/other/file2", map[string]string{ + "key3": "bar2", + "Key4": "foo2", + })).To(BeNil()) + validateListObjectsByPrefix() + }) + + It("UploadStream stores metadata about objects", func() { + fileReader, err := os.Open(file1Path) + Expect(err).Should(BeNil()) + err = client.UploadStreamWithMetadata(ctx, fileReader, "dir/other/file1", map[string]string{ + "Key1": "bar", + "key2": "foo", + }) + Expect(err).Should(BeNil()) + fileReader.Close() + + fileReader, err = os.Open(file2Path) + Expect(err).Should(BeNil()) + err = client.UploadStreamWithMetadata(ctx, fileReader, "dir/other/file2", map[string]string{ + "key3": "bar2", + "Key4": "foo2", + }) + Expect(err).Should(BeNil()) + fileReader.Close() + + validateListObjectsByPrefix() + }) }) }) - }) + } AfterEach(func() { os.RemoveAll(baseDir) }) }) -func setxattr(path string, name string, data []byte, flags int) error { +type xattrwriter func(basedir string, path string, name string, data []byte, flags int) + +func setxattrnative(basedir string, path string, name string, data []byte, flags int) { + key := strings.ToLower(fmt.Sprintf("user.%s", name)) + err := unix.Setxattr(path, key, data, flags) + Expect(err).ToNot(HaveOccurred()) +} + +func setxattrfilebased(basedir string, path string, name string, data []byte, flags int) { + relativePath := path[len(basedir):] + fileName := filepath.Join(basedir, filesystemXattrMetaDataDirectoryName, relativePath) + directory := filepath.Dir(fileName) key := strings.ToLower(fmt.Sprintf("user.%s", name)) - return unix.Setxattr(path, key, data, flags) + err := os.MkdirAll(directory, 0o700) + Expect(err).ToNot(HaveOccurred()) + err = os.WriteFile(fmt.Sprintf("%s%s%s", fileName, delimiter, key), data, 0o600) + Expect(err).ToNot(HaveOccurred()) } -func createFileObject(baseDir, objKey string, imgCreatedAt time.Time, metadata map[string]string) (string, os.FileInfo) { +func createFileObject(baseDir, objKey string, imgCreatedAt time.Time, metadata map[string]string, xattrWriter xattrwriter) (string, os.FileInfo) { filePath := filepath.Join(baseDir, objKey) Expect(os.MkdirAll(filepath.Dir(filePath), 0755)).To(Succeed()) err := os.WriteFile(filePath, []byte("Hello world"), 0600) @@ -348,7 +379,7 @@ func createFileObject(baseDir, objKey string, imgCreatedAt time.Time, metadata m info, err := os.Stat(filePath) Expect(err).Should(BeNil()) for k, v := range metadata { - err := setxattr(filePath, k, []byte(v), 0) + xattrWriter(baseDir, filePath, k, []byte(v), 0) Expect(err).Should(BeNil()) } return filePath, info diff --git a/pkg/s3wrapper/filesystem_xattr_client.go b/pkg/s3wrapper/filesystem_xattr_client.go new file mode 100644 index 00000000000..4559614e817 --- /dev/null +++ b/pkg/s3wrapper/filesystem_xattr_client.go @@ -0,0 +1,117 @@ +package s3wrapper + +import ( + "fmt" + "os" + "path/filepath" + "strings" + + "github.com/pkg/errors" + "github.com/sirupsen/logrus" +) + +func NewFilesystemBasedXattrClient( + log logrus.FieldLogger, + rootDirectory string, +) *FilesystemBasedXattrClient { + return &FilesystemBasedXattrClient{ + log: log, + rootDirectory: rootDirectory, + } +} + +const ( + filesystemXattrMetaDataDirectoryName = "metadata" + delimiter = "::" +) + +type FilesystemBasedXattrClient struct { + log logrus.FieldLogger + rootDirectory string +} + +func (xattrClient *FilesystemBasedXattrClient) getPathWithKey(path string, key string) string { + key = getKeyWithUserAttributePrefix(key) + return fmt.Sprintf("%s%s%s", path, delimiter, key) +} + +func (xattrClient *FilesystemBasedXattrClient) IsSupported() (bool, error) { + return true, nil +} + +// getMetaFilenameForManifestFile Finds the base of the filename and the directory for the manifest metadata files +func (xattrClient *FilesystemBasedXattrClient) getMetaFilenamePrefixForManifestFile(path string) (fileName string, directory string) { + relativePath := path[len(xattrClient.rootDirectory):] + fileName = filepath.Join(xattrClient.rootDirectory, filesystemXattrMetaDataDirectoryName, relativePath) + directory = filepath.Dir(fileName) + return +} + +func (c *FilesystemBasedXattrClient) SetRootDirectory(path string) { + c.rootDirectory = path +} +func (c *FilesystemBasedXattrClient) GetRootDirectory() string { + return c.rootDirectory +} + +func (c *FilesystemBasedXattrClient) Set(path, key string, value string) error { + newPath, directory := c.getMetaFilenamePrefixForManifestFile(path) + err := os.MkdirAll(directory, 0o700) + if err != nil { + return err + } + return os.WriteFile(c.getPathWithKey(newPath, key), []byte(value), 0o600) +} + +func (c *FilesystemBasedXattrClient) Get(path, key string) (string, bool, error) { + newPath, _ := c.getMetaFilenamePrefixForManifestFile(path) + data, err := os.ReadFile(c.getPathWithKey(newPath, key)) + if err != nil { + if errors.Is(err, os.ErrNotExist) { + err = errors.Wrapf(err, "Attribute %s not found for file %s", key, path) + } + return "", false, err + } + return string(data), true, nil +} + +func (c *FilesystemBasedXattrClient) List(path string) ([]string, error) { + var keys []string + _, directory := c.getMetaFilenamePrefixForManifestFile(path) + files, err := os.ReadDir(directory) + if errors.Is(err, os.ErrNotExist) { + return keys, nil + } + for _, file := range files { + prefix := c.getPathWithKey(filepath.Base(path), "") + if strings.HasPrefix(file.Name(), prefix) { + key := strings.TrimPrefix(file.Name(), prefix) + keys = append(keys, key) + } + } + return keys, nil +} + +func (c *FilesystemBasedXattrClient) Remove(path string, key string) error { + newPath, _ := c.getMetaFilenamePrefixForManifestFile(path) + err := os.Remove(c.getPathWithKey(newPath, key)) + if !errors.Is(err, os.ErrNotExist) { + return errors.Wrapf(err, "Could not delete attribute %s for file %s", key, path) + } + //TODO: clean up parent directories. + return nil +} + +func (c *FilesystemBasedXattrClient) RemoveAll(path string) error { + keys, err := c.List(path) + if err != nil { + return errors.Wrapf(err, "could not delete keys in path %s", path) + } + for _, key := range keys { + err = c.Remove(path, key) + if err != nil { + return errors.Wrapf(err, "could not delete key %s in path %s", key, path) + } + } + return nil +} diff --git a/pkg/s3wrapper/mock_xattr_client.go b/pkg/s3wrapper/mock_xattr_client.go new file mode 100644 index 00000000000..4e91dbe40fc --- /dev/null +++ b/pkg/s3wrapper/mock_xattr_client.go @@ -0,0 +1,148 @@ +// Code generated by MockGen. DO NOT EDIT. +// Source: github.com/openshift/assisted-service/pkg/s3wrapper (interfaces: XattrClient) + +// Package s3wrapper is a generated GoMock package. +package s3wrapper + +import ( + reflect "reflect" + + gomock "github.com/golang/mock/gomock" +) + +// MockXattrClient is a mock of XattrClient interface. +type MockXattrClient struct { + ctrl *gomock.Controller + recorder *MockXattrClientMockRecorder +} + +// MockXattrClientMockRecorder is the mock recorder for MockXattrClient. +type MockXattrClientMockRecorder struct { + mock *MockXattrClient +} + +// NewMockXattrClient creates a new mock instance. +func NewMockXattrClient(ctrl *gomock.Controller) *MockXattrClient { + mock := &MockXattrClient{ctrl: ctrl} + mock.recorder = &MockXattrClientMockRecorder{mock} + return mock +} + +// EXPECT returns an object that allows the caller to indicate expected use. +func (m *MockXattrClient) EXPECT() *MockXattrClientMockRecorder { + return m.recorder +} + +// Get mocks base method. +func (m *MockXattrClient) Get(arg0, arg1 string) (string, bool, error) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "Get", arg0, arg1) + ret0, _ := ret[0].(string) + ret1, _ := ret[1].(bool) + ret2, _ := ret[2].(error) + return ret0, ret1, ret2 +} + +// Get indicates an expected call of Get. +func (mr *MockXattrClientMockRecorder) Get(arg0, arg1 interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Get", reflect.TypeOf((*MockXattrClient)(nil).Get), arg0, arg1) +} + +// GetRootDirectory mocks base method. +func (m *MockXattrClient) GetRootDirectory() string { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "GetRootDirectory") + ret0, _ := ret[0].(string) + return ret0 +} + +// GetRootDirectory indicates an expected call of GetRootDirectory. +func (mr *MockXattrClientMockRecorder) GetRootDirectory() *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetRootDirectory", reflect.TypeOf((*MockXattrClient)(nil).GetRootDirectory)) +} + +// IsSupported mocks base method. +func (m *MockXattrClient) IsSupported() (bool, error) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "IsSupported") + ret0, _ := ret[0].(bool) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// IsSupported indicates an expected call of IsSupported. +func (mr *MockXattrClientMockRecorder) IsSupported() *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "IsSupported", reflect.TypeOf((*MockXattrClient)(nil).IsSupported)) +} + +// List mocks base method. +func (m *MockXattrClient) List(arg0 string) ([]string, error) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "List", arg0) + ret0, _ := ret[0].([]string) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// List indicates an expected call of List. +func (mr *MockXattrClientMockRecorder) List(arg0 interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "List", reflect.TypeOf((*MockXattrClient)(nil).List), arg0) +} + +// Remove mocks base method. +func (m *MockXattrClient) Remove(arg0, arg1 string) error { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "Remove", arg0, arg1) + ret0, _ := ret[0].(error) + return ret0 +} + +// Remove indicates an expected call of Remove. +func (mr *MockXattrClientMockRecorder) Remove(arg0, arg1 interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Remove", reflect.TypeOf((*MockXattrClient)(nil).Remove), arg0, arg1) +} + +// RemoveAll mocks base method. +func (m *MockXattrClient) RemoveAll(arg0 string) error { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "RemoveAll", arg0) + ret0, _ := ret[0].(error) + return ret0 +} + +// RemoveAll indicates an expected call of RemoveAll. +func (mr *MockXattrClientMockRecorder) RemoveAll(arg0 interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "RemoveAll", reflect.TypeOf((*MockXattrClient)(nil).RemoveAll), arg0) +} + +// Set mocks base method. +func (m *MockXattrClient) Set(arg0, arg1, arg2 string) error { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "Set", arg0, arg1, arg2) + ret0, _ := ret[0].(error) + return ret0 +} + +// Set indicates an expected call of Set. +func (mr *MockXattrClientMockRecorder) Set(arg0, arg1, arg2 interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Set", reflect.TypeOf((*MockXattrClient)(nil).Set), arg0, arg1, arg2) +} + +// SetRootDirectory mocks base method. +func (m *MockXattrClient) SetRootDirectory(arg0 string) { + m.ctrl.T.Helper() + m.ctrl.Call(m, "SetRootDirectory", arg0) +} + +// SetRootDirectory indicates an expected call of SetRootDirectory. +func (mr *MockXattrClientMockRecorder) SetRootDirectory(arg0 interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "SetRootDirectory", reflect.TypeOf((*MockXattrClient)(nil).SetRootDirectory), arg0) +} diff --git a/pkg/s3wrapper/xattr_client.go b/pkg/s3wrapper/xattr_client.go new file mode 100644 index 00000000000..ee1cf0502c6 --- /dev/null +++ b/pkg/s3wrapper/xattr_client.go @@ -0,0 +1,164 @@ +package s3wrapper + +import ( + "fmt" + "path/filepath" + "strings" + "syscall" + + "github.com/google/renameio" + "github.com/pkg/errors" + "github.com/pkg/xattr" + "github.com/sirupsen/logrus" +) + +//go:generate mockgen --build_flags=--mod=mod -package=s3wrapper -destination=mock_xattr_client.go . XattrClient +type XattrClient interface { + // IsSupported will determine if the client is supported in the current configuration + IsSupported() (bool, error) + + SetRootDirectory(path string) + GetRootDirectory() string + + // Set applies an extended attribute to a given path + // parameter: key - The key of the extended attribute. + // parameter: value - The value of the extended attribute. + // returns: an error if this fails, otherwise nil. + Set(path, key string, value string) error + + // Get returns a value for an extended attribute key on a given given path. + // parameter: path - The path for which the key is to be fetched. + // parameter: key - The key of the extended attribute. + // returns the attribute value as a string, a boolean to indicate whether the value is ok, returns an error if there was one + Get(path, key string) (string, bool, error) + + // List obtains a list of extended attribute keys for a given path + // parameter: path - The path for which extended attributes are to be fetched. + // returns: a list of extended attribute keys for the path or an error if this fails. + List(path string) ([]string, error) + + // Removes an extended attribute for the file at the given path. + // parameter: path - The path for which the extended attribute is to be deleted. + // parameter: key - The key of the extended attribute. + // returns an error if there was an error + Remove(path string, key string) error + + // Removes all extended attributes for the file at the given path. + // parameter: path - The path for which the extended attributes are to be deleted. + // returns an error if there was an error + RemoveAll(path string) error +} + +func NewOSxAttrClient( + log logrus.FieldLogger, + rootDirectory string, +) *OSxattrClient { + return &OSxattrClient{ + log: log, + rootDirectory: rootDirectory, + } +} + +// OSxattrClient represents an xattr client that uses the OS native xattr functionality (if available.) +type OSxattrClient struct { + log logrus.FieldLogger + rootDirectory string +} + +const ( + xattrUserAttributePrefix = "user." +) + +func getKeyWithUserAttributePrefix(key string) string { + return fmt.Sprintf("%s%s", xattrUserAttributePrefix, key) +} + +func removeUserAttributePrefixFromKey(key string) string { + return strings.TrimPrefix(key, xattrUserAttributePrefix) +} + +func hasUserAttributePrefix(key string) bool { + return strings.HasPrefix(key, xattrUserAttributePrefix) +} + +func (c *OSxattrClient) IsSupported() (bool, error) { + t, err := renameio.TempFile("", filepath.Join(c.rootDirectory, "testXattr")) + if err != nil { + return false, errors.Wrap(err, "Unable to create temp file to detect xattr capabilities") + } + defer func() { + if err := t.Cleanup(); err != nil { + c.log.Warn("Unable to clean up temp file %s", t.Name()) + } + }() + err = c.Set(t.Name(), "user.test-xattr-attribute-set", "foobar") + if err != nil { + c.log.WithError(err).Warn("could not set xattr attribute during xattr detection") + return false, nil + } + return true, nil +} + +func (c *OSxattrClient) SetRootDirectory(path string) { + c.rootDirectory = path +} +func (c *OSxattrClient) GetRootDirectory() string { + return c.rootDirectory +} + +func (c *OSxattrClient) Set(path, key string, value string) error { + key = getKeyWithUserAttributePrefix(key) + return xattr.Set(path, key, []byte(value)) +} + +func (c *OSxattrClient) Get(path, key string) (string, bool, error) { + key = getKeyWithUserAttributePrefix(key) + value, err := xattr.Get(path, key) + if errors.Is(err, syscall.ENODATA) { + return "", false, nil + } + if err != nil { + return "", false, errors.Wrap(err, "Unable to obtain extended file attributes while retrieving file metadata") + } + return string(value), true, nil +} + +func (c *OSxattrClient) List(path string) ([]string, error) { + keys := []string{} + result, err := xattr.List(path) + if err != nil { + return nil, err + } + for i := range result { + if hasUserAttributePrefix(result[i]) { + keys = append(keys, removeUserAttributePrefixFromKey(result[i])) + } + } + return keys, nil +} + +func (c *OSxattrClient) Remove(path string, key string) error { + key = getKeyWithUserAttributePrefix(key) + err := xattr.Remove(path, key) + if errors.Is(err, syscall.ENODATA) { + return nil + } + if err != nil { + return err + } + return nil +} + +func (c *OSxattrClient) RemoveAll(path string) error { + keys, err := c.List(path) + if err != nil { + return errors.Wrapf(err, "could not delete keys in path %s", path) + } + for _, key := range keys { + err = c.Remove(path, key) + if err != nil { + return errors.Wrapf(err, "could not delete key %s in path %s", key, path) + } + } + return nil +} diff --git a/vendor/github.com/openshift/assisted-service/api/common/common_types.go b/vendor/github.com/openshift/assisted-service/api/common/common_types.go index 6eff7525735..19afd847d2e 100644 --- a/vendor/github.com/openshift/assisted-service/api/common/common_types.go +++ b/vendor/github.com/openshift/assisted-service/api/common/common_types.go @@ -16,4 +16,3 @@ type ValidationsStatus map[string]ValidationResults // +kubebuilder:object:generate=true type ValidationResults []ValidationResult -