From 45b4e393dfbeb553eaff11646a49ec0e11fb910b Mon Sep 17 00:00:00 2001 From: Chunyi Lyu Date: Tue, 28 May 2024 05:19:10 -0400 Subject: [PATCH 1/6] Clean up workload files when filepath.mode: none Co-authored-by: Jake Klein --- api/v1alpha1/destination_types.go | 12 +- controllers/destination_controller.go | 28 +- controllers/workplacement_controller.go | 89 ++++-- controllers/workplacement_controller_test.go | 132 ++++++--- go.mod | 15 +- go.sum | 38 ++- lib/writers/git.go | 169 +++++------ lib/writers/s3.go | 76 +++-- lib/writers/s3_test.go | 1 + lib/writers/statestore_writer.go | 10 +- .../writersfakes/fake_state_store_writer.go | 264 ++++++++++++------ test/system/system_test.go | 5 +- 12 files changed, 535 insertions(+), 304 deletions(-) diff --git a/api/v1alpha1/destination_types.go b/api/v1alpha1/destination_types.go index 3176c6ae..b8115e9e 100644 --- a/api/v1alpha1/destination_types.go +++ b/api/v1alpha1/destination_types.go @@ -62,10 +62,10 @@ type DestinationSpec struct { } const ( - //if modifying these dont forget to edit below where they are written as a - //kubebuilder comment for setting the default and Enum values. - FilepathExpressionTypeNone = "none" - FilepathExpressionTypeNestedByMetadata = "nestedByMetadata" + // if modifying these dont forget to edit below where they are written as a + // kubebuilder comment for setting the default and Enum values. + FilepathModeNone = "none" + FilepathModeNestedByMetadata = "nestedByMetadata" ) type Filepath struct { @@ -80,9 +80,9 @@ type Filepath struct { // it gets defaulted by the K8s API, but for unit testing it wont be defaulted // since its not a real k8s api, so it may be empty when running unit tests. -func (d *Destination) GetFilepathExpressionType() string { +func (d *Destination) GetFilepathMode() string { if d.Spec.Filepath.Mode == "" { - return FilepathExpressionTypeNestedByMetadata + return FilepathModeNestedByMetadata } return d.Spec.Filepath.Mode } diff --git a/controllers/destination_controller.go b/controllers/destination_controller.go index 224ab953..cf9b1fdb 100644 --- a/controllers/destination_controller.go +++ b/controllers/destination_controller.go @@ -18,21 +18,23 @@ package controllers import ( "context" - "path/filepath" - - "k8s.io/apimachinery/pkg/api/errors" - + "fmt" v1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - ctrl "sigs.k8s.io/controller-runtime" - "sigs.k8s.io/controller-runtime/pkg/client" + "path/filepath" "sigs.k8s.io/yaml" + "k8s.io/apimachinery/pkg/api/errors" + "github.com/go-logr/logr" "github.com/syntasso/kratix/api/v1alpha1" "github.com/syntasso/kratix/lib/writers" + ctrl "sigs.k8s.io/controller-runtime" + "sigs.k8s.io/controller-runtime/pkg/client" ) +const canaryWorkload = "kratix-canary" + // DestinationReconciler reconciles a Destination object type DestinationReconciler struct { Client client.Client @@ -117,10 +119,9 @@ func (r *DestinationReconciler) createResourcePathWithExample(writer writers.Sta } nsBytes, _ := yaml.Marshal(kratixConfigMap) - return writer.WriteDirWithObjects(writers.PreserveExistingContentsInDir, resourcesDir, v1alpha1.Workload{ - Filepath: "kratix-canary-configmap.yaml", - Content: string(nsBytes), - }) + return writer.UpdateFiles(canaryWorkload, []v1alpha1.Workload{{ + Filepath: fmt.Sprintf("%s/kratix-canary-configmap.yaml", resourcesDir), + Content: string(nsBytes)}}, nil) } func (r *DestinationReconciler) createDependenciesPathWithExample(writer writers.StateStoreWriter) error { @@ -133,10 +134,9 @@ func (r *DestinationReconciler) createDependenciesPathWithExample(writer writers } nsBytes, _ := yaml.Marshal(kratixNamespace) - return writer.WriteDirWithObjects(writers.PreserveExistingContentsInDir, dependenciesDir, v1alpha1.Workload{ - Filepath: "kratix-canary-namespace.yaml", - Content: string(nsBytes), - }) + return writer.UpdateFiles(canaryWorkload, []v1alpha1.Workload{{ + Filepath: fmt.Sprintf("%s/kratix-canary-namespace.yaml", dependenciesDir), + Content: string(nsBytes)}}, nil) } // SetupWithManager sets up the controller with the Manager. diff --git a/controllers/workplacement_controller.go b/controllers/workplacement_controller.go index 70049981..a5d3cd55 100644 --- a/controllers/workplacement_controller.go +++ b/controllers/workplacement_controller.go @@ -18,11 +18,12 @@ package controllers import ( "context" - "path/filepath" - + "fmt" "github.com/go-logr/logr" + "gopkg.in/yaml.v2" "k8s.io/apimachinery/pkg/api/errors" "k8s.io/apimachinery/pkg/types" + "path/filepath" ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil" @@ -37,6 +38,10 @@ const ( dependenciesDir = "dependencies" ) +type StateFile struct { + Files []string `json:"files"` +} + // WorkPlacementReconciler reconciles a WorkPlacement object type WorkPlacementReconciler struct { Client client.Client @@ -94,7 +99,7 @@ func (r *WorkPlacementReconciler) Reconcile(ctx context.Context, req ctrl.Reques } if !workPlacement.DeletionTimestamp.IsZero() { - return r.deleteWorkPlacement(ctx, writer, workPlacement, logger) + return r.deleteWorkPlacement(ctx, writer, workPlacement, destination.GetFilepathMode(), logger) } if resourceutil.FinalizersAreMissing(workPlacement, workPlacementFinalizers) { @@ -110,13 +115,28 @@ func (r *WorkPlacementReconciler) Reconcile(ctx context.Context, req ctrl.Reques return ctrl.Result{}, nil } -func (r *WorkPlacementReconciler) deleteWorkPlacement(ctx context.Context, writer writers.StateStoreWriter, workPlacement *v1alpha1.WorkPlacement, logger logr.Logger) (ctrl.Result, error) { +func (r *WorkPlacementReconciler) deleteWorkPlacement(ctx context.Context, writer writers.StateStoreWriter, workPlacement *v1alpha1.WorkPlacement, filePathMode string, logger logr.Logger) (ctrl.Result, error) { if !controllerutil.ContainsFinalizer(workPlacement, repoCleanupWorkPlacementFinalizer) { return ctrl.Result{}, nil } logger.Info("cleaning up files on repository", "repository", workPlacement.Name) - err := r.removeWorkFromRepository(writer, *workPlacement, logger) + + var err error + if filePathMode == v1alpha1.FilepathModeNone { + var kratixFile []byte + if kratixFile, err = writer.ReadFile(fmt.Sprintf(".kratix/%s-%s.yaml", workPlacement.Namespace, workPlacement.Name)); err != nil { + return defaultRequeue, err + } + stateFile := StateFile{} + if err = yaml.Unmarshal(kratixFile, &stateFile); err != nil { + return defaultRequeue, err + } + err = writer.UpdateFiles(workPlacement.Name, nil, stateFile.Files) + } else { + err = writer.UpdateInDir(getDir(*workPlacement)+"/", workPlacement.Name, nil) + } + if err != nil { logger.Error(err, "error removing work from repository, will try again in 5 seconds") return defaultRequeue, err @@ -131,27 +151,62 @@ func (r *WorkPlacementReconciler) deleteWorkPlacement(ctx context.Context, write } func (r *WorkPlacementReconciler) writeWorkloadsToStateStore(writer writers.StateStoreWriter, workPlacement v1alpha1.WorkPlacement, destination v1alpha1.Destination, logger logr.Logger) error { - dir := getDir(workPlacement) - if destination.GetFilepathExpressionType() == v1alpha1.FilepathExpressionTypeNone { - dir = "" + var err error + if destination.GetFilepathMode() == v1alpha1.FilepathModeNone { + var kratixFile []byte + if kratixFile, err = writer.ReadFile(fmt.Sprintf(".kratix/%s-%s.yaml", workPlacement.Namespace, workPlacement.Name)); err != nil { + return err + } + oldStateFile := StateFile{} + if err = yaml.Unmarshal(kratixFile, &oldStateFile); err != nil { + return err + } + + newStateFile := StateFile{ + Files: workLoadsFilenames(workPlacement.Spec.Workloads), + } + stateFileContent, err := yaml.Marshal(newStateFile) + if err != nil { + return err + } + + stateFileWorkload := v1alpha1.Workload{ + Filepath: fmt.Sprintf(".kratix/%s-%s.yaml", workPlacement.Namespace, workPlacement.Name), + Content: string(stateFileContent), + } + + err = writer.UpdateFiles(workPlacement.Name, append(workPlacement.Spec.Workloads, stateFileWorkload), cleanupWorkloads(oldStateFile.Files, workPlacement.Spec.Workloads)) + } else { + err = writer.UpdateInDir(getDir(workPlacement), workPlacement.Name, workPlacement.Spec.Workloads) } - err := writer.WriteDirWithObjects(writers.DeleteExistingContentsInDir, dir, workPlacement.Spec.Workloads...) + if err != nil { logger.Error(err, "Error writing resources to repository") return err } - return nil } -func (r *WorkPlacementReconciler) removeWorkFromRepository(writer writers.StateStoreWriter, workPlacement v1alpha1.WorkPlacement, logger logr.Logger) error { - //MinIO needs a trailing slash to delete a directory - dir := getDir(workPlacement) + "/" - if err := writer.RemoveObject(dir); err != nil { - logger.Error(err, "Error removing workloads from repository", "dir", dir) - return err +func workLoadsFilenames(works []v1alpha1.Workload) []string { + var result []string + for _, w := range works { + result = append(result, w.Filepath) } - return nil + return result +} + +func cleanupWorkloads(old []string, new []v1alpha1.Workload) []string { + works := make(map[string]bool) + for _, w := range new { + works[w.Filepath] = true + } + var result []string + for _, w := range old { + if _, ok := works[w]; !ok { + result = append(result, w) + } + } + return result } func getDir(workPlacement v1alpha1.WorkPlacement) string { diff --git a/controllers/workplacement_controller_test.go b/controllers/workplacement_controller_test.go index 8b753f6f..c668919d 100644 --- a/controllers/workplacement_controller_test.go +++ b/controllers/workplacement_controller_test.go @@ -18,9 +18,7 @@ package controllers_test import ( "context" - - corev1 "k8s.io/api/core/v1" - v1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "fmt" "k8s.io/apimachinery/pkg/types" "github.com/go-logr/logr" @@ -30,14 +28,14 @@ import ( "github.com/syntasso/kratix/lib/hash" "github.com/syntasso/kratix/lib/writers" "github.com/syntasso/kratix/lib/writers/writersfakes" + corev1 "k8s.io/api/core/v1" + v1 "k8s.io/apimachinery/pkg/apis/meta/v1" ctrl "sigs.k8s.io/controller-runtime" "github.com/syntasso/kratix/api/v1alpha1" //+kubebuilder:scaffold:imports ) -var workplacement *v1alpha1.Work - var _ = Describe("WorkplacementReconciler", func() { var ( ctx context.Context @@ -66,7 +64,8 @@ var _ = Describe("WorkplacementReconciler", func() { workloads = []v1alpha1.Workload{ { - Content: "{someApi: foo, someValue: bar}", + Filepath: "fruit.yaml", + Content: "{someApi: foo, someValue: bar}", }, } workPlacement = v1alpha1.WorkPlacement{ @@ -99,19 +98,15 @@ var _ = Describe("WorkplacementReconciler", func() { }, Spec: v1alpha1.DestinationSpec{ Filepath: v1alpha1.Filepath{ - Mode: v1alpha1.FilepathExpressionTypeNone, - }, - StateStoreRef: &v1alpha1.StateStoreReference{ - //Set in the tests beforeach - // Kind: "Git/BucketStateStore", - // Name: "test-state-store", + Mode: v1alpha1.FilepathModeNone, }, + StateStoreRef: &v1alpha1.StateStoreReference{}, }, } }) When("the destination statestore is s3", func() { - When("the destination has filepathExpression type none", func() { + When("the destination has filepath mode of none", func() { BeforeEach(func() { Expect(fakeK8sClient.Create(ctx, &corev1.Secret{ TypeMeta: v1.TypeMeta{ @@ -161,45 +156,104 @@ var _ = Describe("WorkplacementReconciler", func() { return fakeWriter, nil }) }) - It("calls the writer with an empty dir", func() { + + It("reconciles", func() { result, err := t.reconcileUntilCompletion(reconciler, &workPlacement) Expect(err).NotTo(HaveOccurred()) Expect(result).To(Equal(ctrl.Result{})) - Expect(fakeWriter.WriteDirWithObjectsCallCount()).To(Equal(1)) - deleteExistingDir, dir, workloadsToCreate := fakeWriter.WriteDirWithObjectsArgsForCall(0) - Expect(deleteExistingDir).To(BeTrue()) - Expect(dir).To(Equal("")) - Expect(workloadsToCreate).To(Equal(workloads)) - }) + By("calling UpdateFiles()") + Expect(fakeWriter.UpdateFilesCallCount()).To(Equal(1)) + Expect(fakeWriter.UpdateInDirCallCount()).To(Equal(0)) + workPlacementName, workloadsToCreate, workloadsToDelete := fakeWriter.UpdateFilesArgsForCall(0) + Expect(workPlacementName).To(Equal(workPlacement.Name)) - It("constructs the writer using the statestore and destination", func() { - result, err := t.reconcileUntilCompletion(reconciler, &workPlacement) - Expect(err).NotTo(HaveOccurred()) - Expect(result).To(Equal(ctrl.Result{})) + By("writing workloads files and kratix state file") + Expect(workloadsToCreate).To(ConsistOf(append(workloads, v1alpha1.Workload{ + Filepath: fmt.Sprintf(".kratix/%s-%s.yaml", workPlacement.Namespace, workPlacement.Name), + Content: `files: +- fruit.yaml +`, + }))) + Expect(workloadsToDelete).To(BeNil()) + By("constructing the writer using the statestore and destination") Expect(argCreds).To(Equal(map[string][]byte{ "accessKeyID": []byte("test-access"), "secretAccessKey": []byte("test-secret"), })) Expect(argDestination).To(Equal(destination)) Expect(argBucketStateStoreSpec).To(Equal(bucketStateStore.Spec)) - }) - It("sets the finalizer", func() { - result, err := t.reconcileUntilCompletion(reconciler, &workPlacement) - Expect(err).NotTo(HaveOccurred()) - Expect(result).To(Equal(ctrl.Result{})) + By("setting the finalizer") workplacement := &v1alpha1.WorkPlacement{} Expect(fakeK8sClient.Get(ctx, types.NamespacedName{Name: workplacementName, Namespace: "default"}, workplacement)). To(Succeed()) Expect(workplacement.GetFinalizers()).To(ContainElement("finalizers.workplacement.kratix.io/repo-cleanup")) + + }) + + When("deleting a workplacement", func() { + BeforeEach(func() { + result, err := t.reconcileUntilCompletion(reconciler, &workPlacement) + Expect(err).NotTo(HaveOccurred()) + Expect(result).To(Equal(ctrl.Result{})) + }) + + It("calls UpdateFiles()", func() { + fakeWriter.ReadFileReturns([]byte(` +files: + - fruit.yaml`), nil) + Expect(fakeK8sClient.Delete(ctx, &workPlacement)).To(Succeed()) + result, err := t.reconcileUntilCompletion(reconciler, &workPlacement) + Expect(err).NotTo(HaveOccurred()) + Expect(result).To(Equal(ctrl.Result{})) + + Expect(fakeWriter.UpdateFilesCallCount()).To(Equal(2)) + Expect(fakeWriter.UpdateInDirCallCount()).To(Equal(0)) + Expect(fakeWriter.ReadFileCallCount()).To(Equal(2)) + Expect(fakeWriter.ReadFileArgsForCall(1)).To(Equal(fmt.Sprintf(".kratix/%s-%s.yaml", workPlacement.Namespace, workPlacement.Name))) + + workPlacementName, workloadsToCreate, workloadsToDelete := fakeWriter.UpdateFilesArgsForCall(1) + Expect(workPlacementName).To(Equal(workPlacement.Name)) + Expect(workloadsToCreate).To(BeNil()) + Expect(workloadsToDelete).To(ConsistOf("fruit.yaml")) + }) + }) + + When("statestore and workplacement.spec.workloads has diverged", func() { + It("reflects workplacement.spec.workloads", func() { + fakeWriter.ReadFileReturns([]byte(` +files: + - banana.yaml + - apple.yaml + - fruit.yaml`), nil) + + result, err := t.reconcileUntilCompletion(reconciler, &workPlacement) + Expect(err).NotTo(HaveOccurred()) + Expect(result).To(Equal(ctrl.Result{})) + + Expect(fakeWriter.ReadFileCallCount()).To(Equal(1)) + Expect(fakeWriter.ReadFileArgsForCall(0)).To(Equal(fmt.Sprintf(".kratix/%s-%s.yaml", workPlacement.Namespace, workPlacement.Name))) + + Expect(fakeWriter.UpdateFilesCallCount()).To(Equal(1)) + Expect(fakeWriter.UpdateInDirCallCount()).To(Equal(0)) + workPlacementName, workloadsToCreate, workloadsToDelete := fakeWriter.UpdateFilesArgsForCall(0) + Expect(workPlacementName).To(Equal(workPlacement.Name)) + Expect(workloadsToCreate).To(ConsistOf(append(workloads, v1alpha1.Workload{ + Filepath: fmt.Sprintf(".kratix/%s-%s.yaml", workPlacement.Namespace, workPlacement.Name), + Content: `files: +- fruit.yaml +`, + }))) + Expect(workloadsToDelete).To(ConsistOf("banana.yaml", "apple.yaml")) + }) }) }) }) When("the destination statestore is git", func() { - When("the destination has filepathExpression type nestedByMetdata", func() { + When("the destination has filepath mode of nestedByMetadata", func() { BeforeEach(func() { Expect(fakeK8sClient.Create(ctx, &corev1.Secret{ TypeMeta: v1.TypeMeta{ @@ -239,7 +293,7 @@ var _ = Describe("WorkplacementReconciler", func() { destination.Spec.StateStoreRef.Kind = "GitStateStore" destination.Spec.StateStoreRef.Name = "test-state-store" - destination.Spec.Filepath.Mode = v1alpha1.FilepathExpressionTypeNestedByMetadata + destination.Spec.Filepath.Mode = v1alpha1.FilepathModeNestedByMetadata Expect(fakeK8sClient.Create(ctx, &destination)).To(Succeed()) controllers.SetNewGitWriter(func(logger logr.Logger, stateStoreSpec v1alpha1.GitStateStoreSpec, destination v1alpha1.Destination, creds map[string][]byte) (writers.StateStoreWriter, error) { @@ -255,10 +309,11 @@ var _ = Describe("WorkplacementReconciler", func() { Expect(err).NotTo(HaveOccurred()) Expect(result).To(Equal(ctrl.Result{})) - Expect(fakeWriter.WriteDirWithObjectsCallCount()).To(Equal(1)) - deleteExistingDir, dir, workloadsToCreate := fakeWriter.WriteDirWithObjectsArgsForCall(0) - Expect(deleteExistingDir).To(BeTrue()) + Expect(fakeWriter.UpdateInDirCallCount()).To(Equal(1)) + Expect(fakeWriter.UpdateFilesCallCount()).To(Equal(0)) + dir, workPlacementName, workloadsToCreate := fakeWriter.UpdateInDirArgsForCall(0) Expect(dir).To(Equal("resources/default/test-promise/test-resource/5058f")) + Expect(workPlacementName).To(Equal(workPlacement.Name)) Expect(workloadsToCreate).To(Equal(workloads)) }) @@ -283,12 +338,15 @@ var _ = Describe("WorkplacementReconciler", func() { Expect(err).NotTo(HaveOccurred()) Expect(result).To(Equal(ctrl.Result{})) - Expect(fakeWriter.WriteDirWithObjectsCallCount()).To(Equal(1)) - deleteExistingDir, dir, _ := fakeWriter.WriteDirWithObjectsArgsForCall(0) - Expect(deleteExistingDir).To(BeTrue()) + Expect(fakeWriter.UpdateInDirCallCount()).To(Equal(1)) + Expect(fakeWriter.UpdateFilesCallCount()).To(Equal(0)) + dir, workPlacementName, workloadsToCreate := fakeWriter.UpdateInDirArgsForCall(0) Expect(dir).To(Equal("dependencies/test-promise/5058f")) + Expect(workPlacementName).To(Equal(workPlacement.Name)) + Expect(workloadsToCreate).To(Equal(workloads)) }) }) }) + }) }) diff --git a/go.mod b/go.mod index 4a52dcb7..48e0d60a 100644 --- a/go.mod +++ b/go.mod @@ -7,8 +7,8 @@ require ( github.com/go-logr/logr v1.4.1 github.com/google/uuid v1.6.0 github.com/minio/minio-go/v7 v7.0.68 - github.com/onsi/ginkgo/v2 v2.14.0 - github.com/onsi/gomega v1.30.0 + github.com/onsi/ginkgo/v2 v2.17.2 + github.com/onsi/gomega v1.33.1 github.com/pkg/errors v0.9.1 go.uber.org/zap v1.26.0 gopkg.in/yaml.v2 v2.4.0 @@ -44,7 +44,7 @@ require ( github.com/go-openapi/jsonpointer v0.19.6 // indirect github.com/go-openapi/jsonreference v0.20.2 // indirect github.com/go-openapi/swag v0.22.3 // indirect - github.com/go-task/slim-sprig v0.0.0-20230315185526-52ccab3ef572 // indirect + github.com/go-task/slim-sprig/v3 v3.0.0 // indirect github.com/gobuffalo/flect v1.0.2 // indirect github.com/gogo/protobuf v1.3.2 // indirect github.com/golang/groupcache v0.0.0-20210331224755-41bb18bfe9da // indirect @@ -52,7 +52,7 @@ require ( github.com/google/gnostic-models v0.6.8 // indirect github.com/google/go-cmp v0.6.0 // indirect github.com/google/gofuzz v1.2.0 // indirect - github.com/google/pprof v0.0.0-20210720184732-4bb14d4b1be1 // indirect + github.com/google/pprof v0.0.0-20240424215950-a892ee059fd6 // indirect github.com/imdario/mergo v0.3.16 // indirect github.com/jbenet/go-context v0.0.0-20150711004518-d14ea06fba99 // indirect github.com/josharian/intern v1.0.0 // indirect @@ -81,17 +81,18 @@ require ( go.uber.org/multierr v1.11.0 // indirect golang.org/x/crypto v0.22.0 // indirect golang.org/x/exp v0.0.0-20220722155223-a9213eeb770e // indirect - golang.org/x/mod v0.14.0 // indirect + golang.org/x/mod v0.17.0 // indirect golang.org/x/net v0.24.0 // indirect golang.org/x/oauth2 v0.12.0 // indirect + golang.org/x/sync v0.7.0 // indirect golang.org/x/sys v0.19.0 // indirect golang.org/x/term v0.19.0 // indirect golang.org/x/text v0.14.0 // indirect golang.org/x/time v0.3.0 // indirect - golang.org/x/tools v0.17.0 // indirect + golang.org/x/tools v0.20.0 // indirect gomodules.xyz/jsonpatch/v2 v2.4.0 // indirect google.golang.org/appengine v1.6.7 // indirect - google.golang.org/protobuf v1.31.0 // indirect + google.golang.org/protobuf v1.33.0 // indirect gopkg.in/inf.v0 v0.9.1 // indirect gopkg.in/ini.v1 v1.67.0 // indirect gopkg.in/warnings.v0 v0.1.2 // indirect diff --git a/go.sum b/go.sum index 6bb7bb0e..1967a54a 100644 --- a/go.sum +++ b/go.sum @@ -18,9 +18,6 @@ github.com/blang/semver/v4 v4.0.0/go.mod h1:IbckMUScFkM3pff0VJDNKRiT6TG/YpiHIM2y github.com/bwesterb/go-ristretto v1.2.3/go.mod h1:fUIoIZaG73pV5biE2Blr2xEzDoMj7NFEuV9ekS419A0= github.com/cespare/xxhash/v2 v2.2.0 h1:DC2CZ1Ep5Y4k3ZQ899DldepgrayRUGE6BBZ/cd9Cj44= github.com/cespare/xxhash/v2 v2.2.0/go.mod h1:VGX0DQ3Q6kWi7AoAeZDth3/j3BFtOZR5XLFGgcrjCOs= -github.com/chzyer/logex v1.1.10/go.mod h1:+Ywpsq7O8HXn0nuIou7OrIPyXbp3wmkHB+jjWRnGsAI= -github.com/chzyer/readline v0.0.0-20180603132655-2972be24d48e/go.mod h1:nSuG5e5PlCu98SY8svDHJxuZscDgtXS6KTTbou5AhLI= -github.com/chzyer/test v0.0.0-20180213035817-a1ea475d72b1/go.mod h1:Q3SI9o4m/ZMnBNeIyt5eFwwo7qiLfzFZmjNmxjkiQlU= github.com/cloudflare/circl v1.3.3 h1:fE/Qz0QdIGqeWfnwq0RE0R7MI51s0M2E4Ga9kq5AEMs= github.com/cloudflare/circl v1.3.3/go.mod h1:5XYMA4rFBvNIrhs50XuiBJ15vF2pZn4nnUKZrLbUZFA= github.com/creack/pty v1.1.9/go.mod h1:oKZEueFk5CKHvIhNR5MUki03XCEU+Q6VDXinZuGJ33E= @@ -64,8 +61,8 @@ github.com/go-openapi/jsonreference v0.20.2 h1:3sVjiK66+uXK/6oQ8xgcRKcFgQ5KXa2Kv github.com/go-openapi/jsonreference v0.20.2/go.mod h1:Bl1zwGIM8/wsvqjsOQLJ/SH+En5Ap4rVB5KVcIDZG2k= github.com/go-openapi/swag v0.22.3 h1:yMBqmnQ0gyZvEb/+KzuWZOXgllrXT4SADYbvDaXHv/g= github.com/go-openapi/swag v0.22.3/go.mod h1:UzaqsxGiab7freDnrUUra0MwWfN/q7tE4j+VcZ0yl14= -github.com/go-task/slim-sprig v0.0.0-20230315185526-52ccab3ef572 h1:tfuBGBXKqDEevZMzYi5KSi8KkcZtzBcTgAUUtapy0OI= -github.com/go-task/slim-sprig v0.0.0-20230315185526-52ccab3ef572/go.mod h1:9Pwr4B2jHnOSGXyyzV8ROjYa2ojvAY6HCGYYfMoC3Ls= +github.com/go-task/slim-sprig/v3 v3.0.0 h1:sUs3vkvUymDpBKi3qH1YSqBQk9+9D/8M2mN1vB6EwHI= +github.com/go-task/slim-sprig/v3 v3.0.0/go.mod h1:W848ghGpv3Qj3dhTPRyJypKRiqCdHZiAzKg9hl15HA8= github.com/gobuffalo/flect v1.0.2 h1:eqjPGSo2WmjgY2XlpGwo2NXgL3RucAKo4k4qQMNA5sA= github.com/gobuffalo/flect v1.0.2/go.mod h1:A5msMlrHtLqh9umBSnvabjsMrCcCpAyzglnDvkbYKHs= github.com/gogo/protobuf v1.3.2 h1:Ov1cvc58UF3b5XjBnZv7+opcTcQFZebYjWzi34vdm4Q= @@ -85,11 +82,10 @@ github.com/google/go-cmp v0.6.0/go.mod h1:17dUlkBOakJ0+DkrSSNjCkIjxS6bF9zb3elmeN github.com/google/gofuzz v1.0.0/go.mod h1:dBl0BpW6vV/+mYPU4Po3pmUjxk6FQPldtuIdl/M65Eg= github.com/google/gofuzz v1.2.0 h1:xRy4A+RhZaiKjJ1bPfwQ8sedCA+YS2YcCHW6ec7JMi0= github.com/google/gofuzz v1.2.0/go.mod h1:dBl0BpW6vV/+mYPU4Po3pmUjxk6FQPldtuIdl/M65Eg= -github.com/google/pprof v0.0.0-20210720184732-4bb14d4b1be1 h1:K6RDEckDVWvDI9JAJYCmNdQXq6neHJOYx3V6jnqNEec= -github.com/google/pprof v0.0.0-20210720184732-4bb14d4b1be1/go.mod h1:kpwsk12EmLew5upagYY7GY0pfYCcupk39gWOCRROcvE= +github.com/google/pprof v0.0.0-20240424215950-a892ee059fd6 h1:k7nVchz72niMH6YLQNvHSdIE7iqsQxK1P41mySCvssg= +github.com/google/pprof v0.0.0-20240424215950-a892ee059fd6/go.mod h1:kf6iHlnVGwgKolg33glAes7Yg/8iWP8ukqeldJSO7jw= github.com/google/uuid v1.6.0 h1:NIvaJDMOsjHA8n1jAhLSgzrAzy1Hgr+hNrb57e+94F0= github.com/google/uuid v1.6.0/go.mod h1:TIyPZe4MgqvfeYDBFedMoGGpEw/LqOeaOT+nhxU+yHo= -github.com/ianlancetaylor/demangle v0.0.0-20200824232613-28f6c0f3b639/go.mod h1:aSSvb/t6k1mPoxDqO4vJh6VOCGPwU4O0C2/Eqndh1Sc= github.com/imdario/mergo v0.3.16 h1:wwQJbIsHYGMUyLSPrEq1CT16AhnhNJQ51+4fdHUnCl4= github.com/imdario/mergo v0.3.16/go.mod h1:WBLT9ZmE3lPoWsEzCh9LPo3TiwVN+ZKEjmz+hD27ysY= github.com/jbenet/go-context v0.0.0-20150711004518-d14ea06fba99 h1:BQSFePA1RWJOlocH6Fxy8MmwDt+yVQYULKfN0RoTN8A= @@ -134,10 +130,10 @@ github.com/modern-go/reflect2 v1.0.2 h1:xBagoLtFs94CBntxluKeaWgTMpvLxC4ur3nMaC9G github.com/modern-go/reflect2 v1.0.2/go.mod h1:yWuevngMOJpCy52FWWMvUC8ws7m/LJsjYzDa0/r8luk= github.com/munnerz/goautoneg v0.0.0-20191010083416-a7dc8b61c822 h1:C3w9PqII01/Oq1c1nUAm88MOHcQC9l5mIlSMApZMrHA= github.com/munnerz/goautoneg v0.0.0-20191010083416-a7dc8b61c822/go.mod h1:+n7T8mK8HuQTcFwEeznm/DIxMOiR9yIdICNftLE1DvQ= -github.com/onsi/ginkgo/v2 v2.14.0 h1:vSmGj2Z5YPb9JwCWT6z6ihcUvDhuXLc3sJiqd3jMKAY= -github.com/onsi/ginkgo/v2 v2.14.0/go.mod h1:JkUdW7JkN0V6rFvsHcJ478egV3XH9NxpD27Hal/PhZw= -github.com/onsi/gomega v1.30.0 h1:hvMK7xYz4D3HapigLTeGdId/NcfQx1VHMJc60ew99+8= -github.com/onsi/gomega v1.30.0/go.mod h1:9sxs+SwGrKI0+PWe4Fxa9tFQQBG5xSsSbMXOI8PPpoQ= +github.com/onsi/ginkgo/v2 v2.17.2 h1:7eMhcy3GimbsA3hEnVKdw/PQM9XN9krpKVXsZdph0/g= +github.com/onsi/ginkgo/v2 v2.17.2/go.mod h1:nP2DPOQoNsQmsVyv5rDA8JkXQoCs6goXIvr/PRJ1eCc= +github.com/onsi/gomega v1.33.1 h1:dsYjIxxSR755MDmKVsaFQTE22ChNBcuuTWgkUDSubOk= +github.com/onsi/gomega v1.33.1/go.mod h1:U4R44UsT+9eLIaYRB2a5qajjtQYn0hauxvRm16AVYg0= github.com/pjbgf/sha1cd v0.3.0 h1:4D5XXmUUBUl/xQ6IjCkEAbqXskkq/4O7LmGn0AqMDs4= github.com/pjbgf/sha1cd v0.3.0/go.mod h1:nZ1rrWOcGJ5uZgEEVL1VUM9iRQiZvWdbZjkKyFzPPsI= github.com/pkg/errors v0.9.1 h1:FEBLx1zS214owpjy7qsBeixbURkuhQAwrK5UwLGTwt4= @@ -169,7 +165,6 @@ github.com/stretchr/objx v0.5.0/go.mod h1:Yh+to48EsGEfYuaHDzXPcE3xhTkx73EhmCGUpE github.com/stretchr/testify v1.2.2/go.mod h1:a8OnRcib4nhh0OaRAV+Yts87kKdq0PP7pXfy6kDkUVs= github.com/stretchr/testify v1.3.0/go.mod h1:M5WIy9Dh21IEIfnGCwXGc5bZfKNJtfHm1UVUgZn+9EI= github.com/stretchr/testify v1.4.0/go.mod h1:j7eGeouHqKxXV5pUuKE4zz7dFj8WfuZ+81PSLYec5m4= -github.com/stretchr/testify v1.6.1/go.mod h1:6Fq8oRcR53rry900zMqJjRRixrwX3KX962/h/Wwjteg= github.com/stretchr/testify v1.7.1/go.mod h1:6Fq8oRcR53rry900zMqJjRRixrwX3KX962/h/Wwjteg= github.com/stretchr/testify v1.8.0/go.mod h1:yNjHg4UonilssWZ8iaSj1OCr/vHnekPRkoO+kdMU+MU= github.com/stretchr/testify v1.8.1/go.mod h1:w2LPCIKwWwSfY2zedu0+kehJoqGctiVI29o6fzry7u4= @@ -201,8 +196,8 @@ golang.org/x/mod v0.2.0/go.mod h1:s0Qsj1ACt9ePp/hMypM3fl4fZqREWJwdYDEqhRiZZUA= golang.org/x/mod v0.3.0/go.mod h1:s0Qsj1ACt9ePp/hMypM3fl4fZqREWJwdYDEqhRiZZUA= golang.org/x/mod v0.6.0-dev.0.20220419223038-86c51ed26bb4/go.mod h1:jJ57K6gSWd91VN4djpZkiMVwK6gcyfeH4XE8wZrZaV4= golang.org/x/mod v0.8.0/go.mod h1:iBbtSCu2XBx23ZKBPSOrRkjjQPZFPuis4dIYUhu/chs= -golang.org/x/mod v0.14.0 h1:dGoOF9QVLYng8IHTm7BAyWqCqSheQ5pYWGhzW00YJr0= -golang.org/x/mod v0.14.0/go.mod h1:hTbmBsO62+eylJbnUtE2MGJUyE7QWk4xUqPFrRgJ+7c= +golang.org/x/mod v0.17.0 h1:zY54UmvipHiNd+pm+m0x9KhZ9hl1/7QNMyxXbc6ICqA= +golang.org/x/mod v0.17.0/go.mod h1:hTbmBsO62+eylJbnUtE2MGJUyE7QWk4xUqPFrRgJ+7c= golang.org/x/net v0.0.0-20190404232315-eb5bcb51f2a3/go.mod h1:t9HGtf8HONx5eT2rtn7q6eTqICYqUVnKs3thJo3Qplg= golang.org/x/net v0.0.0-20190603091049-60506f45cf65/go.mod h1:HSz+uSET+XFnRR8LxR5pz3Of3rY3CfYBVs4xY44aLks= golang.org/x/net v0.0.0-20190620200207-3b0461eec859/go.mod h1:z5CRVTTTmAJ677TzLLGU+0bjPO0LkuOLi4/5GtJWs/s= @@ -223,12 +218,11 @@ golang.org/x/sync v0.0.0-20190911185100-cd5d95a43a6e/go.mod h1:RxMgew5VJxzue5/jJ golang.org/x/sync v0.0.0-20201020160332-67f06af15bc9/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM= golang.org/x/sync v0.0.0-20220722155255-886fb9371eb4/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM= golang.org/x/sync v0.1.0/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM= -golang.org/x/sync v0.6.0 h1:5BMeUDZ7vkXGfEr1x9B4bRcTH4lpkTkpdh0T/J+qjbQ= -golang.org/x/sync v0.6.0/go.mod h1:Czt+wKu1gCyEFDUtn0jG5QVvpJ6rzVqr5aXyt9drQfk= +golang.org/x/sync v0.7.0 h1:YsImfSBoP9QPYL0xyKJPq0gcaJdG3rInoqxTWbfQu9M= +golang.org/x/sync v0.7.0/go.mod h1:Czt+wKu1gCyEFDUtn0jG5QVvpJ6rzVqr5aXyt9drQfk= golang.org/x/sys v0.0.0-20190215142949-d0b11bdaac8a/go.mod h1:STP8DvDyc/dI5b8T5hshtkjS+E42TnysNCUPdjciGhY= golang.org/x/sys v0.0.0-20190412213103-97732733099d/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= golang.org/x/sys v0.0.0-20191026070338-33540a1f6037/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= -golang.org/x/sys v0.0.0-20191204072324-ce4227a45e2e/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= golang.org/x/sys v0.0.0-20200930185726-fdedc70b468f/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= golang.org/x/sys v0.0.0-20201119102817-f84b799fce68/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= golang.org/x/sys v0.0.0-20210124154548-22da62e12c0c/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= @@ -268,8 +262,8 @@ golang.org/x/tools v0.0.0-20200619180055-7c47624df98f/go.mod h1:EkVYQZoAsY45+roY golang.org/x/tools v0.0.0-20210106214847-113979e3529a/go.mod h1:emZCQorbCU4vsT4fOWvOPXz4eW1wZW4PmDk9uLelYpA= golang.org/x/tools v0.1.12/go.mod h1:hNGJHUnrk76NpqgfD5Aqm5Crs+Hm0VOH/i9J2+nxYbc= golang.org/x/tools v0.6.0/go.mod h1:Xwgl3UAJ/d3gWutnCtw505GrjyAbvKui8lOU390QaIU= -golang.org/x/tools v0.17.0 h1:FvmRgNOcs3kOa+T20R1uhfP9F6HgG2mfxDv1vrx1Htc= -golang.org/x/tools v0.17.0/go.mod h1:xsh6VxdV005rRVaS6SSAf9oiAqljS7UZUacMZ8Bnsps= +golang.org/x/tools v0.20.0 h1:hz/CVckiOxybQvFw6h7b/q80NTr9IUQb4s1IIzW7KNY= +golang.org/x/tools v0.20.0/go.mod h1:WvitBU7JJf6A4jOdg4S1tviW9bhUxkgeCui/0JHctQg= golang.org/x/xerrors v0.0.0-20190717185122-a985d3407aa7/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0= golang.org/x/xerrors v0.0.0-20191011141410-1b5146add898/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0= golang.org/x/xerrors v0.0.0-20191204190536-9bdfabe68543/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0= @@ -280,8 +274,8 @@ google.golang.org/appengine v1.6.7 h1:FZR1q0exgwxzPzp/aF+VccGrSfxfPpkBqjIIEq3ru6 google.golang.org/appengine v1.6.7/go.mod h1:8WjMMxjGQR8xUklV/ARdw2HLXBOI7O7uCIDZVag1xfc= google.golang.org/protobuf v1.26.0-rc.1/go.mod h1:jlhhOSvTdKEhbULTjvd4ARK9grFBp09yW+WbY/TyQbw= google.golang.org/protobuf v1.26.0/go.mod h1:9q0QmTI4eRPtz6boOQmLYwt+qCgq0jsYwAQnmE0givc= -google.golang.org/protobuf v1.31.0 h1:g0LDEJHgrBl9N9r17Ru3sqWhkIx2NB67okBHPwC7hs8= -google.golang.org/protobuf v1.31.0/go.mod h1:HV8QOd/L58Z+nl8r43ehVNZIU/HEI6OcFqwMG9pJV4I= +google.golang.org/protobuf v1.33.0 h1:uNO2rsAINq/JlFpSdYEKIZ0uKD/R9cpdv0T+yoGwGmI= +google.golang.org/protobuf v1.33.0/go.mod h1:c6P6GXX6sHbq/GpV6MGZEdwhWPcYBgnhAHhKbcUYpos= gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0= gopkg.in/check.v1 v1.0.0-20190902080502-41f04d3bba15/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0= gopkg.in/check.v1 v1.0.0-20201130134442-10cb98267c6c h1:Hei/4ADfdWqJk1ZMxUNpqntNwaWcugrBjAiHlqqRiVk= diff --git a/lib/writers/git.go b/lib/writers/git.go index 3f101de5..b0ba9ec5 100644 --- a/lib/writers/git.go +++ b/lib/writers/git.go @@ -2,7 +2,6 @@ package writers import ( "fmt" - "io/fs" "os" "path/filepath" "strings" @@ -36,15 +35,7 @@ type gitAuthor struct { Email string } -const ( - Add string = "Add" - Delete string = "Delete" -) - func NewGitWriter(logger logr.Logger, stateStoreSpec v1alpha1.GitStateStoreSpec, destination v1alpha1.Destination, creds map[string][]byte) (StateStoreWriter, error) { - - //if basic auth check this - //otherwise check ssh and build using ssh lib var authMethod transport.AuthMethod switch stateStoreSpec.AuthMethod { case v1alpha1.SSHAuthMethod: @@ -113,61 +104,40 @@ func NewGitWriter(logger logr.Logger, stateStoreSpec v1alpha1.GitStateStoreSpec, }, nil } -func (g *GitWriter) setupLocalDirectoryWithRepo(logger logr.Logger) (string, *git.Repository, *git.Worktree, error) { - localTmpDir, err := createLocalDirectory(logger) - if err != nil { - logger.Error(err, "could not create temporary repository directory") - return "", nil, nil, err - } +func (g *GitWriter) UpdateFiles(workPlacementName string, workloadsToCreate []v1alpha1.Workload, workloadsToDelete []string) error { + return g.update("", workPlacementName, workloadsToCreate, workloadsToDelete) +} - repo, err := g.cloneRepo(localTmpDir, logger) - if err != nil { - logger.Error(err, "could not clone repository") - return "", nil, nil, err - } +func (g *GitWriter) UpdateInDir(subDir, workPlacementName string, workloadsToCreate []v1alpha1.Workload) error { + return g.update(subDir, workPlacementName, workloadsToCreate, nil) +} - worktree, err := repo.Worktree() - if err != nil { - logger.Error(err, "could not access repo worktree") - return "", nil, nil, err +func (g *GitWriter) update(subDir, workPlacementName string, workloadsToCreate []v1alpha1.Workload, workloadsToDelete []string) error { + if len(workloadsToCreate) == 0 && len(workloadsToDelete) == 0 && subDir == "" { + return nil } - return localTmpDir, repo, worktree, nil -} -func (g *GitWriter) WriteDirWithObjects(deleteExistingContentsInDir bool, subDir string, toWrite ...v1alpha1.Workload) error { dirInGitRepo := filepath.Join(g.path, subDir) logger := g.Log.WithValues( "dir", dirInGitRepo, "branch", g.gitServer.Branch, ) - if len(toWrite) == 0 { - logger.Info("Empty workloads. Nothing to write to Git") - return nil - } - localTmpDir, repo, worktree, err := g.setupLocalDirectoryWithRepo(logger) if err != nil { return err } defer os.RemoveAll(filepath.Dir(localTmpDir)) - if deleteExistingContentsInDir { - logger.Info("checking if any existing directories needs to be deleted") - if _, err := worktree.Filesystem.Lstat(dirInGitRepo); err == nil { - logger.Info("deleting existing content") - if _, err := worktree.Remove(dirInGitRepo); err != nil { - logger.Error(err, "could not add directory deletion to worktree", "dir", dirInGitRepo) - return err - } - } + err = g.deleteExistingFiles(subDir == "", dirInGitRepo, workloadsToDelete, worktree, logger) + if err != nil { + return err } - var filesCommitted []string - for _, item := range toWrite { + for _, file := range workloadsToCreate { //worker-cluster/resources////foo/bar/baz.yaml - worktreeFilePath := filepath.Join(dirInGitRepo, item.Filepath) - logger := logger.WithValues( + worktreeFilePath := filepath.Join(dirInGitRepo, file.Filepath) + log := logger.WithValues( "filepath", worktreeFilePath, ) @@ -176,69 +146,102 @@ func (g *GitWriter) WriteDirWithObjects(deleteExistingContentsInDir bool, subDir //We need to protect against paths containing `..` //filepath.Join expands any '../' in the path to the actual, e.g. /tmp/foo/../ resolves to /tmp/ - //To ensure they can't write to files on disk outside of the tmp git repository we check the absolute path + //To ensure they can't write to files on disk outside the tmp git repository we check the absolute path //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) { - logger.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 repostiory", "absolutePath", absoluteFilePath, "tmpDir", localTmpDir) return nil //We don't want to retry as this isn't a recoverable error. Log error and return nil. } if err := os.MkdirAll(filepath.Dir(absoluteFilePath), 0700); err != nil { - logger.Error(err, "could not generate local directories") + log.Error(err, "could not generate local directories") return err } - if err := os.WriteFile(absoluteFilePath, []byte(item.Content), 0644); err != nil { - logger.Error(err, "could not write to file") + if err := os.WriteFile(absoluteFilePath, []byte(file.Content), 0644); err != nil { + log.Error(err, "could not write to file") return err } if _, err := worktree.Add(worktreeFilePath); err != nil { - logger.Error(err, "could not add file to worktree") + log.Error(err, "could not add file to worktree") return err } - filesCommitted = append(filesCommitted, worktreeFilePath) } - return g.commitAndPush(repo, worktree, Add, filesCommitted, logger) + action := "Delete" + if len(workloadsToCreate) > 0 { + action = "Update" + } + return g.commitAndPush(repo, worktree, action, workPlacementName, logger) } -func (g *GitWriter) RemoveObject(filePath string) error { - logger := g.Log.WithValues("dir", g.path, "filepath", filePath) +// deleteExistingFiles removes all files in dir when removeDirectory is set to true +// else it removes files listed in workloadsToDelete +func (g *GitWriter) deleteExistingFiles(removeDirectory bool, dir string, workloadsToDelete []string, worktree *git.Worktree, logger logr.Logger) error { + if removeDirectory { + if _, err := worktree.Filesystem.Lstat(dir); err == nil { + logger.Info("deleting existing content") + if _, err := worktree.Remove(dir); err != nil { + logger.Error(err, "could not add directory deletion to worktree", "dir", dir) + return err + } + } + } else { + for _, file := range workloadsToDelete { + worktreeFilePath := filepath.Join(dir, file) + log := logger.WithValues( + "filepath", worktreeFilePath, + ) + if _, err := worktree.Filesystem.Lstat(worktreeFilePath); err != nil { + log.Info("file requested to be deleted from worktree but does not exist") + continue + } + if _, err := worktree.Remove(worktreeFilePath); err != nil { + logger.Error(err, "could not remove file from worktree") + return err + } + logger.Info("successfully deleted file from worktree") + } + } + return nil +} - localTmpDir, repo, worktree, err := g.setupLocalDirectoryWithRepo(logger) +func (g *GitWriter) ReadFile(filePath string) ([]byte, error) { + logger := g.Log.WithValues( + "path", filePath, + "branch", g.gitServer.Branch, + ) + + localTmpDir, _, _, err := g.setupLocalDirectoryWithRepo(logger) if err != nil { - return err + return nil, err } defer os.RemoveAll(filepath.Dir(localTmpDir)) - worktreeFilepath := filepath.Join(g.path, filePath) - if _, err := worktree.Filesystem.Lstat(worktreeFilepath); err == nil { - if _, err := worktree.Remove(worktreeFilepath); err != nil { - logger.Error(err, "could not remove file from worktree") - return err - } - logger.Info("successfully deleted file from worktree") - } else { - // Added for debugging purposes to help with bug #186921254 - files := []string{} - walkFunc := func(s string, d fs.DirEntry, err error) error { - if err != nil { - return nil - } - files = append(files, s) - return nil - } - filepath.WalkDir(localTmpDir, walkFunc) - logger.Info("file does not exist on worktree, nothing to delete", "lstatErr", err, "allFiles", files) - return nil + return os.ReadFile(filepath.Join(localTmpDir, filePath)) +} + +func (g *GitWriter) setupLocalDirectoryWithRepo(logger logr.Logger) (string, *git.Repository, *git.Worktree, error) { + localTmpDir, err := createLocalDirectory(logger) + if err != nil { + logger.Error(err, "could not create temporary repository directory") + return "", nil, nil, err } - if err := g.commitAndPush(repo, worktree, Delete, []string{worktreeFilepath}, logger); err != nil { - return err + repo, err := g.cloneRepo(localTmpDir, logger) + if err != nil { + logger.Error(err, "could not clone repository") + return "", nil, nil, err } - return nil + + worktree, err := repo.Worktree() + if err != nil { + logger.Error(err, "could not access repo worktree") + return "", nil, nil, err + } + return localTmpDir, repo, worktree, nil } func (g *GitWriter) push(repo *git.Repository, logger logr.Logger) error { @@ -267,7 +270,7 @@ func (g *GitWriter) cloneRepo(localRepoFilePath string, logger logr.Logger) (*gi }) } -func (g *GitWriter) commitAndPush(repo *git.Repository, worktree *git.Worktree, action string, filesToAdd []string, logger logr.Logger) error { +func (g *GitWriter) commitAndPush(repo *git.Repository, worktree *git.Worktree, action, workPlacementName string, logger logr.Logger) error { status, err := worktree.Status() if err != nil { logger.Error(err, "could not get worktree status") @@ -279,9 +282,7 @@ func (g *GitWriter) commitAndPush(repo *git.Repository, worktree *git.Worktree, return nil } - //should fileToAdd be here at all? is it valuable? specifically the fileToAdd parameter - logger.Info("committing changes", "filesAdded", filesToAdd) - _, err = worktree.Commit(fmt.Sprintf("%s: %v", action, filesToAdd), &git.CommitOptions{ + _, err = worktree.Commit(fmt.Sprintf("%s from: %s", action, workPlacementName), &git.CommitOptions{ Author: &object.Signature{ Name: g.author.Name, Email: g.author.Email, diff --git a/lib/writers/s3.go b/lib/writers/s3.go index 62ace408..f7d98edd 100644 --- a/lib/writers/s3.go +++ b/lib/writers/s3.go @@ -72,22 +72,53 @@ func NewS3Writer(logger logr.Logger, stateStoreSpec v1alpha1.BucketStateStoreSpe }, nil } -func (b *S3Writer) WriteDirWithObjects(deleteExistingContentsInDir bool, dir string, toWrite ...v1alpha1.Workload) error { - logger := b.Log.WithValues( - "bucketName", b.BucketName, - "path", b.path, - ) +func (b *S3Writer) ReadFile(filename string) ([]byte, error) { + obj, err := b.RepoClient.GetObject(context.Background(), b.BucketName, filepath.Join(b.path, filename), minio.GetObjectOptions{}) + if err != nil { + return nil, err + } + defer obj.Close() - objectsToDeleteMap := map[string]minio.ObjectInfo{} + var content []byte + if _, err = obj.Read(content); err != nil { + return nil, err + } + return content, nil +} + +func (b *S3Writer) UpdateFiles(_ string, workloadsToCreate []v1alpha1.Workload, workloadsToDelete []string) error { + return b.update("", workloadsToCreate, workloadsToDelete) +} + +func (b *S3Writer) UpdateInDir(subDir, _ string, workloadsToCreate []v1alpha1.Workload) error { + return b.update(subDir, workloadsToCreate, nil) +} + +func (b *S3Writer) update(subDir string, workloadsToCreate []v1alpha1.Workload, workloadsToDelete []string) error { ctx := context.Background() + logger := b.Log.WithValues("bucketName", b.BucketName, "path", b.path) + + objectsToDeleteMap := map[string]minio.ObjectInfo{} //Get a list of all the old workload files, we delete any that aren't part of the new workload at the end of this function. - if deleteExistingContentsInDir { + if subDir != "" { var err error - objectsToDeleteMap, err = b.getObjectsInDir(ctx, dir, logger) + objectsToDeleteMap, err = b.getObjectsInDir(ctx, subDir, logger) if err != nil { return err } + } else { + for _, work := range workloadsToDelete { + objStat, err := b.RepoClient.StatObject(ctx, b.BucketName, filepath.Join(b.path, work), minio.GetObjectOptions{}) + if err != nil { + if minio.ToErrorResponse(err).Code != "NoSuchKey" { + logger.Error(err, "Error fetching object") + return err + } + logger.Info("Object does not exist yet") + } + objectsToDeleteMap[objStat.Key] = objStat + } } exists, errBucketExists := b.RepoClient.BucketExists(ctx, b.BucketName) @@ -97,41 +128,34 @@ func (b *S3Writer) WriteDirWithObjects(deleteExistingContentsInDir bool, dir str logger.Info("Bucket provided does not exist (or the provided keys don't have permissions)") } - for _, item := range toWrite { - objectFullPath := filepath.Join(b.path, dir, item.Filepath) - // Make sure we don't delete this object, remove this object from the map + for _, work := range workloadsToCreate { + objectFullPath := filepath.Join(b.path, subDir, work.Filepath) delete(objectsToDeleteMap, objectFullPath) + log := logger.WithValues("objectName", objectFullPath) - logger := b.Log.WithValues( - "objectName", objectFullPath, - ) - - ctx := context.Background() - - reader := bytes.NewReader([]byte(item.Content)) - + reader := bytes.NewReader([]byte(work.Content)) objStat, err := b.RepoClient.StatObject(ctx, b.BucketName, objectFullPath, minio.GetObjectOptions{}) if err != nil { if minio.ToErrorResponse(err).Code != "NoSuchKey" { - logger.Error(err, "Error fetching object") + log.Error(err, "Error fetching object") return err } - logger.Info("Object does not exist yet") + log.Info("Object does not exist yet") } else { - contentMd5 := fmt.Sprintf("%x", md5.Sum([]byte(item.Content))) + contentMd5 := fmt.Sprintf("%x", md5.Sum([]byte(work.Content))) if objStat.ETag == contentMd5 { - logger.Info("Content has not changed, will not re-write to bucket") + log.Info("Content has not changed, will not re-write to bucket") continue } } - logger.Info("Writing object to bucket") + log.Info("Writing object to bucket") _, err = b.RepoClient.PutObject(ctx, b.BucketName, objectFullPath, reader, reader.Size(), minio.PutObjectOptions{}) if err != nil { - logger.Error(err, "Error writing object to bucket") + log.Error(err, "Error writing object to bucket") return err } - logger.Info("Object written to bucket") + log.Info("Object written to bucket") } return b.deleteObjects(ctx, objectsToDeleteMap, logger) diff --git a/lib/writers/s3_test.go b/lib/writers/s3_test.go index 644adfd7..e9c3a5dc 100644 --- a/lib/writers/s3_test.go +++ b/lib/writers/s3_test.go @@ -113,6 +113,7 @@ var _ = Describe("S3", func() { Expect(err).To(MatchError("unknown authMethod foo")) }) }) + }) }) diff --git a/lib/writers/statestore_writer.go b/lib/writers/statestore_writer.go index 9243d7c8..aa331dea 100644 --- a/lib/writers/statestore_writer.go +++ b/lib/writers/statestore_writer.go @@ -2,13 +2,9 @@ package writers import "github.com/syntasso/kratix/api/v1alpha1" -const ( - DeleteExistingContentsInDir = true - PreserveExistingContentsInDir = false -) - //go:generate go run github.com/maxbrunsfeld/counterfeiter/v6 . StateStoreWriter type StateStoreWriter interface { - WriteDirWithObjects(deleteExistingContentsInDir bool, dir string, workloads ...v1alpha1.Workload) error - RemoveObject(objectName string) error + UpdateFiles(workPlacementName string, workloadsToCreate []v1alpha1.Workload, workloadsToDelete []string) error + UpdateInDir(subDir, workPlacementName string, workloadsToCreate []v1alpha1.Workload) error + ReadFile(filename string) ([]byte, error) } diff --git a/lib/writers/writersfakes/fake_state_store_writer.go b/lib/writers/writersfakes/fake_state_store_writer.go index 50c859c4..38bd2b43 100644 --- a/lib/writers/writersfakes/fake_state_store_writer.go +++ b/lib/writers/writersfakes/fake_state_store_writer.go @@ -9,109 +9,205 @@ import ( ) type FakeStateStoreWriter struct { - RemoveObjectStub func(string) error - removeObjectMutex sync.RWMutex - removeObjectArgsForCall []struct { + ReadFileStub func(string) ([]byte, error) + readFileMutex sync.RWMutex + readFileArgsForCall []struct { arg1 string } - removeObjectReturns struct { + readFileReturns struct { + result1 []byte + result2 error + } + readFileReturnsOnCall map[int]struct { + result1 []byte + result2 error + } + UpdateFilesStub func(string, []v1alpha1.Workload, []string) error + updateFilesMutex sync.RWMutex + updateFilesArgsForCall []struct { + arg1 string + arg2 []v1alpha1.Workload + arg3 []string + } + updateFilesReturns struct { result1 error } - removeObjectReturnsOnCall map[int]struct { + updateFilesReturnsOnCall map[int]struct { result1 error } - WriteDirWithObjectsStub func(bool, string, ...v1alpha1.Workload) error - writeDirWithObjectsMutex sync.RWMutex - writeDirWithObjectsArgsForCall []struct { - arg1 bool + UpdateInDirStub func(string, string, []v1alpha1.Workload) error + updateInDirMutex sync.RWMutex + updateInDirArgsForCall []struct { + arg1 string arg2 string arg3 []v1alpha1.Workload } - writeDirWithObjectsReturns struct { + updateInDirReturns struct { result1 error } - writeDirWithObjectsReturnsOnCall map[int]struct { + updateInDirReturnsOnCall map[int]struct { result1 error } invocations map[string][][]interface{} invocationsMutex sync.RWMutex } -func (fake *FakeStateStoreWriter) RemoveObject(arg1 string) error { - fake.removeObjectMutex.Lock() - ret, specificReturn := fake.removeObjectReturnsOnCall[len(fake.removeObjectArgsForCall)] - fake.removeObjectArgsForCall = append(fake.removeObjectArgsForCall, struct { +func (fake *FakeStateStoreWriter) ReadFile(arg1 string) ([]byte, error) { + fake.readFileMutex.Lock() + ret, specificReturn := fake.readFileReturnsOnCall[len(fake.readFileArgsForCall)] + fake.readFileArgsForCall = append(fake.readFileArgsForCall, struct { arg1 string }{arg1}) - stub := fake.RemoveObjectStub - fakeReturns := fake.removeObjectReturns - fake.recordInvocation("RemoveObject", []interface{}{arg1}) - fake.removeObjectMutex.Unlock() + stub := fake.ReadFileStub + fakeReturns := fake.readFileReturns + fake.recordInvocation("ReadFile", []interface{}{arg1}) + fake.readFileMutex.Unlock() if stub != nil { return stub(arg1) } + if specificReturn { + return ret.result1, ret.result2 + } + return fakeReturns.result1, fakeReturns.result2 +} + +func (fake *FakeStateStoreWriter) ReadFileCallCount() int { + fake.readFileMutex.RLock() + defer fake.readFileMutex.RUnlock() + return len(fake.readFileArgsForCall) +} + +func (fake *FakeStateStoreWriter) ReadFileCalls(stub func(string) ([]byte, error)) { + fake.readFileMutex.Lock() + defer fake.readFileMutex.Unlock() + fake.ReadFileStub = stub +} + +func (fake *FakeStateStoreWriter) ReadFileArgsForCall(i int) string { + fake.readFileMutex.RLock() + defer fake.readFileMutex.RUnlock() + argsForCall := fake.readFileArgsForCall[i] + return argsForCall.arg1 +} + +func (fake *FakeStateStoreWriter) ReadFileReturns(result1 []byte, result2 error) { + fake.readFileMutex.Lock() + defer fake.readFileMutex.Unlock() + fake.ReadFileStub = nil + fake.readFileReturns = struct { + result1 []byte + result2 error + }{result1, result2} +} + +func (fake *FakeStateStoreWriter) ReadFileReturnsOnCall(i int, result1 []byte, result2 error) { + fake.readFileMutex.Lock() + defer fake.readFileMutex.Unlock() + fake.ReadFileStub = nil + if fake.readFileReturnsOnCall == nil { + fake.readFileReturnsOnCall = make(map[int]struct { + result1 []byte + result2 error + }) + } + fake.readFileReturnsOnCall[i] = struct { + result1 []byte + result2 error + }{result1, result2} +} + +func (fake *FakeStateStoreWriter) UpdateFiles(arg1 string, arg2 []v1alpha1.Workload, arg3 []string) error { + var arg2Copy []v1alpha1.Workload + if arg2 != nil { + arg2Copy = make([]v1alpha1.Workload, len(arg2)) + copy(arg2Copy, arg2) + } + var arg3Copy []string + if arg3 != nil { + arg3Copy = make([]string, len(arg3)) + copy(arg3Copy, arg3) + } + fake.updateFilesMutex.Lock() + ret, specificReturn := fake.updateFilesReturnsOnCall[len(fake.updateFilesArgsForCall)] + fake.updateFilesArgsForCall = append(fake.updateFilesArgsForCall, struct { + arg1 string + arg2 []v1alpha1.Workload + arg3 []string + }{arg1, arg2Copy, arg3Copy}) + stub := fake.UpdateFilesStub + fakeReturns := fake.updateFilesReturns + fake.recordInvocation("UpdateFiles", []interface{}{arg1, arg2Copy, arg3Copy}) + fake.updateFilesMutex.Unlock() + if stub != nil { + return stub(arg1, arg2, arg3) + } if specificReturn { return ret.result1 } return fakeReturns.result1 } -func (fake *FakeStateStoreWriter) RemoveObjectCallCount() int { - fake.removeObjectMutex.RLock() - defer fake.removeObjectMutex.RUnlock() - return len(fake.removeObjectArgsForCall) +func (fake *FakeStateStoreWriter) UpdateFilesCallCount() int { + fake.updateFilesMutex.RLock() + defer fake.updateFilesMutex.RUnlock() + return len(fake.updateFilesArgsForCall) } -func (fake *FakeStateStoreWriter) RemoveObjectCalls(stub func(string) error) { - fake.removeObjectMutex.Lock() - defer fake.removeObjectMutex.Unlock() - fake.RemoveObjectStub = stub +func (fake *FakeStateStoreWriter) UpdateFilesCalls(stub func(string, []v1alpha1.Workload, []string) error) { + fake.updateFilesMutex.Lock() + defer fake.updateFilesMutex.Unlock() + fake.UpdateFilesStub = stub } -func (fake *FakeStateStoreWriter) RemoveObjectArgsForCall(i int) string { - fake.removeObjectMutex.RLock() - defer fake.removeObjectMutex.RUnlock() - argsForCall := fake.removeObjectArgsForCall[i] - return argsForCall.arg1 +func (fake *FakeStateStoreWriter) UpdateFilesArgsForCall(i int) (string, []v1alpha1.Workload, []string) { + fake.updateFilesMutex.RLock() + defer fake.updateFilesMutex.RUnlock() + argsForCall := fake.updateFilesArgsForCall[i] + return argsForCall.arg1, argsForCall.arg2, argsForCall.arg3 } -func (fake *FakeStateStoreWriter) RemoveObjectReturns(result1 error) { - fake.removeObjectMutex.Lock() - defer fake.removeObjectMutex.Unlock() - fake.RemoveObjectStub = nil - fake.removeObjectReturns = struct { +func (fake *FakeStateStoreWriter) UpdateFilesReturns(result1 error) { + fake.updateFilesMutex.Lock() + defer fake.updateFilesMutex.Unlock() + fake.UpdateFilesStub = nil + fake.updateFilesReturns = struct { result1 error }{result1} } -func (fake *FakeStateStoreWriter) RemoveObjectReturnsOnCall(i int, result1 error) { - fake.removeObjectMutex.Lock() - defer fake.removeObjectMutex.Unlock() - fake.RemoveObjectStub = nil - if fake.removeObjectReturnsOnCall == nil { - fake.removeObjectReturnsOnCall = make(map[int]struct { +func (fake *FakeStateStoreWriter) UpdateFilesReturnsOnCall(i int, result1 error) { + fake.updateFilesMutex.Lock() + defer fake.updateFilesMutex.Unlock() + fake.UpdateFilesStub = nil + if fake.updateFilesReturnsOnCall == nil { + fake.updateFilesReturnsOnCall = make(map[int]struct { result1 error }) } - fake.removeObjectReturnsOnCall[i] = struct { + fake.updateFilesReturnsOnCall[i] = struct { result1 error }{result1} } -func (fake *FakeStateStoreWriter) WriteDirWithObjects(arg1 bool, arg2 string, arg3 ...v1alpha1.Workload) error { - fake.writeDirWithObjectsMutex.Lock() - ret, specificReturn := fake.writeDirWithObjectsReturnsOnCall[len(fake.writeDirWithObjectsArgsForCall)] - fake.writeDirWithObjectsArgsForCall = append(fake.writeDirWithObjectsArgsForCall, struct { - arg1 bool +func (fake *FakeStateStoreWriter) UpdateInDir(arg1 string, arg2 string, arg3 []v1alpha1.Workload) error { + var arg3Copy []v1alpha1.Workload + if arg3 != nil { + arg3Copy = make([]v1alpha1.Workload, len(arg3)) + copy(arg3Copy, arg3) + } + fake.updateInDirMutex.Lock() + ret, specificReturn := fake.updateInDirReturnsOnCall[len(fake.updateInDirArgsForCall)] + fake.updateInDirArgsForCall = append(fake.updateInDirArgsForCall, struct { + arg1 string arg2 string arg3 []v1alpha1.Workload - }{arg1, arg2, arg3}) - stub := fake.WriteDirWithObjectsStub - fakeReturns := fake.writeDirWithObjectsReturns - fake.recordInvocation("WriteDirWithObjects", []interface{}{arg1, arg2, arg3}) - fake.writeDirWithObjectsMutex.Unlock() + }{arg1, arg2, arg3Copy}) + stub := fake.UpdateInDirStub + fakeReturns := fake.updateInDirReturns + fake.recordInvocation("UpdateInDir", []interface{}{arg1, arg2, arg3Copy}) + fake.updateInDirMutex.Unlock() if stub != nil { - return stub(arg1, arg2, arg3...) + return stub(arg1, arg2, arg3) } if specificReturn { return ret.result1 @@ -119,44 +215,44 @@ func (fake *FakeStateStoreWriter) WriteDirWithObjects(arg1 bool, arg2 string, ar return fakeReturns.result1 } -func (fake *FakeStateStoreWriter) WriteDirWithObjectsCallCount() int { - fake.writeDirWithObjectsMutex.RLock() - defer fake.writeDirWithObjectsMutex.RUnlock() - return len(fake.writeDirWithObjectsArgsForCall) +func (fake *FakeStateStoreWriter) UpdateInDirCallCount() int { + fake.updateInDirMutex.RLock() + defer fake.updateInDirMutex.RUnlock() + return len(fake.updateInDirArgsForCall) } -func (fake *FakeStateStoreWriter) WriteDirWithObjectsCalls(stub func(bool, string, ...v1alpha1.Workload) error) { - fake.writeDirWithObjectsMutex.Lock() - defer fake.writeDirWithObjectsMutex.Unlock() - fake.WriteDirWithObjectsStub = stub +func (fake *FakeStateStoreWriter) UpdateInDirCalls(stub func(string, string, []v1alpha1.Workload) error) { + fake.updateInDirMutex.Lock() + defer fake.updateInDirMutex.Unlock() + fake.UpdateInDirStub = stub } -func (fake *FakeStateStoreWriter) WriteDirWithObjectsArgsForCall(i int) (bool, string, []v1alpha1.Workload) { - fake.writeDirWithObjectsMutex.RLock() - defer fake.writeDirWithObjectsMutex.RUnlock() - argsForCall := fake.writeDirWithObjectsArgsForCall[i] +func (fake *FakeStateStoreWriter) UpdateInDirArgsForCall(i int) (string, string, []v1alpha1.Workload) { + fake.updateInDirMutex.RLock() + defer fake.updateInDirMutex.RUnlock() + argsForCall := fake.updateInDirArgsForCall[i] return argsForCall.arg1, argsForCall.arg2, argsForCall.arg3 } -func (fake *FakeStateStoreWriter) WriteDirWithObjectsReturns(result1 error) { - fake.writeDirWithObjectsMutex.Lock() - defer fake.writeDirWithObjectsMutex.Unlock() - fake.WriteDirWithObjectsStub = nil - fake.writeDirWithObjectsReturns = struct { +func (fake *FakeStateStoreWriter) UpdateInDirReturns(result1 error) { + fake.updateInDirMutex.Lock() + defer fake.updateInDirMutex.Unlock() + fake.UpdateInDirStub = nil + fake.updateInDirReturns = struct { result1 error }{result1} } -func (fake *FakeStateStoreWriter) WriteDirWithObjectsReturnsOnCall(i int, result1 error) { - fake.writeDirWithObjectsMutex.Lock() - defer fake.writeDirWithObjectsMutex.Unlock() - fake.WriteDirWithObjectsStub = nil - if fake.writeDirWithObjectsReturnsOnCall == nil { - fake.writeDirWithObjectsReturnsOnCall = make(map[int]struct { +func (fake *FakeStateStoreWriter) UpdateInDirReturnsOnCall(i int, result1 error) { + fake.updateInDirMutex.Lock() + defer fake.updateInDirMutex.Unlock() + fake.UpdateInDirStub = nil + if fake.updateInDirReturnsOnCall == nil { + fake.updateInDirReturnsOnCall = make(map[int]struct { result1 error }) } - fake.writeDirWithObjectsReturnsOnCall[i] = struct { + fake.updateInDirReturnsOnCall[i] = struct { result1 error }{result1} } @@ -164,10 +260,12 @@ func (fake *FakeStateStoreWriter) WriteDirWithObjectsReturnsOnCall(i int, result func (fake *FakeStateStoreWriter) Invocations() map[string][][]interface{} { fake.invocationsMutex.RLock() defer fake.invocationsMutex.RUnlock() - fake.removeObjectMutex.RLock() - defer fake.removeObjectMutex.RUnlock() - fake.writeDirWithObjectsMutex.RLock() - defer fake.writeDirWithObjectsMutex.RUnlock() + fake.readFileMutex.RLock() + defer fake.readFileMutex.RUnlock() + fake.updateFilesMutex.RLock() + defer fake.updateFilesMutex.RUnlock() + fake.updateInDirMutex.RLock() + defer fake.updateInDirMutex.RUnlock() copiedInvocations := map[string][][]interface{}{} for key, value := range fake.invocations { copiedInvocations[key] = value diff --git a/test/system/system_test.go b/test/system/system_test.go index dc0e387f..34c90f58 100644 --- a/test/system/system_test.go +++ b/test/system/system_test.go @@ -84,7 +84,6 @@ spec: %s container1Cmd: | %s` - storeType string templateImperativePlatformNamespace = "%s-platform-imperative" templateDeclarativePlatformNamespace = "%s-platform-declarative" @@ -403,6 +402,10 @@ var _ = Describe("Kratix", func() { platform.kubectl("delete", "namespace", oldRRImperativePlatformNamespace) Eventually(platform.kubectl("get", "promise")).ShouldNot(ContainSubstring(bashPromiseName)) }) + + XContext("filepathMode set to none", func() { + + }) }) When("A Promise is updated", func() { From 2b60a66d04a7cecb9ca599a632943a1305bf6c4a Mon Sep 17 00:00:00 2001 From: Chunyi Lyu Date: Tue, 28 May 2024 12:44:23 -0400 Subject: [PATCH 2/6] fix: read from bucket stateStore correctly - add system tests for filePath.mode: none Co-authored-by: Jake Klein --- controllers/workplacement_controller.go | 32 ++++--- lib/writers/git.go | 6 +- lib/writers/s3.go | 18 ++-- lib/writers/statestore_writer.go | 7 +- .../git/platform_kratix_destination.yaml | 13 +++ test/system/system_test.go | 89 ++++++++++++++----- 6 files changed, 125 insertions(+), 40 deletions(-) diff --git a/controllers/workplacement_controller.go b/controllers/workplacement_controller.go index a5d3cd55..1aff33c4 100644 --- a/controllers/workplacement_controller.go +++ b/controllers/workplacement_controller.go @@ -18,10 +18,11 @@ package controllers import ( "context" + "errors" "fmt" "github.com/go-logr/logr" "gopkg.in/yaml.v2" - "k8s.io/apimachinery/pkg/api/errors" + k8sErrors "k8s.io/apimachinery/pkg/api/errors" "k8s.io/apimachinery/pkg/types" "path/filepath" ctrl "sigs.k8s.io/controller-runtime" @@ -66,7 +67,7 @@ func (r *WorkPlacementReconciler) Reconcile(ctx context.Context, req ctrl.Reques workPlacement := &v1alpha1.WorkPlacement{} err := r.Client.Get(context.Background(), req.NamespacedName, workPlacement) if err != nil { - if errors.IsNotFound(err) { + if k8sErrors.IsNotFound(err) { return ctrl.Result{}, nil } logger.Error(err, "Error getting WorkPlacement", "workPlacement", req.Name) @@ -92,7 +93,7 @@ func (r *WorkPlacementReconciler) Reconcile(ctx context.Context, req ctrl.Reques //Mock this out writer, err := newWriter(opts, *destination) if err != nil { - if errors.IsNotFound(err) { + if k8sErrors.IsNotFound(err) { return defaultRequeue, nil } return ctrl.Result{}, err @@ -119,17 +120,18 @@ func (r *WorkPlacementReconciler) deleteWorkPlacement(ctx context.Context, write if !controllerutil.ContainsFinalizer(workPlacement, repoCleanupWorkPlacementFinalizer) { return ctrl.Result{}, nil } - - logger.Info("cleaning up files on repository", "repository", workPlacement.Name) + logger.Info("cleaning up work on repository", "workplacement", workPlacement.Name) var err error if filePathMode == v1alpha1.FilepathModeNone { var kratixFile []byte if kratixFile, err = writer.ReadFile(fmt.Sprintf(".kratix/%s-%s.yaml", workPlacement.Namespace, workPlacement.Name)); err != nil { + logger.Error(err, "failed to read .kratix state file") return defaultRequeue, err } stateFile := StateFile{} if err = yaml.Unmarshal(kratixFile, &stateFile); err != nil { + logger.Error(err, "failed to unmarshall .kratix state file") return defaultRequeue, err } err = writer.UpdateFiles(workPlacement.Name, nil, stateFile.Files) @@ -154,27 +156,26 @@ func (r *WorkPlacementReconciler) writeWorkloadsToStateStore(writer writers.Stat var err error if destination.GetFilepathMode() == v1alpha1.FilepathModeNone { var kratixFile []byte - if kratixFile, err = writer.ReadFile(fmt.Sprintf(".kratix/%s-%s.yaml", workPlacement.Namespace, workPlacement.Name)); err != nil { - return err + if kratixFile, err = writer.ReadFile(fmt.Sprintf(".kratix/%s-%s.yaml", workPlacement.Namespace, workPlacement.Name)); ignoreNotFound(err) != nil { + return fmt.Errorf("failed to read .kratix state file: %s", err) } oldStateFile := StateFile{} if err = yaml.Unmarshal(kratixFile, &oldStateFile); err != nil { - return err + return fmt.Errorf("failed to unmarshall .kratix state file: %s", err) } newStateFile := StateFile{ Files: workLoadsFilenames(workPlacement.Spec.Workloads), } - stateFileContent, err := yaml.Marshal(newStateFile) - if err != nil { - return err + stateFileContent, marshalErr := yaml.Marshal(newStateFile) + if marshalErr != nil { + return fmt.Errorf("failed to marshal new .kratix state file: %s", err) } stateFileWorkload := v1alpha1.Workload{ Filepath: fmt.Sprintf(".kratix/%s-%s.yaml", workPlacement.Namespace, workPlacement.Name), Content: string(stateFileContent), } - err = writer.UpdateFiles(workPlacement.Name, append(workPlacement.Spec.Workloads, stateFileWorkload), cleanupWorkloads(oldStateFile.Files, workPlacement.Spec.Workloads)) } else { err = writer.UpdateInDir(getDir(workPlacement), workPlacement.Name, workPlacement.Spec.Workloads) @@ -187,6 +188,13 @@ func (r *WorkPlacementReconciler) writeWorkloadsToStateStore(writer writers.Stat return nil } +func ignoreNotFound(err error) error { + if errors.Is(err, writers.FileNotFound) { + return nil + } + return err +} + func workLoadsFilenames(works []v1alpha1.Workload) []string { var result []string for _, w := range works { diff --git a/lib/writers/git.go b/lib/writers/git.go index b0ba9ec5..706477e2 100644 --- a/lib/writers/git.go +++ b/lib/writers/git.go @@ -214,12 +214,16 @@ func (g *GitWriter) ReadFile(filePath string) ([]byte, error) { "branch", g.gitServer.Branch, ) - localTmpDir, _, _, err := g.setupLocalDirectoryWithRepo(logger) + localTmpDir, _, worktree, err := g.setupLocalDirectoryWithRepo(logger) if err != nil { return nil, err } defer os.RemoveAll(filepath.Dir(localTmpDir)) + if _, err := worktree.Filesystem.Lstat(filePath); err != nil { + return nil, FileNotFound + } + return os.ReadFile(filepath.Join(localTmpDir, filePath)) } diff --git a/lib/writers/s3.go b/lib/writers/s3.go index f7d98edd..1b5383bc 100644 --- a/lib/writers/s3.go +++ b/lib/writers/s3.go @@ -73,17 +73,25 @@ func NewS3Writer(logger logr.Logger, stateStoreSpec v1alpha1.BucketStateStoreSpe } func (b *S3Writer) ReadFile(filename string) ([]byte, error) { - obj, err := b.RepoClient.GetObject(context.Background(), b.BucketName, filepath.Join(b.path, filename), minio.GetObjectOptions{}) + _, err := b.RepoClient.StatObject(context.Background(), b.BucketName, filepath.Join(b.path, filename), minio.GetObjectOptions{}) if err != nil { + if minio.ToErrorResponse(err).Code == "NoSuchKey" { + return nil, FileNotFound + } return nil, err } - defer obj.Close() - var content []byte - if _, err = obj.Read(content); err != nil { + obj, err := b.RepoClient.GetObject(context.Background(), b.BucketName, filepath.Join(b.path, filename), minio.GetObjectOptions{}) + if err != nil { return nil, err } - return content, nil + defer obj.Close() + + buf := new(bytes.Buffer) + buf.ReadFrom(obj) + + return buf.Bytes(), nil + } func (b *S3Writer) UpdateFiles(_ string, workloadsToCreate []v1alpha1.Workload, workloadsToDelete []string) error { diff --git a/lib/writers/statestore_writer.go b/lib/writers/statestore_writer.go index aa331dea..aa482686 100644 --- a/lib/writers/statestore_writer.go +++ b/lib/writers/statestore_writer.go @@ -1,6 +1,9 @@ package writers -import "github.com/syntasso/kratix/api/v1alpha1" +import ( + "fmt" + "github.com/syntasso/kratix/api/v1alpha1" +) //go:generate go run github.com/maxbrunsfeld/counterfeiter/v6 . StateStoreWriter type StateStoreWriter interface { @@ -8,3 +11,5 @@ type StateStoreWriter interface { UpdateInDir(subDir, workPlacementName string, workloadsToCreate []v1alpha1.Workload) error ReadFile(filename string) ([]byte, error) } + +var FileNotFound = fmt.Errorf("file not found") diff --git a/test/system/assets/git/platform_kratix_destination.yaml b/test/system/assets/git/platform_kratix_destination.yaml index dbe7cb1c..28c3e672 100644 --- a/test/system/assets/git/platform_kratix_destination.yaml +++ b/test/system/assets/git/platform_kratix_destination.yaml @@ -9,3 +9,16 @@ spec: stateStoreRef: name: default kind: GitStateStore +--- +apiVersion: platform.kratix.io/v1alpha1 +kind: Destination +metadata: + name: terraform + labels: + environment: terraform +spec: + filepath: + mode: none + stateStoreRef: + name: default + kind: BucketStateStore diff --git a/test/system/system_test.go b/test/system/system_test.go index 34c90f58..56b16cd8 100644 --- a/test/system/system_test.go +++ b/test/system/system_test.go @@ -323,8 +323,9 @@ var _ = Describe("Kratix", func() { By("mirroring the directory and files from /kratix/output to the statestore", func() { if getEnvOrDefault("TEST_SKIP_BUCKET_CHECK", "false") != "true" { - Expect(listFilesMinIOInStateStore(worker.name, "default", bashPromiseName, rrName, firstPipelineName)).To(ConsistOf("5058f/foo/example.json", "5058f/namespace.yaml")) - Expect(listFilesMinIOInStateStore(worker.name, "default", bashPromiseName, rrName, secondPipelineName)).To(ConsistOf("5058f/configmap.yaml")) + Expect(listFilesInMinIOStateStore(filepath.Join(worker.name, "resources", "default", bashPromiseName, rrName, firstPipelineName))).To(ConsistOf("5058f/foo/example.json", + fmt.Sprintf("5058f/namespace-%s.yaml", rrName))) + Expect(listFilesInMinIOStateStore(filepath.Join(worker.name, "resources", "default", bashPromiseName, rrName, secondPipelineName))).To(ConsistOf("5058f/configmap.yaml")) } }) @@ -402,10 +403,6 @@ var _ = Describe("Kratix", func() { platform.kubectl("delete", "namespace", oldRRImperativePlatformNamespace) Eventually(platform.kubectl("get", "promise")).ShouldNot(ContainSubstring(bashPromiseName)) }) - - XContext("filepathMode set to none", func() { - - }) }) When("A Promise is updated", func() { @@ -685,8 +682,60 @@ var _ = Describe("Kratix", func() { platform.kubectl("label", "destination", platform.name, "security-", removeBashPromiseUniqueLabel) }) }) + + Describe("filepathMode set to none", func() { + It("manages output files from multiple resource requests", func() { + bashPromise.Spec.DestinationSelectors = []v1alpha1.PromiseScheduling{{ + MatchLabels: map[string]string{ + "environment": "terraform", + }, + }} + + platform.eventuallyKubectl("apply", "-f", cat(bashPromise)) + platform.eventuallyKubectl("get", "crd", crd.Name) + rrNameOne := bashPromiseName + "terraform-1" + platform.kubectl("apply", "-f", terraformRequeset(rrNameOne)) + rrNameTwo := bashPromiseName + "terraform-2" + platform.kubectl("apply", "-f", terraformRequeset(rrNameTwo)) + + By("writing output files to the root of stateStore") + destinationName := "terraform" + Eventually(func() []string { + return listFilesInMinIOStateStore(destinationName) + }, shortTimeout, interval).Should(ContainElements( + 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) + }, shortTimeout, interval).ShouldNot(ContainElements( + fmt.Sprintf("%s.yaml", rrNameOne))) + Expect(listFilesInMinIOStateStore(destinationName)).To(ContainElements(fmt.Sprintf("%s.yaml", rrNameTwo))) + }) + }) }) +func terraformRequeset(name string) string { + request := unstructured.Unstructured{ + Object: map[string]interface{}{ + "apiVersion": "test.kratix.io/v1alpha1", + "kind": bashPromiseName, + "metadata": map[string]interface{}{ + "name": name, + }, + "spec": map[string]interface{}{ + "container0Cmd": fmt.Sprintf(` + touch /kratix/output/%s.yaml + `, name), + "container1Cmd": "exit 0", + }, + }, + } + return asFile(request) +} + func exampleBashRequest(name, namespaceSuffix string) string { request := unstructured.Unstructured{ Object: map[string]interface{}{ @@ -710,11 +759,11 @@ func exampleBashRequest(name, namespaceSuffix string) string { kubectl delete namespace imperative-$(yq '.metadata.name' /kratix/input/object.yaml)-%[1]s `, namespaceSuffix, namespaceSuffix), "container1Cmd": fmt.Sprintf(` - kubectl create namespace declarative-$(yq '.metadata.name' /kratix/input/object.yaml)-%[1]s --dry-run=client -oyaml > /kratix/output/namespace.yaml + kubectl create namespace declarative-$(yq '.metadata.name' /kratix/input/object.yaml)-%[1]s --dry-run=client -oyaml > /kratix/output/namespace-%[2]s.yaml mkdir /kratix/output/platform/ - kubectl create namespace declarative-platform-only-$(yq '.metadata.name' /kratix/input/object.yaml)-%[1]s --dry-run=client -oyaml > /kratix/output/platform/namespace.yaml + kubectl create namespace declarative-platform-only-$(yq '.metadata.name' /kratix/input/object.yaml)-%[1]s --dry-run=client -oyaml > /kratix/output/platform/namespace-%[2]s.yaml echo "[{\"matchLabels\":{\"environment\":\"platform\"}, \"directory\":\"platform\"}]" > /kratix/metadata/destination-selectors.yaml - `, namespaceSuffix), + `, namespaceSuffix, name), }, }, } @@ -856,31 +905,29 @@ func (c destination) withExitCode(code int) destination { newDestination.exitCode = code return newDestination } -func listFilesMinIOInStateStore(destinationName, namespace, promiseName, resourceName, pipelineName string) []string { - paths := []string{} - resourceSubDir := filepath.Join(destinationName, "resources", namespace, promiseName, resourceName, pipelineName) + +func listFilesInMinIOStateStore(path string) []string { + files := []string{} // Initialize minio client object. minioClient, err := minio.New(endpoint, &minio.Options{ Creds: credentials.NewStaticV4(accessKeyID, secretAccessKey, ""), Secure: useSSL, }) - Expect(err).ToNot(HaveOccurred()) + ExpectWithOffset(1, err).ToNot(HaveOccurred()) - //worker-1/resources/default/redis/rr-test/configure/ objectCh := minioClient.ListObjects(context.TODO(), bucketName, minio.ListObjectsOptions{ - Prefix: resourceSubDir, + Prefix: path, Recursive: true, }) for object := range objectCh { - Expect(object.Err).NotTo(HaveOccurred()) - - path, err := filepath.Rel(resourceSubDir, object.Key) - Expect(err).ToNot(HaveOccurred()) - paths = append(paths, path) + ExpectWithOffset(1, object.Err).NotTo(HaveOccurred()) + path, err := filepath.Rel(path, object.Key) + ExpectWithOffset(1, err).ToNot(HaveOccurred()) + files = append(files, path) } - return paths + return files } func generateUniquePromise(promisePath string) *v1alpha1.Promise { From 4dd0492fa1a4d8748f7c573f26fa94a87c0aef8b Mon Sep 17 00:00:00 2001 From: Chunyi Lyu Date: Wed, 29 May 2024 07:07:24 -0400 Subject: [PATCH 3/6] feat: clean up kratix file at workplacement delete --- controllers/workplacement_controller.go | 5 +++-- controllers/workplacement_controller_test.go | 2 +- test/system/system_test.go | 2 ++ 3 files changed, 6 insertions(+), 3 deletions(-) diff --git a/controllers/workplacement_controller.go b/controllers/workplacement_controller.go index 1aff33c4..f502c0b8 100644 --- a/controllers/workplacement_controller.go +++ b/controllers/workplacement_controller.go @@ -125,7 +125,8 @@ func (r *WorkPlacementReconciler) deleteWorkPlacement(ctx context.Context, write var err error if filePathMode == v1alpha1.FilepathModeNone { var kratixFile []byte - if kratixFile, err = writer.ReadFile(fmt.Sprintf(".kratix/%s-%s.yaml", workPlacement.Namespace, workPlacement.Name)); err != nil { + kratixFilePath := fmt.Sprintf(".kratix/%s-%s.yaml", workPlacement.Namespace, workPlacement.Name) + if kratixFile, err = writer.ReadFile(kratixFilePath); err != nil { logger.Error(err, "failed to read .kratix state file") return defaultRequeue, err } @@ -134,7 +135,7 @@ func (r *WorkPlacementReconciler) deleteWorkPlacement(ctx context.Context, write logger.Error(err, "failed to unmarshall .kratix state file") return defaultRequeue, err } - err = writer.UpdateFiles(workPlacement.Name, nil, stateFile.Files) + err = writer.UpdateFiles(workPlacement.Name, nil, append(stateFile.Files, kratixFilePath)) } else { err = writer.UpdateInDir(getDir(*workPlacement)+"/", workPlacement.Name, nil) } diff --git a/controllers/workplacement_controller_test.go b/controllers/workplacement_controller_test.go index c668919d..6948640d 100644 --- a/controllers/workplacement_controller_test.go +++ b/controllers/workplacement_controller_test.go @@ -217,7 +217,7 @@ files: workPlacementName, workloadsToCreate, workloadsToDelete := fakeWriter.UpdateFilesArgsForCall(1) Expect(workPlacementName).To(Equal(workPlacement.Name)) Expect(workloadsToCreate).To(BeNil()) - Expect(workloadsToDelete).To(ConsistOf("fruit.yaml")) + Expect(workloadsToDelete).To(ConsistOf("fruit.yaml", ".kratix/default-test-workplacement.yaml")) }) }) diff --git a/test/system/system_test.go b/test/system/system_test.go index 56b16cd8..2135df34 100644 --- a/test/system/system_test.go +++ b/test/system/system_test.go @@ -713,6 +713,8 @@ var _ = Describe("Kratix", func() { }, shortTimeout, interval).ShouldNot(ContainElements( fmt.Sprintf("%s.yaml", rrNameOne))) Expect(listFilesInMinIOStateStore(destinationName)).To(ContainElements(fmt.Sprintf("%s.yaml", rrNameTwo))) + + platform.eventuallyKubectlDelete("promise", bashPromiseName) }) }) }) From 5f04227c23d499fbd3089b72087d02f2e6f0d855 Mon Sep 17 00:00:00 2001 From: Chunyi Lyu Date: Wed, 29 May 2024 07:35:23 -0400 Subject: [PATCH 4/6] fix: revert change to exampleBashRequest test helper --- api/v1alpha1/destination_types.go | 2 +- config/crd/bases/platform.kratix.io_destinations.yaml | 2 +- test/system/system_test.go | 11 +++++------ 3 files changed, 7 insertions(+), 8 deletions(-) diff --git a/api/v1alpha1/destination_types.go b/api/v1alpha1/destination_types.go index b8115e9e..0b12733a 100644 --- a/api/v1alpha1/destination_types.go +++ b/api/v1alpha1/destination_types.go @@ -71,7 +71,7 @@ const ( type Filepath struct { // +kubebuilder:validation:Enum:={nestedByMetadata,none} // +kubebuilder:validation:XValidation:rule="self == oldSelf",message="filepath.mode is immutable" - // The of filepathExpression, either: + // The of filepathExpression, either: // - nestedByMetadata (default): files from the pipeline will be placed in a nested directory structure // - none: file from the pipeline will be placed in a flat directory structure // filepath.mode is immutable diff --git a/config/crd/bases/platform.kratix.io_destinations.yaml b/config/crd/bases/platform.kratix.io_destinations.yaml index 6e6f2d5b..99080d68 100644 --- a/config/crd/bases/platform.kratix.io_destinations.yaml +++ b/config/crd/bases/platform.kratix.io_destinations.yaml @@ -48,7 +48,7 @@ spec: properties: mode: description: |- - The of filepathExpression, either: + The of filepathExpression, either: - nestedByMetadata (default): files from the pipeline will be placed in a nested directory structure - none: file from the pipeline will be placed in a flat directory structure filepath.mode is immutable diff --git a/test/system/system_test.go b/test/system/system_test.go index 2135df34..ad66c0ba 100644 --- a/test/system/system_test.go +++ b/test/system/system_test.go @@ -323,8 +323,7 @@ var _ = Describe("Kratix", func() { By("mirroring the directory and files from /kratix/output to the statestore", func() { if getEnvOrDefault("TEST_SKIP_BUCKET_CHECK", "false") != "true" { - Expect(listFilesInMinIOStateStore(filepath.Join(worker.name, "resources", "default", bashPromiseName, rrName, firstPipelineName))).To(ConsistOf("5058f/foo/example.json", - fmt.Sprintf("5058f/namespace-%s.yaml", rrName))) + Expect(listFilesInMinIOStateStore(filepath.Join(worker.name, "resources", "default", bashPromiseName, rrName, firstPipelineName))).To(ConsistOf("5058f/foo/example.json", "5058f/namespace.yaml")) Expect(listFilesInMinIOStateStore(filepath.Join(worker.name, "resources", "default", bashPromiseName, rrName, secondPipelineName))).To(ConsistOf("5058f/configmap.yaml")) } }) @@ -759,13 +758,13 @@ func exampleBashRequest(name, namespaceSuffix string) string { exit 0 fi kubectl delete namespace imperative-$(yq '.metadata.name' /kratix/input/object.yaml)-%[1]s - `, namespaceSuffix, namespaceSuffix), + `, namespaceSuffix), "container1Cmd": fmt.Sprintf(` - kubectl create namespace declarative-$(yq '.metadata.name' /kratix/input/object.yaml)-%[1]s --dry-run=client -oyaml > /kratix/output/namespace-%[2]s.yaml + kubectl create namespace declarative-$(yq '.metadata.name' /kratix/input/object.yaml)-%[1]s --dry-run=client -oyaml > /kratix/output/namespace.yaml mkdir /kratix/output/platform/ - kubectl create namespace declarative-platform-only-$(yq '.metadata.name' /kratix/input/object.yaml)-%[1]s --dry-run=client -oyaml > /kratix/output/platform/namespace-%[2]s.yaml + kubectl create namespace declarative-platform-only-$(yq '.metadata.name' /kratix/input/object.yaml)-%[1]s --dry-run=client -oyaml > /kratix/output/platform/namespace.yaml echo "[{\"matchLabels\":{\"environment\":\"platform\"}, \"directory\":\"platform\"}]" > /kratix/metadata/destination-selectors.yaml - `, namespaceSuffix, name), + `, namespaceSuffix), }, }, } From d24b6e235f58db5bd1d43b3c842b369bf82c0c52 Mon Sep 17 00:00:00 2001 From: Chunyi Lyu Date: Wed, 29 May 2024 08:49:42 -0400 Subject: [PATCH 5/6] fix: clean up directory when subdir is provided - bool assertion is the other way around --- lib/writers/git.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/writers/git.go b/lib/writers/git.go index 706477e2..3973bcd6 100644 --- a/lib/writers/git.go +++ b/lib/writers/git.go @@ -129,7 +129,7 @@ func (g *GitWriter) update(subDir, workPlacementName string, workloadsToCreate [ } defer os.RemoveAll(filepath.Dir(localTmpDir)) - err = g.deleteExistingFiles(subDir == "", dirInGitRepo, workloadsToDelete, worktree, logger) + err = g.deleteExistingFiles(subDir != "", dirInGitRepo, workloadsToDelete, worktree, logger) if err != nil { return err } From f9360e3b20be6f62522eaccd362355eafc2f5f95 Mon Sep 17 00:00:00 2001 From: Chunyi Lyu Date: Wed, 29 May 2024 09:52:24 -0400 Subject: [PATCH 6/6] chore: fixing some typos Co-authored-by: Rich Barton-Cooper --- api/v1alpha1/destination_types.go | 2 +- config/crd/bases/platform.kratix.io_destinations.yaml | 2 +- controllers/workplacement_controller.go | 8 ++++---- ...platform_kratix_destination.yaml => destinations.yaml} | 0 test/system/system_suite_test.go | 2 +- test/system/system_test.go | 6 +++--- 6 files changed, 10 insertions(+), 10 deletions(-) rename test/system/assets/git/{platform_kratix_destination.yaml => destinations.yaml} (100%) diff --git a/api/v1alpha1/destination_types.go b/api/v1alpha1/destination_types.go index 0b12733a..d61c0b95 100644 --- a/api/v1alpha1/destination_types.go +++ b/api/v1alpha1/destination_types.go @@ -71,7 +71,7 @@ const ( type Filepath struct { // +kubebuilder:validation:Enum:={nestedByMetadata,none} // +kubebuilder:validation:XValidation:rule="self == oldSelf",message="filepath.mode is immutable" - // The of filepathExpression, either: + // filepath.mode can be set to either: // - nestedByMetadata (default): files from the pipeline will be placed in a nested directory structure // - none: file from the pipeline will be placed in a flat directory structure // filepath.mode is immutable diff --git a/config/crd/bases/platform.kratix.io_destinations.yaml b/config/crd/bases/platform.kratix.io_destinations.yaml index 99080d68..7272a03f 100644 --- a/config/crd/bases/platform.kratix.io_destinations.yaml +++ b/config/crd/bases/platform.kratix.io_destinations.yaml @@ -48,7 +48,7 @@ spec: properties: mode: description: |- - The of filepathExpression, either: + filepath.mode can be set to either: - nestedByMetadata (default): files from the pipeline will be placed in a nested directory structure - none: file from the pipeline will be placed in a flat directory structure filepath.mode is immutable diff --git a/controllers/workplacement_controller.go b/controllers/workplacement_controller.go index f502c0b8..8c498da2 100644 --- a/controllers/workplacement_controller.go +++ b/controllers/workplacement_controller.go @@ -132,7 +132,7 @@ func (r *WorkPlacementReconciler) deleteWorkPlacement(ctx context.Context, write } stateFile := StateFile{} if err = yaml.Unmarshal(kratixFile, &stateFile); err != nil { - logger.Error(err, "failed to unmarshall .kratix state file") + logger.Error(err, "failed to unmarshal .kratix state file") return defaultRequeue, err } err = writer.UpdateFiles(workPlacement.Name, nil, append(stateFile.Files, kratixFilePath)) @@ -162,11 +162,11 @@ func (r *WorkPlacementReconciler) writeWorkloadsToStateStore(writer writers.Stat } oldStateFile := StateFile{} if err = yaml.Unmarshal(kratixFile, &oldStateFile); err != nil { - return fmt.Errorf("failed to unmarshall .kratix state file: %s", err) + return fmt.Errorf("failed to unmarshal .kratix state file: %s", err) } newStateFile := StateFile{ - Files: workLoadsFilenames(workPlacement.Spec.Workloads), + Files: workloadsFilenames(workPlacement.Spec.Workloads), } stateFileContent, marshalErr := yaml.Marshal(newStateFile) if marshalErr != nil { @@ -196,7 +196,7 @@ func ignoreNotFound(err error) error { return err } -func workLoadsFilenames(works []v1alpha1.Workload) []string { +func workloadsFilenames(works []v1alpha1.Workload) []string { var result []string for _, w := range works { result = append(result, w.Filepath) diff --git a/test/system/assets/git/platform_kratix_destination.yaml b/test/system/assets/git/destinations.yaml similarity index 100% rename from test/system/assets/git/platform_kratix_destination.yaml rename to test/system/assets/git/destinations.yaml diff --git a/test/system/system_suite_test.go b/test/system/system_suite_test.go index 3a72a959..cdbdc7d8 100644 --- a/test/system/system_suite_test.go +++ b/test/system/system_suite_test.go @@ -51,7 +51,7 @@ var _ = SynchronizedBeforeSuite(func() { platform.kubectl("apply", "-f", "../../hack/destination/gitops-tk-install.yaml") platform.kubectl("apply", "-f", "./assets/bash-promise/deployment.yaml") platform.kubectl("apply", "-f", catAndReplaceFluxResources(tmpDir, "./assets/git/platform_gitops-tk-resources.yaml")) - platform.kubectl("apply", "-f", catAndReplaceFluxResources(tmpDir, "./assets/git/platform_kratix_destination.yaml")) + platform.kubectl("apply", "-f", catAndReplaceFluxResources(tmpDir, "./assets/git/destinations.yaml")) os.RemoveAll(tmpDir) }, func() { //this runs before each test diff --git a/test/system/system_test.go b/test/system/system_test.go index ad66c0ba..9a4473f8 100644 --- a/test/system/system_test.go +++ b/test/system/system_test.go @@ -693,9 +693,9 @@ var _ = Describe("Kratix", func() { platform.eventuallyKubectl("apply", "-f", cat(bashPromise)) platform.eventuallyKubectl("get", "crd", crd.Name) rrNameOne := bashPromiseName + "terraform-1" - platform.kubectl("apply", "-f", terraformRequeset(rrNameOne)) + platform.kubectl("apply", "-f", terraformRequest(rrNameOne)) rrNameTwo := bashPromiseName + "terraform-2" - platform.kubectl("apply", "-f", terraformRequeset(rrNameTwo)) + platform.kubectl("apply", "-f", terraformRequest(rrNameTwo)) By("writing output files to the root of stateStore") destinationName := "terraform" @@ -718,7 +718,7 @@ var _ = Describe("Kratix", func() { }) }) -func terraformRequeset(name string) string { +func terraformRequest(name string) string { request := unstructured.Unstructured{ Object: map[string]interface{}{ "apiVersion": "test.kratix.io/v1alpha1",