gitrepo: add .status.observedVerificationMode to track Git verification

Add `.status.observedVerificationMode` to record the mode used to verify
the Git object(s) and use that to determine whether we need to re-verify
and trigger a full reconciliation.

Signed-off-by: Sanskar Jaiswal <jaiswalsanskar078@gmail.com>

api/v1/gitrepository_types.go

@@ -246,6 +246,10 @@ type GitRepositoryStatus struct {
 	// +optional
 	ObservedInclude []GitRepositoryInclude `json:"observedInclude,omitempty"`
 
+	// ObservedVerificationMode is the observed mode indicating which Git
+	// object(s) have been verified.
+	ObservedVerificationMode *GitVerificationMode `json:"observedVerificationMode,omitempty"`
+
 	meta.ReconcileRequestStatus `json:",inline"`
 }
 

config/crd/bases/source.toolkit.fluxcd.io_gitrepositories.yaml

@@ -410,6 +410,10 @@ spec:
                 description: ObservedRecurseSubmodules is the observed resource submodules
                   configuration used to produce the current Artifact.
                 type: boolean
+              observedVerificationMode:
+                description: ObservedVerificationMode is the observed mode indicating
+                  which Git object(s) have been verified.
+                type: string
             type: object
         type: object
     served: true

docs/api/v1/source.md

@@ -796,6 +796,20 @@ produce the current Artifact.</p>
 </tr>
 <tr>
 <td>
+<code>observedVerificationMode</code><br>
+<em>
+<a href="#source.toolkit.fluxcd.io/v1.GitVerificationMode">
+GitVerificationMode
+</a>
+</em>
+</td>
+<td>
+<p>ObservedVerificationMode is the observed mode indicating which Git
+object(s) have been verified.</p>
+</td>
+</tr>
+<tr>
+<td>
 <code>ReconcileRequestStatus</code><br>
 <em>
 <a href="https://pkg.go.dev/github.com/fluxcd/pkg/apis/meta#ReconcileRequestStatus">
@@ -870,6 +884,7 @@ authors.</p>
 (<code>string</code> alias)</h3>
 <p>
 (<em>Appears on:</em>
+<a href="#source.toolkit.fluxcd.io/v1.GitRepositoryStatus">GitRepositoryStatus</a>, 
 <a href="#source.toolkit.fluxcd.io/v1.GitRepositoryVerification">GitRepositoryVerification</a>)
 </p>
 <p>GitVerificationMode specifies the verification mode for a Git repository.</p>

go.mod

@@ -14,9 +14,9 @@ replace github.com/opencontainers/go-digest => github.com/opencontainers/go-dige
 // Check again when oras.land/oras-go is updated, which is a dependency of Helm.
 replace github.com/docker/docker => github.com/docker/docker v23.0.6+incompatible
 
-replace github.com/fluxcd/pkg/git => github.com/fluxcd/pkg/git v0.12.4-0.20230731130338-0830185ed818
+replace github.com/fluxcd/pkg/git => github.com/fluxcd/pkg/git v0.12.5-0.20230810075312-c35892e27d72
 
-replace github.com/fluxcd/pkg/git/gogit => github.com/fluxcd/pkg/git/gogit v0.12.2-0.20230731130338-0830185ed818
+replace github.com/fluxcd/pkg/git/gogit => github.com/fluxcd/pkg/git/gogit v0.12.2-0.20230810075312-c35892e27d72
 
 require (
 	cloud.google.com/go/storage v1.31.0
@@ -32,7 +32,7 @@ require (
 	github.com/fluxcd/pkg/apis/event v0.5.2
 	github.com/fluxcd/pkg/apis/meta v1.1.2
 	github.com/fluxcd/pkg/git v0.12.4
-	github.com/fluxcd/pkg/git/gogit v0.12.1
+	github.com/fluxcd/pkg/git/gogit v0.12.2-0.20230810075312-c35892e27d72
 	github.com/fluxcd/pkg/gittestserver v0.8.5
 	github.com/fluxcd/pkg/helmtestserver v0.13.2
 	github.com/fluxcd/pkg/lockedfile v0.1.0

go.sum

@@ -354,7 +354,7 @@ github.com/docker/libtrust v0.0.0-20150114040149-fa567046d9b1/go.mod h1:cyGadeNE
 github.com/docopt/docopt-go v0.0.0-20180111231733-ee0de3bc6815/go.mod h1:WwZ+bS3ebgob9U8Nd0kOddGdZWjyMGR8Wziv+TBNwSE=
 github.com/dustin/go-humanize v1.0.1 h1:GzkhY7T5VNhEkwH0PVJgjz+fX1rhBrR7pRT3mDkpeCY=
 github.com/dustin/go-humanize v1.0.1/go.mod h1:Mu1zIs6XwVuF/gI1OepvI0qD18qycQx+mFykh5fBlto=
-github.com/elazarl/goproxy v0.0.0-20221015165544-a0805db90819 h1:RIB4cRk+lBqKK3Oy0r2gRX4ui7tuhiZq2SuTtTCi0/0=
+github.com/elazarl/goproxy v0.0.0-20230731152917-f99041a5c027 h1:1L0aalTpPz7YlMxETKpmQoWMBkeiuorElZIXoNmgiPE=
 github.com/emicklei/go-restful/v3 v3.10.2 h1:hIovbnmBTLjHXkqEBUz3HGpXZdM7ZrE9fJIZIqlJLqE=
 github.com/emicklei/go-restful/v3 v3.10.2/go.mod h1:6n3XBCmQQb25CM2LCACGz8ukIrRry+4bhvbpWn3mrbc=
 github.com/emirpasic/gods v1.18.1 h1:FXtiHYKDGKCW2KzwZKx0iC0PQmdlorYgdFG9jPXJ1Bc=
@@ -393,10 +393,10 @@ github.com/fluxcd/pkg/apis/event v0.5.2 h1:WtnCOeWglf7wR3dpyiWxb1JtYkw1G5OXcERb1
 github.com/fluxcd/pkg/apis/event v0.5.2/go.mod h1:5l6SSxVTkqrXrYjgEqAajOOHkl4x0TPocAuSdu+3AEs=
 github.com/fluxcd/pkg/apis/meta v1.1.2 h1:Unjo7hxadtB2dvGpeFqZZUdsjpRA08YYSBb7dF2WIAM=
 github.com/fluxcd/pkg/apis/meta v1.1.2/go.mod h1:BHQyRHCskGMEDf6kDGbgQ+cyiNpUHbLsCOsaMYM2maI=
-github.com/fluxcd/pkg/git v0.12.4-0.20230731130338-0830185ed818 h1:5RiT/G6Lmbg5eRHh4cjTM9t2v/txc5Yx5rs7vDe3dVk=
-github.com/fluxcd/pkg/git v0.12.4-0.20230731130338-0830185ed818/go.mod h1:VBPwN/pjvkqWmGUzou53fAQDT2tRJAUJ45SC7TyW5TI=
-github.com/fluxcd/pkg/git/gogit v0.12.2-0.20230731130338-0830185ed818 h1:G01A3JDlqrh6JzK/5ydFtdIQE7Nkfaw8+WEshMwMLZQ=
-github.com/fluxcd/pkg/git/gogit v0.12.2-0.20230731130338-0830185ed818/go.mod h1:Z4Ysp8VifKTvWpjJMKncJsgb2iBqHuIeK80VGjlU41Y=
+github.com/fluxcd/pkg/git v0.12.5-0.20230810075312-c35892e27d72 h1:NGeVmHjeSlb2q0jo8QYKJfuHd0mkoHz8lHC+E1NG6Y8=
+github.com/fluxcd/pkg/git v0.12.5-0.20230810075312-c35892e27d72/go.mod h1:rKB1puk7sbC4AYF1oZDBrkvu3cr0aibkd4I5yNbxSQg=
+github.com/fluxcd/pkg/git/gogit v0.12.2-0.20230810075312-c35892e27d72 h1:bfk1G3hynmx4rYN/s1A8hvbDK/QS3BGxXimGZpOZSt8=
+github.com/fluxcd/pkg/git/gogit v0.12.2-0.20230810075312-c35892e27d72/go.mod h1:cS/hPj9E2cONT2bI9RD97mmtWnIsusSBuZ+HXI64be4=
 github.com/fluxcd/pkg/gittestserver v0.8.5 h1:EGqDF4240xPRgW1FFrQAs0Du7fZb8OGXC5qKDIqyXD8=
 github.com/fluxcd/pkg/gittestserver v0.8.5/go.mod h1:SyGEh+OBzFpdlTWWqv3XBkiLB42Iu+mijfIQ4hPlEZQ=
 github.com/fluxcd/pkg/helmtestserver v0.13.2 h1:Wypmc8kr9UrUwB32v2InK8oRDb9tGaixATAXqaZFurI=

internal/controller/gitrepository_controller.go

@@ -552,12 +552,8 @@ func (r *GitRepositoryReconciler) reconcileSource(ctx context.Context, sp *patch
 	// 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) {
-		// If we need to verify a Git object that we previously haven't verified, then
-		// we need to do a full reconciliation.
-		verificationRequired := requiresVerification(obj)
-
 		// Check if the content config contributing to the artifact has changed.
-		if !gitContentConfigChanged(obj, includes) && !verificationRequired {
+		if !gitContentConfigChanged(obj, includes) {
 			ge := serror.NewGeneric(
 				fmt.Errorf("no changes since last reconcilation: observed revision '%s'",
 					commitReference(obj, commit)), sourcev1.GitOperationSucceedReason,
@@ -590,7 +586,7 @@ func (r *GitRepositoryReconciler) reconcileSource(ctx context.Context, sp *patch
 	conditions.Delete(obj, sourcev1.FetchFailedCondition)
 
 	// Verify commit signature
-	if result, err := r.verifyCommitSignature(ctx, obj, *commit); err != nil || result == sreconcile.ResultEmpty {
+	if result, err := r.verifySignature(ctx, obj, *commit); err != nil || result == sreconcile.ResultEmpty {
 		return result, err
 	}
 
@@ -927,14 +923,14 @@ func (r *GitRepositoryReconciler) fetchIncludes(ctx context.Context, obj *source
 	return &artifacts, nil
 }
 
-// verifyCommitSignature verifies the signature of the given Git commit and/or its parent tag
+// verifySignature verifies the signature of the given Git commit and/or its parent tag
 // depedning on the verification mode specified on the object.
 // If the signature can not be verified or the verification fails, it records
 // v1beta2.SourceVerifiedCondition=False and returns.
 // When successful, it records v1beta2.SourceVerifiedCondition=True.
 // If no verification mode is specified on the object, the
 // v1beta2.SourceVerifiedCondition Condition is removed.
-func (r *GitRepositoryReconciler) verifyCommitSignature(ctx context.Context, obj *sourcev1.GitRepository, commit git.Commit) (sreconcile.Result, error) {
+func (r *GitRepositoryReconciler) verifySignature(ctx context.Context, obj *sourcev1.GitRepository, commit git.Commit) (sreconcile.Result, error) {
 	// Check if there is a commit verification is configured and remove any old
 	// observations if there is none
 	if obj.Spec.Verification == nil || obj.Spec.Verification.Mode == "" {
@@ -975,12 +971,20 @@ func (r *GitRepositoryReconciler) verifyCommitSignature(ctx context.Context, obj
 		verifyTag = true
 	}
 
-	if verifyTag && obj.Spec.Reference.Tag == "" {
-		err := errors.New("cannot verify tag object's signature if a tag reference is not specified")
-		conditions.MarkStalled(obj, "InvalidVerificationMode", err.Error())
+	// check if a refname points to a tag, since that can also be used to
+	// fetch and verify the tag object.
+	isRefnameTag := func(name string) bool {
+		return strings.HasPrefix(name, "refs/tags") && !strings.HasSuffix(name, "^{}")
+	}
+	if verifyTag && ((obj.Spec.Reference == nil) || (obj.Spec.Reference.Tag == "" && obj.Spec.Reference.SemVer == "" &&
+		!isRefnameTag(obj.Spec.Reference.Name))) {
+		err := serror.NewStalling(
+			errors.New("cannot verify tag object's signature if a tag reference is not specified"),
+			"InvalidVerificationMode",
+		)
+		conditions.MarkFalse(obj, sourcev1.SourceVerifiedCondition, err.Reason, err.Err.Error())
 		return sreconcile.ResultEmpty, err
 	}
-	conditions.Delete(obj, meta.StalledCondition)
 
 	var message strings.Builder
 	var reason string
@@ -998,6 +1002,7 @@ func (r *GitRepositoryReconciler) verifyCommitSignature(ctx context.Context, obj
 		}
 		message.WriteString(fmt.Sprintf("verified signature of commit '%s' with key '%s'", commit.Hash.String(), headEntity))
 		reason = sourcev1.VerifiedCommitReason
+		obj.Status.ObservedVerificationMode = ptrToVerificationMode(sourcev1.GitVerificationMode(sourcev1.VerifiedCommitReason))
 	}
 
 	if verifyTag {
@@ -1017,9 +1022,11 @@ func (r *GitRepositoryReconciler) verifyCommitSignature(ctx context.Context, obj
 			if verifyHead {
 				message.WriteString(fmt.Sprintf(" and signature of tag '%s' with key '%s'", tag.String(), tagEntity))
 				reason = sourcev1.VerifiedTagAndCommitReason
+				obj.Status.ObservedVerificationMode = ptrToVerificationMode(sourcev1.GitVerificationMode(sourcev1.VerifiedTagAndCommitReason))
 			} else {
 				message.WriteString(fmt.Sprintf("verified signature of tag '%s' with key '%s'", tag.String(), tagEntity))
 				reason = sourcev1.VerifiedTagReason
+				obj.Status.ObservedVerificationMode = ptrToVerificationMode(sourcev1.GitVerificationMode(sourcev1.VerifiedTagReason))
 			}
 		}
 	}
@@ -1100,7 +1107,8 @@ func (r *GitRepositoryReconciler) eventLogf(ctx context.Context, obj runtime.Obj
 
 // gitContentConfigChanged evaluates the current spec with the observations of
 // the artifact in the status to determine if artifact content configuration has
-// changed and requires rebuilding the artifact.
+// changed and requires rebuilding the artifact. Rebuilding the artifact is also
+// required if the object needs to be (re)verified.
 func gitContentConfigChanged(obj *sourcev1.GitRepository, includes *artifactSet) bool {
 	if !pointer.StringEqual(obj.Spec.Ignore, obj.Status.ObservedIgnore) {
 		return true
@@ -1111,6 +1119,9 @@ func gitContentConfigChanged(obj *sourcev1.GitRepository, includes *artifactSet)
 	if len(obj.Spec.Include) != len(obj.Status.ObservedInclude) {
 		return true
 	}
+	if requiresVerification(obj) {
+		return true
+	}
 
 	// Convert artifactSet to index addressable artifacts and ensure that it and
 	// the included artifacts include all the include from the spec.
@@ -1171,25 +1182,29 @@ func commitReference(obj *sourcev1.GitRepository, commit *git.Commit) string {
 // first checking if the GitRepository has a verification spec. If it does, then
 // it returns true based on the following three conditions:
 //
-//   - If the object does not have a SourceVerifiedCondition in its status.
-//   - If the current reason for the SourceVerifiedCondition indicates that only the
-//     the tag has been verified earlier and the HEAD also needs to be verified now.
-//   - If the current reason for the SourceVerifiedCondition indicates that only the
-//     the HEAD has been verified earlier and the tag also needs to be verified now.
+//   - If the object does not have a observed verification mode in its status.
+//   - If the observed verification mode indicates that only the tag had been
+//     verified earlier and the HEAD also needs to be verified now.
+//   - If the observed verification mode indicates that only the HEAD had been
+//     verified earlier and the tag also needs to be verified now.
 func requiresVerification(obj *sourcev1.GitRepository) bool {
 	var verificationRequired bool
 	if obj.Spec.Verification != nil {
-		sourceVerifiedCondition := conditions.Get(obj, sourcev1.SourceVerifiedCondition)
+		observedVerificationMode := obj.Status.ObservedVerificationMode
 		mode := obj.Spec.Verification.GetMode()
-		if sourceVerifiedCondition == nil {
+		if observedVerificationMode == nil {
 			verificationRequired = true
-		} else if sourceVerifiedCondition.Reason == sourcev1.VerifiedTagReason &&
+		} else if *observedVerificationMode == sourcev1.ModeTag &&
 			(mode == sourcev1.ModeHEAD || mode == sourcev1.ModeTagAndHEAD) {
 			verificationRequired = true
-		} else if sourceVerifiedCondition.Reason == sourcev1.VerifiedCommitReason &&
+		} else if *observedVerificationMode == sourcev1.ModeHEAD &&
 			(mode == sourcev1.ModeTag || mode == sourcev1.ModeTagAndHEAD) {
 			verificationRequired = true
 		}
 	}
 	return verificationRequired
 }
+
+func ptrToVerificationMode(mode sourcev1.GitVerificationMode) *sourcev1.GitVerificationMode {
+	return &mode
+}