diff --git a/api/v1/gitrepository_types.go b/api/v1/gitrepository_types.go index 569c6c91d..d37ce3137 100644 --- a/api/v1/gitrepository_types.go +++ b/api/v1/gitrepository_types.go @@ -246,6 +246,14 @@ type GitRepositoryVerification struct { // and SSH public keys must be stored under keys with the .sshpub suffix. // +required SecretRef meta.LocalObjectReference `json:"secretRef"` + + // ForwardOnly, when true, rejects a revision that is not a descendant of + // the last verified revision recorded in + // .status.sourceVerificationRevision, such as a branch force-pushed to an + // older commit or a tag moved backwards. Not supported with + // .spec.ref.semver. + // +optional + ForwardOnly bool `json:"forwardOnly,omitempty"` } // GitRepositoryStatus records the observed state of a Git repository. @@ -300,7 +308,15 @@ type GitRepositoryStatus struct { // +optional SourceVerificationMode *GitVerificationMode `json:"sourceVerificationMode,omitempty"` + // SourceVerificationRevision is the last revision whose Git object(s) + // were successfully verified. It is the baseline for forward-only + // verification: when .spec.verify.forwardOnly is set, a new revision + // must be a descendant of this revision. + // +optional + SourceVerificationRevision string `json:"sourceVerificationRevision,omitempty"` + meta.ReconcileRequestStatus `json:",inline"` + meta.ForceRequestStatus `json:",inline"` } const ( @@ -335,6 +351,18 @@ func (in *GitRepository) GetArtifact() *meta.Artifact { return in.Status.Artifact } +// GetLastHandledReconcileRequest returns the last handled reconcile request +// value recorded in the status. +func (in *GitRepository) GetLastHandledReconcileRequest() string { + return in.Status.GetLastHandledReconcileRequest() +} + +// GetLastHandledForceRequestStatus returns a pointer to the last handled +// force request value recorded in the status. +func (in *GitRepository) GetLastHandledForceRequestStatus() *string { + return &in.Status.LastHandledForceAt +} + // GetProvider returns the Git authentication provider. func (v *GitRepository) GetProvider() string { if v.Spec.Provider == "" { diff --git a/api/v1/zz_generated.deepcopy.go b/api/v1/zz_generated.deepcopy.go index 2ecc0eb7b..c0e321afd 100644 --- a/api/v1/zz_generated.deepcopy.go +++ b/api/v1/zz_generated.deepcopy.go @@ -506,6 +506,7 @@ func (in *GitRepositoryStatus) DeepCopyInto(out *GitRepositoryStatus) { **out = **in } out.ReconcileRequestStatus = in.ReconcileRequestStatus + out.ForceRequestStatus = in.ForceRequestStatus } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new GitRepositoryStatus. diff --git a/config/crd/bases/source.toolkit.fluxcd.io_gitrepositories.yaml b/config/crd/bases/source.toolkit.fluxcd.io_gitrepositories.yaml index dfb8628a8..e9aa67d42 100644 --- a/config/crd/bases/source.toolkit.fluxcd.io_gitrepositories.yaml +++ b/config/crd/bases/source.toolkit.fluxcd.io_gitrepositories.yaml @@ -214,6 +214,14 @@ spec: Verification specifies the configuration to verify the Git commit signature(s). properties: + forwardOnly: + description: |- + ForwardOnly, when true, rejects a revision that is not a descendant of + the last verified revision recorded in + .status.sourceVerificationRevision, such as a branch force-pushed to an + older commit or a tag moved backwards. Not supported with + .spec.ref.semver. + type: boolean mode: default: HEAD description: |- @@ -414,6 +422,12 @@ spec: - url type: object type: array + lastHandledForceAt: + description: |- + LastHandledForceAt holds the value of the most recent + force request value, so a change of the annotation value + can be detected. + type: string lastHandledReconcileAt: description: |- LastHandledReconcileAt holds the value of the most recent @@ -489,6 +503,13 @@ spec: SourceVerificationMode is the last used verification mode indicating which Git object(s) have been verified. type: string + sourceVerificationRevision: + description: |- + SourceVerificationRevision is the last revision whose Git object(s) + were successfully verified. It is the baseline for forward-only + verification: when .spec.verify.forwardOnly is set, a new revision + must be a descendant of this revision. + type: string type: object type: object served: true diff --git a/docs/api/v1/source.md b/docs/api/v1/source.md index 3fe0791b0..28cd5fa89 100644 --- a/docs/api/v1/source.md +++ b/docs/api/v1/source.md @@ -2464,6 +2464,21 @@ which Git object(s) have been verified.

+sourceVerificationRevision
+ +string + + + +(Optional) +

SourceVerificationRevision is the last revision whose Git object(s) +were successfully verified. It is the baseline for forward-only +verification: when .spec.verify.forwardOnly is set, a new revision +must be a descendant of this revision.

+ + + + ReconcileRequestStatus
@@ -2477,6 +2492,21 @@ github.com/fluxcd/pkg/apis/meta.ReconcileRequestStatus

+ + +ForceRequestStatus
+ + +github.com/fluxcd/pkg/apis/meta.ForceRequestStatus + + + + +

+(Members of ForceRequestStatus are embedded into this type.) +

+ + @@ -2531,6 +2561,22 @@ authors. PGP public keys must be stored under keys with the .asc suffix, and SSH public keys must be stored under keys with the .sshpub suffix.

+ + +forwardOnly
+ +bool + + + +(Optional) +

ForwardOnly, when true, rejects a revision that is not a descendant of +the last verified revision recorded in +.status.sourceVerificationRevision, such as a branch force-pushed to an +older commit or a tag moved backwards. Not supported with +.spec.ref.semver.

+ + diff --git a/docs/spec/v1/gitrepositories.md b/docs/spec/v1/gitrepositories.md index a700c460a..952f67a15 100644 --- a/docs/spec/v1/gitrepositories.md +++ b/docs/spec/v1/gitrepositories.md @@ -630,7 +630,7 @@ spec: ### Verification `.spec.verify` is an optional field to enable the verification of Git commit -signatures. The field offers two subfields: +signatures. The field offers the following subfields: - `.mode`, to specify what Git object(s) should be verified. Supported values are: @@ -649,6 +649,10 @@ signatures. The field offers two subfields: keys must be stored under keys with the `.sshpub` suffix. Keys without a recognized suffix are treated as PGP key rings for backward compatibility. +- `.forwardOnly`, to additionally require that every new revision descends from + the last verified revision. Defaults to `false`. See [forward-only + verification](#forward-only-verification). + ```yaml --- apiVersion: source.toolkit.fluxcd.io/v1 @@ -741,6 +745,90 @@ PGP verification reports the PGP key ID in the success message (e.g. `5982D0279C227FFD`), while SSH verification reports the SHA256 fingerprint (e.g. `SHA256:uNiVztksCsDhcc0u9e8BgrJXVGDaf6s7kOsTmI9N7sM`). +#### Forward-only verification + +`.spec.verify.forwardOnly` is an optional boolean field. When set to `true`, a +revision that passes signature verification is only accepted if it descends +from the last verified revision, as recorded in +[`.status.sourceVerificationRevision`](#source-verification-revision). This +rejects a branch that was force-pushed to an older commit, a tag that was moved +to an older commit, and any rewritten history, even when the resulting HEAD +carries a valid signature of a trusted author. + +```yaml +--- +apiVersion: source.toolkit.fluxcd.io/v1 +kind: GitRepository +metadata: + name: podinfo + namespace: default +spec: + interval: 1m + url: https://github.com/stefanprodan/podinfo + ref: + branch: master + verify: + mode: HEAD + forwardOnly: true + secretRef: + name: pgp-public-keys +``` + +The check compares history and nothing else: any signed descendant is accepted, +including a work-in-progress commit of a trusted author that someone with push +access fast-forwarded the branch to. It does not replace code review, and it +does not protect against a compromised trusted key. A legitimate rollback is +done with `git revert`, which produces a new signed descendant and is accepted +without further action. + +The signature of the HEAD and/or tag object is verified before the history is +inspected, so an unsigned revision is rejected without additional Git +operations. + +To inspect the history, the controller clones the last 100 commits of the +reference instead of only its tip whenever the reference changed. If the last +verified revision is not among them, the clone is discarded and the last 500 +commits are cloned into a fresh directory. If it is still not found, the +revision is rejected. Each clone runs within [`.spec.timeout`](#timeout), so a +reconciliation of a large repository takes longer than before and may take up +to twice the timeout; raise the timeout if a clone does not complete in time. A revision checked out by `.spec.ref.commit`, or by a +`.spec.ref.name` that is neither a branch nor a tag (for example +`refs/pull/420/head`), is cloned with its full history as before, and the whole +history is inspected. A rebased pull request branch is therefore rejected. + +`.spec.ref.semver` is not supported together with `forwardOnly`, because +release tags cut from release branches are frequently not descendants of each +other. The GitRepository is marked as stalled with reason +`InvalidVerificationMode`. + +`.status.sourceVerificationRevision` is recorded on every successful +verification, whether `forwardOnly` is set or not, so enabling the field on an +existing object takes effect from the next reconciliation on. When `forwardOnly` +is enabled and no such revision has been recorded yet, the controller verifies +the current revision again, records it as the baseline, and emits the event +`forward-only baseline established at ''`. The baseline is lost, and +the next revision is accepted without a history check, when the object is +recreated, when it is restored without its status, or when `.spec.verify` is +removed and the repository is checked out again. + +When a revision is rejected, the controller sets `SourceVerified=False` and +`Ready=False` with reason `NonFastForwardRevision`, emits a warning event, and +keeps the current Artifact and baseline. Controllers consuming the Artifact, +such as kustomize-controller, keep applying the last verified revision. The +reconciliation is retried with an exponential backoff; for a branch or tag +reference that does not change in the meantime, the retries do not clone the +repository again. + +To accept a revision that does not descend from the baseline, for example after +history was deliberately rewritten, verify the revision on the Git server and +request a [forced reconciliation](#triggering-a-reconcile). The revision on the +server at that moment is verified, recorded as the new baseline, and the event +`forward-only baseline reset to '' by force request` is emitted. The +annotations are not part of the manifest in Git, so this also works for a +GitRepository that is itself applied by Flux, such as `flux-system`, where +pinning `.spec.ref.commit` would require a commit in the rejected history or +would be reverted by kustomize-controller. + ### Ignore `.spec.ignore` is an optional field to specify rules in [the `.gitignore` @@ -939,6 +1027,24 @@ Using `flux`: flux reconcile source git ``` +When [forward-only verification](#forward-only-verification) has rejected a +revision that should be accepted anyway, the baseline can be reset by +additionally annotating the GitRepository with `reconcile.fluxcd.io/forceAt` +set to the same value. Both annotations must be set in the same operation. The +revision on the remote at the time of the reconciliation is verified and +recorded as the new baseline, and the value is reported in +[`.status.lastHandledForceAt`](#last-handled-force-at). The value is consumed +by the reconciliation it triggers; if that reconciliation fails before the +revision is verified, for example because the clone times out, request it again +with a new value: + +```sh +TOKEN="$(date +%s)"; \ +kubectl annotate --field-manager=flux-client-side-apply --overwrite gitrepository/ \ +"reconcile.fluxcd.io/requestedAt=$TOKEN" \ +"reconcile.fluxcd.io/forceAt=$TOKEN" +``` + ### Waiting for `Ready` When a change is applied, it is possible to wait for the GitRepository to reach @@ -1212,6 +1318,8 @@ factors: non-existing Secret. - A specified Include is unavailable. - The verification of the Git commit signature failed. +- The revision does not descend from the last verified revision while + [forward-only verification](#forward-only-verification) is enabled. - The credentials in the referenced Secret are invalid. - The GitRepository spec contains a generic misconfiguration. - A storage related failure when storing the artifact. @@ -1237,6 +1345,10 @@ the following attributes is added to the GitRepository's `.status.conditions`: - `status: "False"` - `reason: Failed` +When [forward-only verification](#forward-only-verification) rejects a +revision, the `reason` is `NonFastForwardRevision` and the message names the +rejected revision and the last verified revision. + While the GitRepository has one or more of these Conditions, the controller will continue to attempt to produce an Artifact for the resource with an exponential backoff, until it succeeds and the GitRepository is marked as @@ -1356,6 +1468,16 @@ Secret data key names. It is used by the controller to detect a change in the verification policy, such as a key rotation, that requires the current revision to be verified again even when its revision did not change. +### Source Verification Revision + +The source-controller reports the last revision whose Git object(s) were +successfully verified in the GitRepository's +`.status.sourceVerificationRevision`, in the same format as the [Artifact +revision](#artifact). It is recorded whenever verification succeeds and serves +as the baseline for [forward-only verification](#forward-only-verification): +when `.spec.verify.forwardOnly` is set, a new revision must descend from it. +The field is cleared when `.spec.verify` is removed. + ### Observed Generation The source-controller reports an [observed generation][typical-status-properties] @@ -1372,5 +1494,13 @@ annotation value it acted on in the `.status.lastHandledReconcileAt` field. For practical information about this field, see [triggering a reconcile](#triggering-a-reconcile). +### Last Handled Force At + +The source-controller reports the last `reconcile.fluxcd.io/forceAt` +annotation value it handled in the `.status.lastHandledForceAt` field. See +[triggering a reconcile](#triggering-a-reconcile) for how the annotation is +used to reset the [forward-only verification](#forward-only-verification) +baseline. + [typical-status-properties]: https://github.com/kubernetes/community/blob/master/contributors/devel/sig-architecture/api-conventions.md#typical-status-properties [kstatus-spec]: https://github.com/kubernetes-sigs/cli-utils/tree/master/pkg/kstatus diff --git a/go.mod b/go.mod index 630ece7bb..0c3255f8c 100644 --- a/go.mod +++ b/go.mod @@ -412,3 +412,5 @@ require ( ) retract v0.32.0 // Refers to incorrect ./api version. + +replace github.com/fluxcd/pkg/git => github.com/ruslan-shaydullin/pkg/git v0.55.1-0.20261015144952-74aba21a4653 diff --git a/go.sum b/go.sum index d4cceb29c..58b5c8cb6 100644 --- a/go.sum +++ b/go.sum @@ -360,8 +360,6 @@ github.com/fluxcd/pkg/auth v0.62.0 h1:ZhrkfoLTaH5Q095sNGLZ47zLYw/AHw4nVnPAspGwT/ github.com/fluxcd/pkg/auth v0.62.0/go.mod h1:46lKYjDvx5Q7fsq09ZC1BxXJcstTOfm2X/NFLLFn7wA= github.com/fluxcd/pkg/cache v0.17.0 h1:197ST7bSJD2Nw4G01y5cMjtWQpwQsyjpl2ox4MaGeU8= github.com/fluxcd/pkg/cache v0.17.0/go.mod h1:OW3fncHhPnPZFZav5FdEa//cyMHSndI0JS+IM69LXOU= -github.com/fluxcd/pkg/git v0.55.0 h1:jMdW0uAYuzcwIL5rJq5pmDd+jSrgOe6eIOZfdrcAvO8= -github.com/fluxcd/pkg/git v0.55.0/go.mod h1:s9AK9ZEG9CtAgkcX4QhQphLsaDBTje32tT5V3knpj1I= github.com/fluxcd/pkg/gittestserver v0.32.0 h1:y1wbnEaOXpEYqAxGMxcsXZTH6W4QjFyTj6m1d1ytjY0= github.com/fluxcd/pkg/gittestserver v0.32.0/go.mod h1:Cp4hpAePUXnSaeESHKitnvORCn+xT4fOKuZljkRBqVo= github.com/fluxcd/pkg/helmtestserver v0.44.0 h1:VaJegpUI6KXxGQkiVoMAwzVkvWHNmqdEk77Y74S2vRE= @@ -838,6 +836,8 @@ github.com/rs/xid v1.6.0 h1:fV591PaemRlL6JfRxGDEPl69wICngIQ3shQtzfy2gxU= github.com/rs/xid v1.6.0/go.mod h1:7XoLgs4eV+QndskICGsho+ADou8ySMSjJKDIan90Nz0= github.com/rubenv/sql-migrate v1.8.1 h1:EPNwCvjAowHI3TnZ+4fQu3a915OpnQoPAjTXCGOy2U0= github.com/rubenv/sql-migrate v1.8.1/go.mod h1:BTIKBORjzyxZDS6dzoiw6eAFYJ1iNlGAtjn4LGeVjS8= +github.com/ruslan-shaydullin/pkg/git v0.55.1-0.20261015144952-74aba21a4653 h1:k7kScfYftV0S46GpcjXU7kduFYXV2HCgMcVYUsDX5W8= +github.com/ruslan-shaydullin/pkg/git v0.55.1-0.20261015144952-74aba21a4653/go.mod h1:s9AK9ZEG9CtAgkcX4QhQphLsaDBTje32tT5V3knpj1I= github.com/russross/blackfriday/v2 v2.1.0 h1:JIOH55/0cWyOuilr9/qlrm0BSXldqnqwMsf35Ld67mk= github.com/russross/blackfriday/v2 v2.1.0/go.mod h1:+Rmxgy9KzJVeS9/2gXHxylqXiyQDYRxCVz55jmeOWTM= github.com/ryanuber/go-glob v1.0.0 h1:iQh3xXAumdQ+4Ufa5b25cRpC5TYKlno6hsv6Cb3pkBk= diff --git a/internal/controller/gitrepository_controller.go b/internal/controller/gitrepository_controller.go index 8228037e3..a61b25646 100644 --- a/internal/controller/gitrepository_controller.go +++ b/internal/controller/gitrepository_controller.go @@ -25,6 +25,7 @@ import ( "path/filepath" "sort" "strings" + "sync" "time" securejoin "github.com/cyphar/filepath-securejoin" @@ -177,6 +178,11 @@ type GitRepositoryReconciler struct { features map[string]bool patchOptions []patch.Option + + // rejectedRevisions caches, per object, the last revision rejected by + // forward-only verification (forwardOnlyRejection) so that retries do + // not clone while the remote reference still points to it. + rejectedRevisions sync.Map } type GitRepositoryReconcilerOptions struct { @@ -516,6 +522,23 @@ func (r *GitRepositoryReconciler) reconcileSource(ctx context.Context, sp *patch conditions.Delete(obj, sourcev1.SourceVerifiedCondition) } + // A force request lets forward-only verification accept a revision that + // does not descend from the current baseline. It is evaluated once per + // reconciliation, which also records the handled annotation value. + forceRequested := meta.ShouldHandleForceRequest(obj) + if forceRequested { + r.rejectedRevisions.Delete(client.ObjectKeyFromObject(obj)) + } + + if forwardOnlyEnabled(obj) && isSemVerReference(obj.Spec.Reference) { + e := serror.NewStalling( + errors.New("forward-only verification is not supported with .spec.ref.semver"), + "InvalidVerificationMode", + ) + conditions.MarkFalse(obj, sourcev1.SourceVerifiedCondition, e.Reason, "%s", e) + return sreconcile.ResultEmpty, e + } + var proxyOpts *transport.ProxyOptions var proxyURL *url.URL if obj.Spec.ProxySecretRef != nil { @@ -575,7 +598,14 @@ func (r *GitRepositoryReconciler) reconcileSource(ctx context.Context, sp *patch // Persist the ArtifactSet. *includes = *artifacts - c, err := r.gitCheckout(ctx, obj, authOpts, proxyOpts, dir, true) + // With forward-only verification and a baseline to look for, clone + // enough history to find the baseline instead of only the tip. + var depth int + if forwardOnlyEnabled(obj) && obj.Status.SourceVerificationRevision != "" && !forceRequested { + depth = forwardOnlyCloneDepth + } + + c, err := r.gitCheckout(ctx, obj, authOpts, proxyOpts, dir, true, depth) if err != nil { return sreconcile.ResultEmpty, err } @@ -590,6 +620,14 @@ func (r *GitRepositoryReconciler) reconcileSource(ctx context.Context, sp *patch // Assign the commit to the shared commit reference. *commit = *c + // Repeat a cached forward-only rejection while the remote reference + // still points to the rejected revision. + if rej, ok := r.cachedForwardOnlyRejection(obj); ok && rej.revision == commitReference(obj, commit) { + e := serror.NewGeneric(errors.New(rej.message), nonFastForwardRevisionReason) + conditions.MarkFalse(obj, sourcev1.SourceVerifiedCondition, e.Reason, "%s", e) + return sreconcile.ResultEmpty, e + } + // If it's a partial commit obtained from an existing artifact, check if the // reconciliation can be skipped if other configurations have not changed. if !git.IsConcreteCommit(*commit) { @@ -620,8 +658,12 @@ func (r *GitRepositoryReconciler) reconcileSource(ctx context.Context, sp *patch } // If we can't skip the reconciliation, checkout again without any - // optimization. - c, err := r.gitCheckout(ctx, obj, authOpts, proxyOpts, dir, false) + // optimization. An unchanged revision that is the baseline needs no + // history. + if commitReference(obj, commit) == obj.Status.SourceVerificationRevision { + depth = 0 + } + c, err := r.gitCheckout(ctx, obj, authOpts, proxyOpts, dir, false, depth) if err != nil { return sreconcile.ResultEmpty, err } @@ -640,8 +682,9 @@ func (r *GitRepositoryReconciler) reconcileSource(ctx context.Context, sp *patch return sreconcile.ResultEmpty, e } - // Verify commit signature - if result, err := r.verifySignature(ctx, obj, *commit); err != nil || result == sreconcile.ResultEmpty { + // Verify the commit signature and, if enabled, that the revision + // descends from the last verified revision. + if result, err := r.verifySource(ctx, obj, authOpts, proxyOpts, dir, commit, forceRequested); err != nil || result == sreconcile.ResultEmpty { return result, err } @@ -1012,14 +1055,16 @@ func (r *GitRepositoryReconciler) reconcileInclude(ctx context.Context, sp *patc } // gitCheckout builds checkout options with the given configurations and -// performs a git checkout. +// performs a git checkout. A positive depth limits the history of the clone +// to that number of commits; otherwise only the tip is cloned. func (r *GitRepositoryReconciler) gitCheckout(ctx context.Context, obj *sourcev1.GitRepository, - authOpts *git.AuthOptions, proxyOpts *transport.ProxyOptions, dir string, optimized bool) (*git.Commit, error) { + authOpts *git.AuthOptions, proxyOpts *transport.ProxyOptions, dir string, optimized bool, depth int) (*git.Commit, error) { // Configure checkout strategy. cloneOpts := repository.CloneConfig{ RecurseSubmodules: obj.Spec.RecurseSubmodules, ShallowClone: true, + Depth: depth, } if ref := obj.Spec.Reference; ref != nil { cloneOpts.Branch = ref.Branch @@ -1044,6 +1089,13 @@ func (r *GitRepositoryReconciler) gitCheckout(ctx context.Context, obj *sourcev1 cloneOpts.LastObservedCommit = artifact.Revision } } + // A revision rejected by forward-only verification is retried without a + // clone for as long as the remote reference still points to it. + if optimized { + if rej, ok := r.cachedForwardOnlyRejection(obj); ok { + cloneOpts.LastObservedCommit = rej.revision + } + } gitCtx, cancel := context.WithTimeout(ctx, obj.Spec.Timeout.Duration) defer cancel() @@ -1166,6 +1218,11 @@ func (r *GitRepositoryReconciler) verificationPolicyChanged(ctx context.Context, if obj.Spec.Verification == nil || obj.Spec.Verification.Mode == "" { return false } + // Forward-only verification needs a baseline, which only a full + // verification records. + if obj.Spec.Verification.ForwardOnly && obj.Status.SourceVerificationRevision == "" { + return true + } secret, err := r.getSecret(ctx, obj.Spec.Verification.SecretRef.Name, obj.GetNamespace()) if err != nil { // Return true so the full reconciliation surfaces the error. @@ -1187,6 +1244,7 @@ func (r *GitRepositoryReconciler) verifySignature(ctx context.Context, obj *sour if obj.Spec.Verification == nil || obj.Spec.Verification.Mode == "" { obj.Status.SourceVerificationMode = nil obj.Status.SourceVerificationFingerprint = "" + obj.Status.SourceVerificationRevision = "" conditions.Delete(obj, sourcev1.SourceVerifiedCondition) return sreconcile.ResultSuccess, nil } @@ -1291,6 +1349,7 @@ func (r *GitRepositoryReconciler) reconcileDelete(ctx context.Context, obj *sour // Cleanup caches. r.TokenCache.DeleteEventsForObject(sourcev1.GitRepositoryKind, obj.GetName(), obj.GetNamespace(), cache.OperationReconcile) + r.rejectedRevisions.Delete(client.ObjectKeyFromObject(obj)) // Stop reconciliation as the object is being deleted return sreconcile.ResultEmpty, nil diff --git a/internal/controller/gitrepository_controller_test.go b/internal/controller/gitrepository_controller_test.go index 4a18c8e37..accbebf30 100644 --- a/internal/controller/gitrepository_controller_test.go +++ b/internal/controller/gitrepository_controller_test.go @@ -18,7 +18,10 @@ package controller import ( "context" + "crypto/ed25519" + "crypto/rand" "encoding/json" + "encoding/pem" "errors" "fmt" "net/http" @@ -26,6 +29,7 @@ import ( "os" "path/filepath" "strings" + "sync/atomic" "testing" "time" @@ -36,9 +40,11 @@ import ( "github.com/go-git/go-git/v5/plumbing/object" "github.com/go-git/go-git/v5/storage/memory" . "github.com/onsi/gomega" + gossh "golang.org/x/crypto/ssh" sshtestdata "golang.org/x/crypto/ssh/testdata" corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/types" "k8s.io/utils/ptr" ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/client" @@ -51,6 +57,7 @@ import ( "github.com/fluxcd/pkg/auth" "github.com/fluxcd/pkg/auth/githubapp" "github.com/fluxcd/pkg/git" + "github.com/fluxcd/pkg/git/signature" "github.com/fluxcd/pkg/gittestserver" "github.com/fluxcd/pkg/runtime/conditions" conditionscheck "github.com/fluxcd/pkg/runtime/conditions/check" @@ -2169,6 +2176,7 @@ func TestGitRepositoryReconciler_verifySignature(t *testing.T) { beforeFunc: func(obj *sourcev1.GitRepository) { obj.Status.SourceVerificationMode = ptrToVerificationMode(sourcev1.ModeGitHEAD) obj.Status.SourceVerificationFingerprint = "stale-fingerprint" + obj.Status.SourceVerificationRevision = "main@sha1:shasum" obj.Spec.Verification = nil }, want: sreconcile.ResultSuccess, @@ -2815,6 +2823,7 @@ func TestGitRepositoryReconciler_verifySignature(t *testing.T) { } else { g.Expect(obj.Status.SourceVerificationMode).To(BeNil()) g.Expect(obj.Status.SourceVerificationFingerprint).To(BeEmpty()) + g.Expect(obj.Status.SourceVerificationRevision).To(BeEmpty()) } }) } @@ -4228,6 +4237,27 @@ func TestGitRepositoryReconciler_verificationPolicyChanged(t *testing.T) { secret: secret, want: true, }, + { + name: "forward-only without a baseline requires verification", + obj: func() *sourcev1.GitRepository { + obj := newObj(sourcev1.ModeGitHEAD, verificationFingerprint(secret)) + obj.Spec.Verification.ForwardOnly = true + return obj + }(), + secret: secret, + want: true, + }, + { + name: "forward-only with a baseline and unchanged keys does not require verification", + obj: func() *sourcev1.GitRepository { + obj := newObj(sourcev1.ModeGitHEAD, verificationFingerprint(secret)) + obj.Spec.Verification.ForwardOnly = true + obj.Status.SourceVerificationRevision = "main@sha1:shasum" + return obj + }(), + secret: secret, + want: false, + }, } for _, tt := range tests { @@ -4344,6 +4374,523 @@ func TestGitRepositoryReconciler_reconcileSource_verificationPolicyChange(t *tes } } +// forwardOnlyTest hosts a Git server and a repository of signed commits for +// forward-only verification tests. The scenarios share one upstream +// repository and run in order; each pushes the references it needs. +type forwardOnlyTest struct { + server *gittestserver.GitServer + repoPath string + repo *gogit.Repository + signer signature.Signer + secret *corev1.Secret + clones atomic.Int32 +} + +func newForwardOnlyTest(t *testing.T) *forwardOnlyTest { + t.Helper() + g := NewWithT(t) + + f := &forwardOnlyTest{repoPath: "/forward-only.git"} + + server, err := gittestserver.NewTempGitServer() + g.Expect(err).NotTo(HaveOccurred()) + t.Cleanup(func() { os.RemoveAll(server.Root()) }) + server.AutoCreate() + // Count the clones: a clone fetches a pack with a POST, while the + // LastObservedCommit fast path only lists the references with a GET. + server.AddHTTPMiddlewares(func(next http.Handler) http.Handler { + return http.HandlerFunc(func(w http.ResponseWriter, req *http.Request) { + if req.Method == http.MethodPost && strings.HasSuffix(req.URL.Path, "/git-upload-pack") { + f.clones.Add(1) + } + next.ServeHTTP(w, req) + }) + }) + g.Expect(server.StartHTTP()).To(Succeed()) + t.Cleanup(server.StopHTTP) + f.server = server + + pub, priv, err := ed25519.GenerateKey(rand.Reader) + g.Expect(err).NotTo(HaveOccurred()) + block, err := gossh.MarshalPrivateKey(priv, "") + g.Expect(err).NotTo(HaveOccurred()) + f.signer, err = signature.NewSSHSigner(pem.EncodeToMemory(block), nil) + g.Expect(err).NotTo(HaveOccurred()) + sshPub, err := gossh.NewPublicKey(pub) + g.Expect(err).NotTo(HaveOccurred()) + f.secret = &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{Name: "trusted-keys", Namespace: "default"}, + Data: map[string][]byte{"signer.sshpub": gossh.MarshalAuthorizedKey(sshPub)}, + } + + f.repo, err = gogit.Init(memory.NewStorage(), memfs.New()) + g.Expect(err).NotTo(HaveOccurred()) + g.Expect(f.repo.Storer.SetReference(plumbing.NewSymbolicReference(plumbing.HEAD, plumbing.NewBranchReferenceName("main")))).To(Succeed()) + _, err = f.repo.CreateRemote(&config.RemoteConfig{ + Name: gogit.DefaultRemoteName, + URLs: []string{server.HTTPAddressWithCredentials() + f.repoPath}, + }) + g.Expect(err).NotTo(HaveOccurred()) + return f +} + +// commit adds a file and commits it on the checked out branch, signed with +// the trusted key unless signed is false. Parents override HEAD. +func (f *forwardOnlyTest) commit(t *testing.T, name string, signed bool, parents ...plumbing.Hash) plumbing.Hash { + t.Helper() + g := NewWithT(t) + wt, err := f.repo.Worktree() + g.Expect(err).NotTo(HaveOccurred()) + file, err := wt.Filesystem.Create(name) + g.Expect(err).NotTo(HaveOccurred()) + _, err = file.Write([]byte(name + "\n")) + g.Expect(err).NotTo(HaveOccurred()) + g.Expect(file.Close()).To(Succeed()) + _, err = wt.Add(name) + g.Expect(err).NotTo(HaveOccurred()) + opts := &gogit.CommitOptions{ + Author: &object.Signature{Name: "Trusted Signer", Email: "signer@example.com", When: time.Now()}, + Parents: parents, + } + if signed { + opts.Signer = f.signer + } + h, err := wt.Commit("add "+name, opts) + g.Expect(err).NotTo(HaveOccurred()) + return h +} + +// checkout points the given branch at the given commit and checks it out. +func (f *forwardOnlyTest) checkout(t *testing.T, branch string, hash plumbing.Hash) { + t.Helper() + g := NewWithT(t) + wt, err := f.repo.Worktree() + g.Expect(err).NotTo(HaveOccurred()) + ref := plumbing.NewBranchReferenceName(branch) + g.Expect(f.repo.Storer.SetReference(plumbing.NewHashReference(ref, hash))).To(Succeed()) + g.Expect(wt.Checkout(&gogit.CheckoutOptions{Branch: ref, Force: true})).To(Succeed()) +} + +// push points the local reference at the given hash and force-pushes it. +func (f *forwardOnlyTest) push(t *testing.T, ref string, hash plumbing.Hash) { + t.Helper() + g := NewWithT(t) + g.Expect(f.repo.Storer.SetReference(plumbing.NewHashReference(plumbing.ReferenceName(ref), hash))).To(Succeed()) + err := f.repo.Push(&gogit.PushOptions{ + RefSpecs: []config.RefSpec{config.RefSpec("+" + ref + ":" + ref)}, + Force: true, + }) + if !errors.Is(err, gogit.NoErrAlreadyUpToDate) { + g.Expect(err).NotTo(HaveOccurred()) + } +} + +// signedTag creates a tag object signed with the trusted key that points to +// the given commit, and force-pushes it. go-git can only sign tags with a PGP +// key, so the object is assembled by hand. +func (f *forwardOnlyTest) signedTag(t *testing.T, name string, target plumbing.Hash) { + t.Helper() + g := NewWithT(t) + tag := &object.Tag{ + Name: name, + Tagger: object.Signature{Name: "Trusted Signer", Email: "signer@example.com", When: time.Now()}, + Message: name + "\n", + TargetType: plumbing.CommitObject, + Target: target, + } + unsigned := &plumbing.MemoryObject{} + g.Expect(tag.EncodeWithoutSignature(unsigned)).To(Succeed()) + payload, err := unsigned.Reader() + g.Expect(err).NotTo(HaveOccurred()) + sig, err := f.signer.Sign(payload) + g.Expect(err).NotTo(HaveOccurred()) + tag.PGPSignature = string(sig) + encoded := f.repo.Storer.NewEncodedObject() + g.Expect(tag.Encode(encoded)).To(Succeed()) + h, err := f.repo.Storer.SetEncodedObject(encoded) + g.Expect(err).NotTo(HaveOccurred()) + f.push(t, plumbing.NewTagReferenceName(name).String(), h) +} + +// object returns a GitRepository with HEAD verification enabled. With an +// artifact revision, the object looks like one that has produced an Artifact +// for that revision; the baseline is the last verified revision. +func (f *forwardOnlyTest) object(forwardOnly bool, ref *sourcev1.GitRepositoryRef, artifact, baseline string) *sourcev1.GitRepository { + obj := &sourcev1.GitRepository{ + ObjectMeta: metav1.ObjectMeta{ + Name: "forward-only-" + randStringRunes(5), + Namespace: "default", + UID: types.UID(randStringRunes(10)), + Generation: 1, + }, + Spec: sourcev1.GitRepositorySpec{ + Interval: metav1.Duration{Duration: interval}, + Timeout: &metav1.Duration{Duration: timeout}, + URL: f.server.HTTPAddress() + f.repoPath, + Reference: ref, + Verification: &sourcev1.GitRepositoryVerification{ + Mode: sourcev1.ModeGitHEAD, + SecretRef: meta.LocalObjectReference{Name: f.secret.Name}, + ForwardOnly: forwardOnly, + }, + }, + Status: sourcev1.GitRepositoryStatus{ + SourceVerificationRevision: baseline, + }, + } + if artifact != "" { + obj.Status.Artifact = &meta.Artifact{Revision: artifact, Path: randStringRunes(10)} + obj.Status.SourceVerificationMode = ptrToVerificationMode(sourcev1.ModeGitHEAD) + obj.Status.SourceVerificationFingerprint = verificationFingerprint(f.secret) + conditions.MarkTrue(obj, sourcev1.ArtifactInStorageCondition, meta.SucceededReason, "stored artifact for revision '%s'", artifact) + } + return obj +} + +// reconciler returns a reconciler whose client holds the trusted keys and the +// object, and its event recorder. +func (f *forwardOnlyTest) reconciler(obj *sourcev1.GitRepository) (*GitRepositoryReconciler, *events.FakeRecorder) { + recorder := events.NewFakeRecorder(64, false) + c := fakeclient.NewClientBuilder(). + WithScheme(testEnv.GetScheme()). + WithObjects(obj, f.secret). + WithStatusSubresource(&sourcev1.GitRepository{}). + Build() + return &GitRepositoryReconciler{ + Client: c, + Recorder: recorder, + Storage: testStorage, + patchOptions: getPatchOptions(gitRepositoryReadyCondition.Owned, "sc"), + }, recorder +} + +// reconcile runs the source reconciliation for the object and returns the +// observed commit, the result, the error and the number of clones performed. +func (f *forwardOnlyTest) reconcile(t *testing.T, r *GitRepositoryReconciler, obj *sourcev1.GitRepository) (git.Commit, sreconcile.Result, error, int32) { + t.Helper() + var commit git.Commit + var includes artifactSet + before := f.clones.Load() + sp := patch.NewSerialPatcher(obj, r.Client) + res, err := r.reconcileSource(ctx, sp, obj, &commit, &includes, t.TempDir()) + return commit, res, err, f.clones.Load() - before +} + +// errorReason returns the reason of a reconciler error. +func errorReason(err error) string { + var ge *serror.Generic + var se *serror.Stalling + switch { + case errors.As(err, &ge): + return ge.Reason + case errors.As(err, &se): + return se.Reason + } + return "" +} + +// eventMessages returns the messages of the events recorded so far. +func eventMessages(recorder *events.FakeRecorder) []string { + var messages []string + for { + select { + case e := <-recorder.Events: + messages = append(messages, e.Message) + default: + return messages + } + } +} + +func TestGitRepositoryReconciler_reconcileSource_forwardOnly(t *testing.T) { + f := newForwardOnlyTest(t) + + const main, other, pull, token = "refs/heads/main", "refs/heads/other", "refs/pull/1/head", "token" + mainRef := &sourcev1.GitRepositoryRef{Branch: "main"} + rev := func(name string, h plumbing.Hash) string { return name + "@sha1:" + h.String() } + + // History: c1 <- c2 <- c3 <- c4 (unsigned); c2 <- s1 merged into m(c3, s1); + // 150 signed commits on m up to tip150, an unsigned u on tip150, and + // forwardOnlyDeepCloneDepth+1 signed commits on m up to tipDeep; d1 on + // branch other from c1. + c1 := f.commit(t, "c1", true) + c2 := f.commit(t, "c2", true) + c3 := f.commit(t, "c3", true) + c4 := f.commit(t, "c4", false) + f.checkout(t, "side", c2) + s1 := f.commit(t, "s1", true) + f.checkout(t, "main", c3) + m := f.commit(t, "m", true, c3, s1) + tip := m + for i := 1; i <= 150; i++ { + tip = f.commit(t, fmt.Sprintf("deep-%d", i), true) + } + tip150 := tip + u := f.commit(t, "unsigned-deep", false) + f.checkout(t, "main", tip150) + for i := 151; i <= forwardOnlyDeepCloneDepth+1; i++ { + tip = f.commit(t, fmt.Sprintf("deep-%d", i), true) + } + tipDeep := tip + f.checkout(t, "other", c1) + d1 := f.commit(t, "d1", true) + f.push(t, main, c2) + // The remote HEAD of the test server points to master; a clone by commit + // needs it to exist. + f.push(t, "refs/heads/master", c2) + + tagRef := &sourcev1.GitRepositoryRef{Tag: "v1.0.0"} + tagObject := func() *sourcev1.GitRepository { + obj := f.object(true, tagRef, rev("v1.0.0", c2), rev("v1.0.0", c2)) + obj.Spec.Verification.Mode = sourcev1.ModeGitTagAndHEAD + obj.Status.SourceVerificationMode = ptrToVerificationMode(sourcev1.ModeGitTagAndHEAD) + return obj + } + forceRequest := func(obj *sourcev1.GitRepository) { + obj.SetAnnotations(map[string]string{meta.ReconcileRequestAnnotation: token, meta.ForceRequestAnnotation: token}) + } + const nonFastForward = "NonFastForwardRevision" + notDescendant := "is not a descendant of the last verified revision" + + steps := []struct { + name string + push map[string]plumbing.Hash // references to force-push first + setup func(t *testing.T) // other changes to the remote + object *sourcev1.GitRepository // nil reuses the previous object and reconciler + mutate func(obj *sourcev1.GitRepository) + reason string // expected error reason; empty for success + message string // substring expected in the error + revision plumbing.Hash // expected observed commit + baseline string // expected status.sourceVerificationRevision + clones int32 // expected number of clones + event string // substring expected in a recorded event + check func(g Gomega, obj *sourcev1.GitRepository) + }{ + { + name: "rollback is accepted when forwardOnly is disabled", push: map[string]plumbing.Hash{main: c1}, + object: f.object(false, mainRef, rev("main", c2), rev("main", c2)), + revision: c1, baseline: rev("main", c1), clones: 1, + }, + { + name: "rollback is rejected", object: f.object(true, mainRef, rev("main", c2), rev("main", c2)), + reason: nonFastForward, message: "revision '" + rev("main", c1) + "' " + notDescendant + " '" + rev("main", c2) + "'; request a forced reconciliation with the reconcile.fluxcd.io/forceAt annotation", + baseline: rev("main", c2), clones: 1, + check: func(g Gomega, obj *sourcev1.GitRepository) { + g.Expect(obj.Status.Artifact.Revision).To(Equal(rev("main", c2))) + g.Expect(obj.Status.SourceVerificationFingerprint).To(Equal(verificationFingerprint(f.secret))) + g.Expect(conditions.GetReason(obj, sourcev1.SourceVerifiedCondition)).To(Equal(nonFastForward)) + }, + }, + { + name: "rejection is repeated without a clone while the remote is unchanged", + reason: nonFastForward, message: notDescendant, baseline: rev("main", c2), clones: 0, + check: func(g Gomega, obj *sourcev1.GitRepository) { + g.Expect(conditions.GetMessage(obj, sourcev1.SourceVerifiedCondition)).To(ContainSubstring(notDescendant)) + }, + }, + { + name: "return to the verified revision clears the rejection", push: map[string]plumbing.Hash{main: c2}, + revision: c2, baseline: rev("main", c2), clones: 1, + check: func(g Gomega, obj *sourcev1.GitRepository) { + g.Expect(conditions.IsTrue(obj, sourcev1.SourceVerifiedCondition)).To(BeTrue()) + g.Expect(conditions.Has(obj, sourcev1.ArtifactOutdatedCondition)).To(BeFalse()) + }, + }, + { + name: "unchanged revision takes the fast path again", + reason: sourcev1.GitOperationSucceedReason, message: "no changes since last reconciliation", clones: 0, + }, + { + name: "cached rejection does not outlive the baseline", push: map[string]plumbing.Hash{main: c1}, + object: f.object(true, mainRef, rev("main", c2), rev("main", c2)), + reason: nonFastForward, message: notDescendant, clones: 1, + }, + { + name: "a cleared baseline is established at the rejected revision", + mutate: func(obj *sourcev1.GitRepository) { obj.Status.SourceVerificationRevision = "" }, + revision: c1, baseline: rev("main", c1), clones: 1, event: "forward-only baseline established at '" + rev("main", c1) + "'", + }, + { + name: "cached rejection does not outlive the object generation", + object: f.object(true, mainRef, rev("main", c2), rev("main", c2)), + reason: nonFastForward, message: notDescendant, clones: 1, + }, + { + name: "a new generation without forwardOnly accepts the revision", + mutate: func(obj *sourcev1.GitRepository) { + obj.Generation = 2 + obj.Spec.Verification.ForwardOnly = false + }, + revision: c1, baseline: rev("main", c1), clones: 1, + }, + { + name: "signed descendant is accepted", push: map[string]plumbing.Hash{main: c3}, + object: f.object(true, mainRef, rev("main", c2), rev("main", c2)), + revision: c3, baseline: rev("main", c3), clones: 1, + }, + { + name: "unsigned descendant is rejected by signature verification", push: map[string]plumbing.Hash{main: c4}, + object: f.object(true, mainRef, rev("main", c3), rev("main", c3)), + reason: "InvalidCommitSignature", baseline: rev("main", c3), clones: 1, + }, + { + name: "merge commit is accepted with the baseline on the first parent", push: map[string]plumbing.Hash{main: m}, + object: f.object(true, mainRef, rev("main", c3), rev("main", c3)), + revision: m, baseline: rev("main", m), clones: 1, + }, + { + name: "merge commit is accepted with the baseline on the second parent", + object: f.object(true, mainRef, rev("main", s1), rev("main", s1)), + revision: m, baseline: rev("main", m), clones: 1, + }, + { + name: "baseline beyond the first clone depth is found by a second clone", push: map[string]plumbing.Hash{main: tip150}, + object: f.object(true, mainRef, rev("main", m), rev("main", m)), + revision: tip150, baseline: rev("main", tip150), clones: 2, + }, + { + name: "unsigned HEAD beyond the first clone depth does not clone again", push: map[string]plumbing.Hash{main: u}, + object: f.object(true, mainRef, rev("main", m), rev("main", m)), + reason: "InvalidCommitSignature", baseline: rev("main", m), clones: 1, + }, + { + name: "baseline beyond the second clone depth is rejected", push: map[string]plumbing.Hash{main: tipDeep}, + object: f.object(true, mainRef, rev("main", m), rev("main", m)), + reason: nonFastForward, message: fmt.Sprintf("the latter was not found within the last %d commits", forwardOnlyDeepCloneDepth), + baseline: rev("main", m), clones: 2, + }, + { + name: "rollback is rejected without an artifact in storage", push: map[string]plumbing.Hash{main: m}, + object: f.object(true, mainRef, "", rev("main", tipDeep)), + reason: nonFastForward, message: notDescendant, baseline: rev("main", tipDeep), clones: 1, + }, + { + name: "first verification establishes the baseline", push: map[string]plumbing.Hash{main: tipDeep}, + object: f.object(true, mainRef, "", ""), + revision: tipDeep, baseline: rev("main", tipDeep), clones: 1, event: "forward-only baseline established at '" + rev("main", tipDeep) + "'", + check: func(g Gomega, obj *sourcev1.GitRepository) { + g.Expect(conditions.GetMessage(obj, sourcev1.SourceVerifiedCondition)).To(ContainSubstring("forward-only baseline established at")) + }, + }, + { + name: "missing baseline is established on an unchanged revision", + object: f.object(true, mainRef, rev("main", tipDeep), ""), + revision: tipDeep, baseline: rev("main", tipDeep), clones: 1, event: "forward-only baseline established at", + }, + { + name: "switching to a branch that does not descend from the baseline is rejected", push: map[string]plumbing.Hash{other: d1}, + object: f.object(true, &sourcev1.GitRepositoryRef{Branch: "other"}, rev("main", tipDeep), rev("main", tipDeep)), + reason: nonFastForward, message: notDescendant, baseline: rev("main", tipDeep), clones: 1, + }, + { + name: "a force request makes the revision the new baseline", mutate: forceRequest, + revision: d1, baseline: rev("other", d1), clones: 1, event: "forward-only baseline reset to '" + rev("other", d1) + "' by force request", + check: func(g Gomega, obj *sourcev1.GitRepository) { + g.Expect(obj.Status.LastHandledForceAt).To(Equal(token)) + }, + }, + { + name: "the same force request does not apply again", push: map[string]plumbing.Hash{other: c1}, + mutate: func(obj *sourcev1.GitRepository) { obj.Status.Artifact.Revision = rev("other", d1) }, + reason: nonFastForward, message: notDescendant, baseline: rev("other", d1), clones: 1, + }, + { + name: "tag moved to an older commit is rejected", setup: func(t *testing.T) { f.signedTag(t, "v1.0.0", c1) }, + object: tagObject(), reason: nonFastForward, message: notDescendant, baseline: rev("v1.0.0", c2), clones: 1, + }, + { + name: "tag moved to a descendant is accepted", setup: func(t *testing.T) { f.signedTag(t, "v1.0.0", c3) }, + object: tagObject(), revision: c3, baseline: rev("v1.0.0", c3), clones: 1, + }, + { + name: "commit pinned to an older commit is rejected", + object: f.object(true, &sourcev1.GitRepositoryRef{Commit: c1.String()}, rev("main", c2), rev("main", c2)), + reason: nonFastForward, message: notDescendant, baseline: rev("main", c2), clones: 1, + }, + { + name: "cached rejection is repeated for a pinned commit after its clone", + reason: nonFastForward, message: notDescendant, baseline: rev("main", c2), clones: 1, + }, + { + name: "commit pinned to a descendant is accepted", + object: f.object(true, &sourcev1.GitRepositoryRef{Commit: c3.String()}, rev("main", c2), rev("main", c2)), + revision: c3, baseline: "sha1:" + c3.String(), clones: 1, + }, + { + // d1 must stay reachable from a branch for the full clone. + name: "pull request reference pointed at a non-descendant is rejected", push: map[string]plumbing.Hash{pull: d1, other: d1}, + object: f.object(true, &sourcev1.GitRepositoryRef{Name: pull}, rev(pull, c2), rev(pull, c2)), + reason: nonFastForward, message: notDescendant, baseline: rev(pull, c2), clones: 1, + }, + { + name: "pull request reference moved to a descendant is accepted", push: map[string]plumbing.Hash{pull: c3}, + object: f.object(true, &sourcev1.GitRepositoryRef{Name: pull}, rev(pull, c2), rev(pull, c2)), + revision: c3, baseline: rev(pull, c3), clones: 1, + }, + { + name: "baseline without a valid commit hash is rejected", + object: f.object(true, mainRef, rev("main", c2), "main@sha1:not-a-hash"), + reason: nonFastForward, message: "has no valid commit hash", baseline: "main@sha1:not-a-hash", clones: 1, + }, + { + name: "semver reference is not supported", + object: f.object(true, &sourcev1.GitRepositoryRef{SemVer: ">=1.0.0"}, rev("v1.0.0", c3), rev("v1.0.0", c3)), + reason: "InvalidVerificationMode", message: "forward-only verification is not supported with .spec.ref.semver", clones: 0, + check: func(g Gomega, obj *sourcev1.GitRepository) { + g.Expect(conditions.IsFalse(obj, sourcev1.SourceVerifiedCondition)).To(BeTrue()) + }, + }, + } + + var ( + obj *sourcev1.GitRepository + r *GitRepositoryReconciler + recorder *events.FakeRecorder + ) + for _, s := range steps { + t.Run(s.name, func(t *testing.T) { + g := NewWithT(t) + for ref, h := range s.push { + f.push(t, ref, h) + } + if s.setup != nil { + s.setup(t) + } + if s.object != nil { + obj = s.object + r, recorder = f.reconciler(obj) + } + if s.mutate != nil { + s.mutate(obj) + } + + commit, res, err, clones := f.reconcile(t, r, obj) + if s.reason == "" { + g.Expect(err).NotTo(HaveOccurred()) + g.Expect(res).To(Equal(sreconcile.ResultSuccess)) + } else { + g.Expect(err).To(HaveOccurred()) + g.Expect(errorReason(err)).To(Equal(s.reason), err.Error()) + g.Expect(err.Error()).To(ContainSubstring(s.message)) + } + if !s.revision.IsZero() { + g.Expect(commit.Hash.String()).To(Equal(s.revision.String())) + } + if s.baseline != "" { + g.Expect(obj.Status.SourceVerificationRevision).To(Equal(s.baseline)) + } + g.Expect(clones).To(Equal(s.clones)) + if s.event != "" { + g.Expect(eventMessages(recorder)).To(ContainElement(ContainSubstring(s.event))) + } + if s.check != nil { + s.check(g, obj) + } + t.Logf("%s: err=%v", s.name, err) + }) + } +} + func ptrToVerificationMode(mode sourcev1.GitVerificationMode) *sourcev1.GitVerificationMode { return &mode } diff --git a/internal/controller/gitrepository_history.go b/internal/controller/gitrepository_history.go new file mode 100644 index 000000000..7528cf72f --- /dev/null +++ b/internal/controller/gitrepository_history.go @@ -0,0 +1,354 @@ +/* +Copyright 2026 The Flux authors + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package controller + +import ( + "context" + "errors" + "fmt" + "os" + + extgogit "github.com/go-git/go-git/v5" + "github.com/go-git/go-git/v5/plumbing" + "github.com/go-git/go-git/v5/plumbing/object" + "github.com/go-git/go-git/v5/plumbing/storer" + "github.com/go-git/go-git/v5/plumbing/transport" + corev1 "k8s.io/api/core/v1" + "k8s.io/apimachinery/pkg/types" + "sigs.k8s.io/controller-runtime/pkg/client" + + "github.com/fluxcd/pkg/apis/meta" + "github.com/fluxcd/pkg/git" + "github.com/fluxcd/pkg/runtime/conditions" + + sourcev1 "github.com/fluxcd/source-controller/api/v1" + serror "github.com/fluxcd/source-controller/internal/error" + sreconcile "github.com/fluxcd/source-controller/internal/reconcile" +) + +const ( + // forwardOnlyCloneDepth is the number of commits cloned when forward-only + // verification has a baseline to look for. + forwardOnlyCloneDepth = 100 + + // forwardOnlyDeepCloneDepth is the number of commits cloned into a fresh + // directory when the baseline is not within forwardOnlyCloneDepth. A + // baseline further away is rejected, which bounds the memory, traffic + // and time a reference pointed at an old commit can cost per attempt. + forwardOnlyDeepCloneDepth = 500 + + // maxAncestryVisits bounds the number of commits findAncestor loads. A + // clone depth counts generations, so a merge-heavy clone holds more + // commits than its depth; the bound mainly guards the full-history + // clones used for commits and pull request references. + maxAncestryVisits = 100_000 + + // nonFastForwardRevisionReason signals that a revision was rejected by + // forward-only verification because it does not descend from the last + // verified revision. + nonFastForwardRevisionReason = "NonFastForwardRevision" +) + +// ancestryResult is the outcome of searching the history of a commit for an +// ancestor. +type ancestryResult int + +const ( + // ancestryFound means the ancestor was reached. + ancestryFound ancestryResult = iota + // ancestryNotFound means the complete reachable history was traversed + // without reaching the ancestor. + ancestryNotFound + // ancestryTruncated means the traversal stopped at a shallow boundary or + // at the visit limit before reaching the ancestor, which may exist beyond + // the available history. + ancestryTruncated +) + +// forwardOnlyRejection is a revision rejected by forward-only verification. It +// is cached per object so that retries repeat the rejection without +// inspecting the history while the remote reference still points to the +// rejected revision, the object is unchanged and the baseline is the same. +type forwardOnlyRejection struct { + uid types.UID + generation int64 + baseline string + revision string + message string +} + +// cachedForwardOnlyRejection returns the cached rejection for the object if +// it still applies. +func (r *GitRepositoryReconciler) cachedForwardOnlyRejection(obj *sourcev1.GitRepository) (forwardOnlyRejection, bool) { + v, ok := r.rejectedRevisions.Load(client.ObjectKeyFromObject(obj)) + if !ok { + return forwardOnlyRejection{}, false + } + rej, ok := v.(forwardOnlyRejection) + if !ok || !forwardOnlyEnabled(obj) || rej.uid != obj.UID || rej.generation != obj.Generation || + rej.baseline != obj.Status.SourceVerificationRevision { + return forwardOnlyRejection{}, false + } + return rej, true +} + +// forwardOnlyEnabled returns whether forward-only verification is configured. +func forwardOnlyEnabled(obj *sourcev1.GitRepository) bool { + return obj.Spec.Verification != nil && obj.Spec.Verification.Mode != "" && obj.Spec.Verification.ForwardOnly +} + +// isSemVerReference returns whether the reference is resolved by a semver +// range, following the precedence of the Git client: commit, name and tag take +// precedence over semver. +func isSemVerReference(ref *sourcev1.GitRepositoryRef) bool { + return ref != nil && ref.SemVer != "" && ref.Commit == "" && ref.Name == "" && ref.Tag == "" +} + +// cloneDepthApplies returns whether the clone of the configured reference +// honours a clone depth. Branches and tags do, directly or through a reference +// name; a commit, and a reference name that is neither a branch nor a tag, are +// cloned with their full history. +func cloneDepthApplies(ref *sourcev1.GitRepositoryRef) bool { + switch { + case ref == nil: + return true + case ref.Commit != "": + return false + case ref.Name != "": + name := plumbing.ReferenceName(ref.Name) + return name.IsBranch() || name.IsTag() + default: + return true + } +} + +// findAncestor reports whether ancestor is reachable from head by walking the +// parent links breadth-first, loading at most limit commits. Commits listed in +// shallow are shallow boundaries whose parents are not available; a missing +// parent object is treated the same way. +func findAncestor(s storer.EncodedObjectStorer, shallow []plumbing.Hash, head, ancestor plumbing.Hash, limit int) (ancestryResult, error) { + boundary := make(map[plumbing.Hash]struct{}, len(shallow)) + for _, h := range shallow { + boundary[h] = struct{}{} + } + + visited := map[plumbing.Hash]struct{}{head: {}} + queue := []plumbing.Hash{head} + truncated := false + for visits := 0; len(queue) > 0; visits++ { + h := queue[0] + queue = queue[1:] + if h == ancestor { + return ancestryFound, nil + } + if visits >= limit { + return ancestryTruncated, nil + } + if _, ok := boundary[h]; ok { + truncated = true + continue + } + c, err := object.GetCommit(s, h) + if err != nil { + if errors.Is(err, plumbing.ErrObjectNotFound) { + truncated = true + continue + } + return ancestryNotFound, fmt.Errorf("unable to read commit '%s': %w", h, err) + } + for _, p := range c.ParentHashes { + if _, ok := visited[p]; ok { + continue + } + visited[p] = struct{}{} + queue = append(queue, p) + } + } + if truncated { + return ancestryTruncated, nil + } + return ancestryNotFound, nil +} + +// isDescendant opens the Git repository checked out at dir and reports whether +// head descends from ancestor within the available history. +func isDescendant(dir string, head, ancestor plumbing.Hash) (ancestryResult, error) { + repo, err := extgogit.PlainOpen(dir) + if err != nil { + return ancestryNotFound, fmt.Errorf("unable to open repository: %w", err) + } + shallow, err := repo.Storer.Shallow() + if err != nil { + return ancestryNotFound, fmt.Errorf("unable to read shallow boundaries: %w", err) + } + return findAncestor(repo.Storer, shallow, head, ancestor, maxAncestryVisits) +} + +// verifySource verifies the signature of the given commit and, when +// forward-only verification is enabled, that it descends from the last +// verified revision. On success it records the revision as the new baseline. +// A rejection by the history check leaves the verification status of the +// current Artifact untouched. +func (r *GitRepositoryReconciler) verifySource(ctx context.Context, obj *sourcev1.GitRepository, + authOpts *git.AuthOptions, proxyOpts *transport.ProxyOptions, dir string, commit *git.Commit, force bool) (sreconcile.Result, error) { + prevMode, prevFingerprint := obj.Status.SourceVerificationMode, obj.Status.SourceVerificationFingerprint + if result, err := r.verifySignature(ctx, obj, *commit); err != nil || result == sreconcile.ResultEmpty { + return result, err + } + result, err := r.verifyHistory(ctx, obj, authOpts, proxyOpts, dir, commit, force) + if err != nil || result == sreconcile.ResultEmpty { + obj.Status.SourceVerificationMode, obj.Status.SourceVerificationFingerprint = prevMode, prevFingerprint + } + return result, err +} + +// verifyHistory enforces forward-only verification for a commit whose +// signature has been verified, and records it as the last verified revision. +// A force request skips the history check and makes the commit the new +// baseline. +func (r *GitRepositoryReconciler) verifyHistory(ctx context.Context, obj *sourcev1.GitRepository, + authOpts *git.AuthOptions, proxyOpts *transport.ProxyOptions, dir string, commit *git.Commit, force bool) (sreconcile.Result, error) { + if obj.Spec.Verification == nil || obj.Spec.Verification.Mode == "" { + return sreconcile.ResultSuccess, nil + } + + baseline := obj.Status.SourceVerificationRevision + forwardOnly := obj.Spec.Verification.ForwardOnly + if forwardOnly && baseline != "" && !force { + if result, err := r.checkAncestry(ctx, obj, authOpts, proxyOpts, dir, commit, baseline); err != nil || result == sreconcile.ResultEmpty { + return result, err + } + } + + revision := commitReference(obj, commit) + obj.Status.SourceVerificationRevision = revision + r.rejectedRevisions.Delete(client.ObjectKeyFromObject(obj)) + + if forwardOnly { + switch { + case force && baseline != "": + sreconcile.EventLogf(ctx, r, obj, corev1.EventTypeNormal, meta.SucceededReason, sourcev1.ActionVerifySource, + "forward-only baseline reset to '%s' by force request", revision) + case baseline == "": + msg := fmt.Sprintf("forward-only baseline established at '%s'", revision) + conditions.MarkTrue(obj, sourcev1.SourceVerifiedCondition, meta.SucceededReason, "%s\n%s", + conditions.GetMessage(obj, sourcev1.SourceVerifiedCondition), msg) + sreconcile.EventLogf(ctx, r, obj, corev1.EventTypeNormal, meta.SucceededReason, sourcev1.ActionVerifySource, "%s", msg) + } + } + return sreconcile.ResultSuccess, nil +} + +// checkAncestry rejects the commit unless the baseline revision is found in its +// history. When the reference honours a clone depth, dir holds the last +// forwardOnlyCloneDepth commits; if the baseline is beyond them, the directory +// is cleared and the reference is cloned again with forwardOnlyDeepCloneDepth +// commits, verifying the signature again if the remote HEAD moved in between. +func (r *GitRepositoryReconciler) checkAncestry(ctx context.Context, obj *sourcev1.GitRepository, + authOpts *git.AuthOptions, proxyOpts *transport.ProxyOptions, dir string, commit *git.Commit, baseline string) (sreconcile.Result, error) { + const hint = "request a forced reconciliation with the reconcile.fluxcd.io/forceAt annotation to accept it as the new baseline" + ancestor := plumbing.NewHash(git.ExtractHashFromRevision(baseline).String()) + if ancestor.IsZero() { + return r.rejectRevision(obj, baseline, commitReference(obj, commit), fmt.Sprintf( + "revision '%s' could not be verified against the last verified revision '%s', which has no valid commit hash; %s", + commitReference(obj, commit), baseline, hint)) + } + depthApplies := cloneDepthApplies(obj.Spec.Reference) + + res, err := isDescendant(dir, plumbing.NewHash(commit.Hash.String()), ancestor) + if err == nil && res == ancestryTruncated && depthApplies { + if err := os.RemoveAll(dir); err == nil { + err = os.MkdirAll(dir, 0o700) + } + if err != nil { + e := serror.NewGeneric( + fmt.Errorf("failed to reset temporary working directory: %w", err), + sourcev1.DirCreationFailedReason, + ) + conditions.MarkTrue(obj, sourcev1.StorageOperationFailedCondition, e.Reason, "%s", e) + return sreconcile.ResultEmpty, e + } + c, err := r.gitCheckout(ctx, obj, authOpts, proxyOpts, dir, false, forwardOnlyDeepCloneDepth) + if err != nil { + return sreconcile.ResultEmpty, err + } + if c == nil { + e := serror.NewGeneric( + errors.New("git repository is empty"), + "EmptyGitRepository", + ) + conditions.MarkTrue(obj, sourcev1.FetchFailedCondition, e.Reason, "%s", e) + return sreconcile.ResultEmpty, e + } + if err := r.validateSparseCheckoutPaths(obj, dir); err != nil { + e := serror.NewGeneric( + fmt.Errorf("failed to sparse checkout directories : %w", err), + sourcev1.GitOperationFailedReason, + ) + conditions.MarkTrue(obj, sourcev1.FetchFailedCondition, e.Reason, "%s", e) + return sreconcile.ResultEmpty, e + } + if c.Hash.String() != commit.Hash.String() { + if result, err := r.verifySignature(ctx, obj, *c); err != nil || result == sreconcile.ResultEmpty { + return result, err + } + } + *commit = *c + res, err = isDescendant(dir, plumbing.NewHash(commit.Hash.String()), ancestor) + } + revision := commitReference(obj, commit) + if err != nil { + e := serror.NewGeneric( + fmt.Errorf("failed to inspect the history of revision '%s': %w", revision, err), + sourcev1.GitOperationFailedReason, + ) + conditions.MarkTrue(obj, sourcev1.FetchFailedCondition, e.Reason, "%s", e) + return sreconcile.ResultEmpty, e + } + + switch { + case res == ancestryFound: + return sreconcile.ResultSuccess, nil + case res == ancestryTruncated && depthApplies: + return r.rejectRevision(obj, baseline, revision, fmt.Sprintf( + "revision '%s' could not be verified as a descendant of the last verified revision '%s': the latter was not found within the last %d commits; %s", + revision, baseline, forwardOnlyDeepCloneDepth, hint)) + case res == ancestryTruncated: + return r.rejectRevision(obj, baseline, revision, fmt.Sprintf( + "revision '%s' could not be verified as a descendant of the last verified revision '%s': the latter was not found among the first %d commits of its history; %s", + revision, baseline, maxAncestryVisits, hint)) + default: + return r.rejectRevision(obj, baseline, revision, fmt.Sprintf( + "revision '%s' is not a descendant of the last verified revision '%s'; %s", + revision, baseline, hint)) + } +} + +// rejectRevision records the rejection of a revision by forward-only +// verification in the status and in the rejection cache. +func (r *GitRepositoryReconciler) rejectRevision(obj *sourcev1.GitRepository, baseline, revision, msg string) (sreconcile.Result, error) { + e := serror.NewGeneric(errors.New(msg), nonFastForwardRevisionReason) + conditions.MarkFalse(obj, sourcev1.SourceVerifiedCondition, e.Reason, "%s", e) + r.rejectedRevisions.Store(client.ObjectKeyFromObject(obj), forwardOnlyRejection{ + uid: obj.UID, + generation: obj.Generation, + baseline: baseline, + revision: revision, + message: msg, + }) + return sreconcile.ResultEmpty, e +} diff --git a/internal/controller/gitrepository_history_test.go b/internal/controller/gitrepository_history_test.go new file mode 100644 index 000000000..55b723ef8 --- /dev/null +++ b/internal/controller/gitrepository_history_test.go @@ -0,0 +1,101 @@ +/* +Copyright 2026 The Flux authors + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package controller + +import ( + "testing" + "time" + + "github.com/go-git/go-git/v5/plumbing" + "github.com/go-git/go-git/v5/plumbing/object" + "github.com/go-git/go-git/v5/plumbing/storer" + "github.com/go-git/go-git/v5/storage/memory" + . "github.com/onsi/gomega" + + sourcev1 "github.com/fluxcd/source-controller/api/v1" +) + +// storeCommit stores a commit object with the given parents and returns its +// hash. The tree is irrelevant for history traversal and left empty. +func storeCommit(t *testing.T, s storer.EncodedObjectStorer, message string, parents ...plumbing.Hash) plumbing.Hash { + t.Helper() + sig := object.Signature{Name: "Jane Doe", Email: "jane@example.com", When: time.Unix(0, 0)} + o := s.NewEncodedObject() + if err := (&object.Commit{Author: sig, Committer: sig, Message: message, ParentHashes: parents}).Encode(o); err != nil { + t.Fatal(err) + } + h, err := s.SetEncodedObject(o) + if err != nil { + t.Fatal(err) + } + return h +} + +func Test_findAncestor(t *testing.T) { + s := memory.NewStorage() + + // a <- b <- c <- d, with side branch a <- s1 merged into m(d, s1). + a := storeCommit(t, s, "a") + b := storeCommit(t, s, "b", a) + c := storeCommit(t, s, "c", b) + d := storeCommit(t, s, "d", c) + s1 := storeCommit(t, s, "s1", a) + m := storeCommit(t, s, "m", d, s1) + // orphan has a parent that is not in the store. + orphan := storeCommit(t, s, "orphan", plumbing.NewHash("0123456789012345678901234567890123456789")) + + tests := []struct { + name string + shallow []plumbing.Hash + head plumbing.Hash + ancestor plumbing.Hash + limit int + want ancestryResult + }{ + {name: "same commit", head: d, ancestor: d, limit: 1, want: ancestryFound}, + {name: "linear ancestor", head: d, ancestor: a, limit: 10, want: ancestryFound}, + {name: "linear descendant is not an ancestor", head: a, ancestor: d, limit: 10, want: ancestryNotFound}, + {name: "unrelated commit", head: d, ancestor: s1, limit: 10, want: ancestryNotFound}, + {name: "ancestor through the first parent of a merge", head: m, ancestor: c, limit: 10, want: ancestryFound}, + {name: "ancestor through the second parent of a merge", head: m, ancestor: s1, limit: 10, want: ancestryFound}, + {name: "shallow boundary before the ancestor", shallow: []plumbing.Hash{b}, head: d, ancestor: a, limit: 10, want: ancestryTruncated}, + {name: "shallow boundary is the ancestor", shallow: []plumbing.Hash{b}, head: d, ancestor: b, limit: 10, want: ancestryFound}, + {name: "shallow boundary on one parent chain of a merge", shallow: []plumbing.Hash{c}, head: m, ancestor: s1, limit: 10, want: ancestryFound}, + {name: "shallow boundary on both parent chains of a merge", shallow: []plumbing.Hash{c, s1}, head: m, ancestor: a, limit: 10, want: ancestryTruncated}, + {name: "missing parent object", head: orphan, ancestor: a, limit: 10, want: ancestryTruncated}, + {name: "visit limit reached", head: d, ancestor: a, limit: 2, want: ancestryTruncated}, + {name: "visit limit allows the ancestor", head: d, ancestor: a, limit: 3, want: ancestryFound}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + g := NewWithT(t) + got, err := findAncestor(s, tt.shallow, tt.head, tt.ancestor, tt.limit) + g.Expect(err).ToNot(HaveOccurred()) + g.Expect(got).To(Equal(tt.want)) + }) + } +} + +func Test_cloneDepthApplies(t *testing.T) { + g := NewWithT(t) + g.Expect(cloneDepthApplies(nil)).To(BeTrue()) + g.Expect(cloneDepthApplies(&sourcev1.GitRepositoryRef{Branch: "main"})).To(BeTrue()) + g.Expect(cloneDepthApplies(&sourcev1.GitRepositoryRef{Name: "refs/tags/v1.0.0^{}"})).To(BeTrue()) + g.Expect(cloneDepthApplies(&sourcev1.GitRepositoryRef{Name: "refs/pull/420/head"})).To(BeFalse()) + g.Expect(cloneDepthApplies(&sourcev1.GitRepositoryRef{Branch: "main", Commit: "abc"})).To(BeFalse()) +}