Skip to content

Commit a888ebf

Browse files
committed
update helm chart builder to support prov file verification
Signed-off-by: Sanskar Jaiswal <[email protected]>
1 parent e4d0b53 commit a888ebf

File tree

9 files changed

+280
-53
lines changed

9 files changed

+280
-53
lines changed

internal/helm/chart/builder.go

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -85,7 +85,7 @@ type Builder interface {
8585
// Reference and BuildOptions, and writes it to p.
8686
// It returns the Build result, or an error.
8787
// It may return an error for unsupported Reference implementations.
88-
Build(ctx context.Context, ref Reference, p string, opts BuildOptions) (*Build, error)
88+
Build(ctx context.Context, ref Reference, p string, opts BuildOptions, keyring []byte) (*Build, error)
8989
}
9090

9191
// BuildOptions provides a list of options for Builder.Build.
@@ -125,6 +125,10 @@ type Build struct {
125125
// Path is the absolute path to the packaged chart.
126126
// Can be empty, in which case a failure should be assumed.
127127
Path string
128+
// ProvFilePath is the absolute path to a provenance file.
129+
// Can be empty, in which case it should be assumed that the packaged
130+
// chart is not verified.
131+
ProvFilePath string
128132
// ValuesFiles is the list of files used to compose the chart's
129133
// default "values.yaml".
130134
ValuesFiles []string

internal/helm/chart/builder_local.go

Lines changed: 28 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ limitations under the License.
1717
package chart
1818

1919
import (
20+
"bytes"
2021
"context"
2122
"fmt"
2223
"os"
@@ -62,7 +63,7 @@ func NewLocalBuilder(dm *DependencyManager) Builder {
6263
// If the LocalReference.Path refers to a chart directory, dependencies are
6364
// confirmed to be present using the DependencyManager, while attempting to
6465
// resolve any missing.
65-
func (b *localChartBuilder) Build(ctx context.Context, ref Reference, p string, opts BuildOptions) (*Build, error) {
66+
func (b *localChartBuilder) Build(ctx context.Context, ref Reference, p string, opts BuildOptions, keyring []byte) (*Build, error) {
6667
localRef, ok := ref.(LocalReference)
6768
if !ok {
6869
err := fmt.Errorf("expected local chart reference")
@@ -108,16 +109,37 @@ func (b *localChartBuilder) Build(ctx context.Context, ref Reference, p string,
108109
// - Chart name from cached chart matches resolved name
109110
// - Chart version from cached chart matches calculated version
110111
// - BuildOptions.Force is False
112+
var provFilePath string
113+
verifyProvFile := func(chart, provFile string) error {
114+
if keyring != nil {
115+
if _, err := os.Stat(provFile); err != nil {
116+
err = fmt.Errorf("could not load provenance file %s: %w", provFile, err)
117+
return &BuildError{Reason: ErrProvenanceVerification, Err: err}
118+
}
119+
err := VerifyProvenanceFile(bytes.NewReader(keyring), chart, provFile)
120+
if err != nil {
121+
err = fmt.Errorf("failed to verify helm chart using provenance file: %w", err)
122+
return &BuildError{Reason: ErrProvenanceVerification, Err: err}
123+
}
124+
}
125+
return nil
126+
}
111127
if opts.CachedChart != "" && !opts.Force {
112128
if curMeta, err = LoadChartMetadataFromArchive(opts.CachedChart); err == nil {
113129
// If the cached metadata is corrupt, we ignore its existence
114130
// and continue the build
115131
if err = curMeta.Validate(); err == nil {
116132
if result.Name == curMeta.Name && result.Version == curMeta.Version {
133+
if !requiresPackaging {
134+
provFilePath = provenanceFilePath(opts.CachedChart)
135+
if err = verifyProvFile(opts.CachedChart, provFilePath); err != nil {
136+
return nil, err
137+
}
138+
result.ProvFilePath = provFilePath
139+
}
117140
result.Path = opts.CachedChart
118141
result.ValuesFiles = opts.GetValuesFiles()
119142
result.Packaged = requiresPackaging
120-
121143
return result, nil
122144
}
123145
}
@@ -126,10 +148,14 @@ func (b *localChartBuilder) Build(ctx context.Context, ref Reference, p string,
126148

127149
// If the chart at the path is already packaged and no custom values files
128150
// options are set, we can copy the chart without making modifications
151+
provFilePath = provenanceFilePath(localRef.Path)
129152
if !requiresPackaging {
130153
if err = copyFileToPath(localRef.Path, p); err != nil {
131154
return result, &BuildError{Reason: ErrChartPull, Err: err}
132155
}
156+
if err = verifyProvFile(localRef.Path, provFilePath); err != nil {
157+
return result, err
158+
}
133159
result.Path = p
134160
return result, nil
135161
}

internal/helm/chart/builder_local_test.go

Lines changed: 13 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,11 @@ func TestLocalBuilder_Build(t *testing.T) {
4040
chartB, err := os.ReadFile("./../testdata/charts/helmchart-0.1.0.tgz")
4141
g.Expect(err).ToNot(HaveOccurred())
4242
g.Expect(chartB).ToNot(BeEmpty())
43+
44+
keyring, err := os.ReadFile("./../testdata/charts/pub.gpg")
45+
g.Expect(err).ToNot(HaveOccurred())
46+
g.Expect(keyring).ToNot(BeEmpty())
47+
4348
mockRepo := func() *repository.ChartRepository {
4449
return &repository.ChartRepository{
4550
Client: &mockGetter{
@@ -210,7 +215,7 @@ fullnameOverride: "full-foo-name-override"`),
210215
)
211216

212217
b := NewLocalBuilder(dm)
213-
cb, err := b.Build(context.TODO(), tt.reference, targetPath, tt.buildOpts)
218+
cb, err := b.Build(context.TODO(), tt.reference, targetPath, tt.buildOpts, keyring)
214219

215220
if tt.wantErr != "" {
216221
g.Expect(err).To(HaveOccurred())
@@ -243,6 +248,10 @@ func TestLocalBuilder_Build_CachedChart(t *testing.T) {
243248

244249
reference := LocalReference{Path: "./../testdata/charts/helmchart"}
245250

251+
keyring, err := os.ReadFile("./../testdata/charts/pub.gpg")
252+
g.Expect(err).ToNot(HaveOccurred())
253+
g.Expect(keyring).ToNot(BeEmpty())
254+
246255
dm := NewDependencyManager()
247256
b := NewLocalBuilder(dm)
248257

@@ -253,21 +262,21 @@ func TestLocalBuilder_Build_CachedChart(t *testing.T) {
253262
// Build first time.
254263
targetPath := filepath.Join(tmpDir, "chart1.tgz")
255264
buildOpts := BuildOptions{}
256-
cb, err := b.Build(context.TODO(), reference, targetPath, buildOpts)
265+
cb, err := b.Build(context.TODO(), reference, targetPath, buildOpts, keyring)
257266
g.Expect(err).ToNot(HaveOccurred())
258267

259268
// Set the result as the CachedChart for second build.
260269
buildOpts.CachedChart = cb.Path
261270

262271
targetPath2 := filepath.Join(tmpDir, "chart2.tgz")
263272
defer os.RemoveAll(targetPath2)
264-
cb, err = b.Build(context.TODO(), reference, targetPath2, buildOpts)
273+
cb, err = b.Build(context.TODO(), reference, targetPath2, buildOpts, keyring)
265274
g.Expect(err).ToNot(HaveOccurred())
266275
g.Expect(cb.Path).To(Equal(targetPath))
267276

268277
// Rebuild with build option Force.
269278
buildOpts.Force = true
270-
cb, err = b.Build(context.TODO(), reference, targetPath2, buildOpts)
279+
cb, err = b.Build(context.TODO(), reference, targetPath2, buildOpts, keyring)
271280
g.Expect(err).ToNot(HaveOccurred())
272281
g.Expect(cb.Path).To(Equal(targetPath2))
273282
}

internal/helm/chart/builder_remote.go

Lines changed: 56 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -17,9 +17,9 @@ limitations under the License.
1717
package chart
1818

1919
import (
20+
"bytes"
2021
"context"
2122
"fmt"
22-
"io"
2323
"os"
2424
"path/filepath"
2525

@@ -33,6 +33,7 @@ import (
3333

3434
"github.com/fluxcd/source-controller/internal/fs"
3535
"github.com/fluxcd/source-controller/internal/helm/repository"
36+
"github.com/fluxcd/source-controller/internal/util"
3637
)
3738

3839
type remoteChartBuilder struct {
@@ -61,7 +62,7 @@ func NewRemoteBuilder(repository *repository.ChartRepository) Builder {
6162
// After downloading the chart, it is only packaged if required due to BuildOptions
6263
// modifying the chart, otherwise the exact data as retrieved from the repository
6364
// is written to p, after validating it to be a chart.
64-
func (b *remoteChartBuilder) Build(_ context.Context, ref Reference, p string, opts BuildOptions) (*Build, error) {
65+
func (b *remoteChartBuilder) Build(_ context.Context, ref Reference, p string, opts BuildOptions, keyring []byte) (*Build, error) {
6566
remoteRef, ok := ref.(RemoteReference)
6667
if !ok {
6768
err := fmt.Errorf("expected remote chart reference")
@@ -105,6 +106,19 @@ func (b *remoteChartBuilder) Build(_ context.Context, ref Reference, p string, o
105106

106107
requiresPackaging := len(opts.GetValuesFiles()) != 0 || opts.VersionMetadata != ""
107108

109+
verifyProvFile := func(chart, provFile string) error {
110+
if keyring != nil {
111+
err := VerifyProvenanceFile(bytes.NewReader(keyring), chart, provFile)
112+
if err != nil {
113+
err = fmt.Errorf("failed to verify helm chart using provenance file %s: %w", provFile, err)
114+
return &BuildError{Reason: ErrProvenanceVerification, Err: err}
115+
}
116+
}
117+
return nil
118+
}
119+
120+
var provFilePath string
121+
108122
// If all the following is true, we do not need to download and/or build the chart:
109123
// - Chart name from cached chart matches resolved name
110124
// - Chart version from cached chart matches calculated version
@@ -115,6 +129,14 @@ func (b *remoteChartBuilder) Build(_ context.Context, ref Reference, p string, o
115129
// and continue the build
116130
if err = curMeta.Validate(); err == nil {
117131
if result.Name == curMeta.Name && result.Version == curMeta.Version {
132+
// We can only verify with provenance file if we didn't package the chart ourselves.
133+
if !requiresPackaging {
134+
provFilePath = provenanceFilePath(opts.CachedChart)
135+
if err = verifyProvFile(opts.CachedChart, provFilePath); err != nil {
136+
return nil, err
137+
}
138+
result.ProvFilePath = provFilePath
139+
}
118140
result.Path = opts.CachedChart
119141
result.ValuesFiles = opts.GetValuesFiles()
120142
result.Packaged = requiresPackaging
@@ -126,15 +148,39 @@ func (b *remoteChartBuilder) Build(_ context.Context, ref Reference, p string, o
126148

127149
// Download the package for the resolved version
128150
res, err := b.remote.DownloadChart(cv)
151+
// Deal with the underlying byte slice to avoid having to read the buffer multiple times.
152+
chartBuf := res.Bytes()
153+
129154
if err != nil {
130155
err = fmt.Errorf("failed to download chart for remote reference: %w", err)
131156
return result, &BuildError{Reason: ErrChartPull, Err: err}
132157
}
158+
if keyring != nil {
159+
provFilePath = provenanceFilePath(p)
160+
err := b.remote.DownloadProvenanceFile(cv, provFilePath)
161+
if err != nil {
162+
err = fmt.Errorf("failed to download provenance file for remote reference: %w", err)
163+
return nil, &BuildError{Reason: ErrChartPull, Err: err}
164+
}
165+
// Write the remote chart temporarily to verify it with provenance file.
166+
// This is needed, since the verification will work only if the .tgz file is untampered.
167+
// But we write the packaged chart to disk under a different name, so the provenance file
168+
// will not be valid for this _new_ packaged chart.
169+
chart, err := util.WriteBytesToFile(chartBuf, fmt.Sprintf("%s-%s.tgz", result.Name, result.Version), false)
170+
defer os.Remove(chart.Name())
171+
if err != nil {
172+
return nil, err
173+
}
174+
if err = verifyProvFile(chart.Name(), provFilePath); err != nil {
175+
return nil, err
176+
}
177+
result.ProvFilePath = provFilePath
178+
}
133179

134180
// Use literal chart copy from remote if no custom values files options are
135181
// set or version metadata isn't set.
136182
if !requiresPackaging {
137-
if err = validatePackageAndWriteToPath(res, p); err != nil {
183+
if err = validatePackageAndWriteToPath(chartBuf, p); err != nil {
138184
return nil, &BuildError{Reason: ErrChartPull, Err: err}
139185
}
140186
result.Path = p
@@ -143,7 +189,7 @@ func (b *remoteChartBuilder) Build(_ context.Context, ref Reference, p string, o
143189

144190
// Load the chart and merge chart values
145191
var chart *helmchart.Chart
146-
if chart, err = loader.LoadArchive(res); err != nil {
192+
if chart, err = loader.LoadArchive(bytes.NewBuffer(chartBuf)); err != nil {
147193
err = fmt.Errorf("failed to load downloaded chart: %w", err)
148194
return result, &BuildError{Reason: ErrChartPackage, Err: err}
149195
}
@@ -166,6 +212,7 @@ func (b *remoteChartBuilder) Build(_ context.Context, ref Reference, p string, o
166212
if err = packageToPath(chart, p); err != nil {
167213
return nil, &BuildError{Reason: ErrChartPackage, Err: err}
168214
}
215+
169216
result.Path = p
170217
result.Packaged = true
171218
return result, nil
@@ -202,18 +249,12 @@ func mergeChartValues(chart *helmchart.Chart, paths []string) (map[string]interf
202249

203250
// validatePackageAndWriteToPath atomically writes the packaged chart from reader
204251
// to out while validating it by loading the chart metadata from the archive.
205-
func validatePackageAndWriteToPath(reader io.Reader, out string) error {
206-
tmpFile, err := os.CreateTemp("", filepath.Base(out))
207-
if err != nil {
208-
return fmt.Errorf("failed to create temporary file for chart: %w", err)
209-
}
252+
func validatePackageAndWriteToPath(b []byte, out string) error {
253+
tmpFile, err := util.WriteBytesToFile(b, out, true)
210254
defer os.Remove(tmpFile.Name())
211-
if _, err = tmpFile.ReadFrom(reader); err != nil {
212-
_ = tmpFile.Close()
213-
return fmt.Errorf("failed to write chart to file: %w", err)
214-
}
215-
if err = tmpFile.Close(); err != nil {
216-
return err
255+
256+
if err != nil {
257+
return fmt.Errorf("failed to write packaged chart to temp file: %w", err)
217258
}
218259
meta, err := LoadChartMetadataFromArchive(tmpFile.Name())
219260
if err != nil {

0 commit comments

Comments
 (0)