controllers: use TransformLegacyRevision helper

Signed-off-by: Hidde Beydals <hello@hidde.co>
This commit is contained in:
Hidde Beydals 2023-01-17 13:25:40 +00:00
parent eaa4a4ff31
commit f00aeae09a
8 changed files with 39 additions and 83 deletions

View File

@ -21,19 +21,15 @@ import sourcev1 "github.com/fluxcd/source-controller/api/v1beta2"
type artifactSet []*sourcev1.Artifact
// Diff returns true if any of the revisions in the artifactSet does not match any of the given artifacts.
func (s artifactSet) Diff(set artifactSet, comp func(x, y *sourcev1.Artifact) bool) bool {
func (s artifactSet) Diff(set artifactSet) bool {
if len(s) != len(set) {
return true
}
if comp == nil {
comp = defaultCompare
}
outer:
for _, j := range s {
for _, k := range set {
if comp(j, k) {
if k.HasRevision(j.Revision) {
continue outer
}
}
@ -41,11 +37,3 @@ outer:
}
return false
}
func defaultCompare(x, y *sourcev1.Artifact) bool {
if y == nil {
return false
}
return x.HasRevision(y.Revision)
}

View File

@ -115,7 +115,7 @@ func Test_artifactSet_Diff(t *testing.T) {
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
result := tt.current.Diff(tt.updated, nil)
result := tt.current.Diff(tt.updated)
if result != tt.expected {
t.Errorf("Archive() result = %v, wantResult %v", result, tt.expected)
}

View File

@ -466,8 +466,8 @@ func (r *BucketReconciler) reconcileSource(ctx context.Context, sp *patch.Serial
// Check if index has changed compared to current Artifact revision.
var changed bool
if artifact := obj.Status.Artifact; artifact != nil && artifact.Revision != "" {
curRev := backwardsCompatibleDigest(artifact.Revision)
changed = curRev != index.Digest(curRev.Algorithm())
curRev := digest.Digest(sourcev1.TransformLegacyRevision(artifact.Revision))
changed = curRev.Validate() != nil || curRev != index.Digest(curRev.Algorithm())
}
// Fetch the bucket objects if required to.
@ -519,8 +519,8 @@ func (r *BucketReconciler) reconcileArtifact(ctx context.Context, sp *patch.Seri
// Set the ArtifactInStorageCondition if there's no drift.
defer func() {
if curArtifact := obj.GetArtifact(); curArtifact != nil && curArtifact.Revision != "" {
curRev := backwardsCompatibleDigest(curArtifact.Revision)
if index.Digest(curRev.Algorithm()) == curRev {
curRev := digest.Digest(sourcev1.TransformLegacyRevision(curArtifact.Revision))
if curRev.Validate() == nil && index.Digest(curRev.Algorithm()) == curRev {
conditions.Delete(obj, sourcev1.ArtifactOutdatedCondition)
conditions.MarkTrue(obj, sourcev1.ArtifactInStorageCondition, meta.SucceededReason,
"stored artifact: revision '%s'", artifact.Revision)
@ -530,8 +530,8 @@ func (r *BucketReconciler) reconcileArtifact(ctx context.Context, sp *patch.Seri
// The artifact is up-to-date
if curArtifact := obj.GetArtifact(); curArtifact != nil && curArtifact.Revision != "" {
curRev := backwardsCompatibleDigest(curArtifact.Revision)
if index.Digest(curRev.Algorithm()) == curRev {
curRev := digest.Digest(sourcev1.TransformLegacyRevision(curArtifact.Revision))
if curRev.Validate() == nil && index.Digest(curRev.Algorithm()) == curRev {
r.eventLogf(ctx, obj, eventv1.EventTypeTrace, sourcev1.ArtifactUpToDateReason, "artifact up-to-date with remote revision: '%s'", artifact.Revision)
return sreconcile.ResultSuccess, nil
}
@ -797,10 +797,3 @@ func fetchIndexFiles(ctx context.Context, provider BucketProvider, obj *sourcev1
return nil
}
func backwardsCompatibleDigest(d string) digest.Digest {
if !strings.Contains(d, ":") {
d = digest.SHA256.String() + ":" + d
}
return digest.Digest(d)
}

View File

@ -516,7 +516,7 @@ func (r *GitRepositoryReconciler) reconcileSource(ctx context.Context, sp *patch
}
// Observe if the artifacts still match the previous included ones
if artifacts.Diff(obj.Status.IncludedArtifacts, gitArtifactRevisionEqual) {
if artifacts.Diff(obj.Status.IncludedArtifacts) {
message := fmt.Sprintf("included artifacts differ from last observed includes")
if obj.Status.IncludedArtifacts != nil {
conditions.MarkTrue(obj, sourcev1.ArtifactOutdatedCondition, "IncludeChange", message)
@ -593,8 +593,7 @@ func (r *GitRepositoryReconciler) reconcileSource(ctx context.Context, sp *patch
}
// Mark observations about the revision on the object
if curArtifact := obj.Status.Artifact; curArtifact == nil ||
git.TransformRevision(curArtifact.Revision) != commit.String() {
if !obj.GetArtifact().HasRevision(commit.String()) {
message := fmt.Sprintf("new upstream revision '%s'", commit.String())
if obj.GetArtifact() != nil {
conditions.MarkTrue(obj, sourcev1.ArtifactOutdatedCondition, "NewRevision", message)
@ -627,9 +626,8 @@ func (r *GitRepositoryReconciler) reconcileArtifact(ctx context.Context, sp *pat
// Set the ArtifactInStorageCondition if there's no drift.
defer func() {
if curArtifact := obj.GetArtifact(); curArtifact != nil &&
git.TransformRevision(curArtifact.Revision) == artifact.Revision &&
!includes.Diff(obj.Status.IncludedArtifacts, gitArtifactRevisionEqual) &&
if curArtifact := obj.GetArtifact(); curArtifact.HasRevision(artifact.Revision) &&
!includes.Diff(obj.Status.IncludedArtifacts) &&
!gitContentConfigChanged(obj, includes) {
conditions.Delete(obj, sourcev1.ArtifactOutdatedCondition)
conditions.MarkTrue(obj, sourcev1.ArtifactInStorageCondition, meta.SucceededReason,
@ -638,9 +636,8 @@ func (r *GitRepositoryReconciler) reconcileArtifact(ctx context.Context, sp *pat
}()
// The artifact is up-to-date
if curArtifact := obj.GetArtifact(); curArtifact != nil &&
git.TransformRevision(curArtifact.Revision) == artifact.Revision &&
!includes.Diff(obj.Status.IncludedArtifacts, gitArtifactRevisionEqual) &&
if curArtifact := obj.GetArtifact(); curArtifact.HasRevision(artifact.Revision) &&
!includes.Diff(obj.Status.IncludedArtifacts) &&
!gitContentConfigChanged(obj, includes) {
r.eventLogf(ctx, obj, eventv1.EventTypeTrace, sourcev1.ArtifactUpToDateReason, "artifact up-to-date with remote revision: '%s'", curArtifact.Revision)
return sreconcile.ResultSuccess, nil
@ -1027,7 +1024,7 @@ func gitContentConfigChanged(obj *sourcev1.GitRepository, includes *artifactSet)
}
// Check if the included repositories are still the same.
if git.TransformRevision(observedInclArtifact.Revision) != git.TransformRevision(currentIncl.Revision) {
if !observedInclArtifact.HasRevision(currentIncl.Revision) {
return true
}
if observedInclArtifact.Checksum != currentIncl.Checksum {
@ -1050,10 +1047,3 @@ func gitRepositoryIncludeEqual(a, b sourcev1.GitRepositoryInclude) bool {
}
return true
}
func gitArtifactRevisionEqual(x, y *sourcev1.Artifact) bool {
if x == nil || y == nil {
return false
}
return git.TransformRevision(x.Revision) == git.TransformRevision(y.Revision)
}

View File

@ -2268,7 +2268,7 @@ func TestGitRepositoryReconciler_fetchIncludes(t *testing.T) {
g.Expect(err != nil).To(Equal(tt.wantErr))
g.Expect(obj.GetConditions()).To(conditions.MatchConditions(tt.assertConditions))
if !tt.wantErr && gotArtifactSet != nil {
g.Expect(gotArtifactSet.Diff(tt.wantArtifactSet, gitArtifactRevisionEqual)).To(BeFalse())
g.Expect(gotArtifactSet.Diff(tt.wantArtifactSet)).To(BeFalse())
}
})
}

View File

@ -22,6 +22,7 @@ import (
"errors"
"fmt"
"github.com/fluxcd/pkg/git"
"github.com/opencontainers/go-digest"
"net/url"
"os"
"path/filepath"
@ -793,7 +794,9 @@ func (r *HelmChartReconciler) buildFromTarballArtifact(ctx context.Context, obj
rev = git.ExtractHashFromRevision(rev).String()
}
if obj.Spec.SourceRef.Kind == sourcev1.BucketKind {
rev = backwardsCompatibleDigest(rev).Hex()
if dig := digest.Digest(sourcev1.TransformLegacyRevision(rev)); dig.Validate() == nil {
rev = dig.Hex()
}
}
if kind := obj.Spec.SourceRef.Kind; kind == sourcev1.GitRepositoryKind || kind == sourcev1.BucketKind {
// The SemVer from the metadata is at times used in e.g. the label metadata for a resource
@ -1244,10 +1247,9 @@ func (r *HelmChartReconciler) requestsForGitRepositoryChange(o client.Object) []
return nil
}
revision := git.TransformRevision(repo.GetArtifact().Revision)
var reqs []reconcile.Request
for _, i := range list.Items {
if git.TransformRevision(i.Status.ObservedSourceArtifactRevision) != revision {
if !repo.GetArtifact().HasRevision(i.Status.ObservedSourceArtifactRevision) {
reqs = append(reqs, reconcile.Request{NamespacedName: client.ObjectKeyFromObject(&i)})
}
}
@ -1272,10 +1274,9 @@ func (r *HelmChartReconciler) requestsForBucketChange(o client.Object) []reconci
return nil
}
revision := backwardsCompatibleDigest(bucket.GetArtifact().Revision)
var reqs []reconcile.Request
for _, i := range list.Items {
if backwardsCompatibleDigest(i.Status.ObservedSourceArtifactRevision) != revision {
if !bucket.GetArtifact().HasRevision(i.Status.ObservedSourceArtifactRevision) {
reqs = append(reqs, reconcile.Request{NamespacedName: client.ObjectKeyFromObject(&i)})
}
}

View File

@ -22,7 +22,6 @@ import (
"crypto/x509"
"errors"
"fmt"
"github.com/fluxcd/pkg/git"
"io"
"net/http"
"os"
@ -391,7 +390,7 @@ func (r *OCIRepositoryReconciler) reconcileSource(ctx context.Context, sp *patch
return sreconcile.ResultEmpty, e
}
// Get the upstream revision from the artifact revision
// Get the upstream revision from the artifact digest
revision, err := r.getRevision(url, opts.craneOpts)
if err != nil {
e := serror.NewGeneric(
@ -406,7 +405,7 @@ func (r *OCIRepositoryReconciler) reconcileSource(ctx context.Context, sp *patch
// Mark observations about the revision on the object
defer func() {
if obj.GetArtifact() == nil || git.TransformRevision(obj.GetArtifact().Revision) != git.TransformRevision(revision) {
if !obj.GetArtifact().HasRevision(revision) {
message := fmt.Sprintf("new revision '%s' for '%s'", revision, url)
if obj.GetArtifact() != nil {
conditions.MarkTrue(obj, sourcev1.ArtifactOutdatedCondition, "NewRevision", message)
@ -426,7 +425,7 @@ func (r *OCIRepositoryReconciler) reconcileSource(ctx context.Context, sp *patch
if obj.Spec.Verify == nil {
// Remove old observations if verification was disabled
conditions.Delete(obj, sourcev1.SourceVerifiedCondition)
} else if (obj.GetArtifact() == nil || git.TransformRevision(obj.GetArtifact().Revision) != git.TransformRevision(revision)) ||
} else if !obj.GetArtifact().HasRevision(revision) ||
conditions.GetObservedGeneration(obj, sourcev1.SourceVerifiedCondition) != obj.Generation ||
conditions.IsFalse(obj, sourcev1.SourceVerifiedCondition) {
@ -459,9 +458,7 @@ func (r *OCIRepositoryReconciler) reconcileSource(ctx context.Context, sp *patch
// Skip pulling if the artifact revision and the source configuration has
// not changed.
if (obj.GetArtifact() != nil &&
git.TransformRevision(obj.GetArtifact().Revision) == git.TransformRevision(revision)) &&
!ociContentConfigChanged(obj) {
if obj.GetArtifact().HasRevision(revision) && !ociContentConfigChanged(obj) {
conditions.Delete(obj, sourcev1.FetchFailedCondition)
return sreconcile.ResultSuccess, nil
}
@ -585,7 +582,8 @@ func (r *OCIRepositoryReconciler) selectLayer(obj *sourcev1.OCIRepository, image
return blob, nil
}
// getRevision fetches the upstream revision and returns the revision in the format `<tag>/<revision>`
// getRevision fetches the upstream digest, returning the revision in the
// format '<tag>@<digest>'.
func (r *OCIRepositoryReconciler) getRevision(url string, options []crane.Option) (string, error) {
ref, err := name.ParseReference(url)
if err != nil {
@ -619,14 +617,15 @@ func (r *OCIRepositoryReconciler) getRevision(url string, options []crane.Option
return revision, nil
}
// digestFromRevision extract the revision from the revision string
// digestFromRevision extracts the digest from the revision string.
func (r *OCIRepositoryReconciler) digestFromRevision(revision string) string {
parts := strings.Split(revision, "@")
return parts[len(parts)-1]
}
// verifySignature verifies the authenticity of the given image reference url. First, it tries using a key
// if a secret with a valid public key is provided. If not, it falls back to a keyless approach for verification.
// verifySignature verifies the authenticity of the given image reference URL.
// First, it tries to use a key if a Secret with a valid public key is provided.
// If not, it falls back to a keyless approach for verification.
func (r *OCIRepositoryReconciler) verifySignature(ctx context.Context, obj *sourcev1.OCIRepository, url string, opt ...remote.Option) error {
ctxTimeout, cancel := context.WithTimeout(ctx, obj.Spec.Timeout.Duration)
defer cancel()
@ -954,11 +953,9 @@ func (r *OCIRepositoryReconciler) reconcileStorage(ctx context.Context, sp *patc
// and the symlink in the Storage is updated to its path.
func (r *OCIRepositoryReconciler) reconcileArtifact(ctx context.Context, sp *patch.SerialPatcher,
obj *sourcev1.OCIRepository, metadata *sourcev1.Artifact, dir string) (sreconcile.Result, error) {
revision := metadata.Revision
// Create artifact
artifact := r.Storage.NewArtifactFor(obj.Kind, obj, revision,
fmt.Sprintf("%s.tar.gz", r.digestFromRevision(revision)))
artifact := r.Storage.NewArtifactFor(obj.Kind, obj, metadata.Revision,
fmt.Sprintf("%s.tar.gz", r.digestFromRevision(metadata.Revision)))
// Set the ArtifactInStorageCondition if there's no drift.
defer func() {
@ -970,9 +967,7 @@ func (r *OCIRepositoryReconciler) reconcileArtifact(ctx context.Context, sp *pat
}()
// The artifact is up-to-date
if (obj.GetArtifact() != nil &&
git.TransformRevision(obj.GetArtifact().Revision) == git.TransformRevision(revision)) &&
!ociContentConfigChanged(obj) {
if obj.GetArtifact().HasRevision(artifact.Revision) && !ociContentConfigChanged(obj) {
r.eventLogf(ctx, obj, eventv1.EventTypeTrace, sourcev1.ArtifactUpToDateReason,
"artifact up-to-date with remote revision: '%s'", artifact.Revision)
return sreconcile.ResultSuccess, nil

View File

@ -1281,7 +1281,7 @@ func TestOCIRepository_reconcileSource_verifyOCISourceSignature(t *testing.T) {
func TestOCIRepository_reconcileSource_noop(t *testing.T) {
g := NewWithT(t)
testRevision := "6.1.5@sha256:8a0eed109e056ab1f7e70e8fb47e00cf6f560ca5cd910c83451882e07edb77fa"
testRevision := "6.1.5@sha256:8e4057c22d531d40e12b065443cb0d80394b7257c4dc557cb1fbd4dce892b86d"
tmpDir := t.TempDir()
server, err := setupRegistryServer(ctx, tmpDir, registryOptions{})
@ -1319,18 +1319,7 @@ func TestOCIRepository_reconcileSource_noop(t *testing.T) {
name: "noop - artifact revisions match (legacy)",
beforeFunc: func(obj *sourcev1.OCIRepository) {
obj.Status.Artifact = &sourcev1.Artifact{
Revision: "6.1.5/8a0eed109e056ab1f7e70e8fb47e00cf6f560ca5cd910c83451882e07edb77fa",
}
},
afterFunc: func(g *WithT, artifact *sourcev1.Artifact) {
g.Expect(artifact.Metadata).To(BeEmpty())
},
},
{
name: "noop - artifact revisions match (legacy: digest)",
beforeFunc: func(obj *sourcev1.OCIRepository) {
obj.Status.Artifact = &sourcev1.Artifact{
Revision: "8a0eed109e056ab1f7e70e8fb47e00cf6f560ca5cd910c83451882e07edb77fa",
Revision: "6.1.5/8e4057c22d531d40e12b065443cb0d80394b7257c4dc557cb1fbd4dce892b86d",
}
},
afterFunc: func(g *WithT, artifact *sourcev1.Artifact) {
@ -2257,7 +2246,7 @@ func pushMultiplePodinfoImages(serverURL string, versions ...string) (map[string
func setPodinfoImageAnnotations(img gcrv1.Image, tag string) gcrv1.Image {
metadata := map[string]string{
oci.SourceAnnotation: "https://github.com/stefanprodan/podinfo",
oci.RevisionAnnotation: fmt.Sprintf("%s@sha256:8a0eed109e056ab1f7e70e8fb47e00cf6f560ca5cd910c83451882e07edb77fa", tag),
oci.RevisionAnnotation: fmt.Sprintf("%s@sha1:b3b00fe35424a45d373bf4c7214178bc36fd7872", tag),
}
return mutate.Annotations(img, metadata).(gcrv1.Image)
}