From 1a60455cdb482be3055b90d245c3bd520c582440 Mon Sep 17 00:00:00 2001 From: Chunyi Lyu Date: Thu, 13 Jun 2024 15:20:26 +0100 Subject: [PATCH] fix: read .kratix with correct path at deletion (#164) * fix: read .kratix with correct path at deletion * fix: dont delete files belonging to other tests - relates to #163 Co-authored-by: Jake Klein Co-authored-by: Chunyi Lyu --- lib/writers/git.go | 15 ++++-- test/system/assets/git/destinations.yaml | 18 ++++++- test/system/system_test.go | 64 ++++++++++++++++++++++-- 3 files changed, 86 insertions(+), 11 deletions(-) diff --git a/lib/writers/git.go b/lib/writers/git.go index d150b2d9..6a635754 100644 --- a/lib/writers/git.go +++ b/lib/writers/git.go @@ -146,7 +146,7 @@ func (g *GitWriter) update(subDir, workPlacementName string, workloadsToCreate [ //returned by `filepath.Join` is still contained with the git repository: // Note: This means `../` can still be used, but only if the end result is still contained within the git repository if !strings.HasPrefix(absoluteFilePath, localTmpDir) { - log.Error(nil, "Path of file to write is not located within the git repostiory", "absolutePath", absoluteFilePath, "tmpDir", localTmpDir) + log.Error(nil, "Path of file to write is not located within the git repository", "absolutePath", absoluteFilePath, "tmpDir", localTmpDir) return "", nil //We don't want to retry as this isn't a recoverable error. Log error and return nil. } @@ -205,8 +205,9 @@ func (g *GitWriter) deleteExistingFiles(removeDirectory bool, dir string, worklo } func (g *GitWriter) ReadFile(filePath string) ([]byte, error) { + fullPath := filepath.Join(g.Path, filePath) logger := g.Log.WithValues( - "Path", filePath, + "Path", fullPath, "branch", g.GitServer.Branch, ) @@ -216,11 +217,17 @@ func (g *GitWriter) ReadFile(filePath string) ([]byte, error) { } defer os.RemoveAll(filepath.Dir(localTmpDir)) - if _, err := worktree.Filesystem.Lstat(filePath); err != nil { + if _, err := worktree.Filesystem.Lstat(fullPath); err != nil { + logger.Error(err, "could not stat file") return nil, FileNotFound } - return os.ReadFile(filepath.Join(localTmpDir, filePath)) + var content []byte + if content, err = os.ReadFile(filepath.Join(localTmpDir, fullPath)); err != nil { + logger.Error(err, "could not read file") + return nil, err + } + return content, nil } func (g *GitWriter) setupLocalDirectoryWithRepo(logger logr.Logger) (string, *git.Repository, *git.Worktree, error) { diff --git a/test/system/assets/git/destinations.yaml b/test/system/assets/git/destinations.yaml index 28c3e672..dbc43019 100644 --- a/test/system/assets/git/destinations.yaml +++ b/test/system/assets/git/destinations.yaml @@ -13,12 +13,26 @@ spec: apiVersion: platform.kratix.io/v1alpha1 kind: Destination metadata: - name: terraform + name: filepathmode-none-bucket labels: - environment: terraform + environment: filepathmode-none + type: bucket spec: filepath: mode: none stateStoreRef: name: default kind: BucketStateStore +--- +apiVersion: platform.kratix.io/v1alpha1 +kind: Destination +metadata: + name: filepathmode-none-git + labels: + environment: filepathmode-none +spec: + filepath: + mode: none + stateStoreRef: + name: default + kind: GitStateStore diff --git a/test/system/system_test.go b/test/system/system_test.go index 9a4473f8..7cb2e54b 100644 --- a/test/system/system_test.go +++ b/test/system/system_test.go @@ -11,6 +11,10 @@ import ( "strings" "time" + "github.com/go-git/go-git/v5" + "github.com/go-git/go-git/v5/plumbing" + "github.com/go-git/go-git/v5/plumbing/transport/http" + "github.com/minio/minio-go/v7" "github.com/minio/minio-go/v7/pkg/credentials" . "github.com/onsi/ginkgo/v2" @@ -686,7 +690,7 @@ var _ = Describe("Kratix", func() { It("manages output files from multiple resource requests", func() { bashPromise.Spec.DestinationSelectors = []v1alpha1.PromiseScheduling{{ MatchLabels: map[string]string{ - "environment": "terraform", + "environment": "filepathmode-none", }, }} @@ -698,22 +702,40 @@ var _ = Describe("Kratix", func() { platform.kubectl("apply", "-f", terraformRequest(rrNameTwo)) By("writing output files to the root of stateStore") - destinationName := "terraform" + promiseDestName := "filepathmode-none-git" + Eventually(func() []string { + return listFilesInGitStateStore(promiseDestName) + }, shortTimeout, interval).Should(ContainElements("configmap.yaml")) + + resourceDestName := "filepathmode-none-bucket" Eventually(func() []string { - return listFilesInMinIOStateStore(destinationName) + return listFilesInMinIOStateStore(resourceDestName) }, shortTimeout, interval).Should(ContainElements( + "configmap.yaml", fmt.Sprintf("%s.yaml", rrNameOne), fmt.Sprintf("%s.yaml", rrNameTwo))) By("removing only files associated with the resource request at deletion") platform.kubectl("delete", crd.Name, rrNameOne) Eventually(func() []string { - return listFilesInMinIOStateStore(destinationName) + return listFilesInMinIOStateStore(resourceDestName) }, shortTimeout, interval).ShouldNot(ContainElements( fmt.Sprintf("%s.yaml", rrNameOne))) - Expect(listFilesInMinIOStateStore(destinationName)).To(ContainElements(fmt.Sprintf("%s.yaml", rrNameTwo))) + Expect(listFilesInMinIOStateStore(resourceDestName)).To(ContainElements(fmt.Sprintf("%s.yaml", rrNameTwo))) + By("cleaning up files from state store at deletion") platform.eventuallyKubectlDelete("promise", bashPromiseName) + Eventually(func() []string { + return listFilesInGitStateStore(promiseDestName) + }, shortTimeout, interval).ShouldNot(ContainElements("configmap.yaml", ".kratix/")) + + Eventually(func() []string { + return listFilesInMinIOStateStore(resourceDestName) + }, shortTimeout, interval).ShouldNot(ContainElements( + "configmap.yaml", + ".kratix/", + fmt.Sprintf("%s.yaml", rrNameOne), + fmt.Sprintf("%s.yaml", rrNameTwo))) }) }) }) @@ -729,6 +751,7 @@ func terraformRequest(name string) string { "spec": map[string]interface{}{ "container0Cmd": fmt.Sprintf(` touch /kratix/output/%s.yaml + echo "[{\"matchLabels\":{\"type\":\"bucket\"}}]" > /kratix/metadata/destination-selectors.yaml `, name), "container1Cmd": "exit 0", }, @@ -907,6 +930,37 @@ func (c destination) withExitCode(code int) destination { return newDestination } +func listFilesInGitStateStore(subDir string) []string { + dir, err := os.MkdirTemp(testTempDir, "git-state-store") + ExpectWithOffset(1, err).NotTo(HaveOccurred()) + + _, err = git.PlainClone(dir, false, &git.CloneOptions{ + Auth: &http.BasicAuth{ + Username: "gitea_admin", + Password: "r8sA8CPHD9!bt6d", + }, + URL: "https://localhost:31333/gitea_admin/kratix", + ReferenceName: plumbing.NewBranchReferenceName("main"), + SingleBranch: true, + Depth: 1, + NoCheckout: false, + InsecureSkipTLS: true, + }) + ExpectWithOffset(1, err).NotTo(HaveOccurred()) + var paths []string + absoluteDir := filepath.Join(dir, subDir) + err = filepath.Walk(absoluteDir, func(path string, info os.FileInfo, err error) error { + if !info.IsDir() { + path, err := filepath.Rel(absoluteDir, path) + Expect(err).NotTo(HaveOccurred()) + paths = append(paths, path) + } + return nil + }) + ExpectWithOffset(1, err).NotTo(HaveOccurred()) + return paths +} + func listFilesInMinIOStateStore(path string) []string { files := []string{}