Fixed permission denied regression (#257)

Fixed an error which would occur if a user attempted to `install` a
component which already previously been installed. This was caused do to
being unable to overwrite existing files in the installed components
`.git` directory. The previously installed component is now deleted
prior to copying from the clone cache.

cmd/find_test.go

@@ -3,8 +3,8 @@ package cmd
 import (
 	"testing"
 
-	"github.com/stretchr/testify/assert"
 	"github.com/google/go-github/v28/github"
+	"github.com/stretchr/testify/assert"
 )
 
 func TestGetFabrikateComponents(t *testing.T) {
@@ -17,8 +17,8 @@ func TestGetFabrikateComponents(t *testing.T) {
 		"samples/kafka-strimzi-portworx/config/common.yaml",
 	}
 
-	for _, path := range paths{
-		var p string = path
+	for _, path := range paths {
+		var p = path
 		githubCodeResults = append(githubCodeResults, github.CodeResult{Path: &p})
 	}
 
@@ -31,4 +31,4 @@ func TestGetFabrikateComponentsEmpty(t *testing.T) {
 
 	components := GetFabrikateComponents(githubCodeResults)
 	assert.Equal(t, 0, len(components))
-}
+}

cmd/install_test.go

@@ -23,6 +23,9 @@ func TestInstallJSON(t *testing.T) {
 	// Change cwd to component directory
 	assert.Nil(t, os.Chdir(componentDir))
 	assert.Nil(t, Install("./"))
+
+	// Installing again should not cause errors
+	assert.Nil(t, Install("./"))
 }
 
 func TestInstallYAML(t *testing.T) {
@@ -37,6 +40,9 @@ func TestInstallYAML(t *testing.T) {
 	// Change cwd to component directory
 	assert.Nil(t, os.Chdir(componentDir))
 	assert.Nil(t, Install("./"))
+
+	// Installing again should not cause errors
+	assert.Nil(t, Install("./"))
 }
 
 func TestInstallWithHooks(t *testing.T) {
@@ -50,7 +56,6 @@ func TestInstallWithHooks(t *testing.T) {
 
 	// Change cwd to component directory
 	assert.Nil(t, os.Chdir(componentDir))
-
 	assert.Nil(t, Install("./"))
 }
 
@@ -91,6 +96,9 @@ func TestInstallHelmMethod(t *testing.T) {
 	assert.Nil(t, os.Chdir(componentDir))
 	assert.Nil(t, Install("./"))
 
+	// Installing again should not cause errors
+	assert.Nil(t, Install("./"))
+
 	// Grafana chart should be version 3.7.0
 	grafanaChartYaml := path.Join("helm_repos", "grafana", "Chart.yaml")
 	grafanaChartBytes, err := ioutil.ReadFile(grafanaChartYaml)

core/component.go

@@ -6,9 +6,9 @@ import (
 	"fmt"
 	"io"
 	"io/ioutil"
+	"net/http"
 	"os"
 	"os/exec"
-	"net/http"
 	"path"
 	"path/filepath"
 	"sort"
@@ -195,39 +195,34 @@ func (c *Component) afterInstall() (err error) {
 // InstallRemoteStaticComponent installs a component by downloading the remote resource manifest
 func (c *Component) InstallRemoteStaticComponent(componentPath string) (err error) {
 	if !IsValidRemoteComponentConfig(*c) {
-		return nil;
+		return nil
 	}
 
-	componentsPath := path.Join(componentPath, "components/", c.Name)
-
-	if err := os.MkdirAll(componentsPath, 0777); err != nil {
+	response, err := http.Get(c.Source)
+	if err != nil {
 		return err
 	}
 
-	response, err := http.Get(c.Source);
-
-	if err != nil {
+	componentsPath := path.Join(componentPath, "components/", c.Name)
+	if err := os.MkdirAll(componentsPath, 0777); err != nil {
 		return err
 	}
 
 	// Write the downloaded resource manifest file
 	defer response.Body.Close()
-	out, err := os.Create(path.Join(componentsPath, c.Name + ".yaml"))
-
+	out, err := os.Create(path.Join(componentsPath, c.Name+".yaml"))
 	if err != nil {
 		logger.Error(emoji.Sprintf(":no_entry_sign: Error occurred in install for component '%s'\nError: %s", c.Name, err))
 		return err
 	}
-
 	defer out.Close()
-	_, err = io.Copy(out, response.Body)
 
-	if (err != nil) {
+	if _, err = io.Copy(out, response.Body); err != nil {
 		logger.Error(emoji.Sprintf(":no_entry_sign: Error occurred in writing manifest file for component '%s'\nError: %s", c.Name, err))
 		return err
 	}
 
-	return nil;
+	return nil
 }
 
 // InstallComponent installs the component (if needed) utilizing its Method.
@@ -245,7 +240,7 @@ func (c *Component) InstallComponent(componentPath string) (err error) {
 
 		logger.Info(emoji.Sprintf(":helicopter: Installing component '%s' with git from '%s'", c.Name, c.Source))
 
-		if err = CloneRepo(c.Source, c.Version, subcomponentPath, c.Branch); err != nil {
+		if err = Git.CloneRepo(c.Source, c.Version, subcomponentPath, c.Branch); err != nil {
 			return err
 		}
 	} else if IsValidRemoteComponentConfig(*c) {
@@ -538,7 +533,7 @@ func (c *Component) GetAccessTokens() (tokens map[string]string, err error) {
 }
 
 // GetStaticComponentPath returns the static path if a component is of type 'static', if not, it returns an empty string
-func (c *Component) GetStaticComponentPath(startPath string)(componentPath string){
+func (c *Component) GetStaticComponentPath(startPath string) (componentPath string) {
 	if c.ComponentType != "static" {
 		return ""
 	}
@@ -550,32 +545,10 @@ func (c *Component) GetStaticComponentPath(startPath string)(componentPath strin
 	return path.Join(c.PhysicalPath, c.Path)
 }
 
-
-// CreateDirectory a directory in the given path and reports no error if the directory already exists
-func CreateDirectory(cmdDir string, dirPath string) (componentPath string, err error) {
-
-	cmd := exec.Command("sh", "-c", "mkdir -p " + dirPath)
-	cmd.Dir = cmdDir
-
-	output, err := cmd.CombinedOutput()
-
-	if err != nil {
-		logger.Error(emoji.Sprintf(":no_entry_sign: Error occurred in creating directory for component\n"))
-		return "", err
-	}
-	if len(output) > 0 {
-		outstring := emoji.Sprintf(":mag_right: Completed creating directory for component\n")
-		logger.Trace(strings.TrimSpace(outstring))
-	}
-
-	return path.Join(cmdDir, dirPath), nil
-}
-
 // IsValidRemoteComponentConfig checks if the given component configuration is valid for a remote component
-func IsValidRemoteComponentConfig(c Component) (bool){
-	return (
-		(c.ComponentType == "static" &&
-		 c.Method == "http")) &&
+func IsValidRemoteComponentConfig(c Component) bool {
+	return (c.ComponentType == "static" &&
+		c.Method == "http") &&
 		(strings.HasSuffix(c.Source, "yaml") ||
-		 strings.HasSuffix(c.Source, "yml"))
+			strings.HasSuffix(c.Source, "yml"))
 }

core/git.go

@@ -199,16 +199,26 @@ func (cache *gitCache) cloneRepo(repo string, commit string, branch string) chan
 	return cloneResultChan
 }
 
+type git struct{}
+
+// Git is function wrapper to expose host git functionality
+var Git = git{}
+
 // CloneRepo is a helper func to centralize cloning a repository with the spec
 // provided by its arguments.
-func CloneRepo(repo string, commit string, intoPath string, branch string) (err error) {
+func (g git) CloneRepo(repo string, commit string, intoPath string, branch string) (err error) {
 	// Clone and get the location of where it was cloned to in tmp
 	result := <-cache.cloneRepo(repo, commit, branch)
 	clonePath := result.get()
 	if result.Error != nil {
 		return result.Error
 	}
 
+	// Remove the into directory if it already exists
+	if err = os.RemoveAll(intoPath); err != nil {
+		return err
+	}
+
 	// copy the repo from tmp cache to component path
 	absIntoPath, err := filepath.Abs(intoPath)
 	if err != nil {
@@ -224,7 +234,7 @@ func CloneRepo(repo string, commit string, intoPath string, branch string) (err
 
 // CleanGitCache deletes all temporary folders created as temporary cache for
 // git clones.
-func CleanGitCache() (err error) {
+func (g git) CleanGitCache() (err error) {
 	logger.Info(emoji.Sprintf(":bomb: Cleaning up git cache..."))
 	cache.mu.Lock()
 	for key, value := range cache.cache {

generators/helm.go

@@ -236,7 +236,7 @@ func (hg *HelmGenerator) Install(c *core.Component) (err error) {
 		case "git":
 			// Clone whole repo into helm repo path
 			logger.Info(emoji.Sprintf(":helicopter: Component '%s' requesting helm chart in path '%s' from git repository '%s'", c.Name, c.Source, c.PhysicalPath))
-			if err = core.CloneRepo(c.Source, c.Version, helmRepoPath, c.Branch); err != nil {
+			if err = core.Git.CloneRepo(c.Source, c.Version, helmRepoPath, c.Branch); err != nil {
 				return err
 			}
 			// Update chart dependencies in chart path -- this is manually done here but automatically done in downloadChart in the case of `method: helm`
@@ -315,6 +315,11 @@ func (hd *helmDownloader) downloadChart(repo, chart, version, into string) (err
 	}
 	hd.mu.Unlock()
 
+	// Remove the into directory if it already exists
+	if err = os.RemoveAll(into); err != nil {
+		return err
+	}
+
 	// copy chart to target `into` dir
 	chartDirectoryInRandomDir := path.Join(randomDir, chart)
 	if err = copy.Copy(chartDirectoryInRandomDir, into); err != nil {