Use document filesystem in config object to read/write files

It allows us to have more clear and controllable unit tests and
to move towards bringing all file operations in airshipctl to a
common standard for all source files (using only document
filesystem module).

Change-Id: I560edb7c8e9cd682f3c188527245fcd40045e16c
Signed-off-by: Ruslan Aliev <raliev@mirantis.com>
Relates-To: #415

pkg/config/config.go

@@ -16,13 +16,13 @@ package config
 
 import (
 	"fmt"
-	"io/ioutil"
 	"os"
 	"path/filepath"
 	"sort"
 
 	"sigs.k8s.io/yaml"
 
+	"opendev.org/airship/airshipctl/pkg/fs"
 	"opendev.org/airship/airshipctl/pkg/log"
 	"opendev.org/airship/airshipctl/pkg/util"
 )
@@ -62,6 +62,7 @@ type Config struct {
 	// file from which this config was loaded
 	// +not persisted in file
 	loadedConfigPath string
+	fileSystem       fs.FileSystem
 }
 
 // Permissions has the permissions for file and directory
@@ -125,11 +126,12 @@ func (c *Config) initConfigPath(airshipConfigPath string) {
 func (c *Config) LoadConfig() error {
 	// If I can read from the file, load from it
 	// throw an error otherwise
-	if _, err := os.Stat(c.loadedConfigPath); err != nil {
+	data, err := c.fileSystem.ReadFile(c.loadedConfigPath)
+	if err != nil {
 		return err
 	}
 
-	return util.ReadYAMLFile(c.loadedConfigPath, c)
+	return yaml.Unmarshal(data, c)
 }
 
 // EnsureComplete verifies that a Config object is ready to use.
@@ -189,26 +191,26 @@ func (c *Config) PersistConfig(overwrite bool) error {
 	}
 
 	// WriteFile doesn't create the directory, create it if needed
-	configDir := filepath.Dir(c.loadedConfigPath)
-	err = os.MkdirAll(configDir, os.FileMode(c.Permissions.DirectoryPermission))
-	if err != nil {
-		return err
-	}
-
-	// Write the Airship Config file
-	err = ioutil.WriteFile(c.loadedConfigPath, airshipConfigYaml, os.FileMode(c.Permissions.FilePermission))
+	dir := c.fileSystem.Dir(c.loadedConfigPath)
+	err = c.fileSystem.MkdirAll(dir)
 	if err != nil {
 		return err
 	}
 
 	// Change the permission of directory
-	err = os.Chmod(configDir, os.FileMode(c.Permissions.DirectoryPermission))
+	err = c.fileSystem.Chmod(dir, os.FileMode(c.Permissions.DirectoryPermission))
+	if err != nil {
+		return err
+	}
+
+	// Write the Airship Config file
+	err = c.fileSystem.WriteFile(c.loadedConfigPath, airshipConfigYaml)
 	if err != nil {
 		return err
 	}
 
 	// Change the permission of config file
-	err = os.Chmod(c.loadedConfigPath, os.FileMode(c.Permissions.FilePermission))
+	err = c.fileSystem.Chmod(c.loadedConfigPath, os.FileMode(c.Permissions.FilePermission))
 	if err != nil {
 		return err
 	}
@@ -551,7 +553,7 @@ func (c *Config) CurrentContextManagementConfig() (*ManagementConfiguration, err
 
 // Purge removes the config file
 func (c *Config) Purge() error {
-	return os.Remove(c.loadedConfigPath)
+	return c.fileSystem.RemoveAll(c.loadedConfigPath)
 }
 
 // CurrentContextManifestMetadata gets manifest metadata
@@ -569,7 +571,13 @@ func (c *Config) CurrentContextManifestMetadata() (*Metadata, error) {
 		Inventory: &InventoryMeta{},
 		PhaseMeta: &PhaseMeta{},
 	}
-	err = util.ReadYAMLFile(filepath.Join(manifest.TargetPath, phaseRepoDir, manifest.MetadataPath), meta)
+
+	data, err := c.fileSystem.ReadFile(filepath.Join(manifest.TargetPath, phaseRepoDir, manifest.MetadataPath))
+	if err != nil {
+		return nil, err
+	}
+
+	err = yaml.Unmarshal(data, meta)
 	if err != nil {
 		return nil, err
 	}

pkg/config/config_test.go

@@ -16,7 +16,9 @@ package config_test
 
 import (
 	"fmt"
+	"io/ioutil"
 	"os"
+	"path/filepath"
 	"strings"
 	"testing"
 
@@ -96,19 +98,14 @@ func TestLoadConfig(t *testing.T) {
 }
 
 func TestPersistConfig(t *testing.T) {
-	conf, cleanup := testutil.InitConfig(t)
-	defer cleanup(t)
-
-	conf.SetLoadedConfigPath(conf.LoadedConfigPath() + ".new")
-
-	err := conf.PersistConfig(true)
+	testDir, err := ioutil.TempDir("", "airship-test")
+	require.NoError(t, err)
+	configPath := filepath.Join(testDir, "config")
+	err = config.CreateConfig(configPath, true)
+	require.NoError(t, err)
+	assert.FileExists(t, configPath)
+	err = os.RemoveAll(testDir)
 	require.NoError(t, err)
-
-	// Check that the files were created
-	assert.FileExists(t, conf.LoadedConfigPath())
-
-	err = conf.PersistConfig(false)
-	require.Error(t, err, config.ErrConfigFileExists{Path: conf.LoadedConfigPath()})
 }
 
 func TestEnsureComplete(t *testing.T) {

pkg/config/utils.go

@@ -16,6 +16,8 @@ package config
 
 import (
 	"encoding/base64"
+
+	"opendev.org/airship/airshipctl/pkg/fs"
 )
 
 // NewConfig returns a newly initialized Config object
@@ -53,6 +55,7 @@ func NewConfig() *Config {
 				MetadataPath:            DefaultManifestMetadataFile,
 			},
 		},
+		fileSystem: fs.NewDocumentFs(),
 	}
 }
 

pkg/phase/helper_test.go

@@ -555,9 +555,9 @@ manifests:
     metadataPath: valid_site/metadata.yaml
     repositories:
       primary:
-        url: "empty/filename/"
-`
-	conf := &config.Config{}
+        url: "empty/filename/"`
+
+	conf := config.NewConfig()
 	err := yaml.Unmarshal([]byte(confString), conf)
 	require.NoError(t, err)
 	return conf

pkg/util/configreader.go

@@ -1,31 +0,0 @@
-/*
- Licensed under the Apache License, Version 2.0 (the "License");
- you may not use this file except in compliance with the License.
- You may obtain a copy of the License at
-
-     https://www.apache.org/licenses/LICENSE-2.0
-
- Unless required by applicable law or agreed to in writing, software
- distributed under the License is distributed on an "AS IS" BASIS,
- WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
- See the License for the specific language governing permissions and
- limitations under the License.
-*/
-
-package util
-
-import (
-	"io/ioutil"
-
-	"sigs.k8s.io/yaml"
-)
-
-// ReadYAMLFile reads YAML-formatted configuration file and
-// de-serializes it to a given object
-func ReadYAMLFile(filePath string, cfg interface{}) error {
-	data, err := ioutil.ReadFile(filePath)
-	if err != nil {
-		return err
-	}
-	return yaml.Unmarshal(data, cfg)
-}

pkg/util/configreader_test.go

@@ -1,47 +0,0 @@
-/*
- Licensed under the Apache License, Version 2.0 (the "License");
- you may not use this file except in compliance with the License.
- You may obtain a copy of the License at
-
-     https://www.apache.org/licenses/LICENSE-2.0
-
- Unless required by applicable law or agreed to in writing, software
- distributed under the License is distributed on an "AS IS" BASIS,
- WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
- See the License for the specific language governing permissions and
- limitations under the License.
-*/
-
-package util_test
-
-import (
-	"testing"
-
-	"github.com/stretchr/testify/assert"
-
-	"opendev.org/airship/airshipctl/pkg/util"
-)
-
-func TestReadYAMLFile(t *testing.T) {
-	assert := assert.New(t)
-
-	var actual map[string]interface{}
-
-	tests := []struct {
-		testFile string
-		isError  bool
-	}{
-		{"testdata/test.yaml", false},
-		{"testdata/incorrect.yaml", true},
-		{"testdata/notfound.yaml", true},
-	}
-
-	for _, test := range tests {
-		err := util.ReadYAMLFile(test.testFile, &actual)
-		if test.isError {
-			assert.Error(err)
-		} else {
-			assert.NoError(err)
-		}
-	}
-}

pkg/util/testdata/incorrect.yaml

@@ -1,3 +0,0 @@
---
-testString: incorrectYamlSyntax
-...

pkg/util/testdata/test.yaml

@@ -1 +0,0 @@
-testString: test

testutil/testconfig.go

@@ -29,27 +29,26 @@ import (
 
 // DummyConfig used by tests, to initialize min set of data
 func DummyConfig() *config.Config {
-	conf := &config.Config{
-		Kind:       config.AirshipConfigKind,
-		APIVersion: config.AirshipConfigAPIVersion,
-		Permissions: config.Permissions{
-			DirectoryPermission: config.AirshipDefaultDirectoryPermission,
-			FilePermission:      config.AirshipDefaultFilePermission,
-		},
-		Contexts: map[string]*config.Context{
-			"dummy_context": DummyContext(),
-		},
-		Manifests: map[string]*config.Manifest{
-			"dummy_manifest": DummyManifest(),
-		},
-		ManagementConfiguration: map[string]*config.ManagementConfiguration{
-			"dummy_management_config": DummyManagementConfiguration(),
-		},
-		EncryptionConfigs: map[string]*config.EncryptionConfig{
-			"dummy_encryption_config": DummyEncryptionConfig(),
-		},
-		CurrentContext: "dummy_context",
+	conf := config.NewConfig()
+	conf.Kind = config.AirshipConfigKind
+	conf.APIVersion = config.AirshipConfigAPIVersion
+	conf.Permissions = config.Permissions{
+		DirectoryPermission: config.AirshipDefaultDirectoryPermission,
+		FilePermission:      config.AirshipDefaultFilePermission,
 	}
+	conf.Contexts = map[string]*config.Context{
+		"dummy_context": DummyContext(),
+	}
+	conf.Manifests = map[string]*config.Manifest{
+		"dummy_manifest": DummyManifest(),
+	}
+	conf.ManagementConfiguration = map[string]*config.ManagementConfiguration{
+		"dummy_management_config": DummyManagementConfiguration(),
+	}
+	conf.EncryptionConfigs = map[string]*config.EncryptionConfig{
+		"dummy_encryption_config": DummyEncryptionConfig(),
+	}
+	conf.CurrentContext = "dummy_context"
 	return conf
 }