diff --git a/controllers/gitrepository_controller.go b/controllers/gitrepository_controller.go index 0b5e9e749..9f238a2ca 100644 --- a/controllers/gitrepository_controller.go +++ b/controllers/gitrepository_controller.go @@ -30,6 +30,7 @@ import ( "github.com/fluxcd/source-controller/pkg/sourceignore" "github.com/go-logr/logr" corev1 "k8s.io/api/core/v1" + apierrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/types" kerrors "k8s.io/apimachinery/pkg/util/errors" @@ -338,6 +339,7 @@ func (r *GitRepositoryReconciler) reconcileArtifact(ctx context.Context, obj *so // The artifact is up-to-date if obj.GetArtifact().HasRevision(artifact.Revision) && !includes.Diff(obj.Status.IncludedArtifacts) { logr.FromContext(ctx).Info("Artifact is up-to-date") + conditions.MarkTrue(obj, sourcev1.ArtifactAvailableCondition, "ArchivedArtifact", "Artifact revision %s", artifact.Revision) return ctrl.Result{RequeueAfter: obj.GetInterval().Duration}, nil } @@ -383,7 +385,7 @@ func (r *GitRepositoryReconciler) reconcileArtifact(ctx context.Context, obj *so // Record it on the object obj.Status.Artifact = artifact.DeepCopy() obj.Status.IncludedArtifacts = includes - conditions.MarkTrue(obj, sourcev1.ArtifactAvailableCondition, "ArchivedArtifact", "Archived artifact revision %s", artifact.Revision) + conditions.MarkTrue(obj, sourcev1.ArtifactAvailableCondition, "ArchivedArtifact", "Artifact revision %s", artifact.Revision) r.Events.Eventf(ctx, obj, map[string]string{ "revision": obj.GetArtifact().Revision, }, events.EventSeverityInfo, sourcev1.GitOperationSucceedReason, conditions.Get(obj, sourcev1.ArtifactAvailableCondition).Message) @@ -392,7 +394,6 @@ func (r *GitRepositoryReconciler) reconcileArtifact(ctx context.Context, obj *so url, err := r.Storage.Symlink(artifact, "latest.tar.gz") if err != nil { r.Events.Eventf(ctx, obj, nil, events.EventSeverityError, sourcev1.StorageOperationFailedReason, "Failed to update status URL symlink: %s", err) - return ctrl.Result{}, err } if url != "" { obj.Status.URL = url @@ -410,16 +411,21 @@ func (r *GitRepositoryReconciler) reconcileArtifact(ctx context.Context, obj *so // statuses is recorded in a condition on the given object. func (r *GitRepositoryReconciler) reconcileInclude(ctx context.Context, obj *sourcev1.GitRepository, artifacts artifactSet, dir string) (ctrl.Result, error) { includes := make([]conditions.Getter, len(obj.Spec.Include)) + artifacts = make(artifactSet, len(obj.Spec.Include)) for i, incl := range obj.Spec.Include { dep := &sourcev1.GitRepository{} if err := r.Get(ctx, types.NamespacedName{Namespace: obj.Namespace, Name: incl.GitRepositoryRef.Name}, dep); err != nil { - return ctrl.Result{RequeueAfter: r.requeueDependency}, client.IgnoreNotFound(err) + if apierrors.IsNotFound(err){ + conditions.MarkFalse(obj, sourcev1.SourceAvailableCondition, "IncludeNotFound", "Could not find resource for include %q: %s", incl.GitRepositoryRef.Name, err.Error()) + } + //r.Eventf() + return ctrl.Result{RequeueAfter: obj.Spec.Interval.Duration}, client.IgnoreNotFound(err) } // Confirm include has an artifact if dep.GetArtifact() == nil { - conditions.MarkFalse(obj, sourcev1.SourceAvailableCondition, "IncludeFailure", "No artifact available for include %s", incl.GitRepositoryRef) + conditions.MarkFalse(obj, sourcev1.SourceAvailableCondition, "IncludeUnavailable", "No artifact available for include %q", incl.GitRepositoryRef.Name) return ctrl.Result{RequeueAfter: r.requeueDependency}, nil } @@ -428,20 +434,25 @@ func (r *GitRepositoryReconciler) reconcileInclude(ctx context.Context, obj *sou // Copy artifact (sub)contents to configured directory toPath, err := securejoin.SecureJoin(dir, incl.GetToPath()) if err != nil { - conditions.MarkFalse(obj, sourcev1.SourceAvailableCondition, "IncludeFailure", "Failed to calculate path for include %s: %s", incl.GitRepositoryRef, err.Error()) + conditions.MarkFalse(obj, sourcev1.SourceAvailableCondition, "IncludeFailure", "Failed to calculate path for include %q: %s", incl.GitRepositoryRef.Name, err.Error()) return ctrl.Result{}, err } if err = r.Storage.CopyToPath(dep.GetArtifact(), incl.GetFromPath(), toPath); err != nil { - conditions.MarkFalse(obj, sourcev1.SourceAvailableCondition, "IncludeCopyFailure", "Failed to copy %s include from %s to %s: %s", incl.GitRepositoryRef, incl.GetFromPath(), toPath, err.Error()) + conditions.MarkFalse(obj, sourcev1.SourceAvailableCondition, "IncludeCopyFailure", "Failed to copy %q include from %s to %s: %s", incl.GitRepositoryRef.Name, incl.GetFromPath(), incl.GetToPath(), err.Error()) return ctrl.Result{}, err } artifacts[i] = dep.GetArtifact().DeepCopy() } - // Record an aggregation of all includes' Ready state to the object - // condition - conditions.SetAggregate(obj, sourcev1.SourceAvailableCondition, includes) + // Record an aggregation of all includes Stalled or Ready state to + // the object condition + conditions.SetAggregate(obj, sourcev1.SourceAvailableCondition, includes, + conditions.WithConditions(meta.StalledCondition, meta.ReadyCondition), + conditions.WithNegativePolarityConditions(meta.StalledCondition), + conditions.WithSourceRefIf(meta.StalledCondition), + conditions.WithCounter(), + conditions.WithCounterIfOnly(meta.ReadyCondition)) return ctrl.Result{RequeueAfter: obj.Spec.Interval.Duration}, nil } diff --git a/controllers/gitrepository_controller_test.go b/controllers/gitrepository_controller_test.go index f955b3a80..0e6d9d7c1 100644 --- a/controllers/gitrepository_controller_test.go +++ b/controllers/gitrepository_controller_test.go @@ -48,6 +48,7 @@ import ( ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/client" fakeclient "sigs.k8s.io/controller-runtime/pkg/client/fake" + "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil" "sigs.k8s.io/controller-runtime/pkg/log" "github.com/fluxcd/pkg/apis/meta" @@ -591,7 +592,7 @@ func TestGitRepositoryReconciler_reconcileArtifact(t *testing.T) { }, want: ctrl.Result{RequeueAfter: mockInterval}, assertConditions: []metav1.Condition{ - *conditions.TrueCondition(sourcev1.ArtifactAvailableCondition, "ArchivedArtifact", "Archived artifact revision main/revision"), + *conditions.TrueCondition(sourcev1.ArtifactAvailableCondition, "ArchivedArtifact", "Artifact revision main/revision"), }, }, { @@ -662,6 +663,251 @@ func TestGitRepositoryReconciler_reconcileArtifact(t *testing.T) { } } +func TestGitRepositoryReconciler_reconcileInclude(t *testing.T) { + g := NewWithT(t) + + storage, err := newTestStorage() + g.Expect(err).NotTo(HaveOccurred()) + defer os.RemoveAll(storage.BasePath) + + dependencyInterval := 5 * time.Second + + type dependency struct { + name string + withArtifact bool + conditions []metav1.Condition + } + + type include struct { + name string + fromPath string + toPath string + shouldExist bool + } + + tests := []struct { + name string + dependencies []dependency + includes []include + want ctrl.Result + wantErr bool + assertConditions []metav1.Condition + }{ + { + name: "Includes artifacts", + dependencies: []dependency{ + { + name: "a", + withArtifact: true, + conditions: []metav1.Condition{ + *conditions.TrueCondition(meta.ReadyCondition, "Foo", "foo ready"), + }, + }, + { + name: "b", + withArtifact: true, + conditions: []metav1.Condition{ + *conditions.TrueCondition(meta.ReadyCondition, "Bar", "bar ready"), + }, + }, + }, + includes: []include{ + {name: "a", toPath: "a/"}, + {name: "b", toPath: "b/"}, + }, + want: ctrl.Result{RequeueAfter: mockInterval}, + assertConditions: []metav1.Condition{ + *conditions.TrueCondition(sourcev1.SourceAvailableCondition, "Foo", "2 of 2 Ready"), + }, + }, + { + name: "Non existing artifact", + includes: []include{ + {name: "a", toPath: "a/"}, + }, + want: ctrl.Result{RequeueAfter: mockInterval}, + wantErr: false, + assertConditions: []metav1.Condition{ + *conditions.FalseCondition(sourcev1.SourceAvailableCondition, "IncludeNotFound", "Could not find resource for include \"a\": gitrepositories.source.toolkit.fluxcd.io \"a\" not found"), + }, + }, + { + name: "Missing artifact", + dependencies: []dependency{ + { + name: "a", + withArtifact: false, + conditions: []metav1.Condition{ + *conditions.FalseCondition(sourcev1.SourceAvailableCondition, "Foo", "foo unavailable"), + }, + }, + }, + includes: []include{ + {name: "a", toPath: "a/"}, + }, + want: ctrl.Result{RequeueAfter: dependencyInterval}, + assertConditions: []metav1.Condition{ + *conditions.FalseCondition(sourcev1.SourceAvailableCondition, "IncludeUnavailable", "No artifact available for include \"a\""), + }, + }, + { + name: "Invalid FromPath", + dependencies: []dependency{ + { + name: "a", + withArtifact: true, + }, + }, + includes: []include{ + {name: "a", fromPath: "../../../path", shouldExist: false}, + }, + wantErr: true, + assertConditions: []metav1.Condition{ + *conditions.FalseCondition(sourcev1.SourceAvailableCondition, "IncludeCopyFailure", "Failed to copy \"a\" include from ../../../path to a"), + }, + }, + { + name: "Stalled include", + dependencies: []dependency{ + { + name: "a", + withArtifact: true, + conditions: []metav1.Condition{ + *conditions.TrueCondition(meta.ReadyCondition, "Foo", "foo ready"), + }, + }, + { + name: "b", + withArtifact: true, + conditions: []metav1.Condition{ + *conditions.TrueCondition(meta.StalledCondition, "Bar", "bar stalled"), + }, + }, + }, + includes: []include{ + {name: "a", toPath: "a/"}, + {name: "b", toPath: "b/"}, + }, + want: ctrl.Result{RequeueAfter: mockInterval}, + assertConditions: []metav1.Condition{ + *conditions.FalseCondition(sourcev1.SourceAvailableCondition, "Bar @ GitRepository/a", "bar stalled"), + }, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + g := NewWithT(t) + + var depObjs []client.Object + for _, d := range tt.dependencies { + obj := &sourcev1.GitRepository{ + ObjectMeta: metav1.ObjectMeta{ + Name: d.name, + }, + Status: sourcev1.GitRepositoryStatus{ + Conditions: d.conditions, + }, + } + if d.withArtifact { + obj.Status.Artifact = &sourcev1.Artifact{ + Path: d.name + ".tar.gz", + Revision: d.name, + LastUpdateTime: metav1.Now(), + } + g.Expect(storage.Archive(obj.GetArtifact(), "testdata/git/repository", nil)).To(Succeed()) + } + depObjs = append(depObjs, obj) + } + + s := runtime.NewScheme() + utilruntime.Must(sourcev1.AddToScheme(s)) + builder := fakeclient.NewClientBuilder().WithScheme(s) + if len(tt.dependencies) > 0 { + builder.WithObjects(depObjs...) + } + + r := &GitRepositoryReconciler{ + Client: builder.Build(), + Storage: storage, + requeueDependency: dependencyInterval, + } + + obj := &sourcev1.GitRepository{ + ObjectMeta: metav1.ObjectMeta{ + Name: "reconcile-include", + }, + Spec: sourcev1.GitRepositorySpec{ + Interval: metav1.Duration{Duration: mockInterval}, + }, + } + + for i, incl := range tt.includes { + incl := sourcev1.GitRepositoryInclude{ + GitRepositoryRef: meta.LocalObjectReference{Name: incl.name}, + FromPath: incl.fromPath, + ToPath: incl.toPath, + } + tt.includes[i].fromPath = incl.GetFromPath() + tt.includes[i].toPath = incl.GetToPath() + obj.Spec.Include = append(obj.Spec.Include, incl) + } + + tmpDir, err := ioutil.TempDir("", "include-") + g.Expect(err).NotTo(HaveOccurred()) + + var artifacts artifactSet + got, err := r.reconcileInclude(ctx, obj, artifacts, tmpDir) + g.Expect(obj.GetConditions()).To(conditions.MatchConditions(tt.assertConditions)) + g.Expect(err != nil).To(Equal(tt.wantErr)) + g.Expect(got).To(Equal(tt.want)) + for _, i := range tt.includes { + if i.toPath != "" { + expect := g.Expect(filepath.Join(storage.BasePath, i.toPath)) + if i.shouldExist { + expect.To(BeADirectory()) + } else { + expect.NotTo(BeADirectory()) + } + } + if i.shouldExist { + g.Expect(filepath.Join(storage.BasePath, i.toPath)).Should(BeADirectory()) + } else { + g.Expect(filepath.Join(storage.BasePath, i.toPath)).ShouldNot(BeADirectory()) + } + } + }) + } +} + +func TestGitRepositoryReconciler_reconcileDelete(t *testing.T) { + g := NewWithT(t) + + r := &GitRepositoryReconciler{ + Storage: storage, + } + + obj := &sourcev1.GitRepository{ + ObjectMeta: metav1.ObjectMeta{ + Name: "reconcile-delete-", + DeletionTimestamp: &metav1.Time{Time: time.Now()}, + Finalizers: []string{ + sourcev1.SourceFinalizer, + }, + }, + Status: sourcev1.GitRepositoryStatus{}, + } + + artifact := storage.NewArtifactFor(sourcev1.GitRepositoryKind, obj.GetObjectMeta(), "revision", "foo.txt") + obj.Status.Artifact = &artifact + + got, err := r.reconcileDelete(ctx, obj) + g.Expect(err).NotTo(HaveOccurred()) + g.Expect(got).To(Equal(ctrl.Result{})) + g.Expect(controllerutil.ContainsFinalizer(obj, sourcev1.SourceFinalizer)).To(BeFalse()) + g.Expect(obj.Status.Artifact).To(BeNil()) + +} + func TestGitRepositoryReconciler_verifyCommitSignature(t *testing.T) { tests := []struct { name string diff --git a/controllers/storage.go b/controllers/storage.go index ce3b959da..6b0207b15 100644 --- a/controllers/storage.go +++ b/controllers/storage.go @@ -30,6 +30,7 @@ import ( "strings" "time" + securejoin "github.com/cyphar/filepath-securejoin" "github.com/go-git/go-git/v5/plumbing/format/gitignore" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -354,28 +355,32 @@ func (s *Storage) CopyFromPath(artifact *sourcev1.Artifact, path string) (err er return s.Copy(artifact, f) } -// CopyToPath copies the contents of the given atrifact to the path. -func (s *Storage) CopyToPath(atrifact *sourcev1.Artifact, subPath, toPath string) error { +// CopyToPath copies the contents in the (sub)Path of the given artifact to the +// given path. +func (s *Storage) CopyToPath(artifact *sourcev1.Artifact, subPath, toPath string) error { // create a tmp directory to store artifact - tmp, err := ioutil.TempDir("", "flux-include") + tmp, err := ioutil.TempDir("", "flux-include-") if err != nil { return err } defer os.RemoveAll(tmp) // read artifact file content - localPath := s.LocalPath(*atrifact) + localPath := s.LocalPath(*artifact) f, err := os.Open(localPath) if err != nil { return err } defer f.Close() // untar the artifact - untarPath := filepath.Join(tmp, "tar") + untarPath := filepath.Join(tmp, "unpack") if _, err = untar.Untar(f, untarPath); err != nil { return err } // copy the folder to the path - fromPath := filepath.Join(untarPath, subPath) + fromPath, err := securejoin.SecureJoin(untarPath, subPath) + if err != nil { + return err + } if err := fs.RenameWithFallback(fromPath, toPath); err != nil { return err } @@ -426,7 +431,11 @@ func (s *Storage) LocalPath(artifact sourcev1.Artifact) string { if artifact.Path == "" { return "" } - return filepath.Join(s.BasePath, artifact.Path) + path, err := securejoin.SecureJoin(s.BasePath, artifact.Path) + if err != nil { + return "" + } + return path } // newHash returns a new SHA1 hash. diff --git a/go.mod b/go.mod index 654ad596f..28a149c1b 100644 --- a/go.mod +++ b/go.mod @@ -6,14 +6,13 @@ replace github.com/fluxcd/source-controller/api => ./api require ( github.com/Masterminds/semver/v3 v3.1.1 - github.com/blang/semver/v4 v4.0.0 github.com/cyphar/filepath-securejoin v0.2.2 github.com/fluxcd/pkg/apis/meta v0.9.0 github.com/fluxcd/pkg/gittestserver v0.2.1 github.com/fluxcd/pkg/gitutil v0.0.1 github.com/fluxcd/pkg/helmtestserver v0.1.0 github.com/fluxcd/pkg/lockedfile v0.0.5 - github.com/fluxcd/pkg/runtime v0.11.1-0.20210514212714-849f4a7f244f + github.com/fluxcd/pkg/runtime v0.11.1-0.20210521141553-959de5c91c1c github.com/fluxcd/pkg/ssh v0.0.5 github.com/fluxcd/pkg/untar v0.0.5 github.com/fluxcd/pkg/version v0.0.1 diff --git a/go.sum b/go.sum index 20f599177..3ce97db87 100644 --- a/go.sum +++ b/go.sum @@ -130,10 +130,7 @@ github.com/bitly/go-simplejson v0.5.0/go.mod h1:cXHtHw4XUPsvGaxgjIAn8PhEWG9NfngE github.com/bketelsen/crypt v0.0.3-0.20200106085610-5cbc8cc4026c/go.mod h1:MKsuJmJgSg28kpZDP6UIiPt0e0Oz0kqKNGyRaWEPv84= github.com/blang/semver v3.1.0+incompatible/go.mod h1:kRBLl5iJ+tD4TcOOxsy/0fnwebNt5EWlYSAyrTnjyyk= github.com/blang/semver v3.5.0+incompatible/go.mod h1:kRBLl5iJ+tD4TcOOxsy/0fnwebNt5EWlYSAyrTnjyyk= -github.com/blang/semver v3.5.1+incompatible h1:cQNTCjp13qL8KC3Nbxr/y2Bqb63oX6wdnnjpJbkM4JQ= github.com/blang/semver v3.5.1+incompatible/go.mod h1:kRBLl5iJ+tD4TcOOxsy/0fnwebNt5EWlYSAyrTnjyyk= -github.com/blang/semver/v4 v4.0.0 h1:1PFHFE6yCCTv8C1TeyNNarDzntLi7wMI5i/pzqYIsAM= -github.com/blang/semver/v4 v4.0.0/go.mod h1:IbckMUScFkM3pff0VJDNKRiT6TG/YpiHIM2yvyW5YoQ= github.com/bmizerany/assert v0.0.0-20160611221934-b7ed37b82869/go.mod h1:Ekp36dRnpXw/yCqJaO+ZrUyxD+3VXMFFr56k5XYrpB4= github.com/bshuster-repo/logrus-logstash-hook v0.4.1 h1:pgAtgj+A31JBVtEHu2uHuEx0n+2ukqUJnS2vVe5pQNA= github.com/bshuster-repo/logrus-logstash-hook v0.4.1/go.mod h1:zsTqEiSzDgAa/8GZR7E1qaXrhYNDKBYy5/dWPTIflbk= @@ -261,8 +258,8 @@ github.com/fluxcd/pkg/helmtestserver v0.1.0 h1:RiVVxIHD6PJdKinW46feFIYf1LUj6xXSp github.com/fluxcd/pkg/helmtestserver v0.1.0/go.mod h1:3L+tbPn74PsHwHsyhbfk/kZAosrwMFTTA92XEFiwVAE= github.com/fluxcd/pkg/lockedfile v0.0.5 h1:C3T8wfdff1UY1bvplmCkGOLrdMWJHO8Q8+tdlEXJYzQ= github.com/fluxcd/pkg/lockedfile v0.0.5/go.mod h1:uAtPUBId6a2RqO84MTH5HKGX0SbM1kNW3Wr/FhYyDVA= -github.com/fluxcd/pkg/runtime v0.11.1-0.20210514212714-849f4a7f244f h1:LZIsKBl9Px7y/RMl6zU74t11+NTYdbtxzt1JkI8tY50= -github.com/fluxcd/pkg/runtime v0.11.1-0.20210514212714-849f4a7f244f/go.mod h1:vKV1eL4j9TjM6hhWopKeEBmrqXR8SJys0ir3D1GPtKE= +github.com/fluxcd/pkg/runtime v0.11.1-0.20210521141553-959de5c91c1c h1:AeCK2FX1rYUKlUUs/QhIQcCnKNsELodMht9hMr20cxk= +github.com/fluxcd/pkg/runtime v0.11.1-0.20210521141553-959de5c91c1c/go.mod h1:vKV1eL4j9TjM6hhWopKeEBmrqXR8SJys0ir3D1GPtKE= github.com/fluxcd/pkg/ssh v0.0.5 h1:rnbFZ7voy2JBlUfMbfyqArX2FYaLNpDhccGFC3qW83A= github.com/fluxcd/pkg/ssh v0.0.5/go.mod h1:7jXPdXZpc0ttMNz2kD9QuMi3RNn/e0DOFbj0Tij/+Hs= github.com/fluxcd/pkg/testserver v0.0.2 h1:SoaMtO9cE5p/wl2zkGudzflnEHd9mk68CGjZOo7w0Uk=