diff --git a/api/v1/gitrepository_types.go b/api/v1/gitrepository_types.go index 28a610c80..838e77a1a 100644 --- a/api/v1/gitrepository_types.go +++ b/api/v1/gitrepository_types.go @@ -38,6 +38,31 @@ const ( IncludeUnavailableCondition string = "IncludeUnavailable" ) +// GitVerificationMode specifies the verification mode for a Git repository. +type GitVerificationMode string + +// Valid checks the validity of the Git verification mode. +func (m GitVerificationMode) Valid() bool { + switch m { + case ModeGitHEAD, ModeGitTag, ModeGitTagAndHEAD: + return true + default: + return false + } +} + +const ( + // ModeGitHEAD implies that the HEAD of the Git repository (after it has been + // checked out to the required commit) should be verified. + ModeGitHEAD GitVerificationMode = "HEAD" + // ModeGitTag implies that the tag object specified in the checkout configuration + // should be verified. + ModeGitTag GitVerificationMode = "Tag" + // ModeGitTagAndHEAD implies that both the tag object and the commit it points + // to should be verified. + ModeGitTagAndHEAD GitVerificationMode = "TagAndHEAD" +) + // GitRepositorySpec specifies the required configuration to produce an // Artifact for a Git repository. type GitRepositorySpec struct { @@ -172,9 +197,15 @@ type GitRepositoryRef struct { // GitRepositoryVerification specifies the Git commit signature verification // strategy. type GitRepositoryVerification struct { - // Mode specifies what Git object should be verified, currently ('head'). - // +kubebuilder:validation:Enum=head - Mode string `json:"mode"` + // Mode specifies which Git object(s) should be verified. + // + // The variants "head" and "HEAD" both imply the same thing, i.e. verify + // the commit that the HEAD of the Git repository points to. The variant + // "head" solely exists to ensure backwards compatibility. + // +kubebuilder:validation:Enum=head;HEAD;Tag;TagAndHEAD + // +optional + // +kubebuilder:default:=HEAD + Mode GitVerificationMode `json:"mode,omitempty"` // SecretRef specifies the Secret containing the public keys of trusted Git // authors. @@ -217,6 +248,11 @@ type GitRepositoryStatus struct { // +optional ObservedInclude []GitRepositoryInclude `json:"observedInclude,omitempty"` + // SourceVerificationMode is the last used verification mode indicating + // which Git object(s) have been verified. + // +optional + SourceVerificationMode *GitVerificationMode `json:"sourceVerificationMode,omitempty"` + meta.ReconcileRequestStatus `json:",inline"` } @@ -252,6 +288,26 @@ func (in *GitRepository) GetArtifact() *Artifact { return in.Status.Artifact } +// GetMode returns the declared GitVerificationMode, or a ModeGitHEAD default. +func (v *GitRepositoryVerification) GetMode() GitVerificationMode { + if v.Mode.Valid() { + return v.Mode + } + return ModeGitHEAD +} + +// VerifyHEAD returns if the configured mode instructs verification of the +// Git HEAD. +func (v *GitRepositoryVerification) VerifyHEAD() bool { + return v.GetMode() == ModeGitHEAD || v.GetMode() == ModeGitTagAndHEAD +} + +// VerifyTag returns if the configured mode instructs verification of the +// Git tag. +func (v *GitRepositoryVerification) VerifyTag() bool { + return v.GetMode() == ModeGitTag || v.GetMode() == ModeGitTagAndHEAD +} + // +genclient // +genclient:Namespaced // +kubebuilder:storageversion diff --git a/api/v1/zz_generated.deepcopy.go b/api/v1/zz_generated.deepcopy.go index 23630ff9f..8167c7136 100644 --- a/api/v1/zz_generated.deepcopy.go +++ b/api/v1/zz_generated.deepcopy.go @@ -232,6 +232,11 @@ func (in *GitRepositoryStatus) DeepCopyInto(out *GitRepositoryStatus) { *out = make([]GitRepositoryInclude, len(*in)) copy(*out, *in) } + if in.SourceVerificationMode != nil { + in, out := &in.SourceVerificationMode, &out.SourceVerificationMode + *out = new(GitVerificationMode) + **out = **in + } out.ReconcileRequestStatus = in.ReconcileRequestStatus } diff --git a/config/crd/bases/source.toolkit.fluxcd.io_gitrepositories.yaml b/config/crd/bases/source.toolkit.fluxcd.io_gitrepositories.yaml index 223787998..c06124009 100644 --- a/config/crd/bases/source.toolkit.fluxcd.io_gitrepositories.yaml +++ b/config/crd/bases/source.toolkit.fluxcd.io_gitrepositories.yaml @@ -168,10 +168,16 @@ spec: Git commit signature(s). properties: mode: - description: Mode specifies what Git object should be verified, - currently ('head'). + default: HEAD + description: "Mode specifies which Git object(s) should be verified. + \n The variants \"head\" and \"HEAD\" both imply the same thing, + i.e. verify the commit that the HEAD of the Git repository points + to. The variant \"head\" solely exists to ensure backwards compatibility." enum: - head + - HEAD + - Tag + - TagAndHEAD type: string secretRef: description: SecretRef specifies the Secret containing the public @@ -184,7 +190,6 @@ spec: - name type: object required: - - mode - secretRef type: object required: @@ -407,6 +412,10 @@ spec: description: ObservedRecurseSubmodules is the observed resource submodules configuration used to produce the current Artifact. type: boolean + sourceVerificationMode: + description: SourceVerificationMode is the last used verification + mode indicating which Git object(s) have been verified. + type: string type: object type: object served: true diff --git a/docs/api/v1/source.md b/docs/api/v1/source.md index f4ccd92c8..ff34c7e60 100644 --- a/docs/api/v1/source.md +++ b/docs/api/v1/source.md @@ -800,6 +800,21 @@ produce the current Artifact.

+sourceVerificationMode
+ + +GitVerificationMode + + + + +(Optional) +

SourceVerificationMode is the last used verification mode indicating +which Git object(s) have been verified.

+ + + + ReconcileRequestStatus
@@ -839,11 +854,17 @@ strategy.

mode
-string +
+GitVerificationMode + -

Mode specifies what Git object should be verified, currently (‘head’).

+(Optional) +

Mode specifies which Git object(s) should be verified.

+

The variants “head” and “HEAD” both imply the same thing, i.e. verify +the commit that the HEAD of the Git repository points to. The variant +“head” solely exists to ensure backwards compatibility.

@@ -864,6 +885,14 @@ authors.

+

GitVerificationMode +(string alias)

+

+(Appears on: +GitRepositoryStatus, +GitRepositoryVerification) +

+

GitVerificationMode specifies the verification mode for a Git repository.

Source

Source interface must be supported by all API types. diff --git a/docs/spec/v1/gitrepositories.md b/docs/spec/v1/gitrepositories.md index a5e4f74bf..066f765fb 100644 --- a/docs/spec/v1/gitrepositories.md +++ b/docs/spec/v1/gitrepositories.md @@ -366,8 +366,17 @@ spec: `.spec.verify` is an optional field to enable the verification of Git commit signatures. The field offers two subfields: -- `.mode`, to specify what Git commit object should be verified. Only supports - `head` at present. +- `.mode`, to specify what Git object(s) should be verified. Supported + values are: + - `HEAD`: Verifies the commit object pointed to by the HEAD of the repository + after performing a checkout via `.spec.ref`. + - `head`: Same as `HEAD`, supported for backwards compatibility purposes. + - `Tag`: Verifies the tag object pointed to by the specified/inferred tag + reference in `.spec.ref.tag`, `.spec.ref.semver` or `.spec.ref.name`. + - `TagAndHEAD`: Verifies the tag object pointed to by the specified/inferred tag + reference in `.spec.ref.tag`, `.spec.ref.semver` or `.spec.ref.name` and + the commit object pointed to by the tag. + - `.secretRef.name`, to specify a reference to a Secret in the same namespace as the GitRepository. Containing the (PGP) public keys of trusted Git authors. @@ -384,7 +393,7 @@ spec: ref: branch: master verify: - mode: head + mode: HEAD secretRef: name: pgp-public-keys ``` @@ -978,6 +987,15 @@ status: ... ``` +### Source Verification Mode + +The source-controller reports the Git object(s) it verified in the Git +repository to create an artifact in the GitRepository's +`.status.sourceVerificationMode`. This value is the same as the [verification +mode in spec](#verification). The verification status is applicable only to the +latest Git repository revision used to successfully build and store an +artifact. + ### Observed Generation The source-controller reports an [observed generation][typical-status-properties] diff --git a/go.mod b/go.mod index 78cac9575..d454ab8f4 100644 --- a/go.mod +++ b/go.mod @@ -27,16 +27,16 @@ require ( github.com/docker/go-units v0.5.0 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/gittestserver v0.8.5 + github.com/fluxcd/pkg/git v0.13.0 + github.com/fluxcd/pkg/git/gogit v0.13.0 + github.com/fluxcd/pkg/gittestserver v0.8.6 github.com/fluxcd/pkg/helmtestserver v0.13.2 github.com/fluxcd/pkg/lockedfile v0.1.0 github.com/fluxcd/pkg/masktoken v0.2.0 github.com/fluxcd/pkg/oci v0.30.1 github.com/fluxcd/pkg/runtime v0.42.0 github.com/fluxcd/pkg/sourceignore v0.3.5 - github.com/fluxcd/pkg/ssh v0.8.1 + github.com/fluxcd/pkg/ssh v0.8.2 github.com/fluxcd/pkg/tar v0.2.0 github.com/fluxcd/pkg/testserver v0.4.0 github.com/fluxcd/pkg/version v0.2.2 diff --git a/go.sum b/go.sum index 9210fcc57..1740c5c72 100644 --- a/go.sum +++ b/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,12 +393,12 @@ 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 h1:COuVYUL+gqMOYAm6oD32Vwcmy/8WVsT/nMk8ps0lpJI= -github.com/fluxcd/pkg/git v0.12.4/go.mod h1:rKB1puk7sbC4AYF1oZDBrkvu3cr0aibkd4I5yNbxSQg= -github.com/fluxcd/pkg/git/gogit v0.12.1 h1:06jzHOTntYN5xCSQvyFXtLXdqoP8crLh7VYgtXS9+wo= -github.com/fluxcd/pkg/git/gogit v0.12.1/go.mod h1:Z4Ysp8VifKTvWpjJMKncJsgb2iBqHuIeK80VGjlU41Y= -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/git v0.13.0 h1:GcJfldYqw6ELf0vbTCV+iFZgSpK6HZBKx3yAvn1Dqfg= +github.com/fluxcd/pkg/git v0.13.0/go.mod h1:rKB1puk7sbC4AYF1oZDBrkvu3cr0aibkd4I5yNbxSQg= +github.com/fluxcd/pkg/git/gogit v0.13.0 h1:XCwfiB5qbz08djUgo0TII09zibH97Hn56v098pkFpns= +github.com/fluxcd/pkg/git/gogit v0.13.0/go.mod h1:V3g+UyIDSAOysg5KCpHhS+HXBUmNmmbNlVruWkpCJgY= +github.com/fluxcd/pkg/gittestserver v0.8.6 h1:YM8prVKB3LC9LBBe+a2p7l1BlfV9erXCgC1em9sbqW4= +github.com/fluxcd/pkg/gittestserver v0.8.6/go.mod h1:3abUQFRNlfBhn+BD+TI2lfXI/JkdntdQ99spSnItFk4= github.com/fluxcd/pkg/helmtestserver v0.13.2 h1:Wypmc8kr9UrUwB32v2InK8oRDb9tGaixATAXqaZFurI= github.com/fluxcd/pkg/helmtestserver v0.13.2/go.mod h1:Em5iCJ0FU7TgSS1jfOy2rwc0NnsFgz9BHB4QOo186wM= github.com/fluxcd/pkg/lockedfile v0.1.0 h1:YsYFAkd6wawMCcD74ikadAKXA4s2sukdxrn7w8RB5eo= @@ -411,8 +411,8 @@ github.com/fluxcd/pkg/runtime v0.42.0 h1:a5DQ/f90YjoHBmiXZUpnp4bDSLORjInbmqP7K11 github.com/fluxcd/pkg/runtime v0.42.0/go.mod h1:p6A3xWVV8cKLLQW0N90GehKgGMMmbNYv+OSJ/0qB0vg= github.com/fluxcd/pkg/sourceignore v0.3.5 h1:omcHTH5X5tlPr9w1b9T7WuJTOP+o/KdVdarYb4kgkCU= github.com/fluxcd/pkg/sourceignore v0.3.5/go.mod h1:6Xz3jErz8RsidsdrjUBBUGKes24rbdp/F38MnTGibEw= -github.com/fluxcd/pkg/ssh v0.8.1 h1:v35y7Ks/+ABWce8RcnrC7psVIhf3EdCUNFJi5+tYOps= -github.com/fluxcd/pkg/ssh v0.8.1/go.mod h1:M1ouDXuDG+QuhGB4JYEjCNCykNytLJGDhwKn9y4DEOE= +github.com/fluxcd/pkg/ssh v0.8.2 h1:WNfvTmnLnOUyXQDb8luSfmn1X0RIuhJBcKMFtKm6YsQ= +github.com/fluxcd/pkg/ssh v0.8.2/go.mod h1:ewbU9vakYYdGSX92qXhx6Kqi5tVQ3ppmGQakCX1R6Gw= github.com/fluxcd/pkg/tar v0.2.0 h1:HEUHgONQYsJGeZZ4x6h5nQU9Aox1I4T3bOp1faWTqf8= github.com/fluxcd/pkg/tar v0.2.0/go.mod h1:w0/TOC7kwBJhnSJn7TCABkc/I7ib1f2Yz6vOsbLBnhw= github.com/fluxcd/pkg/testserver v0.4.0 h1:pDZ3gistqYhwlf3sAjn1Q8NzN4Qe6I1BEmHMHi46lMg= diff --git a/internal/controller/gitrepository_controller.go b/internal/controller/gitrepository_controller.go index 219663df8..3dfa9c91e 100644 --- a/internal/controller/gitrepository_controller.go +++ b/internal/controller/gitrepository_controller.go @@ -587,7 +587,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 } @@ -924,17 +924,18 @@ func (r *GitRepositoryReconciler) fetchIncludes(ctx context.Context, obj *source return &artifacts, nil } -// verifyCommitSignature verifies the signature of the given Git commit, if a -// verification mode is specified on the object. +// verifySignature verifies the signature of the given Git commit and/or its referencing tag +// depending 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 == "" { + obj.Status.SourceVerificationMode = nil conditions.Delete(obj, sourcev1.SourceVerifiedCondition) return sreconcile.ResultSuccess, nil } @@ -958,22 +959,74 @@ func (r *GitRepositoryReconciler) verifyCommitSignature(ctx context.Context, obj for _, v := range secret.Data { keyRings = append(keyRings, string(v)) } - // Verify commit with GPG data from secret - entity, err := commit.Verify(keyRings...) - if err != nil { - e := serror.NewGeneric( - fmt.Errorf("signature verification of commit '%s' failed: %w", commit.Hash.String(), err), - "InvalidCommitSignature", - ) - conditions.MarkFalse(obj, sourcev1.SourceVerifiedCondition, e.Reason, e.Err.Error()) - // Return error in the hope the secret changes - return sreconcile.ResultEmpty, e + + var message strings.Builder + if obj.Spec.Verification.VerifyTag() { + // If we need to verify a tag object, then the commit must have a tag + // that points to it. If it does not, then its safe to asssume that + // the checkout didn't happen via a tag reference, thus the object can + // be marked as stalled. + tag := commit.ReferencingTag + if tag == nil { + 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 + } + if !git.IsSignedTag(*tag) { + // If the tag was not signed then we can't verify its signature + // but since the upstream tag object can change at any time, we can't + // mark the object as stalled. + err := serror.NewGeneric( + fmt.Errorf("cannot verify signature of tag '%s' since it is not signed", commit.ReferencingTag.String()), + "InvalidGitObject", + ) + conditions.MarkFalse(obj, sourcev1.SourceVerifiedCondition, err.Reason, err.Err.Error()) + return sreconcile.ResultEmpty, err + } + + // Verify tag with GPG data from secret + tagEntity, err := tag.Verify(keyRings...) + if err != nil { + e := serror.NewGeneric( + fmt.Errorf("signature verification of tag '%s' failed: %w", tag.String(), err), + "InvalidTagSignature", + ) + conditions.MarkFalse(obj, sourcev1.SourceVerifiedCondition, e.Reason, e.Err.Error()) + // Return error in the hope the secret changes + return sreconcile.ResultEmpty, e + } + + message.WriteString(fmt.Sprintf("verified signature of\n\t- tag '%s' with key '%s'", tag.String(), tagEntity)) } - conditions.MarkTrue(obj, sourcev1.SourceVerifiedCondition, meta.SucceededReason, - "verified signature of commit '%s' with key '%s'", commit.Hash.String(), entity) - r.eventLogf(ctx, obj, eventv1.EventTypeTrace, "VerifiedCommit", - "verified signature of commit '%s' with key '%s'", commit.Hash.String(), entity) + if obj.Spec.Verification.VerifyHEAD() { + // Verify commit with GPG data from secret + headEntity, err := commit.Verify(keyRings...) + if err != nil { + e := serror.NewGeneric( + fmt.Errorf("signature verification of commit '%s' failed: %w", commit.Hash.String(), err), + "InvalidCommitSignature", + ) + conditions.MarkFalse(obj, sourcev1.SourceVerifiedCondition, e.Reason, e.Err.Error()) + // Return error in the hope the secret changes + return sreconcile.ResultEmpty, e + } + // If we also verified the tag previously, then append to the message. + if message.Len() > 0 { + message.WriteString(fmt.Sprintf("\n\t- commit '%s' with key '%s'", commit.Hash.String(), headEntity)) + } else { + message.WriteString(fmt.Sprintf("verified signature of\n\t- commit '%s' with key '%s'", commit.Hash.String(), headEntity)) + } + } + + reason := meta.SucceededReason + mode := obj.Spec.Verification.GetMode() + obj.Status.SourceVerificationMode = &mode + conditions.MarkTrue(obj, sourcev1.SourceVerifiedCondition, reason, message.String()) + r.eventLogf(ctx, obj, eventv1.EventTypeTrace, reason, message.String()) return sreconcile.ResultSuccess, nil } @@ -1048,7 +1101,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 @@ -1059,6 +1113,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. @@ -1113,3 +1170,28 @@ func commitReference(obj *sourcev1.GitRepository, commit *git.Commit) string { } return commit.String() } + +// requiresVerification inspects a GitRepository's verification spec and its status +// to determine whether the Git repository needs to be verified again. It does so by +// 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 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 { + if obj.Spec.Verification != nil { + observedMode := obj.Status.SourceVerificationMode + mode := obj.Spec.Verification.GetMode() + if observedMode == nil { + return true + } + if (*observedMode == sourcev1.ModeGitTag && (mode == sourcev1.ModeGitHEAD || mode == sourcev1.ModeGitTagAndHEAD)) || + (*observedMode == sourcev1.ModeGitHEAD && (mode == sourcev1.ModeGitTag || mode == sourcev1.ModeGitTagAndHEAD)) { + return true + } + } + return false +} diff --git a/internal/controller/gitrepository_controller_test.go b/internal/controller/gitrepository_controller_test.go index a7740fe40..85c96dcd2 100644 --- a/internal/controller/gitrepository_controller_test.go +++ b/internal/controller/gitrepository_controller_test.go @@ -65,15 +65,20 @@ import ( ) const ( - encodedCommitFixture = `tree f0c522d8cc4c90b73e2bc719305a896e7e3c108a -parent eb167bc68d0a11530923b1f24b4978535d10b879 -author Stefan Prodan 1633681364 +0300 -committer Stefan Prodan 1633681364 +0300 + encodedCommitFixture = `tree 35f0b28987e60d4b8dec1f707fd07fef5ad84abc +parent 8b52742dbc848eb0975e62ae00fbfa4f8108e835 +author Sanskar Jaiswal 1691045123 +0530 +committer Sanskar Jaiswal 1691068951 +0530 -Update containerd and runc to fix CVEs +git/e2e: disable CGO while running e2e tests -Signed-off-by: Stefan Prodan +Disable CGO for Git e2e tests as it was originially required because of +our libgit2 client. Since we no longer maintain a libgit2 client, there +is no need to run the tests with CGO enabled. + +Signed-off-by: Sanskar Jaiswal ` + malformedEncodedCommitFixture = `parent eb167bc68d0a11530923b1f24b4978535d10b879 author Stefan Prodan 1633681364 +0300 committer Stefan Prodan 1633681364 +0300 @@ -84,62 +89,81 @@ Signed-off-by: Stefan Prodan ` signatureCommitFixture = `-----BEGIN PGP SIGNATURE----- -iHUEABEIAB0WIQQHgExUr4FrLdKzpNYyma6w5AhbrwUCYV//1AAKCRAyma6w5Ahb -r7nJAQCQU4zEJu04/Q0ac/UaL6htjhq/wTDNMeUM+aWG/LcBogEAqFUea1oR2BJQ -JCJmEtERFh39zNWSazQmxPAFhEE0kbc= -=+Wlj +iQIzBAABCAAdFiEEOxEY0f3iSZ5rKQ+vWYLQJ5wif/0FAmTLqnEACgkQWYLQJ5wi +f/1mYw/+LRttvfPrfYl7ASUBGYSQuDzjeold8OO1LpmwjrKPpX4ivZbXHh+lJF0F +fqudKuJfJzeQCHsMZjnfgvXHd2VvxPh1jX6h3JLuNu7d4g1DtNQsKJtsLx7JW99X +J9Bb1xj0Ghh2PkrWEB9vpw+uZz4IhFrB+DNNLRNBkon3etrS1q57q8dhQFIhLI1y +ij3rq3kFHjrNNdokIv2ujyVJtWgy2fK2ELW5v2dznpykOo7hQEKgtOIHPBzGBFT0 +dUFjB99Qy4Qgjh3vWaY4fZ3u/vhp3swmw91OlDkFeyndWjDSZhzYnb7wY+U6z35C +aU4Gzc71CquSd/nTdOEkpuolBVWV5cBkM+Nxi8jtVGBeDDFE49j27a3lQ3+qtT7/ +q4FCe5Jw3GSOJvaLBLGmYVn9fc49t/28b5tkGtCHs3ATpsJohzELEIiDP90Me7hQ +Joks3ML38T4J/zZ4/ObbVMkrCEATYe3r1Ep7+e6VmOG9iTg0JIexexddjHX26Tgu +iuVP2GD/8PceqgNW/LPX84Ub32WTKPZJg+NyliDjH5QOvmguK1dRtSb/9eyYcoSF +Fkf0HcgG5jOk0OZJv0QcqXd9PhB4oXeuXgGszo9M+fhr3nWvEooAJtIyLtVtt/u2 +rNNB7xkZ1uWx+52w9RG2gmZh+LaESwd1rNXgUFLNBebNN3jNzsA= +=73xf -----END PGP SIGNATURE-----` + + encodedTagFixture = `object 11525516bd55152ce68848bb14680aad43f18479 +type commit +tag v0.1.0 +tagger Sanskar Jaiswal 1691132850 +0530 + +v0.1.0 +` + + malformedEncodedTagFixture = `object 11525516bd55152ce68848bb14680aad43f18479 +tagger Sanskar Jaiswal 1691132850 +0530 + +v0.1.0 +` + + signatureTagFixture = `-----BEGIN PGP SIGNATURE----- + +iQIzBAABCAAdFiEEOxEY0f3iSZ5rKQ+vWYLQJ5wif/0FAmTMo7IACgkQWYLQJ5wi +f/1uUQ/9F70u8LZZQ3+U2vuYQ8fyVp/AV5h5zwxK5UlkR1crB0gSpdaiIxMMQRc8 +4QQIqCXloSHherUu9SPbDe9Qmr0JL8a57XqThjUSa52IYMDVos9sYwViJit+xGyz +HDot2nQ8MAqkDaiuwAnTqOyTPA89U36lGV/X/25mYxAuED+8xFx1OfvjGkX2eMEr +peWJ8VEfdFr2OmWwFceh6iF/izIaZGttwCyNy4BIh2W0GvUtQAxzqF4IzUvwfJU/ +bgARaHKQhWqFhDNImttsqJBweWavEDDmUgNg80c3cUZKqBtAjElToP9gis/SnPH5 +zaCAH66OzyKIhn6lde7KpOzyqbOyzddTa8SKkAAHyO7onukOktV8W9toeAxlF20q +Bw0MZGzAGisF8EK1HVv8UzrW9vAwdJN/yDIHWkjaeHr2FHmeV3a2QxH9PdwbE3tI +B21TCVULJuM8oR0ZG62xzg5ba5HiZMiilNMJdrBfjk5xYGk3LQU1gB4FVYa7yTsN +YfAokYtUIG187Qb8vPr1P95TzZxKdb7r/PAKEbGPro5D2Rri8OnxO/OaXG/giWS5 +5gRGmsQjvMsbzE/2PVc9+jshtZM49xL9H3DMjAWtO6MFbOqGqdi4MBa0T4qj6sZz +AbSLuRIBpXDES86faDXLRmufc95+iA/fh7W23G6vmd+SjXnCcHc= +=o4nf +-----END PGP SIGNATURE----- +` + armoredKeyRingFixture = `-----BEGIN PGP PUBLIC KEY BLOCK----- -mQSuBF9+HgMRDADKT8UBcSzpTi4JXt/ohhVW3x81AGFPrQvs6MYrcnNJfIkPTJD8 -mY5T7j1fkaN5wcf1wnxM9qTcW8BodkWNGEoEYOtVuigLSxPFqIncxK0PHvdU8ths -TEInBrgZv9t6xIVa4QngOEUd2D/aYni7M+75z7ntgj6eU1xLZ60upRFn05862OvJ -rZFUvzjsZXMAO3enCu2VhG/2axCY/5uI8PgWjyiKV2TH4LBJgzlb0v6SyI+fYf5K -Bg2WzDuLKvQBi9tFSwnUbQoFFlOeiGW8G/bdkoJDWeS1oYgSD3nkmvXvrVESCrbT -C05OtQOiDXjSpkLim81vNVPtI2XEug+9fEA+jeJakyGwwB+K8xqV3QILKCoWHKGx -yWcMHSR6cP9tdXCk2JHZBm1PLSJ8hIgMH/YwBJLYg90u8lLAs9WtpVBKkLplzzgm -B4Z4VxCC+xI1kt+3ZgYvYC+oUXJXrjyAzy+J1f+aWl2+S/79glWgl/xz2VibWMz6 -nZUE+wLMxOQqyOsBALsoE6z81y/7gfn4R/BziBASi1jq/r/wdboFYowmqd39DACX -+i+V0OplP2TN/F5JajzRgkrlq5cwZHinnw+IFwj9RTfOkdGb3YwhBt/h2PP38969 -ZG+y8muNtaIqih1pXj1fz9HRtsiCABN0j+JYpvV2D2xuLL7P1O0dt5BpJ3KqNCRw -mGgO2GLxbwvlulsLidCPxdK/M8g9Eeb/xwA5LVwvjVchHkzHuUT7durn7AT0RWiK -BT8iDfeBB9RKienAbWyybEqRaR6/Tv+mghFIalsDiBPbfm4rsNzsq3ohfByqECiy -yUvs2O3NDwkoaBDkA3GFyKv8/SVpcuL5OkVxAHNCIMhNzSgotQ3KLcQc0IREfFCa -3CsBAC7CsE2bJZ9IA9sbBa3jimVhWUQVudRWiLFeYHUF/hjhqS8IHyFwprjEOLaV -EG0kBO6ELypD/bOsmN9XZLPYyI3y9DM6Vo0KMomE+yK/By/ZMxVfex8/TZreUdhP -VdCLL95Rc4w9io8qFb2qGtYBij2wm0RWLcM0IhXWAtjI3B17IN+6hmv+JpiZccsM -AMNR5/RVdXIl0hzr8LROD0Xe4sTyZ+fm3mvpczoDPQNRrWpmI/9OT58itnVmZ5jM -7djV5y/NjBk63mlqYYfkfWto97wkhg0MnTnOhzdtzSiZQRzj+vf+ilLfIlLnuRr1 -JRV9Skv6xQltcFArx4JyfZCo7JB1ZXcbdFAvIXXS11RTErO0XVrXNm2RenpW/yZA -9f+ESQ/uUB6XNuyqVUnJDAFJFLdzx8sO3DXo7dhIlgpFqgQobUl+APpbU5LT95sm -89UrV0Lt9vh7k6zQtKOjEUhm+dErmuBnJo8MvchAuXLagHjvb58vYBCUxVxzt1KG -2IePwJ/oXIfawNEGad9Lmdo1FYG1u53AKWZmpYOTouu92O50FG2+7dBh0V2vO253 -aIGFRT1r14B1pkCIun7z7B/JELqOkmwmlRrUnxlADZEcQT3z/S8/4+2P7P6kXO7X -/TAX5xBhSqUbKe3DhJSOvf05/RVL5ULc2U2JFGLAtmBOFmnD/u0qoo5UvWliI+v/ -47QnU3RlZmFuIFByb2RhbiA8c3RlZmFuLnByb2RhbkBnbWFpbC5jb20+iJAEExEI -ADgWIQQHgExUr4FrLdKzpNYyma6w5AhbrwUCX34eAwIbAwULCQgHAgYVCgkICwIE -FgIDAQIeAQIXgAAKCRAyma6w5Ahbrzu/AP9l2YpRaWZr6wSQuEn0gMN8DRzsWJPx -pn0akdY7SRP3ngD9GoKgu41FAItnHAJ2KiHv/fHFyHMndNP3kPGPNW4BF+65Aw0E -X34eAxAMAMdYFCHmVA8TZxSTMBDpKYave8RiDCMMMjk26Gl0EPN9f2Y+s5++DhiQ -hojNH9VmJkFwZX1xppxe1y1aLa/U6fBAqMP/IdNH8270iv+A9YIxdsWLmpm99BDO -3suRfsHcOe9T0x/CwRfDNdGM/enGMhYGTgF4VD58DRDE6WntaBhl4JJa300NG6X0 -GM4Gh59DKWDnez/Shulj8demlWmakP5imCVoY+omOEc2k3nH02U+foqaGG5WxZZ+ -GwEPswm2sBxvn8nwjy9gbQwEtzNI7lWYiz36wCj2VS56Udqt+0eNg8WzocUT0XyI -moe1qm8YJQ6fxIzaC431DYi/mCDzgx4EV9ww33SXX3Yp2NL6PsdWJWw2QnoqSMpM -z5otw2KlMgUHkkXEKs0apmK4Hu2b6KD7/ydoQRFUqR38Gb0IZL1tOL6PnbCRUcig -Aypy016W/WMCjBfQ8qxIGTaj5agX2t28hbiURbxZkCkz+Z3OWkO0Rq3Y2hNAYM5s -eTn94JIGGwADBgv/dbSZ9LrBvdMwg8pAtdlLtQdjPiT1i9w5NZuQd7OuKhOxYTEB -NRDTgy4/DgeNThCeOkMB/UQQPtJ3Et45S2YRtnnuvfxgnlz7xlUn765/grtnRk4t -ONjMmb6tZos1FjIJecB/6h4RsvUd2egvtlpD/Z3YKr6MpNjWg4ji7m27e9pcJfP6 -YpTDrq9GamiHy9FS2F2pZlQxriPpVhjCLVn9tFGBIsXNxxn7SP4so6rJBmyHEAlq -iym9wl933e0FIgAw5C1vvprYu2amk+jmVBsJjjCmInW5q/kWAFnFaHBvk+v+/7tX -hywWUI7BqseikgUlkgJ6eU7E9z1DEyuS08x/cViDoNh2ntVUhpnluDu48pdqBvvY -a4uL/D+KI84THUAJ/vZy+q6G3BEb4hI9pFjgrdJpUKubxyZolmkCFZHjV34uOcTc -LQr28P8xW8vQbg5DpIsivxYLqDGXt3OyiItxvLMtw/ypt6PkoeP9A4KDST4StITE -1hrOrPtJ/VRmS2o0iHgEGBEIACAWIQQHgExUr4FrLdKzpNYyma6w5AhbrwUCX34e -AwIbDAAKCRAyma6w5Ahbr6QWAP9/pl2R6r1nuCnXzewSbnH1OLsXf32hFQAjaQ5o -Oomb3gD/TRf/nAdVED+k81GdLzciYdUGtI71/qI47G0nMBluLRE= -=/4e+ +mQINBGQmiZ0BEACwsubUFoWtp6iJDK9oUN4RhPS0bAKpcRTa7P/rTCD/MbTMYdWC +4vod3FMm4+rNF0SESxY67MGmR4M3dSyOZkCijqHm9jDVOvN847LOl5bntkm8Euxm +LkpfsBWng09+gtfwuKxOxPMY017D1jM23OGbrqznHaokerFeDp9sJf1C7Z9jVf39 +oB/MF0bMdUJuxFFBdpoI73DORlAVUI14mfDbFj7v02Spkv1hqS2LtJ/Jl4QR/Vw4 +mR71aFmGFWqLBlkUOjJ2SZGkCmF/qbUdLmVb7yZUtqtua4DVkBPTORfOMhGDbrME +Nmb6Ft5neZwU0ETsT/oc6Np+PDFSUDBxu0CbKG6bw7N2y8RfiVJTaoNLFoFGV5dA +K8OpyTxU4IEPDMpkWs7tpRxPCC02uCfyqlvdF4EURXYXTj54DDLOGQjoqB+iGtVi +y2dQ4cuNhfuIFCFTA16s41DwmB0fQuOg3yfPPo7+jUefD+iAt3CZ9Guvu5+/mGyq +KxSBBRFHc8ED/L7JLPMU6tZglaPch9P4H6Fi2swDryyZQn/a2kYanEh9v1wL94L4 +3gUdjIYP8kjfg7nnS2FX9hl5FtPeM3jvnWjfv9jR+c8HWQZY2wM3Rj5iulu70K2U +pkdRUN0p2D5+Kq6idNreNoPlpQGoUOYrtAfOwtDFgMwuOZ78XkSIbFhtgwARAQAB +tEVTYW5za2FyIEphaXN3YWwgKEdpdEh1YiBHUEcgc2lnaW5nIGtleSkgPGphaXN3 +YWxzYW5za2FyMDc4QGdtYWlsLmNvbT6JAk4EEwEIADgWIQQ7ERjR/eJJnmspD69Z +gtAnnCJ//QUCZCaJnQIbAwULCQgHAgYVCgkICwIEFgIDAQIeAQIXgAAKCRBZgtAn +nCJ//dF4D/0Tl5Wre6KrZvjDs5loulhN8YMYb63jr+x1eVkpMpta51XvZvkZFoiY +9T4MQX+qgAkTrUJsxgWUwtVtDfmbyLXodDRS6JUbCRiMu12VD7mNT+lUfuhR2sJv +rHZoolQp7X4DTea1R64PcttfmlGO2pUNpGNmhojO0PahXqOCHmEUWBJQhI8RvOcs +zRjEzDcAcEgtMGzamq6DR54YxyzGE8V9b5WD/elmEXM6uWW+CkfX8WskKbLdRY0t ++GQ1pOtf3tKxD46I3LIsUEwbyh4Dv4vJbZmyxjI+FKbSCW5tMrz/ZWrPNl0m+pDI +Yn0+GWed2pgTMFh3VAhYCyIVugKynlaToH+D2z3DnuEp3Jfs+b1BdirS/PW79tW7 +rjCJzqofF2UPyK0mzdYL+P3k9Hip5J0bCGoeMdCLsP5fYq3Y1YS4bH4JkDm52y+r +y89AH4LHHQt+A7w19I+6M2jmcNnDUMrpuSo84GeoM59O3fU7hLCC1Jx4hj7EBRrb +QzY5FInrE/WTcgFRljK46zhW4ybmfak/xJV654UqJCDWlVbc68D8JrKNQOj7gdPs +zh1+m2pFDEhWZkaFtQbSEpXMIJ9DsCoyQL4Knl+89VxHsrIyAJsmGb3V8xvtv5w9 +QuWtsDnYbvDHtTpu1NZChVrnr/l1k3C2fcLhV1s583AvhGMkbgSXkQ== +=Tdjz -----END PGP PUBLIC KEY BLOCK----- ` ) @@ -1515,18 +1539,50 @@ func TestGitRepositoryReconciler_reconcileDelete(t *testing.T) { g.Expect(obj.Status.Artifact).To(BeNil()) } -func TestGitRepositoryReconciler_verifyCommitSignature(t *testing.T) { +func TestGitRepositoryReconciler_verifySignature(t *testing.T) { tests := []struct { - name string - secret *corev1.Secret - commit git.Commit - beforeFunc func(obj *sourcev1.GitRepository) - want sreconcile.Result - wantErr bool - assertConditions []metav1.Condition + name string + secret *corev1.Secret + commit git.Commit + beforeFunc func(obj *sourcev1.GitRepository) + want sreconcile.Result + wantErr bool + err error + wantSourceVerificationMode *sourcev1.GitVerificationMode + assertConditions []metav1.Condition }{ { - name: "Valid commit makes SourceVerifiedCondition=True", + name: "Valid commit with mode=HEAD makes SourceVerifiedCondition=True", + secret: &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: "existing", + }, + Data: map[string][]byte{ + "foo": []byte(armoredKeyRingFixture), + }, + }, + commit: git.Commit{ + Hash: []byte("shasum"), + Encoded: []byte(encodedCommitFixture), + Signature: signatureCommitFixture, + }, + beforeFunc: func(obj *sourcev1.GitRepository) { + obj.Spec.Interval = metav1.Duration{Duration: interval} + obj.Spec.Verification = &sourcev1.GitRepositoryVerification{ + Mode: sourcev1.ModeGitHEAD, + SecretRef: meta.LocalObjectReference{ + Name: "existing", + }, + } + }, + want: sreconcile.ResultSuccess, + wantSourceVerificationMode: ptrToVerificationMode(sourcev1.ModeGitHEAD), + assertConditions: []metav1.Condition{ + *conditions.TrueCondition(sourcev1.SourceVerifiedCondition, meta.SucceededReason, "verified signature of\n\t- commit 'shasum' with key '5982D0279C227FFD'"), + }, + }, + { + name: "Valid commit with mode=head makes SourceVerifiedCondition=True", secret: &corev1.Secret{ ObjectMeta: metav1.ObjectMeta{ Name: "existing", @@ -1549,27 +1605,188 @@ func TestGitRepositoryReconciler_verifyCommitSignature(t *testing.T) { }, } }, + want: sreconcile.ResultSuccess, + wantSourceVerificationMode: ptrToVerificationMode(sourcev1.ModeGitHEAD), + assertConditions: []metav1.Condition{ + *conditions.TrueCondition(sourcev1.SourceVerifiedCondition, meta.SucceededReason, "verified signature of\n\t- commit 'shasum' with key '5982D0279C227FFD'"), + }, + }, + { + name: "Valid tag with mode=tag makes SourceVerifiedCondition=True", + secret: &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: "existing", + }, + Data: map[string][]byte{ + "foo": []byte(armoredKeyRingFixture), + }, + }, + commit: git.Commit{ + ReferencingTag: &git.Tag{ + Name: "v0.1.0", + Hash: []byte("shasum"), + Encoded: []byte(encodedTagFixture), + Signature: signatureTagFixture, + }, + }, + beforeFunc: func(obj *sourcev1.GitRepository) { + obj.Spec.Reference = &sourcev1.GitRepositoryRef{ + Tag: "v0.1.0", + } + obj.Spec.Interval = metav1.Duration{Duration: interval} + obj.Spec.Verification = &sourcev1.GitRepositoryVerification{ + Mode: sourcev1.ModeGitTag, + SecretRef: meta.LocalObjectReference{ + Name: "existing", + }, + } + }, + want: sreconcile.ResultSuccess, + wantSourceVerificationMode: ptrToVerificationMode(sourcev1.ModeGitTag), + assertConditions: []metav1.Condition{ + *conditions.TrueCondition(sourcev1.SourceVerifiedCondition, meta.SucceededReason, "verified signature of\n\t- tag 'v0.1.0@shasum' with key '5982D0279C227FFD'"), + }, + }, + { + name: "Valid tag and commit with mode=TagAndHEAD makes SourceVerifiedCondition=True", + secret: &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: "existing", + }, + Data: map[string][]byte{ + "foo": []byte(armoredKeyRingFixture), + }, + }, + commit: git.Commit{ + Hash: []byte("shasum"), + Encoded: []byte(encodedCommitFixture), + Signature: signatureCommitFixture, + ReferencingTag: &git.Tag{ + Name: "v0.1.0", + Hash: []byte("shasum"), + Encoded: []byte(encodedTagFixture), + Signature: signatureTagFixture, + }, + }, + beforeFunc: func(obj *sourcev1.GitRepository) { + obj.Spec.Reference = &sourcev1.GitRepositoryRef{ + Tag: "v0.1.0", + } + obj.Spec.Interval = metav1.Duration{Duration: interval} + obj.Spec.Verification = &sourcev1.GitRepositoryVerification{ + Mode: sourcev1.ModeGitTagAndHEAD, + SecretRef: meta.LocalObjectReference{ + Name: "existing", + }, + } + }, + want: sreconcile.ResultSuccess, + wantSourceVerificationMode: ptrToVerificationMode(sourcev1.ModeGitTagAndHEAD), + assertConditions: []metav1.Condition{ + *conditions.TrueCondition(sourcev1.SourceVerifiedCondition, meta.SucceededReason, "verified signature of\n\t- tag 'v0.1.0@shasum' with key '5982D0279C227FFD'\n\t- commit 'shasum' with key '5982D0279C227FFD'"), + }, + }, + { + name: "Source verification mode in status is unset if there's no verification in spec", + beforeFunc: func(obj *sourcev1.GitRepository) { + obj.Status.SourceVerificationMode = ptrToVerificationMode(sourcev1.ModeGitHEAD) + obj.Spec.Verification = nil + }, want: sreconcile.ResultSuccess, + }, + { + name: "Verification of tag with no tag ref SourceVerifiedCondition=False and returns a stalling error", + secret: &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: "existing", + }, + Data: map[string][]byte{ + "foo": []byte(armoredKeyRingFixture), + }, + }, + beforeFunc: func(obj *sourcev1.GitRepository) { + obj.Spec.Reference = &sourcev1.GitRepositoryRef{ + Branch: "main", + } + obj.Spec.Interval = metav1.Duration{Duration: interval} + obj.Spec.Verification = &sourcev1.GitRepositoryVerification{ + Mode: sourcev1.ModeGitTag, + SecretRef: meta.LocalObjectReference{ + Name: "existing", + }, + } + }, + wantErr: true, + err: serror.NewStalling( + errors.New("cannot verify tag object's signature if a tag reference is not specified"), + "InvalidVerificationMode", + ), + assertConditions: []metav1.Condition{ + *conditions.FalseCondition(sourcev1.SourceVerifiedCondition, "InvalidVerificationMode", "cannot verify tag object's signature if a tag reference is not specified"), + }, + }, + { + name: "Unsigned tag with mode=tag makes SourceVerifiedCondition=False", + secret: &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: "existing", + }, + Data: map[string][]byte{ + "foo": []byte(armoredKeyRingFixture), + }, + }, + commit: git.Commit{ + ReferencingTag: &git.Tag{ + Name: "v0.1.0", + Hash: []byte("shasum"), + Encoded: []byte(encodedTagFixture), + }, + }, + beforeFunc: func(obj *sourcev1.GitRepository) { + obj.Spec.Reference = &sourcev1.GitRepositoryRef{ + Tag: "v0.1.0", + } + obj.Spec.Interval = metav1.Duration{Duration: interval} + obj.Spec.Verification = &sourcev1.GitRepositoryVerification{ + Mode: sourcev1.ModeGitTag, + SecretRef: meta.LocalObjectReference{ + Name: "existing", + }, + } + }, + wantErr: true, assertConditions: []metav1.Condition{ - *conditions.TrueCondition(sourcev1.SourceVerifiedCondition, meta.SucceededReason, "verified signature of commit 'shasum' with key '3299AEB0E4085BAF'"), + *conditions.FalseCondition(sourcev1.SourceVerifiedCondition, "InvalidGitObject", "cannot verify signature of tag 'v0.1.0@shasum' since it is not signed"), }, }, { - name: "Invalid commit sets no SourceVerifiedCondition and returns error", + name: "Partially successful verification makes SourceVerifiedCondition=False", secret: &corev1.Secret{ ObjectMeta: metav1.ObjectMeta{ Name: "existing", }, + Data: map[string][]byte{ + "foo": []byte(armoredKeyRingFixture), + }, }, commit: git.Commit{ Hash: []byte("shasum"), Encoded: []byte(malformedEncodedCommitFixture), Signature: signatureCommitFixture, + ReferencingTag: &git.Tag{ + Name: "v0.1.0", + Hash: []byte("shasum"), + Encoded: []byte(encodedTagFixture), + Signature: signatureTagFixture, + }, }, beforeFunc: func(obj *sourcev1.GitRepository) { + obj.Spec.Reference = &sourcev1.GitRepositoryRef{ + Tag: "v0.1.0", + } obj.Spec.Interval = metav1.Duration{Duration: interval} obj.Spec.Verification = &sourcev1.GitRepositoryVerification{ - Mode: "head", + Mode: sourcev1.ModeGitTagAndHEAD, SecretRef: meta.LocalObjectReference{ Name: "existing", }, @@ -1577,15 +1794,70 @@ func TestGitRepositoryReconciler_verifyCommitSignature(t *testing.T) { }, wantErr: true, assertConditions: []metav1.Condition{ - *conditions.FalseCondition(sourcev1.SourceVerifiedCondition, "InvalidCommitSignature", "signature verification of commit 'shasum' failed: unable to verify commit with any of the given key rings"), + *conditions.FalseCondition(sourcev1.SourceVerifiedCondition, "InvalidCommitSignature", "signature verification of commit 'shasum' failed: unable to verify Git commit: unable to verify payload with any of the given key rings"), }, }, { - name: "Secret get failure sets no SourceVerifiedCondition and returns error", + name: "Invalid commit makes SourceVerifiedCondition=False and returns error", + secret: &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: "existing", + }, + }, + commit: git.Commit{ + Hash: []byte("shasum"), + Encoded: []byte(malformedEncodedCommitFixture), + Signature: signatureCommitFixture, + }, beforeFunc: func(obj *sourcev1.GitRepository) { obj.Spec.Interval = metav1.Duration{Duration: interval} obj.Spec.Verification = &sourcev1.GitRepositoryVerification{ - Mode: "head", + Mode: sourcev1.ModeGitHEAD, + SecretRef: meta.LocalObjectReference{ + Name: "existing", + }, + } + }, + wantErr: true, + assertConditions: []metav1.Condition{ + *conditions.FalseCondition(sourcev1.SourceVerifiedCondition, "InvalidCommitSignature", "signature verification of commit 'shasum' failed: unable to verify Git commit: unable to verify payload with any of the given key rings"), + }, + }, + { + name: "Invalid PGP key makes SourceVerifiedCondition=False and returns error", + secret: &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: "invalid", + }, + Data: map[string][]byte{ + "foo": []byte("invalid PGP public key"), + }, + }, + commit: git.Commit{ + Hash: []byte("shasum"), + Encoded: []byte(malformedEncodedCommitFixture), + Signature: signatureCommitFixture, + }, + beforeFunc: func(obj *sourcev1.GitRepository) { + obj.Spec.Interval = metav1.Duration{Duration: interval} + obj.Spec.Verification = &sourcev1.GitRepositoryVerification{ + Mode: sourcev1.ModeGitHEAD, + SecretRef: meta.LocalObjectReference{ + Name: "invalid", + }, + } + }, + wantErr: true, + assertConditions: []metav1.Condition{ + *conditions.FalseCondition(sourcev1.SourceVerifiedCondition, "InvalidCommitSignature", "signature verification of commit 'shasum' failed: unable to verify Git commit: unable to read armored key ring: openpgp: invalid argument: no armored data found"), + }, + }, + { + name: "Secret get failure makes SourceVerifiedCondition=False and returns error", + beforeFunc: func(obj *sourcev1.GitRepository) { + obj.Spec.Interval = metav1.Duration{Duration: interval} + obj.Spec.Verification = &sourcev1.GitRepositoryVerification{ + Mode: sourcev1.ModeGitHEAD, SecretRef: meta.LocalObjectReference{ Name: "none-existing", }, @@ -1648,10 +1920,18 @@ func TestGitRepositoryReconciler_verifyCommitSignature(t *testing.T) { tt.beforeFunc(obj) } - got, err := r.verifyCommitSignature(context.TODO(), obj, tt.commit) + got, err := r.verifySignature(context.TODO(), obj, tt.commit) g.Expect(obj.Status.Conditions).To(conditions.MatchConditions(tt.assertConditions)) g.Expect(err != nil).To(Equal(tt.wantErr)) + if tt.err != nil { + g.Expect(err).To(Equal(tt.err)) + } g.Expect(got).To(Equal(tt.want)) + if tt.wantSourceVerificationMode != nil { + g.Expect(*obj.Status.SourceVerificationMode).To(Equal(*tt.wantSourceVerificationMode)) + } else { + g.Expect(obj.Status.SourceVerificationMode).To(BeNil()) + } }) } } @@ -2800,3 +3080,124 @@ func TestGitContentConfigChanged(t *testing.T) { }) } } + +func Test_requiresVerification(t *testing.T) { + tests := []struct { + name string + obj *sourcev1.GitRepository + want bool + }{ + { + name: "GitRepository without verification does not require verification", + obj: &sourcev1.GitRepository{ + Spec: sourcev1.GitRepositorySpec{}, + }, + want: false, + }, + { + name: "GitRepository with verification and no observed verification mode in status requires verification", + obj: &sourcev1.GitRepository{ + Spec: sourcev1.GitRepositorySpec{ + Verification: &sourcev1.GitRepositoryVerification{}, + }, + }, + want: true, + }, + { + name: "GitRepository with HEAD verification and a verified tag requires verification", + obj: &sourcev1.GitRepository{ + Spec: sourcev1.GitRepositorySpec{ + Verification: &sourcev1.GitRepositoryVerification{ + Mode: sourcev1.ModeGitHEAD, + }, + }, + Status: sourcev1.GitRepositoryStatus{ + SourceVerificationMode: ptrToVerificationMode(sourcev1.ModeGitTag), + }, + }, + want: true, + }, + { + name: "GitRepository with tag and HEAD verification and a verified tag requires verification", + obj: &sourcev1.GitRepository{ + Spec: sourcev1.GitRepositorySpec{ + Verification: &sourcev1.GitRepositoryVerification{ + Mode: sourcev1.ModeGitTagAndHEAD, + }, + }, + Status: sourcev1.GitRepositoryStatus{ + SourceVerificationMode: ptrToVerificationMode(sourcev1.ModeGitTag), + }, + }, + want: true, + }, + { + name: "GitRepository with tag verification and a verified HEAD requires verification", + obj: &sourcev1.GitRepository{ + Spec: sourcev1.GitRepositorySpec{ + Verification: &sourcev1.GitRepositoryVerification{ + Mode: sourcev1.ModeGitTag, + }, + }, + Status: sourcev1.GitRepositoryStatus{ + SourceVerificationMode: ptrToVerificationMode(sourcev1.ModeGitHEAD), + }, + }, + want: true, + }, + { + name: "GitRepository with tag and HEAD verification and a verified HEAD requires verification", + obj: &sourcev1.GitRepository{ + Spec: sourcev1.GitRepositorySpec{ + Verification: &sourcev1.GitRepositoryVerification{ + Mode: sourcev1.ModeGitTagAndHEAD, + }, + }, + Status: sourcev1.GitRepositoryStatus{ + SourceVerificationMode: ptrToVerificationMode(sourcev1.ModeGitHEAD), + }, + }, + want: true, + }, + { + name: "GitRepository with tag verification and a verified HEAD and tag does not require verification", + obj: &sourcev1.GitRepository{ + Spec: sourcev1.GitRepositorySpec{ + Verification: &sourcev1.GitRepositoryVerification{ + Mode: sourcev1.ModeGitTag, + }, + }, + Status: sourcev1.GitRepositoryStatus{ + SourceVerificationMode: ptrToVerificationMode(sourcev1.ModeGitTagAndHEAD), + }, + }, + want: false, + }, + { + name: "GitRepository with head verification and a verified HEAD and tag does not require verification", + obj: &sourcev1.GitRepository{ + Spec: sourcev1.GitRepositorySpec{ + Verification: &sourcev1.GitRepositoryVerification{ + Mode: sourcev1.ModeGitHEAD, + }, + }, + Status: sourcev1.GitRepositoryStatus{ + SourceVerificationMode: ptrToVerificationMode(sourcev1.ModeGitTagAndHEAD), + }, + }, + want: false, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + g := NewWithT(t) + verificationRequired := requiresVerification(tt.obj) + g.Expect(verificationRequired).To(Equal(tt.want)) + }) + } +} + +func ptrToVerificationMode(mode sourcev1.GitVerificationMode) *sourcev1.GitVerificationMode { + return &mode +}