APPS-1184 Fix remote configuration

cmd/backup/main.go

@@ -53,7 +53,7 @@ func run() int {
 	rootCmd.Flags().BoolVarP(&remote, "remote", "r", false, "use remote config file")
 
 	rootCmd.RunE = func(_ *cobra.Command, _ []string) error {
-		manager, err := service.NewConfigurationManager(configFile, remote)
+		manager, err := service.NewConfigManagerBuilder().NewConfigManager(configFile, remote)
 		if err != nil {
 			return err
 		}

go.mod

@@ -53,6 +53,7 @@ require (
 	github.com/prometheus/procfs v0.12.0 // indirect
 	github.com/rogpeppe/go-internal v1.11.0 // indirect
 	github.com/spf13/pflag v1.0.5 // indirect
+	github.com/stretchr/objx v0.5.2 // indirect
 	github.com/swaggo/files/v2 v2.0.0 // indirect
 	github.com/yuin/gopher-lua v1.1.1 // indirect
 	golang.org/x/net v0.20.0 // indirect

go.sum

@@ -111,6 +111,8 @@ github.com/spf13/cobra v1.8.0/go.mod h1:WXLWApfZ71AjXPya3WOlMsY9yMs7YeiHhFVlvLyh
 github.com/spf13/pflag v1.0.5 h1:iy+VFUOCP1a+8yFto/drg2CJ5u0yRoB7fZw3DKv/JXA=
 github.com/spf13/pflag v1.0.5/go.mod h1:McXfInJRrz4CZXVZOBLb0bTZqETkiAhM9Iw0y3An2Bg=
 github.com/stretchr/objx v0.1.0/go.mod h1:HFkY916IF+rwdDfMAkV7OtwuqBVzrE8GR6GFx+wExME=
+github.com/stretchr/objx v0.5.2 h1:xuMeJ0Sdp5ZMRXx/aWO6RZxdr3beISkG5/G/aIRr3pY=
+github.com/stretchr/objx v0.5.2/go.mod h1:FRsXN1f5AsAjCGJKqEizvkpNtU+EGNCLh3NxZ/8L+MA=
 github.com/stretchr/testify v1.3.0/go.mod h1:M5WIy9Dh21IEIfnGCwXGc5bZfKNJtfHm1UVUgZn+9EI=
 github.com/stretchr/testify v1.6.1/go.mod h1:6Fq8oRcR53rry900zMqJjRRixrwX3KX962/h/Wwjteg=
 github.com/stretchr/testify v1.9.0 h1:HtqpIVDClZ4nwg75+f6Lvsy/wHu+3BoSGCbBAcpTsTg=

pkg/service/configuration_manager.go

@@ -2,6 +2,7 @@ package service
 
 import (
 	"bytes"
+	"fmt"
 	"net/http"
 	"net/url"
 	"os"
@@ -15,38 +16,88 @@ type ConfigurationManager interface {
 	WriteConfiguration(config *model.Config) error
 }
 
-func NewConfigurationManager(configFile string, remote bool) (ConfigurationManager, error) {
-	uri, err := url.Parse(configFile)
+type Reader interface {
+	read(string) ([]byte, error)
+}
+
+type HTTPReader struct{}
+
+func (h HTTPReader) read(url string) ([]byte, error) {
+	resp, err := http.Get(url)
 	if err != nil {
 		return nil, err
 	}
+	defer resp.Body.Close()
+	buf := new(bytes.Buffer)
+	_, err = buf.ReadFrom(resp.Body)
+	if err != nil {
+		return nil, err
+	}
+	return buf.Bytes(), nil
+}
 
-	isDownload := uri.Scheme == "http" || uri.Scheme == "https"
+type FileReader struct{}
 
-	if remote {
-		return remoteConfigurationManager(configFile, isDownload)
+func (f FileReader) read(url string) ([]byte, error) {
+	return os.ReadFile(url)
+}
+
+type ConfigManagerBuilder struct {
+	http      Reader
+	file      Reader
+	s3Builder S3ManagerBuilder
+}
+
+func NewConfigManagerBuilder() *ConfigManagerBuilder {
+	return &ConfigManagerBuilder{
+		http:      HTTPReader{},
+		file:      FileReader{},
+		s3Builder: S3ManagerBuilderImpl{},
 	}
-	if isDownload {
-		return NewHTTPConfigurationManager(configFile), nil
+}
+
+func (b *ConfigManagerBuilder) NewConfigManager(configFile string, remote bool) (ConfigurationManager, error) {
+	configStorage, err := b.makeConfigStorage(configFile, remote)
+	if err != nil {
+		return nil, err
+	}
+
+	switch configStorage.Type {
+	case model.S3:
+		return b.s3Builder.NewS3ConfigurationManager(configStorage)
+	case model.Local:
+		return newLocalConfigurationManager(configStorage)
+	default:
+		return nil, fmt.Errorf("unknown type %d", configStorage.Type)
 	}
-	return NewFileConfigurationManager(configFile), nil
 }
 
-func remoteConfigurationManager(configFile string, isDownload bool) (ConfigurationManager, error) {
-	var buf []byte
-	var err error
+func newLocalConfigurationManager(configStorage *model.Storage) (ConfigurationManager, error) {
+	isHttp, err := isHttpPath(*configStorage.Path)
+	if err != nil {
+		return nil, err
+	}
+	if isHttp {
+		return NewHTTPConfigurationManager(*configStorage.Path), nil
+	}
+	return NewFileConfigurationManager(*configStorage.Path), nil
+}
 
-	if isDownload {
-		buf, err = download(configFile)
-	} else {
-		buf, err = os.ReadFile(configFile)
+func (b *ConfigManagerBuilder) makeConfigStorage(configUri string, remote bool) (*model.Storage, error) {
+	if !remote {
+		return &model.Storage{
+			Type: model.Local,
+			Path: &configUri,
+		}, nil
 	}
+
+	content, err := b.loadFileContent(configUri)
 	if err != nil {
 		return nil, err
 	}
 
 	configStorage := &model.Storage{}
-	err = yaml.Unmarshal(buf, configStorage)
+	err = yaml.Unmarshal(content, configStorage)
 	if err != nil {
 		return nil, err
 	}
@@ -55,27 +106,26 @@ func remoteConfigurationManager(configFile string, isDownload bool) (Configurati
 	if err != nil {
 		return nil, err
 	}
-
-	switch configStorage.Type {
-	case model.S3:
-		return NewS3ConfigurationManager(configStorage)
-	default:
-		return NewFileConfigurationManager(*configStorage.Path), nil
-	}
+	return configStorage, nil
 }
 
-func download(url string) ([]byte, error) {
-	resp, err := http.Get(url)
+func (b *ConfigManagerBuilder) loadFileContent(configFile string) ([]byte, error) {
+	isDownload, err := isHttpPath(configFile)
 	if err != nil {
 		return nil, err
 	}
-	defer resp.Body.Close()
+	if isDownload {
+		return b.http.read(configFile)
+	} else {
+		return b.file.read(configFile)
+	}
+}
 
-	buf := new(bytes.Buffer)
-	_, err = buf.ReadFrom(resp.Body)
+func isHttpPath(path string) (bool, error) {
+	uri, err := url.Parse(path)
 	if err != nil {
-		return nil, err
+		return false, err
 	}
 
-	return buf.Bytes(), nil
+	return uri.Scheme == "http" || uri.Scheme == "https", nil
 }

pkg/service/configuration_manager_s3.go

@@ -6,16 +6,25 @@ import (
 	"github.com/aerospike/backup/pkg/model"
 )
 
-// FileConfigurationManager implements the ConfigurationManager interface,
+// S3ConfigurationManager implements the ConfigurationManager interface,
 // performing I/O operations on AWS S3.
 type S3ConfigurationManager struct {
 	*S3Context
 }
 
 var _ ConfigurationManager = (*S3ConfigurationManager)(nil)
 
-// NewS3ConfigurationManager returns a new S3ConfigurationManager.
-func NewS3ConfigurationManager(configStorage *model.Storage) (ConfigurationManager, error) {
+// S3ManagerBuilder defines the interface for building S3ConfigurationManager.
+type S3ManagerBuilder interface {
+	// NewS3ConfigurationManager returns a new S3ConfigurationManager.
+	NewS3ConfigurationManager(configStorage *model.Storage) (ConfigurationManager, error)
+}
+
+type S3ManagerBuilderImpl struct{}
+
+var _ S3ManagerBuilder = &S3ManagerBuilderImpl{}
+
+func (builder S3ManagerBuilderImpl) NewS3ConfigurationManager(configStorage *model.Storage) (ConfigurationManager, error) {
 	s3Context, err := NewS3Context(configStorage)
 	if err != nil {
 		return nil, err

pkg/service/configuration_manager_test.go

@@ -0,0 +1,157 @@
+package service
+
+import (
+	"errors"
+	"reflect"
+	"testing"
+
+	"github.com/aerospike/backup/pkg/model"
+	"github.com/stretchr/testify/mock"
+	"github.com/stretchr/testify/require"
+)
+
+type MockDownloader struct {
+	mock.Mock
+}
+
+func (m *MockDownloader) read(configFile string) ([]byte, error) {
+	args := m.Called(configFile)
+	return args.Get(0).([]byte), args.Error(1)
+}
+
+type MockS3Builder struct {
+}
+
+func (m *MockS3Builder) NewS3ConfigurationManager(storage *model.Storage) (ConfigurationManager, error) {
+	if storage.Type == model.S3 {
+		return &S3ConfigurationManager{}, nil
+	}
+	return nil, errors.New("wrong type")
+}
+
+func TestConfigManagerBuilder_NewConfigManager(t *testing.T) {
+	mockLocal := new(MockDownloader)
+	mockHttp := new(MockDownloader)
+
+	tests := []struct {
+		name         string
+		configFile   string
+		remote       bool
+		setMock      func()
+		expectError  bool
+		expectedType reflect.Type
+	}{
+		// Configuration file is passed straight to the service.
+		{
+			name:         "local non-remote",
+			configFile:   "config.yaml",
+			remote:       false,
+			setMock:      func() {},
+			expectError:  false,
+			expectedType: reflect.TypeOf(&FileConfigurationManager{}),
+		},
+		{
+			name:         "http non-remote",
+			configFile:   "https://example.com/config.yaml",
+			remote:       false,
+			setMock:      func() {},
+			expectError:  false,
+			expectedType: reflect.TypeOf(&HTTPConfigurationManager{}),
+		},
+		// Open/download remote config file, and based on it's content open/download backup config.
+		{
+			name:       "local remote local configuration",
+			configFile: "/path/to/remote.yaml",
+			remote:     true,
+			setMock: func() {
+				mockLocal.
+					On("read", "/path/to/remote.yaml").
+					Return([]byte("path: config.yaml"), nil)
+			},
+			expectError:  false,
+			expectedType: reflect.TypeOf(&FileConfigurationManager{}),
+		},
+		{
+			name:       "local remote http configuration",
+			configFile: "/path/to/remote.yaml",
+			remote:     true,
+			setMock: func() {
+				mockLocal.
+					On("read", "/path/to/remote.yaml").
+					Return([]byte("path: https://example.com/config.yaml"), nil)
+			},
+			expectError:  false,
+			expectedType: reflect.TypeOf(&HTTPConfigurationManager{}),
+		},
+		{
+			name:       "http remote local configuration",
+			configFile: "https://example.com/config.yaml",
+			remote:     true,
+			setMock: func() {
+				mockHttp.
+					On("read", "https://example.com/config.yaml").
+					Return([]byte("path: config.yaml"), nil)
+			},
+			expectError:  false,
+			expectedType: reflect.TypeOf(&FileConfigurationManager{}),
+		},
+		{
+			name:       "http remote http",
+			configFile: "http://path/to/remote.yaml",
+			remote:     true,
+			setMock: func() {
+				mockHttp.
+					On("read", "http://path/to/remote.yaml").
+					Return([]byte("path: https://example.com/config.yaml"), nil)
+			},
+			expectError:  false,
+			expectedType: reflect.TypeOf(&HTTPConfigurationManager{}),
+		},
+		// S3 configuration, we can read it from local file or by http.
+		{
+			name:       "local s3",
+			configFile: "config.yaml",
+			remote:     true,
+			setMock: func() {
+				mockLocal.
+					On("read", "config.yaml").
+					Return([]byte("type: 1\npath: s3://bucket/config.yaml\ns3-region: europe"), nil)
+			},
+			expectError:  false,
+			expectedType: reflect.TypeOf(&S3ConfigurationManager{}),
+		},
+		{
+			name:       "http s3",
+			configFile: "https://example.com/config.yaml",
+			remote:     true,
+			setMock: func() {
+				mockHttp.
+					On("read", "https://example.com/config.yaml").
+					Return([]byte("type: 1\npath: s3://bucket/config.yaml\ns3-region: europe"), nil)
+			},
+			expectError:  false,
+			expectedType: reflect.TypeOf(&S3ConfigurationManager{}),
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			mockLocal = new(MockDownloader)
+			mockHttp = new(MockDownloader)
+			builder := &ConfigManagerBuilder{
+				http:      mockHttp,
+				file:      mockLocal,
+				s3Builder: &MockS3Builder{},
+			}
+			tt.setMock()
+			config, err := builder.NewConfigManager(tt.configFile, tt.remote)
+			if tt.expectError {
+				require.Error(t, err)
+			} else {
+				require.NoError(t, err)
+			}
+			configType := reflect.TypeOf(config)
+			require.Equal(t, tt.expectedType.String(), configType.String())
+		})
+	}
+}