From d7101bcb2c93aa9857dde5e0464cbdae70c2449d Mon Sep 17 00:00:00 2001
From: Vladislav Kuzmin <vkuzmin@mirantis.com>
Date: Thu, 11 Jun 2020 13:48:10 +0400
Subject: [PATCH] Refactor environment module

Move code from environment module to util and document
modules to prevent circular dependencies.

Relates-To: #264

Change-Id: Ifec9ab3f1ada01cc662e655ff4a6f2cfebe4150a
---
 pkg/document/bundle.go           | 39 +++++++++++++++++++++++++--
 pkg/document/bundle_test.go      | 23 ++++++++++++++++
 pkg/environment/settings.go      | 45 +++-----------------------------
 pkg/environment/settings_test.go |  7 -----
 pkg/util/homedir.go              | 28 ++++++++++++++++++++
 pkg/util/homedir_test.go         | 43 ++++++++++++++++++++++++++++++
 6 files changed, 134 insertions(+), 51 deletions(-)
 create mode 100644 pkg/util/homedir.go
 create mode 100644 pkg/util/homedir_test.go

diff --git a/pkg/document/bundle.go b/pkg/document/bundle.go
index e54176133..876d91ee7 100644
--- a/pkg/document/bundle.go
+++ b/pkg/document/bundle.go
@@ -16,7 +16,10 @@ package document
 
 import (
 	"io"
+	"os"
+	"path/filepath"
 	"strings"
+	"sync"
 
 	"sigs.k8s.io/kustomize/api/k8sdeps/kunstruct"
 	"sigs.k8s.io/kustomize/api/krusty"
@@ -24,7 +27,8 @@ import (
 	"sigs.k8s.io/kustomize/api/resource"
 	"sigs.k8s.io/kustomize/api/types"
 
-	"opendev.org/airship/airshipctl/pkg/environment"
+	"opendev.org/airship/airshipctl/pkg/config"
+	"opendev.org/airship/airshipctl/pkg/util"
 	utilyaml "opendev.org/airship/airshipctl/pkg/util/yaml"
 )
 
@@ -58,6 +62,10 @@ type Bundle interface {
 	Append(Document) error
 }
 
+// A singleton for the kustomize plugin path configuration
+var pluginPath string
+var pluginPathLock = &sync.Mutex{}
+
 // NewBundleByPath helper function that returns new document.Bundle interface based on clusterType and
 // phase, example: helpers.NewBunde(airConfig, "ephemeral", "initinfra")
 func NewBundleByPath(rootPath string) (Bundle, error) {
@@ -89,7 +97,7 @@ func NewBundle(fSys FileSystem, kustomizePath string) (Bundle, error) {
 		LoadRestrictions:     options.LoadRestrictions,
 		DoPrune:              false, // Default
 		PluginConfig: &types.PluginConfig{
-			AbsPluginHome:      environment.PluginPath(),
+			AbsPluginHome:      PluginPath(),
 			PluginRestrictions: types.PluginRestrictionsNone,
 		},
 	}
@@ -103,6 +111,33 @@ func NewBundle(fSys FileSystem, kustomizePath string) (Bundle, error) {
 	return bundle, err
 }
 
+// PluginPath returns the kustomize plugin path
+func PluginPath() string {
+	if pluginPath == "" {
+		InitPluginPath()
+	}
+
+	pluginPathLock.Lock()
+	defer pluginPathLock.Unlock()
+	return pluginPath
+}
+
+// InitPluginPath sets the location to look for kustomize plugins (including airshipctl itself).
+func InitPluginPath() {
+	pluginPathLock.Lock()
+	defer pluginPathLock.Unlock()
+
+	// Check if we got the path via ENVIRONMENT variable
+	pluginPath = os.Getenv(config.AirshipPluginPathEnv)
+	if pluginPath != "" {
+		return
+	}
+
+	// Otherwise, we'll try putting it in the home directory
+	homeDir := util.UserHomeDir()
+	pluginPath = filepath.Join(homeDir, config.AirshipConfigDir, config.AirshipPluginPath)
+}
+
 // GetKustomizeResourceMap returns a Kustomize Resource Map for this bundle
 func (b *BundleFactory) GetKustomizeResourceMap() resmap.ResMap {
 	return b.ResMap
diff --git a/pkg/document/bundle_test.go b/pkg/document/bundle_test.go
index 287f3181f..2b951b82c 100644
--- a/pkg/document/bundle_test.go
+++ b/pkg/document/bundle_test.go
@@ -16,11 +16,14 @@ package document_test
 
 import (
 	"bytes"
+	"os"
+	"path/filepath"
 	"testing"
 
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 
+	"opendev.org/airship/airshipctl/pkg/config"
 	"opendev.org/airship/airshipctl/pkg/document"
 	"opendev.org/airship/airshipctl/testutil"
 )
@@ -202,3 +205,23 @@ func TestBundleOrder(t *testing.T) {
 	assert.Equal(t, "Deployment", doc.GetKind())
 	assert.Equal(t, "workflow-controller", doc.GetName())
 }
+
+func TestPluginPath(t *testing.T) {
+	testDir, cleanup := testutil.TempDir(t, "test-home")
+	defer cleanup(t)
+	defer setHome(testDir)()
+
+	expectedPluginPath := filepath.Join(testDir, config.AirshipConfigDir, config.AirshipPluginPath)
+	document.InitPluginPath()
+	assert.Equal(t, expectedPluginPath, document.PluginPath())
+}
+
+// setHome sets the HOME environment variable to `path`, and returns a function
+// that can be used to reset HOME to its original value
+func setHome(path string) (resetHome func()) {
+	oldHome := os.Getenv("HOME")
+	os.Setenv("HOME", path)
+	return func() {
+		os.Setenv("HOME", oldHome)
+	}
+}
diff --git a/pkg/environment/settings.go b/pkg/environment/settings.go
index ff42538d2..ab51e8aad 100644
--- a/pkg/environment/settings.go
+++ b/pkg/environment/settings.go
@@ -17,7 +17,6 @@ package environment
 import (
 	"os"
 	"path/filepath"
-	"sync"
 
 	"github.com/spf13/cobra"
 
@@ -25,6 +24,7 @@ import (
 
 	"opendev.org/airship/airshipctl/pkg/config"
 	"opendev.org/airship/airshipctl/pkg/log"
+	"opendev.org/airship/airshipctl/pkg/util"
 )
 
 // AirshipCTLSettings is a container for all of the settings needed by airshipctl
@@ -37,10 +37,6 @@ type AirshipCTLSettings struct {
 	Config            *config.Config
 }
 
-// A singleton for the kustomize plugin path configuration
-var pluginPath string
-var pluginPathLock = &sync.Mutex{}
-
 // InitFlags adds the default settings flags to cmd
 func (a *AirshipCTLSettings) InitFlags(cmd *cobra.Command) {
 	flags := cmd.PersistentFlags()
@@ -75,7 +71,6 @@ func (a *AirshipCTLSettings) InitConfig() {
 
 	a.InitAirshipConfigPath()
 	a.InitKubeConfigPath()
-	InitPluginPath()
 
 	err := a.Config.LoadConfig(a.AirshipConfigPath, a.KubeConfigPath, a.Create)
 	if err != nil {
@@ -98,7 +93,7 @@ func (a *AirshipCTLSettings) InitAirshipConfigPath() {
 	}
 
 	// Otherwise, we'll try putting it in the home directory
-	homeDir := userHomeDir()
+	homeDir := util.UserHomeDir()
 	a.AirshipConfigPath = filepath.Join(homeDir, config.AirshipConfigDir, config.AirshipConfig)
 }
 
@@ -123,40 +118,6 @@ func (a *AirshipCTLSettings) InitKubeConfigPath() {
 	}
 
 	// Otherwise, we'll try putting it in the home directory
-	homeDir := userHomeDir()
+	homeDir := util.UserHomeDir()
 	a.KubeConfigPath = filepath.Join(homeDir, config.AirshipConfigDir, config.AirshipKubeConfig)
 }
-
-// InitPluginPath - Sets the location to look for kustomize plugins (including airshipctl itself).
-func InitPluginPath() {
-	pluginPathLock.Lock()
-	defer pluginPathLock.Unlock()
-
-	// Check if we got the path via ENVIRONMENT variable
-	pluginPath = os.Getenv(config.AirshipPluginPathEnv)
-	if pluginPath != "" {
-		return
-	}
-
-	// Otherwise, we'll try putting it in the home directory
-	homeDir := userHomeDir()
-	pluginPath = filepath.Join(homeDir, config.AirshipConfigDir, config.AirshipPluginPath)
-}
-
-// PluginPath returns the kustomize plugin path
-func PluginPath() string {
-	pluginPathLock.Lock()
-	defer pluginPathLock.Unlock()
-	return pluginPath
-}
-
-// userHomeDir is a utility function that wraps os.UserHomeDir and returns no
-// errors. If the user has no home directory, the returned value will be the
-// empty string
-func userHomeDir() string {
-	homeDir, err := os.UserHomeDir()
-	if err != nil {
-		homeDir = ""
-	}
-	return homeDir
-}
diff --git a/pkg/environment/settings_test.go b/pkg/environment/settings_test.go
index 2d7975a10..9aa030e33 100644
--- a/pkg/environment/settings_test.go
+++ b/pkg/environment/settings_test.go
@@ -47,14 +47,11 @@ func TestInitConfig(t *testing.T) {
 		var testSettings environment.AirshipCTLSettings
 		expectedAirshipConfig := filepath.Join(testDir, config.AirshipConfigDir, config.AirshipConfig)
 		expectedKubeConfig := filepath.Join(testDir, config.AirshipConfigDir, config.AirshipKubeConfig)
-		expectedPluginPath := filepath.Join(testDir, config.AirshipConfigDir, config.AirshipPluginPath)
 
 		testSettings.InitAirshipConfigPath()
 		testSettings.InitKubeConfigPath()
-		environment.InitPluginPath()
 		assert.Equal(t, expectedAirshipConfig, testSettings.AirshipConfigPath)
 		assert.Equal(t, expectedKubeConfig, testSettings.KubeConfigPath)
-		assert.Equal(t, expectedPluginPath, environment.PluginPath())
 	})
 
 	t.Run("PreferEnvToDefault", func(subTest *testing.T) {
@@ -66,21 +63,17 @@ func TestInitConfig(t *testing.T) {
 		var testSettings environment.AirshipCTLSettings
 		expectedAirshipConfig := filepath.Join(testDir, "airshipEnv")
 		expectedKubeConfig := filepath.Join(testDir, "kubeEnv")
-		expectedPluginPath := filepath.Join(testDir, "pluginPath")
 
 		os.Setenv(config.AirshipConfigEnv, expectedAirshipConfig)
 		os.Setenv(config.AirshipKubeConfigEnv, expectedKubeConfig)
-		os.Setenv(config.AirshipPluginPathEnv, expectedPluginPath)
 		defer os.Unsetenv(config.AirshipConfigEnv)
 		defer os.Unsetenv(config.AirshipKubeConfigEnv)
-		defer os.Unsetenv(config.AirshipPluginPathEnv)
 
 		testSettings.Create = true
 		testSettings.InitConfig()
 
 		assert.Equal(t, expectedAirshipConfig, testSettings.AirshipConfigPath)
 		assert.Equal(t, expectedKubeConfig, testSettings.KubeConfigPath)
-		assert.Equal(t, expectedPluginPath, environment.PluginPath())
 	})
 
 	t.Run("PreferCmdLineArgToDefault", func(subTest *testing.T) {
diff --git a/pkg/util/homedir.go b/pkg/util/homedir.go
new file mode 100644
index 000000000..de516bec5
--- /dev/null
+++ b/pkg/util/homedir.go
@@ -0,0 +1,28 @@
+/*
+ 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 "os"
+
+// UserHomeDir is a utility function that wraps os.UserHomeDir and returns no
+// errors. If the user has no home directory, the returned value will be the
+// empty string
+func UserHomeDir() string {
+	homeDir, err := os.UserHomeDir()
+	if err != nil {
+		homeDir = ""
+	}
+	return homeDir
+}
diff --git a/pkg/util/homedir_test.go b/pkg/util/homedir_test.go
new file mode 100644
index 000000000..8fd2560cf
--- /dev/null
+++ b/pkg/util/homedir_test.go
@@ -0,0 +1,43 @@
+/*
+ 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 (
+	"os"
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+
+	"opendev.org/airship/airshipctl/pkg/util"
+	"opendev.org/airship/airshipctl/testutil"
+)
+
+func TestUserHomeDir(t *testing.T) {
+	expectedTestDir, cleanup := testutil.TempDir(t, "test-home")
+	defer cleanup(t)
+	defer setHome(expectedTestDir)()
+
+	assert.Equal(t, expectedTestDir, util.UserHomeDir())
+}
+
+// setHome sets the HOME environment variable to `path`, and returns a function
+// that can be used to reset HOME to its original value
+func setHome(path string) (resetHome func()) {
+	oldHome := os.Getenv("HOME")
+	os.Setenv("HOME", path)
+	return func() {
+		os.Setenv("HOME", oldHome)
+	}
+}