From 903d29bf27b84a3851d4a41a1e1cb3555c6a7297 Mon Sep 17 00:00:00 2001 From: Sanskar Jaiswal Date: Mon, 14 Mar 2022 20:59:56 +0530 Subject: [PATCH] refactor code to be more neat Signed-off-by: Sanskar Jaiswal --- api/go.mod | 7 +- api/go.sum | 19 +- controllers/helmchart_controller.go | 68 ++-- controllers/helmchart_controller_test.go | 352 ++++++++---------- controllers/storage.go | 31 +- .../testdata/charts/helmchart-0.1.0.tgz | Bin 3280 -> 3416 bytes .../testdata/charts/helmchart-0.1.0.tgz.prov | 22 +- internal/helm/chart/builder.go | 10 +- internal/helm/chart/builder_local.go | 39 +- internal/helm/chart/builder_local_test.go | 42 ++- internal/helm/chart/builder_remote.go | 35 +- internal/helm/chart/builder_remote_test.go | 39 +- internal/helm/chart/verify.go | 56 ++- internal/helm/repository/chart_repository.go | 2 +- internal/util/file.go | 43 +++ internal/util/temp.go | 26 -- 16 files changed, 458 insertions(+), 333 deletions(-) create mode 100644 internal/util/file.go diff --git a/api/go.mod b/api/go.mod index 46c5284ff..c75967d96 100644 --- a/api/go.mod +++ b/api/go.mod @@ -5,7 +5,7 @@ go 1.17 require ( github.com/fluxcd/pkg/apis/acl v0.0.3 github.com/fluxcd/pkg/apis/meta v0.12.0 - k8s.io/apimachinery v0.23.2 + k8s.io/apimachinery v0.23.4 sigs.k8s.io/controller-runtime v0.11.0 ) @@ -17,10 +17,13 @@ require ( github.com/json-iterator/go v1.1.12 // indirect github.com/modern-go/concurrent v0.0.0-20180306012644-bacd9c7ef1dd // indirect github.com/modern-go/reflect2 v1.0.2 // indirect - golang.org/x/net v0.0.0-20211215060638-4ddde0e984e9 // indirect + golang.org/x/net v0.0.0-20220107192237-5cfca573fb4d // indirect + golang.org/x/sys v0.0.0-20211216021012-1d35b9e2eb4e // indirect golang.org/x/text v0.3.7 // indirect + gopkg.in/check.v1 v1.0.0-20201130134442-10cb98267c6c // indirect gopkg.in/inf.v0 v0.9.1 // indirect gopkg.in/yaml.v2 v2.4.0 // indirect + k8s.io/api v0.23.4 // indirect k8s.io/klog/v2 v2.30.0 // indirect k8s.io/utils v0.0.0-20211208161948-7d6a63dca704 // indirect sigs.k8s.io/json v0.0.0-20211208200746-9f7c6b3444d2 // indirect diff --git a/api/go.sum b/api/go.sum index 7267da4d0..0a854b5fe 100644 --- a/api/go.sum +++ b/api/go.sum @@ -297,6 +297,8 @@ github.com/kr/fs v0.1.0/go.mod h1:FFnZGqtBN9Gxj7eW1uZ42v5BccTP0vu6NEaFoC2HwRg= github.com/kr/logfmt v0.0.0-20140226030751-b84e30acd515/go.mod h1:+0opPa2QZZtGFBFZlji/RkVcI2GknAs/DXo4wKdlNEc= github.com/kr/pretty v0.1.0/go.mod h1:dAy3ld7l9f0ibDNOQOHHMYYIIbhfbHSm3C4ZsoJORNo= github.com/kr/pretty v0.2.0/go.mod h1:ipq/a2n7PKx3OHsz4KJII5eveXtPO4qwEXGdVfWzfnI= +github.com/kr/pretty v0.2.1 h1:Fmg33tUaq4/8ym9TJN1x7sLJnHVwhP33CNkpYV/7rwI= +github.com/kr/pretty v0.2.1/go.mod h1:ipq/a2n7PKx3OHsz4KJII5eveXtPO4qwEXGdVfWzfnI= github.com/kr/pty v1.1.1/go.mod h1:pFQYn66WHrOpPYNljwOMqo10TkYh1fy3cYio2l3bCsQ= github.com/kr/text v0.1.0/go.mod h1:4Jbv+DJW3UT/LiOwJeYQe1efqtUx/iVham/4vfdArNI= github.com/kr/text v0.2.0 h1:5Nx0Ya0ZqY2ygV366QzturHI13Jq95ApcVaJBhpS+AY= @@ -334,7 +336,6 @@ github.com/munnerz/goautoneg v0.0.0-20191010083416-a7dc8b61c822/go.mod h1:+n7T8m github.com/mwitkow/go-conntrack v0.0.0-20161129095857-cc309e4a2223/go.mod h1:qRWi+5nqEBWmkhHvq77mSJWrCKwh8bxhgT7d/eI7P4U= github.com/mwitkow/go-conntrack v0.0.0-20190716064945-2f068394615f/go.mod h1:qRWi+5nqEBWmkhHvq77mSJWrCKwh8bxhgT7d/eI7P4U= github.com/mxk/go-flowrate v0.0.0-20140419014527-cca7078d478f/go.mod h1:ZdcZmHo+o7JKHSa8/e818NopupXU1YMK5fe1lsApnBw= -github.com/niemeyer/pretty v0.0.0-20200227124842-a10e7caefd8e h1:fD57ERR4JtEqsWbfPhv4DMiApHyliiK5xCTNVSPiaAs= github.com/niemeyer/pretty v0.0.0-20200227124842-a10e7caefd8e/go.mod h1:zD1mROLANZcx1PVRCS0qkT7pwLkGfwJo4zjcN/Tysno= github.com/nxadm/tail v1.4.4/go.mod h1:kenIhsEOeOJmVchQTgglprH7qJGnHDVpk1VPCcaMI8A= github.com/nxadm/tail v1.4.8 h1:nPr65rt6Y5JFSKQO7qToXr7pePgD6Gwiw05lkbyAQTE= @@ -569,8 +570,9 @@ golang.org/x/net v0.0.0-20210525063256-abc453219eb5/go.mod h1:9nx3DQGgdP8bBQD5qx golang.org/x/net v0.0.0-20210805182204-aaa1db679c0d/go.mod h1:9nx3DQGgdP8bBQD5qxJ1jj9UTztislL4KSBs9R2vV5Y= golang.org/x/net v0.0.0-20210825183410-e898025ed96a/go.mod h1:9nx3DQGgdP8bBQD5qxJ1jj9UTztislL4KSBs9R2vV5Y= golang.org/x/net v0.0.0-20211209124913-491a49abca63/go.mod h1:9nx3DQGgdP8bBQD5qxJ1jj9UTztislL4KSBs9R2vV5Y= -golang.org/x/net v0.0.0-20211215060638-4ddde0e984e9 h1:kmreh1vGI63l2FxOAYS3Yv6ATsi7lSTuwNSVbGfJV9I= golang.org/x/net v0.0.0-20211215060638-4ddde0e984e9/go.mod h1:9nx3DQGgdP8bBQD5qxJ1jj9UTztislL4KSBs9R2vV5Y= +golang.org/x/net v0.0.0-20220107192237-5cfca573fb4d h1:62NvYBuaanGXR2ZOfwDFkhhl6X1DUgf8qg3GuQvxZsE= +golang.org/x/net v0.0.0-20220107192237-5cfca573fb4d/go.mod h1:9nx3DQGgdP8bBQD5qxJ1jj9UTztislL4KSBs9R2vV5Y= golang.org/x/oauth2 v0.0.0-20180821212333-d2e6202438be/go.mod h1:N/0e6XlmueqKjAGxoOufVs8QHGRruUQn6yWY3a++T0U= golang.org/x/oauth2 v0.0.0-20190226205417-e64efc72b421/go.mod h1:gOpvHmFTYa4IltrdGE7lF6nIHvwfUNPOp7c8zoXwtLw= golang.org/x/oauth2 v0.0.0-20190604053449-0f29369cfe45/go.mod h1:gOpvHmFTYa4IltrdGE7lF6nIHvwfUNPOp7c8zoXwtLw= @@ -660,8 +662,9 @@ golang.org/x/sys v0.0.0-20210616094352-59db8d763f22/go.mod h1:oPkhp1MJrh7nUepCBc golang.org/x/sys v0.0.0-20210630005230-0f9fa26af87c/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/sys v0.0.0-20210809222454-d867a43fc93e/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/sys v0.0.0-20210831042530-f4d43177bf5e/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= -golang.org/x/sys v0.0.0-20211029165221-6e7872819dc8 h1:M69LAlWZCshgp0QSzyDcSsSIejIEeuaCVpmwcKwyLMk= golang.org/x/sys v0.0.0-20211029165221-6e7872819dc8/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= +golang.org/x/sys v0.0.0-20211216021012-1d35b9e2eb4e h1:fLOSk5Q00efkSvAm+4xcoXD+RRmLmmulPn5I3Y9F2EM= +golang.org/x/sys v0.0.0-20211216021012-1d35b9e2eb4e/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/term v0.0.0-20201126162022-7de9c90e9dd1/go.mod h1:bj7SfCRtBDWHUb9snDiAeCFNEtKQo2Wmx5Cou7ajbmo= golang.org/x/term v0.0.0-20210615171337-6886f2dfbf5b/go.mod h1:jbD1KX2456YbFQfuXm/mYQcufACuNUgVhRMnK/tPxf8= golang.org/x/text v0.0.0-20170915032832-14c0d48ead0c/go.mod h1:NqM8EUOU14njkJ3fqMW+pc6Ldnwhi/IjpwHt7yyuwOQ= @@ -857,8 +860,9 @@ gopkg.in/alecthomas/kingpin.v2 v2.2.6/go.mod h1:FMv+mEhP44yOT+4EoQTLFTRgOQ1FBLks gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0= gopkg.in/check.v1 v1.0.0-20180628173108-788fd7840127/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0= gopkg.in/check.v1 v1.0.0-20190902080502-41f04d3bba15/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0= -gopkg.in/check.v1 v1.0.0-20200227125254-8fa46927fb4f h1:BLraFXnmrev5lT+xlilqcH8XK9/i0At2xKjWk4p6zsU= gopkg.in/check.v1 v1.0.0-20200227125254-8fa46927fb4f/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0= +gopkg.in/check.v1 v1.0.0-20201130134442-10cb98267c6c h1:Hei/4ADfdWqJk1ZMxUNpqntNwaWcugrBjAiHlqqRiVk= +gopkg.in/check.v1 v1.0.0-20201130134442-10cb98267c6c/go.mod h1:JHkPIbrfpd72SG/EVd6muEfDQjcINNoR0C8j2r3qZ4Q= gopkg.in/errgo.v2 v2.1.0/go.mod h1:hNsd1EY+bozCKY1Ytp96fpM3vjJbqLJn88ws8XvfDNI= gopkg.in/fsnotify.v1 v1.4.7/go.mod h1:Tz8NjZHkW78fSQdbUxIjBTcgA1z1m8ZHf0WmKUhAMys= gopkg.in/inf.v0 v0.9.1 h1:73M5CoZyi3ZLMOyDlQh031Cx6N9NDJ2Vvfl76EDAgDc= @@ -893,12 +897,14 @@ honnef.co/go/tools v0.0.0-20190523083050-ea95bdfd59fc/go.mod h1:rf3lG4BRIbNafJWh honnef.co/go/tools v0.0.1-2019.2.3/go.mod h1:a3bituU0lyd329TUQxRnasdCoJDkEUEAqEt0JzvZhAg= honnef.co/go/tools v0.0.1-2020.1.3/go.mod h1:X/FiERA/W4tHapMX5mGpAtMSVEeEUOyHaw9vFzvIQ3k= honnef.co/go/tools v0.0.1-2020.1.4/go.mod h1:X/FiERA/W4tHapMX5mGpAtMSVEeEUOyHaw9vFzvIQ3k= -k8s.io/api v0.23.0 h1:WrL1gb73VSC8obi8cuYETJGXEoFNEh3LU0Pt+Sokgro= k8s.io/api v0.23.0/go.mod h1:8wmDdLBHBNxtOIytwLstXt5E9PddnZb0GaMcqsvDBpg= +k8s.io/api v0.23.4 h1:85gnfXQOWbJa1SiWGpE9EEtHs0UVvDyIsSMpEtl2D4E= +k8s.io/api v0.23.4/go.mod h1:i77F4JfyNNrhOjZF7OwwNJS5Y1S9dpwvb9iYRYRczfI= k8s.io/apiextensions-apiserver v0.23.0/go.mod h1:xIFAEEDlAZgpVBl/1VSjGDmLoXAWRG40+GsWhKhAxY4= k8s.io/apimachinery v0.23.0/go.mod h1:fFCTTBKvKcwTPFzjlcxp91uPFZr+JA0FubU4fLzzFYc= -k8s.io/apimachinery v0.23.2 h1:dBmjCOeYBdg2ibcQxMuUq+OopZ9fjfLIR5taP/XKeTs= k8s.io/apimachinery v0.23.2/go.mod h1:zDqeV0AK62LbCI0CI7KbWCAYdLg+E+8UXJ0rIz5gmS8= +k8s.io/apimachinery v0.23.4 h1:fhnuMd/xUL3Cjfl64j5ULKZ1/J9n8NuQEgNL+WXWfdM= +k8s.io/apimachinery v0.23.4/go.mod h1:BEuFMMBaIbcOqVIJqNZJXGFTP4W6AycEpb5+m/97hrM= k8s.io/apiserver v0.23.0/go.mod h1:Cec35u/9zAepDPPFyT+UMrgqOCjgJ5qtfVJDxjZYmt4= k8s.io/client-go v0.23.0/go.mod h1:hrDnpnK1mSr65lHHcUuIZIXDgEbzc7/683c6hyG4jTA= k8s.io/code-generator v0.23.0/go.mod h1:vQvOhDXhuzqiVfM/YHp+dmg10WDZCchJVObc9MvowsE= @@ -911,6 +917,7 @@ k8s.io/klog/v2 v2.30.0/go.mod h1:y1WjHnz7Dj687irZUWR/WLkLc5N1YHtjLdmgWjndZn0= k8s.io/kube-openapi v0.0.0-20211115234752-e816edb12b65/go.mod h1:sX9MT8g7NVZM5lVL/j8QyCCJe8YSMW30QvGZWaCIDIk= k8s.io/utils v0.0.0-20210802155522-efc7438f0176/go.mod h1:jPW/WVKK9YHAvNhRxK0md/EJ228hCsBRufyofKtW8HA= k8s.io/utils v0.0.0-20210930125809-cb0fa318a74b/go.mod h1:jPW/WVKK9YHAvNhRxK0md/EJ228hCsBRufyofKtW8HA= +k8s.io/utils v0.0.0-20211116205334-6203023598ed/go.mod h1:jPW/WVKK9YHAvNhRxK0md/EJ228hCsBRufyofKtW8HA= k8s.io/utils v0.0.0-20211208161948-7d6a63dca704 h1:ZKMMxTvduyf5WUtREOqg5LiXaN1KO/+0oOQPRFrClpo= k8s.io/utils v0.0.0-20211208161948-7d6a63dca704/go.mod h1:jPW/WVKK9YHAvNhRxK0md/EJ228hCsBRufyofKtW8HA= rsc.io/binaryregexp v0.2.0/go.mod h1:qTv7/COck+e2FymRvadv62gMdZztPaShugOCi3I+8D8= diff --git a/controllers/helmchart_controller.go b/controllers/helmchart_controller.go index 586000fc3..29470eb74 100644 --- a/controllers/helmchart_controller.go +++ b/controllers/helmchart_controller.go @@ -73,6 +73,7 @@ var helmChartReadyCondition = summarize.Conditions{ sourcev1.FetchFailedCondition, sourcev1.ArtifactOutdatedCondition, sourcev1.StorageOperationFailedCondition, + sourcev1.SourceVerifiedCondition, meta.ReadyCondition, meta.ReconcilingCondition, meta.StalledCondition, @@ -82,6 +83,7 @@ var helmChartReadyCondition = summarize.Conditions{ sourcev1.FetchFailedCondition, sourcev1.ArtifactOutdatedCondition, sourcev1.StorageOperationFailedCondition, + sourcev1.SourceVerifiedCondition, meta.StalledCondition, meta.ReconcilingCondition, }, @@ -469,16 +471,20 @@ func (r *HelmChartReconciler) buildFromHelmRepository(ctx context.Context, obj * opts.VersionMetadata = strconv.FormatInt(obj.Generation, 10) } - var keyring []byte - keyring, err = r.getProvenanceKeyring(ctx, obj) + keyring, err := r.getProvenanceKeyring(ctx, obj) if err != nil { - conditions.MarkTrue(obj, sourcev1.FetchFailedCondition, sourcev1.AuthenticationFailedReason, err.Error()) - return sreconcile.ResultEmpty, err + e := &serror.Event{ + Err: fmt.Errorf("failed to get public key for chart signature verification: %w", err), + Reason: sourcev1.SourceVerifiedCondition, + } + conditions.MarkFalse(obj, sourcev1.FetchFailedCondition, sourcev1.SourceVerifiedCondition, e.Error()) + return sreconcile.ResultEmpty, e } + opts.Keyring = keyring // Build the chart ref := chart.RemoteReference{Name: obj.Spec.Chart, Version: obj.Spec.Version} - build, err := cb.Build(ctx, ref, util.TempPathForObj("", ".tgz", obj), opts, keyring) + build, err := cb.Build(ctx, ref, util.TempPathForObj("", ".tgz", obj), opts) if err != nil { return sreconcile.ResultEmpty, err @@ -599,19 +605,23 @@ func (r *HelmChartReconciler) buildFromTarballArtifact(ctx context.Context, obj } opts.VersionMetadata += strconv.FormatInt(obj.Generation, 10) } - var keyring []byte - keyring, err = r.getProvenanceKeyring(ctx, obj) + keyring, err := r.getProvenanceKeyring(ctx, obj) if err != nil { - conditions.MarkTrue(obj, sourcev1.FetchFailedCondition, sourcev1.AuthenticationFailedReason, err.Error()) - return sreconcile.ResultEmpty, err + e := &serror.Event{ + Err: fmt.Errorf("failed to get public key for chart signature verification: %w", err), + Reason: sourcev1.SourceVerifiedCondition, + } + conditions.MarkFalse(obj, sourcev1.FetchFailedCondition, sourcev1.SourceVerifiedCondition, e.Error()) + return sreconcile.ResultEmpty, e } + opts.Keyring = keyring // Build chart cb := chart.NewLocalBuilder(dm) build, err := cb.Build(ctx, chart.LocalReference{ WorkDir: sourceDir, Path: chartPath, - }, util.TempPathForObj("", ".tgz", obj), opts, keyring) + }, util.TempPathForObj("", ".tgz", obj), opts) if err != nil { return sreconcile.ResultEmpty, err } @@ -641,6 +651,14 @@ func (r *HelmChartReconciler) reconcileArtifact(ctx context.Context, obj *source conditions.Delete(obj, sourcev1.ArtifactOutdatedCondition) conditions.MarkTrue(obj, meta.ReadyCondition, reasonForBuild(b), b.Summary()) } + if b.VerificationSignature != nil && b.ProvFilePath != "" && obj.GetArtifact() != nil { + var sigVerMsg strings.Builder + sigVerMsg.WriteString(fmt.Sprintf("chart signed by: %v", strings.Join(b.VerificationSignature.Identities[:], ","))) + sigVerMsg.WriteString(fmt.Sprintf(" using key with fingeprint: %X", b.VerificationSignature.KeyFingerprint)) + sigVerMsg.WriteString(fmt.Sprintf(" and hash verified: %s", b.VerificationSignature.FileHash)) + + conditions.MarkTrue(obj, sourcev1.SourceVerifiedCondition, reasonForBuild(b), sigVerMsg.String()) + } }() // Create artifact from build data @@ -692,7 +710,7 @@ func (r *HelmChartReconciler) reconcileArtifact(ctx context.Context, obj *source if err = r.Storage.CopyFromPath(&provArtifact, b.ProvFilePath); err != nil { return sreconcile.ResultEmpty, &serror.Event{ Err: fmt.Errorf("unable to copy Helm chart provenance file to storage: %w", err), - Reason: sourcev1.StorageOperationFailedReason, + Reason: sourcev1.StorageOperationFailedCondition, } } } @@ -790,15 +808,23 @@ func (r *HelmChartReconciler) garbageCollect(ctx context.Context, obj *sourcev1. obj.Status.Artifact = nil return nil } + if obj.GetArtifact() != nil { - if deleted, err := r.Storage.RemoveAllButCurrent(*obj.GetArtifact()); err != nil { + localPath := r.Storage.LocalPath(*obj.GetArtifact()) + provFilePath := localPath + ".prov" + dir := filepath.Dir(localPath) + callbacks := make([]func(path string, info os.FileInfo) bool, 0) + callbacks = append(callbacks, func(path string, info os.FileInfo) bool { + if path != localPath && path != provFilePath && info.Mode()&os.ModeSymlink != os.ModeSymlink { + return true + } + return false + }) + if _, err := r.Storage.RemoveConditionally(dir, callbacks); err != nil { return &serror.Event{ Err: fmt.Errorf("garbage collection of old artifacts failed: %w", err), Reason: "GarbageCollectionFailed", } - } else if len(deleted) > 0 { - r.eventLogf(ctx, obj, events.EventTypeTrace, "GarbageCollectionSucceeded", - "garbage collected old artifacts") } } return nil @@ -1076,20 +1102,12 @@ func (r *HelmChartReconciler) getProvenanceKeyring(ctx context.Context, chart *s var secret corev1.Secret err := r.Client.Get(ctx, name, &secret) if err != nil { - e := &serror.Event{ - Err: fmt.Errorf("failed to get secret '%s': %w", chart.Spec.VerificationKeyring.SecretRef.Name, err), - Reason: sourcev1.AuthenticationFailedReason, - } - return nil, e + return nil, err } key := chart.Spec.VerificationKeyring.Key if val, ok := secret.Data[key]; !ok { err = fmt.Errorf("secret doesn't contain the advertised verification keyring name %s", key) - e := &serror.Event{ - Err: fmt.Errorf("invalid secret '%s': %w", secret.GetName(), err), - Reason: sourcev1.AuthenticationFailedReason, - } - return nil, e + return nil, err } else { return val, nil } diff --git a/controllers/helmchart_controller_test.go b/controllers/helmchart_controller_test.go index 5390f57a4..37b08630f 100644 --- a/controllers/helmchart_controller_test.go +++ b/controllers/helmchart_controller_test.go @@ -67,7 +67,119 @@ func TestHelmChartReconciler_Reconcile(t *testing.T) { g.Expect(err).NotTo(HaveOccurred()) defer os.RemoveAll(server.Root()) + g.Expect(server.PackageChartWithVersion(chartPath, chartVersion)).To(Succeed()) + g.Expect(server.GenerateIndex()).To(Succeed()) + + server.Start() + defer server.Stop() + + ns, err := testEnv.CreateNamespace(ctx, "helmchart") + g.Expect(err).ToNot(HaveOccurred()) + defer func() { g.Expect(testEnv.Delete(ctx, ns)).To(Succeed()) }() + + repository := &sourcev1.HelmRepository{ + ObjectMeta: metav1.ObjectMeta{ + GenerateName: "helmrepository-", + Namespace: ns.Name, + }, + Spec: sourcev1.HelmRepositorySpec{ + URL: server.URL(), + }, + } + g.Expect(testEnv.CreateAndWait(ctx, repository)).To(Succeed()) + + obj := &sourcev1.HelmChart{ + ObjectMeta: metav1.ObjectMeta{ + GenerateName: "helmrepository-reconcile-", + Namespace: ns.Name, + }, + Spec: sourcev1.HelmChartSpec{ + Chart: chartName, + Version: chartVersion, + SourceRef: sourcev1.LocalHelmChartSourceReference{ + Kind: sourcev1.HelmRepositoryKind, + Name: repository.Name, + }, + }, + } + g.Expect(testEnv.Create(ctx, obj)).To(Succeed()) + + key := client.ObjectKey{Name: obj.Name, Namespace: obj.Namespace} + + // Wait for finalizer to be set + g.Eventually(func() bool { + if err := testEnv.Get(ctx, key, obj); err != nil { + return false + } + return len(obj.Finalizers) > 0 + }, timeout).Should(BeTrue()) + + // Wait for HelmChart to be Ready + g.Eventually(func() bool { + if err := testEnv.Get(ctx, key, obj); err != nil { + return false + } + if !conditions.IsReady(obj) || obj.Status.Artifact == nil { + return false + } + readyCondition := conditions.Get(obj, meta.ReadyCondition) + return obj.Generation == readyCondition.ObservedGeneration && + obj.Generation == obj.Status.ObservedGeneration + }, timeout).Should(BeTrue()) + + // Check if the object status is valid. + condns := &status.Conditions{NegativePolarity: helmChartReadyCondition.NegativePolarity} + checker := status.NewChecker(testEnv.Client, testEnv.GetScheme(), condns) + checker.CheckErr(ctx, obj) + + // kstatus client conformance check. + u, err := patch.ToUnstructured(obj) + g.Expect(err).ToNot(HaveOccurred()) + res, err := kstatus.Compute(u) + g.Expect(err).ToNot(HaveOccurred()) + g.Expect(res.Status).To(Equal(kstatus.CurrentStatus)) + + // Patch the object with reconcile request annotation. + patchHelper, err := patch.NewHelper(obj, testEnv.Client) + g.Expect(err).ToNot(HaveOccurred()) + annotations := map[string]string{ + meta.ReconcileRequestAnnotation: "now", + } + obj.SetAnnotations(annotations) + g.Expect(patchHelper.Patch(ctx, obj)).ToNot(HaveOccurred()) + g.Eventually(func() bool { + if err := testEnv.Get(ctx, key, obj); err != nil { + return false + } + return obj.Status.LastHandledReconcileAt == "now" + }, timeout).Should(BeTrue()) + + g.Expect(testEnv.Delete(ctx, obj)).To(Succeed()) + + // Wait for HelmChart to be deleted + g.Eventually(func() bool { + if err := testEnv.Get(ctx, key, obj); err != nil { + return apierrors.IsNotFound(err) + } + return false + }, timeout).Should(BeTrue()) +} + +func TestHelmChartReconciler_ReconcileWithSigVerification(t *testing.T) { + g := NewWithT(t) + + const ( + chartName = "helmchart" + chartVersion = "0.2.0" + chartPath = "testdata/charts/helmchart" + ) + + server, err := helmtestserver.NewTempHelmServer() + g.Expect(err).NotTo(HaveOccurred()) + defer os.RemoveAll(server.Root()) + publicKeyPath := fmt.Sprintf("%s/%s", server.Root(), publicKeyFileName) + g.Expect(server.PackageSignedChartWithVersion(chartPath, chartVersion, publicKeyPath)).To(Succeed()) g.Expect(server.GenerateIndex()).To(Succeed()) @@ -215,7 +327,6 @@ func TestHelmChartReconciler_reconcileStorage(t *testing.T) { if err := testStorage.MkdirAll(*obj.Status.Artifact); err != nil { return err } - if err := testStorage.AtomicWriteFile(obj.Status.Artifact, strings.NewReader(v), 0644); err != nil { return err } @@ -347,13 +458,9 @@ func TestHelmChartReconciler_reconcileSource(t *testing.T) { } g.Expect(storage.Archive(gitArtifact, "testdata/charts", nil)).To(Succeed()) - keyring, err := os.ReadFile("testdata/charts/pub.gpg") - g.Expect(err).ToNot(HaveOccurred()) - tests := []struct { name string source sourcev1.Source - secret *corev1.Secret beforeFunc func(obj *sourcev1.HelmChart) want sreconcile.Result wantErr error @@ -371,27 +478,12 @@ func TestHelmChartReconciler_reconcileSource(t *testing.T) { Artifact: gitArtifact, }, }, - secret: &corev1.Secret{ - ObjectMeta: metav1.ObjectMeta{ - Name: "keyring-secret", - Namespace: "default", - }, - Data: map[string][]byte{ - publicKeyFileName: keyring, - }, - }, beforeFunc: func(obj *sourcev1.HelmChart) { obj.Spec.Chart = "testdata/charts/helmchart-0.1.0.tgz" obj.Spec.SourceRef = sourcev1.LocalHelmChartSourceReference{ Name: "gitrepository", Kind: sourcev1.GitRepositoryKind, } - obj.Spec.VerificationKeyring = &sourcev1.VerificationKeyring{ - SecretRef: meta.LocalObjectReference{ - Name: "keyring-secret", - }, - Key: publicKeyFileName, - } }, want: sreconcile.ResultSuccess, assertFunc: func(g *WithT, build chart.Build, obj sourcev1.HelmChart) { @@ -399,7 +491,6 @@ func TestHelmChartReconciler_reconcileSource(t *testing.T) { g.Expect(build.Name).To(Equal("helmchart")) g.Expect(build.Version).To(Equal("0.1.0")) g.Expect(build.Path).To(BeARegularFile()) - g.Expect(build.ProvFilePath).To(BeARegularFile()) g.Expect(obj.Status.ObservedSourceArtifactRevision).To(Equal(gitArtifact.Revision)) g.Expect(obj.Status.Conditions).To(conditions.MatchConditions([]metav1.Condition{ @@ -408,7 +499,6 @@ func TestHelmChartReconciler_reconcileSource(t *testing.T) { }, cleanFunc: func(g *WithT, build *chart.Build) { g.Expect(os.Remove(build.Path)).To(Succeed()) - g.Expect(os.Remove(build.ProvFilePath)).To(Succeed()) }, }, { @@ -515,9 +605,6 @@ func TestHelmChartReconciler_reconcileSource(t *testing.T) { if tt.source != nil { clientBuilder.WithRuntimeObjects(tt.source) } - if tt.secret != nil { - clientBuilder.WithRuntimeObjects(tt.secret) - } r := &HelmChartReconciler{ Client: clientBuilder.Build(), @@ -568,58 +655,33 @@ func TestHelmChartReconciler_buildFromHelmRepository(t *testing.T) { ) serverFactory, err := helmtestserver.NewTempHelmServer() - publicKeyPath := fmt.Sprintf("%s/%s", serverFactory.Root(), publicKeyFileName) - g.Expect(err).NotTo(HaveOccurred()) defer os.RemoveAll(serverFactory.Root()) for _, ver := range []string{chartVersion, higherChartVersion} { - g.Expect(serverFactory.PackageSignedChartWithVersion(chartPath, ver, publicKeyPath+ver)).To(Succeed()) + g.Expect(serverFactory.PackageChartWithVersion(chartPath, ver)).To(Succeed()) } g.Expect(serverFactory.GenerateIndex()).To(Succeed()) - keyring1, err := os.ReadFile(publicKeyPath + chartVersion) - g.Expect(err).ToNot(HaveOccurred()) - defer os.Remove(publicKeyPath + chartVersion) - - keyring2, err := os.ReadFile(publicKeyPath + higherChartVersion) - g.Expect(err).ToNot(HaveOccurred()) - defer os.Remove(publicKeyPath + higherChartVersion) - type options struct { username string password string } tests := []struct { - name string - server options - secret *corev1.Secret - keyringSecret *corev1.Secret - beforeFunc func(obj *sourcev1.HelmChart, repository *sourcev1.HelmRepository) - want sreconcile.Result - wantErr error - assertFunc func(g *WithT, obj *sourcev1.HelmChart, build chart.Build) - cleanFunc func(g *WithT, build *chart.Build) + name string + server options + secret *corev1.Secret + beforeFunc func(obj *sourcev1.HelmChart, repository *sourcev1.HelmRepository) + want sreconcile.Result + wantErr error + assertFunc func(g *WithT, obj *sourcev1.HelmChart, build chart.Build) + cleanFunc func(g *WithT, build *chart.Build) }{ { name: "Reconciles chart build", beforeFunc: func(obj *sourcev1.HelmChart, repository *sourcev1.HelmRepository) { obj.Spec.Chart = "helmchart" - obj.Spec.VerificationKeyring = &sourcev1.VerificationKeyring{ - SecretRef: meta.LocalObjectReference{ - Name: "keyring-secret-0.3.0", - }, - Key: publicKeyFileName, - } - }, - keyringSecret: &corev1.Secret{ - ObjectMeta: metav1.ObjectMeta{ - Name: "keyring-secret-0.3.0", - }, - Data: map[string][]byte{ - publicKeyFileName: keyring2, - }, }, want: sreconcile.ResultSuccess, assertFunc: func(g *WithT, _ *sourcev1.HelmChart, build chart.Build) { @@ -627,12 +689,9 @@ func TestHelmChartReconciler_buildFromHelmRepository(t *testing.T) { g.Expect(build.Version).To(Equal(higherChartVersion)) g.Expect(build.Path).ToNot(BeEmpty()) g.Expect(build.Path).To(BeARegularFile()) - g.Expect(build.ProvFilePath).ToNot(BeEmpty()) - g.Expect(build.ProvFilePath).To(BeARegularFile()) }, cleanFunc: func(g *WithT, build *chart.Build) { g.Expect(os.Remove(build.Path)).To(Succeed()) - g.Expect(os.Remove(build.ProvFilePath)).To(Succeed()) }, }, { @@ -641,14 +700,6 @@ func TestHelmChartReconciler_buildFromHelmRepository(t *testing.T) { username: "foo", password: "bar", }, - keyringSecret: &corev1.Secret{ - ObjectMeta: metav1.ObjectMeta{ - Name: "keyring-secret-0.2.0", - }, - Data: map[string][]byte{ - publicKeyFileName: keyring1, - }, - }, secret: &corev1.Secret{ ObjectMeta: metav1.ObjectMeta{ Name: "auth", @@ -662,12 +713,6 @@ func TestHelmChartReconciler_buildFromHelmRepository(t *testing.T) { obj.Spec.Chart = chartName obj.Spec.Version = chartVersion repository.Spec.SecretRef = &meta.LocalObjectReference{Name: "auth"} - obj.Spec.VerificationKeyring = &sourcev1.VerificationKeyring{ - SecretRef: meta.LocalObjectReference{ - Name: "keyring-secret-0.2.0", - }, - Key: publicKeyFileName, - } }, want: sreconcile.ResultSuccess, assertFunc: func(g *WithT, _ *sourcev1.HelmChart, build chart.Build) { @@ -675,34 +720,17 @@ func TestHelmChartReconciler_buildFromHelmRepository(t *testing.T) { g.Expect(build.Version).To(Equal(chartVersion)) g.Expect(build.Path).ToNot(BeEmpty()) g.Expect(build.Path).To(BeARegularFile()) - g.Expect(build.ProvFilePath).ToNot(BeEmpty()) - g.Expect(build.ProvFilePath).To(BeARegularFile()) }, cleanFunc: func(g *WithT, build *chart.Build) { g.Expect(os.Remove(build.Path)).To(Succeed()) - g.Expect(os.Remove(build.ProvFilePath)).To(Succeed()) }, }, { name: "Uses artifact as build cache", - keyringSecret: &corev1.Secret{ - ObjectMeta: metav1.ObjectMeta{ - Name: "keyring-secret-0.2.0", - }, - Data: map[string][]byte{ - publicKeyFileName: keyring1, - }, - }, beforeFunc: func(obj *sourcev1.HelmChart, repository *sourcev1.HelmRepository) { obj.Spec.Chart = chartName obj.Spec.Version = chartVersion obj.Status.Artifact = &sourcev1.Artifact{Path: chartName + "-" + chartVersion + ".tgz"} - obj.Spec.VerificationKeyring = &sourcev1.VerificationKeyring{ - SecretRef: meta.LocalObjectReference{ - Name: "keyring-secret-0.2.0", - }, - Key: publicKeyFileName, - } }, want: sreconcile.ResultSuccess, assertFunc: func(g *WithT, obj *sourcev1.HelmChart, build chart.Build) { @@ -710,30 +738,14 @@ func TestHelmChartReconciler_buildFromHelmRepository(t *testing.T) { g.Expect(build.Version).To(Equal(chartVersion)) g.Expect(build.Path).To(Equal(filepath.Join(serverFactory.Root(), obj.Status.Artifact.Path))) g.Expect(build.Path).To(BeARegularFile()) - g.Expect(build.ProvFilePath).To(Equal(filepath.Join(serverFactory.Root(), obj.Status.Artifact.Path+".prov"))) - g.Expect(build.ProvFilePath).To(BeARegularFile()) }, }, { name: "Sets Generation as VersionMetadata with values files", - keyringSecret: &corev1.Secret{ - ObjectMeta: metav1.ObjectMeta{ - Name: "keyring-secret-0.3.0", - }, - Data: map[string][]byte{ - publicKeyFileName: keyring2, - }, - }, beforeFunc: func(obj *sourcev1.HelmChart, repository *sourcev1.HelmRepository) { obj.Spec.Chart = chartName obj.Generation = 3 obj.Spec.ValuesFiles = []string{"values.yaml", "override.yaml"} - obj.Spec.VerificationKeyring = &sourcev1.VerificationKeyring{ - SecretRef: meta.LocalObjectReference{ - Name: "keyring-secret-0.3.0", - }, - Key: publicKeyFileName, - } }, want: sreconcile.ResultSuccess, assertFunc: func(g *WithT, _ *sourcev1.HelmChart, build chart.Build) { @@ -741,24 +753,13 @@ func TestHelmChartReconciler_buildFromHelmRepository(t *testing.T) { g.Expect(build.Version).To(Equal(higherChartVersion + "+3")) g.Expect(build.Path).ToNot(BeEmpty()) g.Expect(build.Path).To(BeARegularFile()) - g.Expect(build.ProvFilePath).ToNot(BeEmpty()) - g.Expect(build.ProvFilePath).To(BeARegularFile()) }, cleanFunc: func(g *WithT, build *chart.Build) { g.Expect(os.Remove(build.Path)).To(Succeed()) - g.Expect(os.Remove(build.ProvFilePath)).To(Succeed()) }, }, { name: "Forces build on generation change", - keyringSecret: &corev1.Secret{ - ObjectMeta: metav1.ObjectMeta{ - Name: "keyring-secret-0.2.0", - }, - Data: map[string][]byte{ - publicKeyFileName: keyring1, - }, - }, beforeFunc: func(obj *sourcev1.HelmChart, repository *sourcev1.HelmRepository) { obj.Generation = 3 obj.Spec.Chart = chartName @@ -766,12 +767,6 @@ func TestHelmChartReconciler_buildFromHelmRepository(t *testing.T) { obj.Status.ObservedGeneration = 2 obj.Status.Artifact = &sourcev1.Artifact{Path: chartName + "-" + chartVersion + ".tgz"} - obj.Spec.VerificationKeyring = &sourcev1.VerificationKeyring{ - SecretRef: meta.LocalObjectReference{ - Name: "keyring-secret-0.2.0", - }, - Key: publicKeyFileName, - } }, want: sreconcile.ResultSuccess, assertFunc: func(g *WithT, obj *sourcev1.HelmChart, build chart.Build) { @@ -779,12 +774,9 @@ func TestHelmChartReconciler_buildFromHelmRepository(t *testing.T) { g.Expect(build.Version).To(Equal(chartVersion)) g.Expect(build.Path).ToNot(Equal(filepath.Join(serverFactory.Root(), obj.Status.Artifact.Path))) g.Expect(build.Path).To(BeARegularFile()) - g.Expect(build.ProvFilePath).ToNot(Equal(filepath.Join(serverFactory.Root(), obj.Status.Artifact.Path+".prov"))) - g.Expect(build.ProvFilePath).To(BeARegularFile()) }, cleanFunc: func(g *WithT, build *chart.Build) { g.Expect(os.Remove(build.Path)).To(Succeed()) - g.Expect(os.Remove(build.ProvFilePath)).To(Succeed()) }, }, { @@ -868,9 +860,6 @@ func TestHelmChartReconciler_buildFromHelmRepository(t *testing.T) { if tt.secret != nil { clientBuilder.WithObjects(tt.secret.DeepCopy()) } - if tt.keyringSecret != nil { - clientBuilder.WithObjects(tt.keyringSecret.DeepCopy()) - } storage, err := newTestStorage(server) g.Expect(err).ToNot(HaveOccurred()) @@ -912,9 +901,6 @@ func TestHelmChartReconciler_buildFromHelmRepository(t *testing.T) { defer tt.cleanFunc(g, &b) } got, err := r.buildFromHelmRepository(context.TODO(), obj, repository, &b) - if err != nil { - t.Logf("error: %s", err) - } g.Expect(err != nil).To(Equal(tt.wantErr != nil)) if tt.wantErr != nil { @@ -952,28 +938,18 @@ func TestHelmChartReconciler_buildFromTarballArtifact(t *testing.T) { g.Expect(storage.CopyFromPath(yamlArtifact, "testdata/charts/helmchart/values.yaml")).To(Succeed()) cachedArtifact := &sourcev1.Artifact{ Revision: "0.1.0", - Path: "helmchart-0.1.0.tgz", + Path: "cached.tgz", } g.Expect(storage.CopyFromPath(cachedArtifact, "testdata/charts/helmchart-0.1.0.tgz")).To(Succeed()) - provArtifact := &sourcev1.Artifact{ - Revision: "1234smth", - Path: "helmchart-0.1.0.tgz.prov", - } - g.Expect(storage.CopyFromPath(provArtifact, "testdata/charts/helmchart-0.1.0.tgz.prov")).To(Succeed()) - - keyring, err := os.ReadFile("testdata/charts/pub.gpg") - g.Expect(err).ToNot(HaveOccurred()) - tests := []struct { - name string - source sourcev1.Artifact - keyringSecret *corev1.Secret - beforeFunc func(obj *sourcev1.HelmChart) - want sreconcile.Result - wantErr error - assertFunc func(g *WithT, build chart.Build) - cleanFunc func(g *WithT, build *chart.Build) + name string + source sourcev1.Artifact + beforeFunc func(obj *sourcev1.HelmChart) + want sreconcile.Result + wantErr error + assertFunc func(g *WithT, build chart.Build) + cleanFunc func(g *WithT, build *chart.Build) }{ { name: "Resolves chart dependencies and builds", @@ -1037,24 +1013,9 @@ func TestHelmChartReconciler_buildFromTarballArtifact(t *testing.T) { { name: "Chart from storage cache", source: *chartsArtifact.DeepCopy(), - keyringSecret: &corev1.Secret{ - ObjectMeta: metav1.ObjectMeta{ - Name: "keyring-secret", - Namespace: "default", - }, - Data: map[string][]byte{ - publicKeyFileName: keyring, - }, - }, beforeFunc: func(obj *sourcev1.HelmChart) { obj.Spec.Chart = "testdata/charts/helmchart-0.1.0.tgz" obj.Status.Artifact = cachedArtifact.DeepCopy() - obj.Spec.VerificationKeyring = &sourcev1.VerificationKeyring{ - SecretRef: meta.LocalObjectReference{ - Name: "keyring-secret", - }, - Key: publicKeyFileName, - } }, want: sreconcile.ResultSuccess, assertFunc: func(g *WithT, build chart.Build) { @@ -1062,21 +1023,11 @@ func TestHelmChartReconciler_buildFromTarballArtifact(t *testing.T) { g.Expect(build.Version).To(Equal("0.1.0")) g.Expect(build.Path).To(Equal(storage.LocalPath(*cachedArtifact.DeepCopy()))) g.Expect(build.Path).To(BeARegularFile()) - g.Expect(build.ProvFilePath).To(Equal(storage.LocalPath(*provArtifact.DeepCopy()))) - g.Expect(build.ProvFilePath).To(BeARegularFile()) }, }, { name: "Generation change forces rebuild", source: *chartsArtifact.DeepCopy(), - keyringSecret: &corev1.Secret{ - ObjectMeta: metav1.ObjectMeta{ - Name: "keyring-secret", - }, - Data: map[string][]byte{ - publicKeyFileName: keyring, - }, - }, beforeFunc: func(obj *sourcev1.HelmChart) { obj.Generation = 2 obj.Spec.Chart = "testdata/charts/helmchart-0.1.0.tgz" @@ -1117,13 +1068,8 @@ func TestHelmChartReconciler_buildFromTarballArtifact(t *testing.T) { t.Run(tt.name, func(t *testing.T) { g := NewWithT(t) - clientBuilder := fake.NewClientBuilder() - if tt.keyringSecret != nil { - clientBuilder.WithObjects(tt.keyringSecret.DeepCopy()) - } - r := &HelmChartReconciler{ - Client: clientBuilder.Build(), + Client: fake.NewClientBuilder().Build(), EventRecorder: record.NewFakeRecorder(32), Storage: storage, Getters: testGetters, @@ -1189,7 +1135,7 @@ func TestHelmChartReconciler_reconcileArtifact(t *testing.T) { }, afterFunc: func(t *WithT, obj *sourcev1.HelmChart) { t.Expect(obj.GetArtifact()).ToNot(BeNil()) - t.Expect(obj.GetArtifact().Checksum).To(Equal("5fabb8b212945e7187a24a5e893d06fe98c83f3c49ed0f7b6df6de633d95a8f3")) + t.Expect(obj.GetArtifact().Checksum).To(Equal("007c7b7446eebcb18caeffe9898a3356ba1795f54df40ad39cfcc7382874a10a")) t.Expect(obj.GetArtifact().Revision).To(Equal("0.1.0")) t.Expect(obj.Status.URL).ToNot(BeEmpty()) t.Expect(obj.Status.ObservedChartName).To(Equal("helmchart")) @@ -1200,24 +1146,21 @@ func TestHelmChartReconciler_reconcileArtifact(t *testing.T) { }, }, { - name: "A build with a non-nil ProvFilePath persists prov file to storage", - build: mockChartBuild("helmchart", "0.1.0", "testdata/charts/helmchart-0.1.0.tgz", "testdata/charts/helmchart-0.1.0.tgz"), + name: "Build with a verified signature sets SourceVerifiedCondition=Truue", + build: mockChartBuild("helmchart", "0.1.0", "testdata/charts/helmchart-0.1.0.tgz", "testdata/charts/helmchart-0.1.0.tgz.prov"), beforeFunc: func(obj *sourcev1.HelmChart) { - conditions.MarkTrue(obj, sourcev1.ArtifactOutdatedCondition, "Foo", "") + obj.Status.Artifact = &sourcev1.Artifact{ + Path: "testdata/charts/helmchart-0.1.0.tgz", + } }, + want: sreconcile.ResultSuccess, afterFunc: func(t *WithT, obj *sourcev1.HelmChart) { - provArtifact := testStorage.NewArtifactFor(obj.Kind, obj.GetObjectMeta(), "0.1.0", fmt.Sprintf("%s-%s.tgz.prov", "helmchart", "0.1.0")) + provArtifact := testStorage.NewArtifactFor(obj.Kind, obj.GetObjectMeta(), "0.1.0", "helmchart-0.1.0.tgz.prov") t.Expect(provArtifact.Path).ToNot(BeEmpty()) - t.Expect(obj.GetArtifact()).ToNot(BeNil()) - fmt.Printf("checksum: %s", obj.GetArtifact().Checksum) - t.Expect(obj.GetArtifact().Checksum).To(Equal("5fabb8b212945e7187a24a5e893d06fe98c83f3c49ed0f7b6df6de633d95a8f3")) - t.Expect(obj.GetArtifact().Revision).To(Equal("0.1.0")) - t.Expect(obj.Status.URL).ToNot(BeEmpty()) - t.Expect(obj.Status.ObservedChartName).To(Equal("helmchart")) }, - want: sreconcile.ResultSuccess, assertConditions: []metav1.Condition{ *conditions.TrueCondition(meta.ReadyCondition, sourcev1.ChartPullSucceededReason, "pulled 'helmchart' chart with version '0.1.0'"), + *conditions.TrueCondition(sourcev1.SourceVerifiedCondition, sourcev1.ChartPullSucceededReason, "chart signed by: TestUser1,TestUser2 using key with fingeprint: 0102000000000000000000000000000000000000 and hash verified: 53gntj23r24asnf0"), }, }, { @@ -1271,7 +1214,7 @@ func TestHelmChartReconciler_reconcileArtifact(t *testing.T) { }, afterFunc: func(t *WithT, obj *sourcev1.HelmChart) { t.Expect(obj.GetArtifact()).ToNot(BeNil()) - t.Expect(obj.GetArtifact().Checksum).To(Equal("5fabb8b212945e7187a24a5e893d06fe98c83f3c49ed0f7b6df6de633d95a8f3")) + t.Expect(obj.GetArtifact().Checksum).To(Equal("007c7b7446eebcb18caeffe9898a3356ba1795f54df40ad39cfcc7382874a10a")) t.Expect(obj.GetArtifact().Revision).To(Equal("0.1.0")) t.Expect(obj.Status.URL).ToNot(BeEmpty()) t.Expect(obj.Status.ObservedChartName).To(Equal("helmchart")) @@ -1786,6 +1729,7 @@ func TestHelmChartReconciler_reconcileSubRecs(t *testing.T) { func mockChartBuild(name, version, path, provFilePath string) *chart.Build { var copyP string var copyPP string + var verSig *chart.VerificationSignature if path != "" { f, err := os.Open(path) if err == nil { @@ -1810,12 +1754,18 @@ func mockChartBuild(name, version, path, provFilePath string) *chart.Build { copyPP = ff.Name() } } + verSig = &chart.VerificationSignature{ + FileHash: "53gntj23r24asnf0", + Identities: []string{"TestUser1", "TestUser2"}, + KeyFingerprint: [20]byte{1, 2}, + } } } return &chart.Build{ - Name: name, - Version: version, - Path: copyP, - ProvFilePath: copyPP, + Name: name, + Version: version, + Path: copyP, + ProvFilePath: copyPP, + VerificationSignature: verSig, } } diff --git a/controllers/storage.go b/controllers/storage.go index f8016f5fe..bd39b66b4 100644 --- a/controllers/storage.go +++ b/controllers/storage.go @@ -120,7 +120,6 @@ func (s *Storage) RemoveAll(artifact sourcev1.Artifact) (string, error) { func (s *Storage) RemoveAllButCurrent(artifact sourcev1.Artifact) ([]string, error) { deletedFiles := []string{} localPath := s.LocalPath(artifact) - localProvPath := localPath + ".prov" dir := filepath.Dir(localPath) var errors []string _ = filepath.Walk(dir, func(path string, info os.FileInfo, err error) error { @@ -129,7 +128,7 @@ func (s *Storage) RemoveAllButCurrent(artifact sourcev1.Artifact) ([]string, err return nil } - if path != localPath && path != localProvPath && !info.IsDir() && info.Mode()&os.ModeSymlink != os.ModeSymlink { + if path != localPath && !info.IsDir() && info.Mode()&os.ModeSymlink != os.ModeSymlink { if err := os.Remove(path); err != nil { errors = append(errors, info.Name()) } else { @@ -146,6 +145,34 @@ func (s *Storage) RemoveAllButCurrent(artifact sourcev1.Artifact) ([]string, err return deletedFiles, nil } +func (s *Storage) RemoveConditionally(dir string, callbacks []func(path string, info os.FileInfo) bool) ([]string, error) { + deletedFiles := []string{} + var errors []string + _ = filepath.Walk(dir, func(path string, info os.FileInfo, err error) error { + if err != nil { + errors = append(errors, err.Error()) + return nil + } + for _, callback := range callbacks { + if callback(path, info) { + if err := os.Remove(path); err != nil { + errors = append(errors, info.Name()) + } else { + // Collect the successfully deleted file paths. + deletedFiles = append(deletedFiles, path) + } + break + } + } + return nil + }) + + if len(errors) > 0 { + return deletedFiles, fmt.Errorf("failed to remove files: %s", strings.Join(errors, " ")) + } + return deletedFiles, nil +} + // ArtifactExist returns a boolean indicating whether the v1beta1.Artifact exists in storage and is a regular file. func (s *Storage) ArtifactExist(artifact sourcev1.Artifact) bool { fi, err := os.Lstat(s.LocalPath(artifact)) diff --git a/controllers/testdata/charts/helmchart-0.1.0.tgz b/controllers/testdata/charts/helmchart-0.1.0.tgz index a43147fd87321deb4d03de1d89d5e16a7d4a6baf..186a1ddb788b83cff243f8ffa684859c1700a8b1 100644 GIT binary patch delta 3390 zcmV-E4Z-rz8Q2<-JAXX=Z`-(%{aJs-oYIGWTrJDaM+*TxpqKXcg4a!hrmqi+Vo}i2 z*ye^JwIt=l=k@*V2a=K{%W~|^W|Lby%@2+&lQY91=Yun3rpVHGN~G?en$KWPQugGY zAq>OtC`F&)+)L}3PLhrH1-ocY6mHy8N%TfIq24IK&ABKaIrv4wEyzcb> zA>aVsLMhF>E3KO7Q3z&;sg2pssM578kPogyory}P%f`RAJluR)403>EYQ7z;=0zRSq z#skQ5#x4Y-@jQaF@ww0!5*2b?bi2qIyTn+cRuO#r*?(H1PcxK~CK!S5d*ht3jhCK6 zIioSYi(|`$2XFwF8OC%>QNi^T^%Nx#DB1yW4M9ys&Jr`kSR&Dwcu+h=pkkWC=y{Y+Br3}ka@uH2QYinb zw|`xLS^z^bL(oR|iP8O7a1~RLJGSDfFl*V^3bO=rg)(r4f-oh3a6${6N{ZuZkmwfmt5K3Q$v7bjDJcIIe%rxFEJH%^ayb-*&IfaSlNs z6OEnm-^p zW+o_oY2k=rxwI`L;S+?X#(_cfcB8d*Y^E(h)jF{5(!HH(oh=rI(9>pL%}g7$Sb%YA z%VpH>FBXoJ#S6>jVgW&g-8J)?kAF;|!5<5*3FQj>bCKYMklJ6dTf`%Q2}T?~yfI-2 zoPT=%;rqp>&tKj?eQvx{tQkyDL$7DTFO`uP!7%X|nQ?}}`ReU*2|WS7D8bEW-};Nd zc(K4>Ro}v&170w(^vkeO8BCPNEKd;pDn2iRKi`h^MT5Dsiw0{3vAPq3RDWY?c~hF` zTm>Xa%tWAlSA1FemVtOG?9@g5{?meu=;=yWTjc&xkmNrJGtq*)P4)m_%u@ts8d5R` zO|B3KjPV*$%5z;hy(vN+C7fLpyAm=Zl$m836!DpbpF|EZ;c!iKJazH99jA%jYkKhP zch9q$#n8LnYxbU1vv>L7^MBW~)0PRe==Pzz4?*blN}~_{V&N|qg%UoeJi!|%=bF_7 zS!y7JFRYFQR?(bF`OB6SPz@zA%r++0j&`dut|I?EwDIE8`|s!Pe*Dl7SSFHv7d!)> zWLdBt{hN+&+l5&P^3`TP%9S<(d)uUE0tOe2!k*f zL~p`3VX>2RTkM(*Hyf2w5_<{2h|z5CuZc|HX(>-IG@>_Q-E=;-)9T>fpe_FUK1G(H zRDsU!B^9_U{yQEVHsZhIhAu|TaJUb3gUE++aXc&JthhKBT=$K++%%kN!@cxMoyKS!x4a2K~5yr$=QzcTY z6*t_7#}rwjfJk&(ezd6)nO|W2Mx)*_WHh%``%&kMRa&NAP;!=AW zt|?Q+@-Rqx*YS+P3Gl6e;1t+X)3tmQr*U-YvItCw}G1W?Jv=m zV6+%k;Nriw7OF^keJoH1P+Q_|sP9$&E;KfFhMQPY!pQ`a z-e?}J#ZQ+-QomJlWy)?3*)<2+YvsZfSa$`ZEgj2XG=JI%{@OV&R&mqPx0f((A6LD? za=y(>tv3sb`bV$W@GTZA+w%POJe(-_voBUGd-GsCoY5WOy8Q@!vy0{rm^XGS#0AyerC+2;Q$&fmd!zqq4NPS9-yN z!tzChs(7ih8iw(kf@X3e~Kaj~%JfggRm=(ed_Ile?w^{a}j%dA}+{#ihvdt=W8?R6Xi+?CLYZovt^H7w;YC?U zU;jOmLW@|i2)>+N)P7@hh8&f;kYa@E)C`p$upw4g_Ku+6d~3-z7?Vg6`X^#9tF^l( zzJDr$w@>+oPSy*vRkO8n?cTb$p^Y2V${KIgV^#INtHy88s1=qgGWBZnT_`=akLzGN zw8ejAk?p>Y0`7?ahT&kik^k$?|2_=tsN}A)JSMa|FETwlBEMB1HH(d#sn>qj>OQrd zpu%*9@>HZ5kqG{Oyd4CCqh9Dka9TTo7JvLbA7SA@gCD3aeW}|nSAi@?eJ$h_<&)s* zjY|UzMySbPeFIqUf;Squc~t9dodt6d@tG4&dpKN4=nqG}ZP8ybFSV0xu4VnGn|F&{ zKH4aSyI6C}{JZ*kabj(~K)qO8pLcHo&RpUs`=2tR0avU8?W5)qnN@ zWX^C0=W9%r=lBc_dw^hrHCuf1>H$c@8*Uz8s7CAJmFh@B1tSt)Ay1kCPN690Tb5T# zQy0;hrlsE1TCT~aU1pDMi8^=;X!HN2k$xcmA08jS-t_;+o&SFbSiApIrhbaF_T%P# z+~Mz*Gb7c}kJ{e1S8V=nIb%|6W`BsBpuTAC6x%kwYrE+6{L&qgJV*a8jqEM@U!?i} zxEa``|A)h7{r~kK9CZ5s5U_V8<~Bo7G~e4^+>$&jEk}FDtw!T7Ux#@tXw!dtW59m) zV!=7aW_$4m%)l=FKM5P>zlY)BQRn|31X|VlLL@D^?C<0JdyQq|-(9C$Hh;H%d9YwA z#FZb}yDdGH$FbR#mw`jGPEe-9uP3PMC6fq7xtfo}&8og&kGDtg?U`x%?B_DeDkmxe z`1Wj{@!xXD)gqK=)|)QV{O34B`Ir`~(*M9b(g_z5@0rHV_SA`?`r%fKqnY~&4Uij2X8iIFR~Qa*VF5*g8ShBoI> zdrx@c9RSCPyJ7M?lQ^a~n7BmOzr6_H6K8V}-1amqL59*^*bBVi{pI&dEhKseP@G8m zdU^>7mC6eyRQK(tW8Vu#|0DbMvwSg~^vy^4Rn2(6LNp@rRi4?ikAKR08K~>bdl`(# zmG?5xY39BBUwfZUQjse-d;dXsK_ThdK`Q-Ea(8t6 z{`UPP?9l&~`~OGb(XiA1hk)+>e|P`CyZ_(a|L^YqclZCh`~UxXP{n_uSPS?6AE0)| zf5Wi3|9c&F@!x~M0XMAf#hxfL#FC1%Q820C)hqLO}cKvHej89dyt^ U2M+`P7XSeN|39#41^`?D02V^B2><{9 delta 3253 zcmV;m3`+CZ8qgV#JAXUvZri$&{jH~%ReI5ntz|iJ(n3HM=qBB>!R<+nrneUdMN!bw z*k(hKIwa-9>w2GkK~l10Sx($EH`xQ2KN49U&J2h1#Tj~nER82b>h6j88_Y<`p4{^Y z!!SJR_wD~M44ePM-s}F8UVp#W>-P`$UmrdRdk4M#>n9N2H-95-BUhTpC*d#CYA)_C zGDt>0qf}Jz2&VfU$+G3gUeG%VLN7rTOPXo>?k)T`vJ|Ywz(`2=dp<$XU?oCV7VbBXg@3Ggu8&958_CF;oNA<680Jqrx{_FiU`|tO{o&7(= z*n?9Xk(_C8`+skZdZ`qFw+EjlQ~^~$;E%VzzweBMOo`T*z=$$50H+u;A`zxUQZi(y zKnoZmWJD=UfO0KhCUOZH(~J>~D)2ms)-9Zfoa+dBo<~zM#>fMZmO5c$_WS{ks>RfJMi}X zd;2VrBp*1NeH23f7iFksN@H3Rh|Kw0buPF8Xukf3LK#7?V8sf`P;e3Cgyl-3JR2BA z5K>3*CiFbY#}bug3b|3cB!UrP%3vj&3vEM=Dt}gDC|xc?RX|0zm>a_|0$kgJ?9HUo*VNK@pA)iW(XSv8+QQW*bowpBna zfPW#GB4}g##Mpi;xQeOB9b5547+p5L!l+=bPzKIX5T*oBZd6#IGOY60{g+yq%?7~% z`IJh*Q{-B~71a}9RBKi!M9Io4PVGhBITYyofUj!B`dHiHf}WyaRLu!Dp8bCKXcNbN7#E#gsNf|0|A*MBAq!TE>Nci#pdK7KlW`rLS@STh)-hEB(XUzL#< z!7%X=nQ?}}`SR^z0UZI~mEdNz$NoGpUMw(J)tB&VzzZf8ewj8XgNgE(zd6i5BuE*#m%aPZ6AHNXZN|xkMl^!YfEA&vlW(8hx&#gtI}Bm69o;%&2Kl z#AiGH5IMwz!xho-#N}&tohEy$*}=1)-e)z9p>ws>?meq^@8aFZ&u1qs8)(sOLw6g3 z(CL&;AN={;pU(>;d`@|S*MCs%HLD4-v_J@7SQ`thp*fTC7cCl4EhRF{Ha6C-cB3&a zGrujYaq!{v+xgqy-!%l5iDX-aXW)}83s$o~Gx4}xn3W)3t?0Dne|=6H8SWCBl1 zd4i!4y$S2C^Ra{0Zrs~w%m2PjkYy-UptE}^1>Kha9rg|y`QPDTe}9+%J;a#LyDwo% z)5umIMwFrPtjDPlOFY2{UUsc}?!NTiU1#=i*eGlig`xtkV~&)Rw>Zj9^}RiE2Cm^? zePZd|>opqq0b1rfhJT|2`$*G^d^Doh;CCuy#vz$!$Ki=9oRB3A6Ypm5XHFO$QA~_` zw7du2Kha^g@Y>KYyeb%COpG^GBE?#9+pTy+ktGUXt<(`nfxSIhEH?kHp~~?-hUw6Dx{Y(sU_yky4MnB zBfwTdH-WnR@YF6KI^KR?YkV$c;jpX*$|bKWZ-1J>Oj52#;Qvi^{-*o}Hb?hoG;P!K ziU!uU)|gDu_b}UPx3|z02gS$`*^OIl|N0vjRF}7VE?PM710-h5gm?eneel2ecd;pv zrh+>z)lFPF2!DshybaW(Yrly$B%{T$0+;`_L8zkW@xDMEKuyKnQr|8cR_43)JZ#x= zGHK7+lppYCE;KeyhU-{T!pRtu&Ttm3*)S#-_vz~+w#9&zuEsg+CS>=^1p`|_5MG}GS!{- zyi3ZH2u_z;;FZVHsBA55RbKF*@O)8WYMr(@Z0Xy70H-`buAy&coAjyZ2bv?Ta?YzT z&3gOLk$);(s3VpV9Z%lZsBfgd9%Cv#jvVUJG78i+VVJ>IsN9Lpo3gf5uGwL+osnhb zv(e~WmcwAJ1-inD7JrbGwUevGJU3z8UvJv#HqRc^5v}$UTUjbrw%(;|;}yze5oK+K zok}0;dC+psjl68&M{Q)Lx4(+n^{m0KDW-iDRDV5eSS+H})MK&mo0tZ*F83PBss+Bv zgDo1buBjBtMC)wji<%4vWg~U<`%DTgV!N1wP<8qbAa@1EsUQ#{|F5kE^KyQee^nX?lfb}VOt&y8YwF>Jzn9GRIoN(IH;Yvck zocXq8f5pDkZZxNqmVsX(l*@p`33}uNF|3(HYRv?rM-LvT2vuV|$|Ac#P2&|4S$RK=D63Jbb+t z{||e+`2P@N<@X8B`bKW^Q}UH)ykGg4jssGWT~#o_OUJ0`_phR8AMLGz^8cJN&} zMX%PE?oj0E|G#vyH`sqs=Kpm!aDSWqAM7{#|F3(!!=3#<#MrtMbCaVez_<1nHxv&` z&(S_{qtp1y_hBAuwAsH#rQP37EI7y594~&s4cun`M`7ds_kMVAw2S`_GFr`gAd(ha z_O}WCorc=@cem-5!>wOlESLy!=|^CRsXUI&vAj$intg(@9DX%MU2mC0Fn`R|Y$&dm z{RMlyJ%TUK4Cu43WtCNqRRr+m**5FH;gYLGDADMfF3bGqI7RuG9;>qdz&z417ZUH8 z#;y7P{!#Pyp9hD#`2Qed4+cbQl-&Mm(``RoO_0Mdr!1j-3>k?p$r#o0$Dk_DY~~GW zf{ekKiJ@z_Qa*kK5*g8Sihs7|QF~8#;_U&)vHOL|^GxE1USr}4UH|qXfDfF_KyW+L zuml-O`-82(3r;V-U1%ZE+k@gp(&v*4NT^g^Fs8a||2p=)VE8|>YyXxnCgZO8Q+`xa z-mMS~Nqm`S_U@zdUIywa^Iir+a_PMcbeefD|JU1t&qPv@D>yrSr!Kr8lj1wZ+6!oc nr0d|M`0fQ$6^jJByA)(Mc4IgG0^|Pz00960yXnmo07d`+0s(b5 diff --git a/controllers/testdata/charts/helmchart-0.1.0.tgz.prov b/controllers/testdata/charts/helmchart-0.1.0.tgz.prov index df5962f5f..7c44d8c25 100644 --- a/controllers/testdata/charts/helmchart-0.1.0.tgz.prov +++ b/controllers/testdata/charts/helmchart-0.1.0.tgz.prov @@ -10,17 +10,17 @@ version: 0.1.0 ... files: - helmchart-0.1.0.tgz: sha256:5fabb8b212945e7187a24a5e893d06fe98c83f3c49ed0f7b6df6de633d95a8f3 + helmchart-0.1.0.tgz: sha256:007c7b7446eebcb18caeffe9898a3356ba1795f54df40ad39cfcc7382874a10a -----BEGIN PGP SIGNATURE----- -wsDcBAEBCgAQBQJiJkq8CRBwNbqX0yqHwQAAJ5sMAKTMqzOLvrunXBQ8TPXcqXGc -bzZ9MSrK3mBzQlhZHxabCZP25vemUwGsvNyS2BS8ow+dDzaug8luZBw1aC5slSyS -uAG9bkcyqHwsHnJ+Z8O5cpYsmOzhA4CDSsq1xICxCKYy2jZPj+I8KBk5INtSi6gP -lB1YV4ry42D2BsxK7IuPr5iUzecsVNsMbvupVLUSqsR3k3A2plGhBw10yCnYxEFg -MYlUdhHKFOSiUEmgif7toEQFfofxoutcPaAHe1zRYE4t1M2AozGx3njXchTdkHbe -WvsNc96wrFAGu852VXC6hyJH2keWhY91vaoELVbkDYnCiHouu/yXE4ox2MgN6Kr0 -u1gNeaEtk+IxYthDuBBRfslQ6O6lIfi3vObanC6Wl1pmC3YyYtnFOz6VQzW9k+Sv -FF9l6ysxoYxGWzAmIhGDIoohKwD1LtRBDjWHPivsOkYyEjmFb0MUsgFHuhVRMV4Z -aTckCGBebQS0bR3wE4GaGV1sLVoZDXph7v7YGa7Enw== -=7DS/ +wsDcBAEBCgAQBQJiKwNBCRBwNbqX0yqHwQAACj8MABCY6mVrWaJdC64PbhTTonVE +97MZZpQBT+CZIRAecfkvcTeMTBeKh/yRwsSmjwo46eKOpNFJ1eQVHqVcKWLfBn3Z +AijuXTaISl8SnQyKPF2Z8n+YrYwh9OWPUX2CpUQstx+snSLDuv5ltWIgRlzfHAUN +hwzsgjs8bpHe8wZTgnASUVbcMMYQXCcovbXB6NATDLkZLHBWWEISicOl6VYLLl2D +kZg7LDcDKPcPmKJ6WtVurkyWXhK3jdYzlaOQWjs2nLIH/CdlmAygELuWexsOZAhY +MEauKEMoVzDQF5oaNA78AzlBLGogxao5fBYtAAHGb5tQdnVRUeSci+7IR0LHsS05 +YF/UnUF69GSESfoKIBvQuzex4BRCLBwayq6CSyrpZQ2+Vg4ARPo7LFg7Wy0zvC9Z +NxGnIeh1az9hltdzPgg6ZahPZB+eMF+t9ouAz9OZ3kxYUDmoE+Z+NqRWsPi27Cxk +CSw9EfJfDsputN/wj4NAxZKfqauMtS5sgaSgtrW+zA== +=mfBq -----END PGP SIGNATURE----- \ No newline at end of file diff --git a/internal/helm/chart/builder.go b/internal/helm/chart/builder.go index 25bc19563..50fa9ffc5 100644 --- a/internal/helm/chart/builder.go +++ b/internal/helm/chart/builder.go @@ -85,7 +85,7 @@ type Builder interface { // Reference and BuildOptions, and writes it to p. // It returns the Build result, or an error. // It may return an error for unsupported Reference implementations. - Build(ctx context.Context, ref Reference, p string, opts BuildOptions, keyring []byte) (*Build, error) + Build(ctx context.Context, ref Reference, p string, opts BuildOptions) (*Build, error) } // BuildOptions provides a list of options for Builder.Build. @@ -104,6 +104,10 @@ type BuildOptions struct { // Force can be set to force the build of the chart, for example // because the list of ValuesFiles has changed. Force bool + + // Keyring can be set to the data of the chart VerificationKeyring secret + // used for verifying a chart's signature using a provenance file. + Keyring []byte } // GetValuesFiles returns BuildOptions.ValuesFiles, except if it equals @@ -129,6 +133,9 @@ type Build struct { // Can be empty, in which case it should be assumed that the packaged // chart is not verified. ProvFilePath string + // VerificationSignature is populated when a chart's signature + // is susccessfully verified using it's provenance file. + VerificationSignature *VerificationSignature // ValuesFiles is the list of files used to compose the chart's // default "values.yaml". ValuesFiles []string @@ -161,7 +168,6 @@ func (b *Build) Summary() string { if len(b.ValuesFiles) > 0 { s.WriteString(fmt.Sprintf(" and merged values files %v", b.ValuesFiles)) } - return s.String() } diff --git a/internal/helm/chart/builder_local.go b/internal/helm/chart/builder_local.go index cbbe840b8..6885e1d40 100644 --- a/internal/helm/chart/builder_local.go +++ b/internal/helm/chart/builder_local.go @@ -26,6 +26,7 @@ import ( "github.com/Masterminds/semver/v3" securejoin "github.com/cyphar/filepath-securejoin" "helm.sh/helm/v3/pkg/chart/loader" + "helm.sh/helm/v3/pkg/provenance" "sigs.k8s.io/yaml" "github.com/fluxcd/pkg/runtime/transform" @@ -63,7 +64,7 @@ func NewLocalBuilder(dm *DependencyManager) Builder { // If the LocalReference.Path refers to a chart directory, dependencies are // confirmed to be present using the DependencyManager, while attempting to // resolve any missing. -func (b *localChartBuilder) Build(ctx context.Context, ref Reference, p string, opts BuildOptions, keyring []byte) (*Build, error) { +func (b *localChartBuilder) Build(ctx context.Context, ref Reference, p string, opts BuildOptions) (*Build, error) { localRef, ok := ref.(LocalReference) if !ok { err := fmt.Errorf("expected local chart reference") @@ -105,37 +106,43 @@ func (b *localChartBuilder) Build(ctx context.Context, ref Reference, p string, isChartDir := pathIsDir(localRef.Path) requiresPackaging := isChartDir || opts.VersionMetadata != "" || len(opts.GetValuesFiles()) != 0 - // If all the following is true, we do not need to package the chart: - // - Chart name from cached chart matches resolved name - // - Chart version from cached chart matches calculated version - // - BuildOptions.Force is False var provFilePath string - verifyProvFile := func(chart, provFile string) error { - if keyring != nil { + verifyProvFile := func(chart, provFile string) (*provenance.Verification, error) { + if opts.Keyring != nil { if _, err := os.Stat(provFile); err != nil { err = fmt.Errorf("could not load provenance file %s: %w", provFile, err) - return &BuildError{Reason: ErrProvenanceVerification, Err: err} + return nil, &BuildError{Reason: ErrProvenanceVerification, Err: err} } - err := VerifyProvenanceFile(bytes.NewReader(keyring), chart, provFile) + ver, err := verifyChartWithProvFile(bytes.NewReader(opts.Keyring), chart, provFile) if err != nil { err = fmt.Errorf("failed to verify helm chart using provenance file: %w", err) - return &BuildError{Reason: ErrProvenanceVerification, Err: err} + return nil, &BuildError{Reason: ErrProvenanceVerification, Err: err} } + return ver, nil } - return nil + return nil, nil } + + // If all the following is true, we do not need to package the chart: + // - Chart name from cached chart matches resolved name + // - Chart version from cached chart matches calculated version + // - BuildOptions.Force is False if opts.CachedChart != "" && !opts.Force { if curMeta, err = LoadChartMetadataFromArchive(opts.CachedChart); err == nil { // If the cached metadata is corrupt, we ignore its existence // and continue the build if err = curMeta.Validate(); err == nil { if result.Name == curMeta.Name && result.Version == curMeta.Version { + // We can only verify a cached chart with provenance file if we didn't + // package the chart ourselves, and instead stored it as is. if !requiresPackaging { provFilePath = provenanceFilePath(opts.CachedChart) - if err = verifyProvFile(opts.CachedChart, provFilePath); err != nil { + if ver, err := verifyProvFile(opts.CachedChart, provFilePath); err != nil { return nil, err + } else { + result.VerificationSignature = buildVerificationSig(ver) + result.ProvFilePath = provFilePath } - result.ProvFilePath = provFilePath } result.Path = opts.CachedChart result.ValuesFiles = opts.GetValuesFiles() @@ -156,11 +163,13 @@ func (b *localChartBuilder) Build(ctx context.Context, ref Reference, p string, if err = copyFileToPath(provenanceFilePath(localRef.Path), provFilePath); err != nil { return result, &BuildError{Reason: ErrChartPull, Err: err} } - if err = verifyProvFile(localRef.Path, provFilePath); err != nil { + if ver, err := verifyProvFile(localRef.Path, provFilePath); err != nil { return result, err + } else { + result.ProvFilePath = provFilePath + result.VerificationSignature = buildVerificationSig(ver) } result.Path = p - result.ProvFilePath = provFilePath return result, nil } diff --git a/internal/helm/chart/builder_local_test.go b/internal/helm/chart/builder_local_test.go index e108250c3..a3543a605 100644 --- a/internal/helm/chart/builder_local_test.go +++ b/internal/helm/chart/builder_local_test.go @@ -110,6 +110,7 @@ func TestLocalBuilder_Build(t *testing.T) { { name: "already packaged chart", reference: LocalReference{Path: "./../testdata/charts/helmchart-0.1.0.tgz"}, + buildOpts: BuildOptions{Keyring: keyring}, wantVersion: "0.1.0", wantPackaged: false, }, @@ -215,7 +216,7 @@ fullnameOverride: "full-foo-name-override"`), ) b := NewLocalBuilder(dm) - cb, err := b.Build(context.TODO(), tt.reference, targetPath, tt.buildOpts, keyring) + cb, err := b.Build(context.TODO(), tt.reference, targetPath, tt.buildOpts) if tt.wantErr != "" { g.Expect(err).To(HaveOccurred()) @@ -226,6 +227,10 @@ fullnameOverride: "full-foo-name-override"`), g.Expect(err).ToNot(HaveOccurred()) g.Expect(cb.Packaged).To(Equal(tt.wantPackaged), "unexpected Build.Packaged value") g.Expect(cb.Path).ToNot(BeEmpty(), "empty Build.Path") + if tt.buildOpts.Keyring != nil { + g.Expect(cb.ProvFilePath).ToNot(BeEmpty(), "empty Build.ProvFilePath") + g.Expect(cb.VerificationSignature).ToNot(BeNil(), "nil Build.VerificationSignature") + } // Load the resulting chart and verify the values. resultChart, err := loader.Load(cb.Path) @@ -262,7 +267,7 @@ func TestLocalBuilder_Build_CachedChart(t *testing.T) { // Build first time. targetPath := filepath.Join(tmpDir, "chart1.tgz") buildOpts := BuildOptions{} - cb, err := b.Build(context.TODO(), reference, targetPath, buildOpts, keyring) + cb, err := b.Build(context.TODO(), reference, targetPath, buildOpts) g.Expect(err).ToNot(HaveOccurred()) // Set the result as the CachedChart for second build. @@ -270,17 +275,46 @@ func TestLocalBuilder_Build_CachedChart(t *testing.T) { targetPath2 := filepath.Join(tmpDir, "chart2.tgz") defer os.RemoveAll(targetPath2) - cb, err = b.Build(context.TODO(), reference, targetPath2, buildOpts, keyring) + cb, err = b.Build(context.TODO(), reference, targetPath2, buildOpts) g.Expect(err).ToNot(HaveOccurred()) g.Expect(cb.Path).To(Equal(targetPath)) // Rebuild with build option Force. buildOpts.Force = true - cb, err = b.Build(context.TODO(), reference, targetPath2, buildOpts, keyring) + cb, err = b.Build(context.TODO(), reference, targetPath2, buildOpts) g.Expect(err).ToNot(HaveOccurred()) g.Expect(cb.Path).To(Equal(targetPath2)) } +func TestLocalBuilder_VerifyCachedChartSig(t *testing.T) { + g := NewWithT(t) + + reference := LocalReference{Path: "./../testdata/charts/helmchart-0.1.0.tgz"} + + keyring, err := os.ReadFile("./../testdata/charts/pub.gpg") + g.Expect(err).ToNot(HaveOccurred()) + g.Expect(keyring).ToNot(BeEmpty()) + + dm := NewDependencyManager() + b := NewLocalBuilder(dm) + + tmpDir, err := os.MkdirTemp("", "local-chart-") + g.Expect(err).ToNot(HaveOccurred()) + defer os.RemoveAll(tmpDir) + + buildOpts := BuildOptions{} + buildOpts.Keyring = keyring + + buildOpts.CachedChart = "./../testdata/charts/helmchart-0.1.0.tgz" + targetPath2 := filepath.Join(tmpDir, "chart2.tgz") + defer os.RemoveAll(targetPath2) + + cb, err := b.Build(context.TODO(), reference, targetPath2, buildOpts) + g.Expect(err).ToNot(HaveOccurred()) + g.Expect(cb.ProvFilePath).ToNot(BeEmpty(), "empty Build.ProvFilePath") + g.Expect(cb.VerificationSignature).ToNot(BeNil(), "nil Build.VerificationSignature") +} + func Test_mergeFileValues(t *testing.T) { tests := []struct { name string diff --git a/internal/helm/chart/builder_remote.go b/internal/helm/chart/builder_remote.go index 54e1bed8c..910ad102a 100644 --- a/internal/helm/chart/builder_remote.go +++ b/internal/helm/chart/builder_remote.go @@ -27,6 +27,7 @@ import ( helmchart "helm.sh/helm/v3/pkg/chart" "helm.sh/helm/v3/pkg/chart/loader" "helm.sh/helm/v3/pkg/chartutil" + "helm.sh/helm/v3/pkg/provenance" "sigs.k8s.io/yaml" "github.com/fluxcd/pkg/runtime/transform" @@ -62,7 +63,7 @@ func NewRemoteBuilder(repository *repository.ChartRepository) Builder { // After downloading the chart, it is only packaged if required due to BuildOptions // modifying the chart, otherwise the exact data as retrieved from the repository // is written to p, after validating it to be a chart. -func (b *remoteChartBuilder) Build(_ context.Context, ref Reference, p string, opts BuildOptions, keyring []byte) (*Build, error) { +func (b *remoteChartBuilder) Build(_ context.Context, ref Reference, p string, opts BuildOptions) (*Build, error) { remoteRef, ok := ref.(RemoteReference) if !ok { err := fmt.Errorf("expected remote chart reference") @@ -106,15 +107,16 @@ func (b *remoteChartBuilder) Build(_ context.Context, ref Reference, p string, o requiresPackaging := len(opts.GetValuesFiles()) != 0 || opts.VersionMetadata != "" - verifyProvFile := func(chart, provFile string) error { - if keyring != nil { - err := VerifyProvenanceFile(bytes.NewReader(keyring), chart, provFile) + verifyProvFile := func(chart, provFile string) (*provenance.Verification, error) { + if opts.Keyring != nil { + ver, err := verifyChartWithProvFile(bytes.NewReader(opts.Keyring), chart, provFile) if err != nil { err = fmt.Errorf("failed to verify helm chart using provenance file %s: %w", provFile, err) - return &BuildError{Reason: ErrProvenanceVerification, Err: err} + return nil, &BuildError{Reason: ErrProvenanceVerification, Err: err} } + return ver, nil } - return nil + return nil, nil } var provFilePath string @@ -129,13 +131,16 @@ func (b *remoteChartBuilder) Build(_ context.Context, ref Reference, p string, o // and continue the build if err = curMeta.Validate(); err == nil { if result.Name == curMeta.Name && result.Version == curMeta.Version { - // We can only verify with provenance file if we didn't package the chart ourselves. + // We can only verify a cached chart with provenance file if we didn't + // package the chart ourselves, and instead stored it as is. if !requiresPackaging { provFilePath = provenanceFilePath(opts.CachedChart) - if err = verifyProvFile(opts.CachedChart, provFilePath); err != nil { + if ver, err := verifyProvFile(opts.CachedChart, provFilePath); err != nil { return nil, err + } else { + result.ProvFilePath = provFilePath + result.VerificationSignature = buildVerificationSig(ver) } - result.ProvFilePath = provFilePath } result.Path = opts.CachedChart result.ValuesFiles = opts.GetValuesFiles() @@ -155,7 +160,7 @@ func (b *remoteChartBuilder) Build(_ context.Context, ref Reference, p string, o err = fmt.Errorf("failed to download chart for remote reference: %w", err) return result, &BuildError{Reason: ErrChartPull, Err: err} } - if keyring != nil { + if opts.Keyring != nil { provFilePath = provenanceFilePath(p) err := b.remote.DownloadProvenanceFile(cv, provFilePath) if err != nil { @@ -166,15 +171,17 @@ func (b *remoteChartBuilder) Build(_ context.Context, ref Reference, p string, o // This is needed, since the verification will work only if the .tgz file is untampered. // But we write the packaged chart to disk under a different name, so the provenance file // will not be valid for this _new_ packaged chart. - chart, err := util.WriteBytesToFile(chartBuf, fmt.Sprintf("%s-%s.tgz", cv.Name, cv.Version), false) + chart, err := util.WriteToFile(chartBuf, fmt.Sprintf("%s-%s.tgz", cv.Name, cv.Version)) defer os.Remove(chart.Name()) if err != nil { return nil, err } - if err = verifyProvFile(chart.Name(), provFilePath); err != nil { + if ver, err := verifyProvFile(chart.Name(), provFilePath); err != nil { return nil, err + } else { + result.ProvFilePath = provFilePath + result.VerificationSignature = buildVerificationSig(ver) } - result.ProvFilePath = provFilePath } // Use literal chart copy from remote if no custom values files options are @@ -250,7 +257,7 @@ func mergeChartValues(chart *helmchart.Chart, paths []string) (map[string]interf // validatePackageAndWriteToPath atomically writes the packaged chart from reader // to out while validating it by loading the chart metadata from the archive. func validatePackageAndWriteToPath(b []byte, out string) error { - tmpFile, err := util.WriteBytesToFile(b, out, true) + tmpFile, err := util.WriteToTempFile(b, out) defer os.Remove(tmpFile.Name()) if err != nil { diff --git a/internal/helm/chart/builder_remote_test.go b/internal/helm/chart/builder_remote_test.go index b9795b352..9afd521b9 100644 --- a/internal/helm/chart/builder_remote_test.go +++ b/internal/helm/chart/builder_remote_test.go @@ -138,16 +138,22 @@ entries: wantErr: "Invalid Metadata string", }, { - name: "with version metadata", - reference: RemoteReference{Name: "grafana"}, - repository: mockRepo(), - buildOpts: BuildOptions{VersionMetadata: "foo"}, + name: "with version metadata", + reference: RemoteReference{Name: "grafana"}, + repository: mockRepo(), + buildOpts: BuildOptions{ + VersionMetadata: "foo", + Keyring: keyring, + }, wantVersion: "0.1.0+foo", wantPackaged: true, }, { - name: "default values", - reference: RemoteReference{Name: "grafana"}, + name: "default values", + reference: RemoteReference{Name: "grafana"}, + buildOpts: BuildOptions{ + Keyring: keyring, + }, repository: mockRepo(), wantVersion: "0.1.0", wantValues: chartutil.Values{ @@ -159,6 +165,7 @@ entries: reference: RemoteReference{Name: "grafana"}, buildOpts: BuildOptions{ ValuesFiles: []string{"a.yaml", "b.yaml", "c.yaml"}, + Keyring: keyring, }, repository: mockRepo(), wantVersion: "0.1.0", @@ -187,10 +194,7 @@ entries: b := NewRemoteBuilder(tt.repository) - if tt.buildOpts.VersionMetadata != "" { - keyring = nil - } - cb, err := b.Build(context.TODO(), tt.reference, targetPath, tt.buildOpts, keyring) + cb, err := b.Build(context.TODO(), tt.reference, targetPath, tt.buildOpts) if tt.wantErr != "" { g.Expect(err).To(HaveOccurred()) @@ -201,6 +205,8 @@ entries: g.Expect(err).ToNot(HaveOccurred()) g.Expect(cb.Packaged).To(Equal(tt.wantPackaged), "unexpected Build.Packaged value") g.Expect(cb.Path).ToNot(BeEmpty(), "empty Build.Path") + g.Expect(cb.ProvFilePath).ToNot(BeEmpty(), "empty Build.ProvFilePath") + g.Expect(cb.VerificationSignature).ToNot(BeNil(), "nil Build.VerificationSignature") // Load the resulting chart and verify the values. resultChart, err := loader.Load(cb.Path) @@ -273,8 +279,11 @@ entries: targetPath := filepath.Join(tmpDir, "helmchart-0.1.0.tgz") defer os.RemoveAll(targetPath) buildOpts := BuildOptions{} - cb, err := b.Build(context.TODO(), reference, targetPath, buildOpts, keyring) + buildOpts.Keyring = keyring + cb, err := b.Build(context.TODO(), reference, targetPath, buildOpts) g.Expect(err).ToNot(HaveOccurred()) + g.Expect(cb.ProvFilePath).ToNot(BeEmpty(), "empty Build.ProvFilePath") + g.Expect(cb.VerificationSignature).ToNot(BeNil(), "nil Build.VerificationSignature") // Set the result as the CachedChart for second build. buildOpts.CachedChart = cb.Path @@ -282,15 +291,19 @@ entries: // Rebuild with a new path. targetPath2 := filepath.Join(tmpDir, "chart2.tgz") defer os.RemoveAll(targetPath2) - cb, err = b.Build(context.TODO(), reference, targetPath2, buildOpts, keyring) + cb, err = b.Build(context.TODO(), reference, targetPath2, buildOpts) g.Expect(err).ToNot(HaveOccurred()) g.Expect(cb.Path).To(Equal(targetPath)) + g.Expect(cb.ProvFilePath).ToNot(BeEmpty(), "empty Build.ProvFilePath") + g.Expect(cb.VerificationSignature).ToNot(BeNil(), "nil Build.VerificationSignature") // Rebuild with build option Force. buildOpts.Force = true - cb, err = b.Build(context.TODO(), reference, targetPath2, buildOpts, keyring) + cb, err = b.Build(context.TODO(), reference, targetPath2, buildOpts) g.Expect(err).ToNot(HaveOccurred()) g.Expect(cb.Path).To(Equal(targetPath2)) + g.Expect(cb.ProvFilePath).ToNot(BeEmpty(), "empty Build.ProvFilePath") + g.Expect(cb.VerificationSignature).ToNot(BeNil(), "nil Build.VerificationSignature") } func Test_mergeChartValues(t *testing.T) { diff --git a/internal/helm/chart/verify.go b/internal/helm/chart/verify.go index 26e91c31e..9f0870b24 100644 --- a/internal/helm/chart/verify.go +++ b/internal/helm/chart/verify.go @@ -1,3 +1,19 @@ +/* +Copyright 2021 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 chart import ( @@ -14,14 +30,14 @@ import ( // Ref: https://github.com/helm/helm/blob/v3.8.0/pkg/downloader/chart_downloader.go#L328 // modified to accept a custom provenance file path and an actual keyring instead of a // path to the file containing the keyring. -func VerifyProvenanceFile(keyring io.Reader, chartPath, provFilePath string) error { +func verifyChartWithProvFile(keyring io.Reader, chartPath, provFilePath string) (*provenance.Verification, error) { switch fi, err := os.Stat(chartPath); { case err != nil: - return err + return nil, err case fi.IsDir(): - return fmt.Errorf("unpacked charts cannot be verified") + return nil, fmt.Errorf("unpacked charts cannot be verified") case !isTar(chartPath): - return fmt.Errorf("chart must be a tgz file") + return nil, fmt.Errorf("chart must be a tgz file") } if provFilePath == "" { @@ -29,20 +45,17 @@ func VerifyProvenanceFile(keyring io.Reader, chartPath, provFilePath string) err } if _, err := os.Stat(provFilePath); err != nil { - return fmt.Errorf("could not load provenance file %s: %w", provFilePath, err) + return nil, fmt.Errorf("could not load provenance file %s: %w", provFilePath, err) } ring, err := openpgp.ReadKeyRing(keyring) if err != nil { - return err + return nil, err } sig := &provenance.Signatory{KeyRing: ring} - _, err = sig.Verify(chartPath, provFilePath) - if err != nil { - return err - } - return nil + verification, err := sig.Verify(chartPath, provFilePath) + return verification, err } // isTar tests whether the given file is a tar file. @@ -55,3 +68,24 @@ func isTar(filename string) bool { func provenanceFilePath(path string) string { return path + ".prov" } + +// ref: https://github.com/helm/helm/blob/v3.8.0/pkg/action/verify.go#L47-L51 +type VerificationSignature struct { + Identities []string + KeyFingerprint [20]byte + FileHash string +} + +func buildVerificationSig(ver *provenance.Verification) *VerificationSignature { + var verSig VerificationSignature + if ver != nil { + if ver.SignedBy != nil { + for name := range ver.SignedBy.Identities { + verSig.Identities = append(verSig.Identities, name) + } + } + verSig.FileHash = ver.FileHash + verSig.KeyFingerprint = ver.SignedBy.PrimaryKey.Fingerprint + } + return &verSig +} diff --git a/internal/helm/repository/chart_repository.go b/internal/helm/repository/chart_repository.go index 58be329e3..b9b6482f2 100644 --- a/internal/helm/repository/chart_repository.go +++ b/internal/helm/repository/chart_repository.go @@ -216,7 +216,7 @@ func (r *ChartRepository) DownloadProvenanceFile(chart *repo.ChartVersion, path if err != nil { return err } - tmpFile, err := util.WriteBytesToFile(res.Bytes(), path, true) + tmpFile, err := util.WriteToTempFile(res.Bytes(), path) defer os.Remove(tmpFile.Name()) if err != nil { diff --git a/internal/util/file.go b/internal/util/file.go new file mode 100644 index 000000000..dc023d318 --- /dev/null +++ b/internal/util/file.go @@ -0,0 +1,43 @@ +package util + +import ( + "fmt" + "os" + "path/filepath" +) + +func writeBytesToFile(bytes []byte, out string, temp bool) (*os.File, error) { + var file *os.File + var err error + + if temp { + file, err = os.CreateTemp("", out) + if err != nil { + return nil, fmt.Errorf("failed to create temporary file %s: %w", filepath.Base(out), err) + } + } else { + file, err = os.Create(out) + if err != nil { + return nil, fmt.Errorf("failed to create temporary file for chart %s: %w", out, err) + } + } + if _, err := file.Write(bytes); err != nil { + _ = file.Close() + return nil, fmt.Errorf("failed to write to file %s: %w", file.Name(), err) + } + if err := file.Close(); err != nil { + return nil, err + } + return file, nil +} + +// Writes the provided bytes to a file at the given path and returns the file handle. +func WriteToFile(bytes []byte, path string) (*os.File, error) { + return writeBytesToFile(bytes, path, false) +} + +// Writes the provided bytes to a temp file with the name provided in the path and +// returns the file handle. +func WriteToTempFile(bytes []byte, out string) (*os.File, error) { + return writeBytesToFile(bytes, filepath.Base(out), true) +} diff --git a/internal/util/temp.go b/internal/util/temp.go index ef07c7bd3..054b12801 100644 --- a/internal/util/temp.go +++ b/internal/util/temp.go @@ -50,29 +50,3 @@ func pattern(obj client.Object) (p string) { kind := strings.ToLower(obj.GetObjectKind().GroupVersionKind().Kind) return fmt.Sprintf("%s-%s-%s-", kind, obj.GetNamespace(), obj.GetName()) } - -// TODO: think of a better name? -func WriteBytesToFile(bytes []byte, out string, temp bool) (*os.File, error) { - var file *os.File - var err error - - if temp { - file, err = os.CreateTemp("", filepath.Base(out)) - if err != nil { - return nil, fmt.Errorf("failed to create temporary file %s: %w", filepath.Base(out), err) - } - } else { - file, err = os.Create(out) - if err != nil { - return nil, fmt.Errorf("failed to create temporary file for chart %s: %w", out, err) - } - } - if _, err := file.Write(bytes); err != nil { - _ = file.Close() - return nil, fmt.Errorf("failed to write to file %s: %w", file.Name(), err) - } - if err := file.Close(); err != nil { - return nil, err - } - return file, nil -}