diff --git a/app/boot_levels.go b/app/boot_levels.go index 9d87471c4..686391929 100644 --- a/app/boot_levels.go +++ b/app/boot_levels.go @@ -4,6 +4,7 @@ import ( "context" "crypto/tls" "fmt" + "os" "strings" authService "github.com/cortezaproject/corteza-server/auth" @@ -76,6 +77,18 @@ func (app *CortezaApp) Setup() (err error) { log.Warn("Should be used only for testing") log.Warn("You may experience unstability and data loss") } + + if _, is := os.LookupEnv("MINIO_BUCKET_SEP"); is { + log.Warn("Found MINIO_BUCKET_SEP in environment variables, it has been removed") + + return fmt.Errorf( + "invalid minio configurtion: " + + "found MINIO_BUCKET_SEP in environment variables, " + + "which is removed due to latest versions of min.io " + + "bucket names can only consist of lowercase letters, numbers, dots (.), and hyphens (-). " + + "so instead use environment variable MINIO_BUCKET, " + + "we have extended it to have more flexibility over minio bucket name") + } } hcd := healthcheck.Defaults() diff --git a/compose/service/service.go b/compose/service/service.go index afffde8cf..2a67fdc8b 100644 --- a/compose/service/service.go +++ b/compose/service/service.go @@ -75,8 +75,8 @@ var ( } ) -// Initializes compose-only services -func Initialize(ctx context.Context, log *zap.Logger, s store.Storer, c Config) (err error) { +// Initialize compose-only services +func Initialize(_ context.Context, log *zap.Logger, s store.Storer, c Config) (err error) { var ( hcd = healthcheck.Defaults() ) @@ -103,29 +103,30 @@ func Initialize(ctx context.Context, log *zap.Logger, s store.Storer, c Config) DefaultResourceTranslation = ResourceTranslationsManager(locale.Global()) if DefaultObjectStore == nil { + var ( + opt = c.Storage + bucket string + ) const svcPath = "compose" - if c.Storage.MinioEndpoint != "" { - var bucket = svcPath - if c.Storage.MinioBucket != "" { - bucket = c.Storage.MinioBucket + c.Storage.MinioBucketSep + svcPath - } + if opt.MinioEndpoint != "" { + bucket = minio.GetBucket(opt.MinioBucket, svcPath) - DefaultObjectStore, err = minio.New(bucket, minio.Options{ - Endpoint: c.Storage.MinioEndpoint, - Secure: c.Storage.MinioSecure, - Strict: c.Storage.MinioStrict, - AccessKeyID: c.Storage.MinioAccessKey, - SecretAccessKey: c.Storage.MinioSecretKey, + DefaultObjectStore, err = minio.New(bucket, opt.MinioPathPrefix, svcPath, minio.Options{ + Endpoint: opt.MinioEndpoint, + Secure: opt.MinioSecure, + Strict: opt.MinioStrict, + AccessKeyID: opt.MinioAccessKey, + SecretAccessKey: opt.MinioSecretKey, - ServerSideEncryptKey: []byte(c.Storage.MinioSSECKey), + ServerSideEncryptKey: []byte(opt.MinioSSECKey), }) log.Info("initializing minio", zap.String("bucket", bucket), - zap.String("endpoint", c.Storage.MinioEndpoint), + zap.String("endpoint", opt.MinioEndpoint), zap.Error(err)) } else { - path := c.Storage.Path + "/" + svcPath + path := opt.Path + "/" + svcPath DefaultObjectStore, err = plain.New(path) log.Info("initializing store", zap.String("path", path), diff --git a/pkg/objstore/minio/store.go b/pkg/objstore/minio/store.go index 0e3a0ec52..ff75079b0 100644 --- a/pkg/objstore/minio/store.go +++ b/pkg/objstore/minio/store.go @@ -4,6 +4,7 @@ import ( "context" "fmt" "io" + "strings" minio "github.com/minio/minio-go/v6" "github.com/minio/minio-go/v6/pkg/encrypt" @@ -23,10 +24,20 @@ type ( ServerSideEncryptKey []byte } - store struct { - bucket string + minioClient interface { + BucketExists(bucketName string) (bool, error) + MakeBucket(bucketName string, location string) (err error) + PutObject(bucketName, objectName string, reader io.Reader, objectSize int64, opts minio.PutObjectOptions) (n int64, err error) + RemoveObject(bucketName, objectName string) error + GetObject(bucketName, objectName string, opts minio.GetObjectOptions) (*minio.Object, error) + } - mc *minio.Client + store struct { + bucket string + pathPrefix string + component string + + mc minioClient sse encrypt.ServerSide originalFn func(id uint64, ext string) string @@ -44,10 +55,20 @@ var ( } ) -func New(bucket string, opt Options) (s *store, err error) { +func New(bucket, pathPrefix, component string, opt Options) (s *store, err error) { + client, err := minio.New(opt.Endpoint, opt.AccessKeyID, opt.SecretAccessKey, opt.Secure) + if err != nil { + return nil, err + } + return newWithClient(client, bucket, pathPrefix, component, opt) +} + +func newWithClient(mc minioClient, bucket, pathPrefix, component string, opt Options) (s *store, err error) { s = &store{ - bucket: bucket, - mc: nil, + bucket: bucket, + pathPrefix: pathPrefix, + component: component, + mc: mc, originalFn: defOriginalFn, previewFn: defPreviewFn, @@ -57,10 +78,6 @@ func New(bucket string, opt Options) (s *store, err error) { return nil, err } - if s.mc, err = minio.New(opt.Endpoint, opt.AccessKeyID, opt.SecretAccessKey, opt.Secure); err != nil { - return nil, err - } - if e, err := s.mc.BucketExists(s.bucket); err != nil { return nil, err } else if !e { @@ -104,7 +121,7 @@ func (s store) Preview(id uint64, ext string) string { } func (s store) Save(name string, f io.Reader) (err error) { - _, err = s.mc.PutObject(s.bucket, name, f, -1, minio.PutObjectOptions{ + _, err = s.mc.PutObject(s.bucket, s.getObjectName(name), f, -1, minio.PutObjectOptions{ ServerSideEncryption: s.sse, }) @@ -112,15 +129,26 @@ func (s store) Save(name string, f io.Reader) (err error) { } func (s store) Remove(name string) error { - return s.mc.RemoveObject(s.bucket, name) + return s.mc.RemoveObject(s.bucket, s.getObjectName(name)) } func (s store) Open(name string) (io.ReadSeeker, error) { - return s.mc.GetObject(s.bucket, name, minio.GetObjectOptions{ + return s.mc.GetObject(s.bucket, s.getObjectName(name), minio.GetObjectOptions{ ServerSideEncryption: s.sse, }) } -func (s *store) Healthcheck(ctx context.Context) error { +func (s *store) Healthcheck(_ context.Context) error { return nil } + +// getObjectName prefix path to object name +func (s *store) getObjectName(name string) (out string) { + path := strings.Replace(s.pathPrefix, "{component}", s.component, 1) + return fmt.Sprintf("%s%s", path, name) +} + +// GetBucket return bucket name based on storage option bucket, separator or bucketName +func GetBucket(bucket, component string) string { + return strings.Replace(bucket, "{component}", component, 1) +} diff --git a/pkg/objstore/minio/store_test.go b/pkg/objstore/minio/store_test.go new file mode 100644 index 000000000..2b218bd59 --- /dev/null +++ b/pkg/objstore/minio/store_test.go @@ -0,0 +1,234 @@ +package minio + +import ( + "bytes" + "fmt" + "github.com/minio/minio-go/v6" + "github.com/minio/minio-go/v6/pkg/s3utils" + "github.com/stretchr/testify/require" + "io" + "testing" +) + +type ( + testMinio struct{} +) + +func (t testMinio) BucketExists(bucketName string) (out bool, err error) { + return +} + +func (t testMinio) MakeBucket(bucketName string, location string) (err error) { + return +} + +func (t testMinio) PutObject(bucketName, objectName string, reader io.Reader, objectSize int64, opts minio.PutObjectOptions) (n int64, err error) { + return +} + +func (t testMinio) RemoveObject(bucketName, objectName string) (err error) { + return +} + +func (t testMinio) GetObject(bucketName, objectName string, opts minio.GetObjectOptions) (out *minio.Object, err error) { + return +} + +func TestBucketName(t *testing.T) { + type ( + tf struct { + // Input + bucketName string + // Expected result + errMsg string + // Flag to indicate whether test should Pass + valid bool + } + ) + + var ( + req = require.New(t) + tcc = []tf{ + { + bucketName: ".testbucket", + errMsg: "Bucket name contains invalid characters", + valid: false, + }, + { + bucketName: "testbucket.", + errMsg: "Bucket name contains invalid characters", + valid: false, + }, + { + bucketName: "testbucket-", + errMsg: "Bucket name contains invalid characters", + valid: false, + }, + { + bucketName: "testbucket/", + errMsg: "Bucket name contains invalid characters", + valid: false, + }, + + { + bucketName: "te", + errMsg: "Bucket name cannot be smaller than 3 characters", + valid: false, + }, + { + bucketName: "", + errMsg: "Bucket name cannot be empty", + valid: false, + }, + { + bucketName: "test..bucket", + errMsg: "Bucket name contains invalid characters", + valid: false, + }, + { + bucketName: "test.bucket.com", + errMsg: "", + valid: true, + }, + { + bucketName: "test-bucket", + errMsg: "", + valid: true, + }, + { + bucketName: "123test-bucket", + errMsg: "", + valid: true, + }, + } + ) + + for i, tc := range tcc { + _ = req + _ = i + + err := s3utils.CheckValidBucketName(tc.bucketName) + if tc.errMsg != "" { + req.Equal(tc.errMsg, err.Error(), tc.errMsg) + } else { + req.NoError(err) + } + } +} + +func TestStore(t *testing.T) { + type ( + tf struct { + name string + bucket string + pathPrefix string + componentName string + expectedBucketName string + expectedError error + } + ) + + var ( + mc testMinio + tcc = []tf{ + { + name: "default bucket", + bucket: "{component}", + componentName: "test", + expectedBucketName: "test", + }, + { + name: "custom bucket", + bucket: "corteza-{component}", + componentName: "test", + expectedBucketName: "corteza-test", + }, + { + name: "custom bucket", + bucket: "corteza-{component}", + componentName: "test", + expectedBucketName: "corteza-test", + }, + { + name: "custom bucket", + bucket: "corteza-{component}", + componentName: "test", + expectedBucketName: "corteza-test", + }, + { + name: "bucket has invalid character(/)", + bucket: "corteza/{component}", + componentName: "test", + expectedError: fmt.Errorf("Bucket name contains invalid characters"), + }, + { + name: "bucket has invalid character(-) at the end of the name", + bucket: "{component}-", + componentName: "test", + expectedError: fmt.Errorf("Bucket name contains invalid characters"), + }, + } + ) + + for _, tc := range tcc { + t.Run(tc.name, func(t *testing.T) { + var ( + req = require.New(t) + bucket = GetBucket(tc.bucket, tc.componentName) + ) + + store, err := newWithClient(mc, bucket, tc.pathPrefix, tc.componentName, Options{}) + req.Equal(tc.expectedError, err) + if tc.expectedError == nil { + req.Equal(tc.expectedBucketName, store.bucket, "Unexpected bucket name") + } + + if store != nil { + { + fn := store.Original(123, "txt") + expected := "123.txt" + req.True(fn == expected, "Unexpected filename returned: %s != %s", expected, fn) + } + + { + fn := store.Preview(123, "txt") + expected := "123_preview.txt" + req.True(fn == expected, "Unexpected filename returned: %s != %s", expected, fn) + } + + // @todo extend below test to check expected path, content of the object + // after write, read and delete + // write a file + { + buf := bytes.NewBuffer([]byte("This is a testing buffer")) + err := store.Save("test/123.txt", buf) + req.True(err == nil, "Error saving file, %+v", err) + + err = store.Save("test123/123.txt", buf) + req.True(err == nil, "Expected error when saving file outside of namespace") + } + + // read a file + { + _, err := store.Open("test/123.txt") + req.True(err == nil, "Unexpected error when reading file: %+v", err) + + _, err = store.Open("test/1234.txt") + req.True(err == nil, "Expected error when opening non-existent file") + _, err = store.Open("test123/123.txt") + req.True(err == nil, "Expected error when opening file outside of namespace") + } + + // delete a file + { + err := store.Remove("test/123.txt") + req.True(err == nil, "Unexpected error when removing file: %+v", err) + err = store.Remove("test/123.txt") + req.True(err == nil, "Expected error when removing missing file") + err = store.Remove("test123/123.txt") + req.True(err == nil, "Expected error when deleting file outside of namespace") + } + } + }) + } +} diff --git a/pkg/options/objectStore.gen.go b/pkg/options/objectStore.gen.go index 447ae0fab..1cccf4a47 100644 --- a/pkg/options/objectStore.gen.go +++ b/pkg/options/objectStore.gen.go @@ -10,25 +10,25 @@ package options type ( ObjectStoreOpt struct { - Path string `env:"STORAGE_PATH"` - MinioEndpoint string `env:"MINIO_ENDPOINT"` - MinioSecure bool `env:"MINIO_SECURE"` - MinioAccessKey string `env:"MINIO_ACCESS_KEY"` - MinioSecretKey string `env:"MINIO_SECRET_KEY"` - MinioSSECKey string `env:"MINIO_SSEC_KEY"` - MinioBucket string `env:"MINIO_BUCKET"` - MinioBucketSep string `env:"MINIO_BUCKET_SEP"` - MinioStrict bool `env:"MINIO_STRICT"` + Path string `env:"STORAGE_PATH"` + MinioEndpoint string `env:"MINIO_ENDPOINT"` + MinioSecure bool `env:"MINIO_SECURE"` + MinioAccessKey string `env:"MINIO_ACCESS_KEY"` + MinioSecretKey string `env:"MINIO_SECRET_KEY"` + MinioSSECKey string `env:"MINIO_SSEC_KEY"` + MinioBucket string `env:"MINIO_BUCKET"` + MinioPathPrefix string `env:"MINIO_PATH_PREFIX"` + MinioStrict bool `env:"MINIO_STRICT"` } ) // ObjectStore initializes and returns a ObjectStoreOpt with default values func ObjectStore() (o *ObjectStoreOpt) { o = &ObjectStoreOpt{ - Path: "var/store", - MinioSecure: true, - MinioBucketSep: "/", - MinioStrict: false, + Path: "var/store", + MinioSecure: true, + MinioBucket: "{component}", + MinioStrict: false, } fill(o) diff --git a/pkg/options/objectStore.yaml b/pkg/options/objectStore.yaml index b8c54fa85..8083d32f9 100644 --- a/pkg/options/objectStore.yaml +++ b/pkg/options/objectStore.yaml @@ -29,11 +29,14 @@ props: - name: minioBucket env: MINIO_BUCKET + default: "{component}" + description: |- + `component` placeholder is replaced with service name (e.g system). - - name: minioBucketSep - env: MINIO_BUCKET_SEP - default: "/" - description: Used between MINIO_BUCKET and the service name (e.g system). Ignored if MINIO_BUCKET is not set. Required in latest versions of min.io since "/" is not accepted anymore in bucket names. + - name: minioPathPrefix + env: MINIO_PATH_PREFIX + description: |- + `component` placeholder is replaced with service name (e.g system). - name: minioStrict type: bool diff --git a/system/service/service.go b/system/service/service.go index f34adb01f..ca12f5591 100644 --- a/system/service/service.go +++ b/system/service/service.go @@ -125,29 +125,30 @@ func Initialize(ctx context.Context, log *zap.Logger, s store.Storer, ws websock DefaultSettings = Settings(ctx, DefaultStore, DefaultLogger, DefaultAccessControl, CurrentSettings) if DefaultObjectStore == nil { + var ( + opt = c.Storage + bucket string + ) const svcPath = "system" - if c.Storage.MinioEndpoint != "" { - var bucket = svcPath - if c.Storage.MinioBucket != "" { - bucket = c.Storage.MinioBucket + c.Storage.MinioBucketSep + svcPath - } + if opt.MinioEndpoint != "" { + bucket = minio.GetBucket(opt.MinioBucket, svcPath) - DefaultObjectStore, err = minio.New(bucket, minio.Options{ - Endpoint: c.Storage.MinioEndpoint, - Secure: c.Storage.MinioSecure, - Strict: c.Storage.MinioStrict, - AccessKeyID: c.Storage.MinioAccessKey, - SecretAccessKey: c.Storage.MinioSecretKey, + DefaultObjectStore, err = minio.New(bucket, opt.MinioPathPrefix, svcPath, minio.Options{ + Endpoint: opt.MinioEndpoint, + Secure: opt.MinioSecure, + Strict: opt.MinioStrict, + AccessKeyID: opt.MinioAccessKey, + SecretAccessKey: opt.MinioSecretKey, - ServerSideEncryptKey: []byte(c.Storage.MinioSSECKey), + ServerSideEncryptKey: []byte(opt.MinioSSECKey), }) log.Info("initializing minio", zap.String("bucket", bucket), - zap.String("endpoint", c.Storage.MinioEndpoint), + zap.String("endpoint", opt.MinioEndpoint), zap.Error(err)) } else { - path := c.Storage.Path + "/" + svcPath + path := opt.Path + "/" + svcPath DefaultObjectStore, err = plain.New(path) log.Info("initializing store", zap.String("path", path),