diff --git a/.gitignore b/.gitignore index 9ec9695351..9f8cad8669 100644 --- a/.gitignore +++ b/.gitignore @@ -10,6 +10,7 @@ webapp/yarn-error.log mattermost.mattermost-license config/mattermost.mattermost-license config/config.json +config/*.crt web/static/js/bundle*.js web/static/js/bundle*.js.map diff --git a/api4/system_test.go b/api4/system_test.go index 91b0bc7d83..dfe0379892 100644 --- a/api4/system_test.go +++ b/api4/system_test.go @@ -509,9 +509,10 @@ func TestS3TestConnection(t *testing.T) { CheckInternalErrorStatus(t, resp) assert.Equal(t, "api.file.test_connection.app_error", resp.Error.Id) - *config.FileSettings.AmazonS3Bucket = "shouldcreatenewbucket" + *config.FileSettings.AmazonS3Bucket = "shouldnotcreatenewbucket" _, resp = th.SystemAdminClient.TestS3Connection(&config) - CheckOKStatus(t, resp) + CheckInternalErrorStatus(t, resp) + assert.Equal(t, "api.file.test_connection.app_error", resp.Error.Id) }) t.Run("as restricted system admin", func(t *testing.T) { diff --git a/app/server.go b/app/server.go index c73642896d..47b16ae03d 100644 --- a/app/server.go +++ b/app/server.go @@ -529,8 +529,14 @@ func NewServer(options ...Option) (*Server, error) { if appErr != nil { mlog.Error("Problem with file storage settings", mlog.Err(appErr)) } else { - if nErr := backend.TestConnection(); nErr != nil { - mlog.Error("Problem with file storage settings", mlog.Err(nErr)) + nErr := backend.TestConnection() + if nErr != nil { + if errors.Is(nErr, filestore.ErrNoS3Bucket) { + nErr = backend.(*filestore.S3FileBackend).MakeBucket() + } + if nErr != nil { + mlog.Error("Problem with file storage settings", mlog.Err(nErr)) + } } } diff --git a/app/server_test.go b/app/server_test.go index 6b14749e61..d25dbd7fa3 100644 --- a/app/server_test.go +++ b/app/server_test.go @@ -26,6 +26,7 @@ import ( "github.com/mattermost/mattermost-server/v5/config" "github.com/mattermost/mattermost-server/v5/model" + "github.com/mattermost/mattermost-server/v5/shared/filestore" "github.com/mattermost/mattermost-server/v5/shared/mlog" "github.com/mattermost/mattermost-server/v5/store/storetest" "github.com/mattermost/mattermost-server/v5/utils/fileutils" @@ -138,6 +139,46 @@ func TestStartServerPortUnavailable(t *testing.T) { require.Error(t, serverErr) } +func TestStartServerNoS3Bucket(t *testing.T) { + s3Host := os.Getenv("CI_MINIO_HOST") + if s3Host == "" { + s3Host = "localhost" + } + + s3Port := os.Getenv("CI_MINIO_PORT") + if s3Port == "" { + s3Port = "9000" + } + + s3Endpoint := fmt.Sprintf("%s:%s", s3Host, s3Port) + + s, err := NewServer(func(server *Server) error { + configStore, _ := config.NewFileStore("config.json", true) + store, _ := config.NewStoreFromBacking(configStore, nil, false) + server.configStore = store + server.UpdateConfig(func(cfg *model.Config) { + cfg.FileSettings = model.FileSettings{ + DriverName: model.NewString(model.IMAGE_DRIVER_S3), + AmazonS3AccessKeyId: model.NewString(model.MINIO_ACCESS_KEY), + AmazonS3SecretAccessKey: model.NewString(model.MINIO_SECRET_KEY), + AmazonS3Bucket: model.NewString("nosuchbucket"), + AmazonS3Endpoint: model.NewString(s3Endpoint), + AmazonS3Region: model.NewString(""), + AmazonS3PathPrefix: model.NewString(""), + AmazonS3SSL: model.NewBool(false), + } + }) + return nil + }) + require.NoError(t, err) + + // ensure that a new bucket was created + backend, err := s.FileBackend() + require.Nil(t, err) + err = backend.(*filestore.S3FileBackend).TestConnection() + require.Nil(t, err) +} + func TestStartServerTLSSuccess(t *testing.T) { s, err := NewServer() require.NoError(t, err) diff --git a/shared/filestore/s3store.go b/shared/filestore/s3store.go index a7a59c7410..6e7c49f0cd 100644 --- a/shared/filestore/s3store.go +++ b/shared/filestore/s3store.go @@ -41,6 +41,8 @@ const ( bucketNotFound = "NoSuchBucket" ) +// ErrNoS3Bucket is returned when testing a connection and no S3 bucket is found +var ErrNoS3Bucket = errors.New("no such bucket") var ( imageExtensions = map[string]bool{".jpg": true, ".jpeg": true, ".gif": true, ".bmp": true, ".png": true, ".tiff": true, "tif": true} imageMimeTypes = map[string]string{".jpg": "image/jpeg", ".jpeg": "image/jpeg", ".gif": "image/gif", ".bmp": "image/bmp", ".png": "image/png", ".tiff": "image/tiff", ".tif": "image/tif"} @@ -157,16 +159,18 @@ func (b *S3FileBackend) TestConnection() error { } } - if exists { - mlog.Debug("Connection to S3 or minio is good. Bucket exists.") - } else { - mlog.Warn("Bucket specified does not exist. Attempting to create...") - err := b.client.MakeBucket(context.Background(), b.bucket, s3.MakeBucketOptions{Region: b.region}) - if err != nil { - return errors.Wrap(err, "unable to create the s3 bucket") - } + if !exists { + return ErrNoS3Bucket } + mlog.Debug("Connection to S3 or minio is good. Bucket exists.") + return nil +} +func (b *S3FileBackend) MakeBucket() error { + err := b.client.MakeBucket(context.Background(), b.bucket, s3.MakeBucketOptions{Region: b.region}) + if err != nil { + return errors.Wrap(err, "unable to create the s3 bucket") + } return nil } diff --git a/shared/filestore/s3store_test.go b/shared/filestore/s3store_test.go index d8b1fb69e8..0bbe6be4e8 100644 --- a/shared/filestore/s3store_test.go +++ b/shared/filestore/s3store_test.go @@ -4,11 +4,23 @@ package filestore import ( + "crypto/rand" + "encoding/base64" + "fmt" + "os" + "strings" "testing" "github.com/stretchr/testify/require" ) +// Copied from model/config.go to avoid an import cycle +const ( + MinioAccessKey = "minioaccesskey" + MinioSecretKey = "miniosecretkey" + ImageDriverS3 = "amazons3" +) + func TestCheckMandatoryS3Fields(t *testing.T) { cfg := FileBackendSettings{} @@ -26,3 +38,42 @@ func TestCheckMandatoryS3Fields(t *testing.T) { require.Equal(t, "s3.amazonaws.com", cfg.AmazonS3Endpoint, "should've set the endpoint to the default") } + +func TestMakeBucket(t *testing.T) { + s3Host := os.Getenv("CI_MINIO_HOST") + if s3Host == "" { + s3Host = "localhost" + } + + s3Port := os.Getenv("CI_MINIO_PORT") + if s3Port == "" { + s3Port = "9000" + } + + s3Endpoint := fmt.Sprintf("%s:%s", s3Host, s3Port) + + // Generate a random bucket name + b := make([]byte, 30) + rand.Read(b) + bucketName := base64.StdEncoding.EncodeToString(b) + bucketName = strings.ToLower(bucketName) + bucketName = strings.Replace(bucketName, "+", "", -1) + bucketName = strings.Replace(bucketName, "/", "", -1) + + cfg := FileBackendSettings{ + DriverName: ImageDriverS3, + AmazonS3AccessKeyId: MinioAccessKey, + AmazonS3SecretAccessKey: MinioSecretKey, + AmazonS3Bucket: bucketName, + AmazonS3Endpoint: s3Endpoint, + AmazonS3Region: "", + AmazonS3PathPrefix: "", + AmazonS3SSL: false, + } + + fileBackend, err := NewS3FileBackend(cfg) + require.NoError(t, err) + + err = fileBackend.MakeBucket() + require.NoError(t, err) +}