From 46740fa32e9ff44a42fb7371260e809c29ce988a Mon Sep 17 00:00:00 2001 From: Evan Louie Date: Mon, 9 Sep 2019 01:13:38 -0700 Subject: [PATCH] Fixed `permission denied` regression 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 | 8 +++---- cmd/install_test.go | 10 +++++++- core/component.go | 57 ++++++++++++--------------------------------- core/git.go | 14 +++++++++-- generators/helm.go | 7 +++++- 5 files changed, 46 insertions(+), 50 deletions(-) diff --git a/cmd/find_test.go b/cmd/find_test.go index 5d25b60..dc34b61 100644 --- a/cmd/find_test.go +++ b/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)) -} \ No newline at end of file +} diff --git a/cmd/install_test.go b/cmd/install_test.go index 5438fc0..2eecd30 100644 --- a/cmd/install_test.go +++ b/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) diff --git a/core/component.go b/core/component.go index 6212c4e..1ebcde5 100644 --- a/core/component.go +++ b/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")) } diff --git a/core/git.go b/core/git.go index 972931b..0a1540c 100644 --- a/core/git.go +++ b/core/git.go @@ -199,9 +199,14 @@ 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() @@ -209,6 +214,11 @@ func CloneRepo(repo string, commit string, intoPath string, branch string) (err 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 { diff --git a/generators/helm.go b/generators/helm.go index ef80c2e..6e3a2cb 100644 --- a/generators/helm.go +++ b/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 {