Skip to content

Commit

Permalink
make doc changes and fix reconilation process
Browse files Browse the repository at this point in the history 
Signed-off-by: Sanskar Jaiswal <[email protected]>
  • Loading branch information
@aryan9600
aryan9600 committed Mar 17, 2022
1 parent ae68b45 commit f347630
Show file tree
Hide file tree
Showing 8 changed files with 155 additions and 105 deletions.
8 changes: 6 additions & 2 deletions api/v1beta2/helmchart_types.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,7 @@ type HelmChartSpec struct {
// +optional
AccessFrom *acl.AccessFrom `json:"accessFrom,omitempty"`

// Keyring information for verifying the packaged chart's signature using a provenance file.
// VerificationKeyring for verifying the packaged chart's signature using a provenance file.
// +optional
VerificationKeyring *VerificationKeyring `json:"verificationKeyring,omitempty"`
}
Expand All @@ -94,7 +94,7 @@ type VerificationKeyring struct {
// +required
SecretRef meta.LocalObjectReference `json:"secretRef,omitempty"`

// The key that corresponds to the keyring value.
// Key in the SecretRef that contains the public keyring in legacy GPG format.
// +kubebuilder:default:=pubring.gpg
// +optional
Key string `json:"key,omitempty"`
Expand Down Expand Up @@ -168,6 +168,10 @@ const (
// ChartPackageSucceededReason signals that the package of the Helm
// chart succeeded.
ChartPackageSucceededReason string = "ChartPackageSucceeded"

// ChartVerifiedSucceededReason signals that the Helm chart's signature
// has been verified using it's provenance file.
ChartVerifiedSucceededReason string = "ChartVerifiedSucceeded"
)

// GetConditions returns the status conditions of the object.
Expand Down
49 changes: 16 additions & 33 deletions controllers/helmchart_controller.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -97,8 +97,6 @@ var helmChartReadyCondition = summarize.Conditions{
},
}

const KeyringFileName = "pubring.gpg"

// +kubebuilder:rbac:groups=source.toolkit.fluxcd.io,resources=helmcharts,verbs=get;list;watch;create;update;patch;delete
// +kubebuilder:rbac:groups=source.toolkit.fluxcd.io,resources=helmcharts/status,verbs=get;update;patch
// +kubebuilder:rbac:groups=source.toolkit.fluxcd.io,resources=helmcharts/finalizers,verbs=get;create;update;patch;delete
Expand Down Expand Up @@ -475,9 +473,9 @@ func (r *HelmChartReconciler) buildFromHelmRepository(ctx context.Context, obj *
if err != nil {
e := &serror.Event{
Err: fmt.Errorf("failed to get public key for chart signature verification: %w", err),
Reason: sourcev1.SourceVerifiedCondition,
Reason: sourcev1.AuthenticationFailedReason,
}
conditions.MarkFalse(obj, sourcev1.FetchFailedCondition, sourcev1.SourceVerifiedCondition, e.Error())
conditions.MarkFalse(obj, sourcev1.SourceVerifiedCondition, e.Reason, e.Error())
return sreconcile.ResultEmpty, e
}
opts.Keyring = keyring
Expand Down Expand Up @@ -609,9 +607,9 @@ func (r *HelmChartReconciler) buildFromTarballArtifact(ctx context.Context, obj
if err != nil {
e := &serror.Event{
Err: fmt.Errorf("failed to get public key for chart signature verification: %w", err),
Reason: sourcev1.SourceVerifiedCondition,
Reason: sourcev1.AuthenticationFailedReason,
}
conditions.MarkFalse(obj, sourcev1.FetchFailedCondition, sourcev1.SourceVerifiedCondition, e.Error())
conditions.MarkFalse(obj, sourcev1.SourceVerifiedCondition, e.Reason, e.Error())
return sreconcile.ResultEmpty, e
}
opts.Keyring = keyring
Expand Down Expand Up @@ -651,14 +649,6 @@ 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
Expand Down Expand Up @@ -914,22 +904,6 @@ func (r *HelmChartReconciler) getHelmRepositorySecret(ctx context.Context, repos
return &secret, nil
}

func (r *HelmChartReconciler) getVerificationKeyringSecret(ctx context.Context, chart *sourcev1.HelmChart) (*corev1.Secret, error) {
if chart.Spec.VerificationKeyring == nil {
return nil, nil
}
name := types.NamespacedName{
Namespace: chart.GetNamespace(),
Name: chart.Spec.VerificationKeyring.SecretRef.Name,
}
var secret corev1.Secret
err := r.Client.Get(ctx, name, &secret)
if err != nil {
return nil, err
}
return &secret, nil
}

func (r *HelmChartReconciler) indexHelmRepositoryByURL(o client.Object) []string {
repo, ok := o.(*sourcev1.HelmRepository)
if !ok {
Expand Down Expand Up @@ -1060,6 +1034,15 @@ func observeChartBuild(obj *sourcev1.HelmChart, build *chart.Build, err error) {
conditions.Delete(obj, sourcev1.BuildFailedCondition)
}

if build.VerificationSignature != nil && build.ProvFilePath != "" {
var sigVerMsg strings.Builder
sigVerMsg.WriteString(fmt.Sprintf("chart signed by: %v", strings.Join(build.VerificationSignature.Identities[:], ",")))
sigVerMsg.WriteString(fmt.Sprintf(" using key with fingeprint: %X", build.VerificationSignature.KeyFingerprint))
sigVerMsg.WriteString(fmt.Sprintf(" and hash verified: %s", build.VerificationSignature.FileHash))

conditions.MarkTrue(obj, sourcev1.SourceVerifiedCondition, sourcev1.ChartVerifiedSucceededReason, sigVerMsg.String())
}

if err != nil {
var buildErr *chart.BuildError
if ok := errors.As(err, &buildErr); !ok {
Expand Down Expand Up @@ -1105,10 +1088,10 @@ func (r *HelmChartReconciler) getProvenanceKeyring(ctx context.Context, chart *s
return nil, err
}
key := chart.Spec.VerificationKeyring.Key
if val, ok := secret.Data[key]; !ok {
val, ok := secret.Data[key]
if !ok {
err = fmt.Errorf("secret doesn't contain the advertised verification keyring name %s", key)
return nil, err
} else {
return val, nil
}
return val, nil
}
131 changes: 76 additions & 55 deletions controllers/helmchart_controller_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ import (
sreconcile "github.com/fluxcd/source-controller/internal/reconcile"
)

const publicKeyFileName = "pub.pgp"
const publicKeyFileName = "pub.gpg"

func TestHelmChartReconciler_Reconcile(t *testing.T) {
g := NewWithT(t)
Expand Down Expand Up @@ -458,14 +458,19 @@ 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())
g.Expect(keyring).ToNot(BeEmpty())

tests := []struct {
name string
source sourcev1.Source
beforeFunc func(obj *sourcev1.HelmChart)
want sreconcile.Result
wantErr error
assertFunc func(g *WithT, build chart.Build, obj sourcev1.HelmChart)
cleanFunc func(g *WithT, build *chart.Build)
name string
source sourcev1.Source
keyringSecret *corev1.Secret
beforeFunc func(obj *sourcev1.HelmChart)
want sreconcile.Result
wantErr error
assertFunc func(g *WithT, build chart.Build, obj sourcev1.HelmChart)
cleanFunc func(g *WithT, build *chart.Build)
}{
{
name: "Observes Artifact revision and build result",
Expand Down Expand Up @@ -501,6 +506,59 @@ func TestHelmChartReconciler_reconcileSource(t *testing.T) {
g.Expect(os.Remove(build.Path)).To(Succeed())
},
},
{
name: "Observes Artifact revision and build result with valid signature",
source: &sourcev1.GitRepository{
ObjectMeta: metav1.ObjectMeta{
Name: "gitrepository",
Namespace: "default",
},
Status: sourcev1.GitRepositoryStatus{
Artifact: gitArtifact,
},
},
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.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) {
g.Expect(build.Complete()).To(BeTrue())
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.VerificationSignature).ToNot(BeNil())
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{
*conditions.TrueCondition(sourcev1.ArtifactOutdatedCondition, "NewChart", "pulled 'helmchart' chart with version '0.1.0'"),
*conditions.TrueCondition(sourcev1.SourceVerifiedCondition, sourcev1.ChartVerifiedSucceededReason, "chart signed by: TestUser using key with fingeprint: 943CB5929ECDA2B5B5EC88BC7035BA97D32A87C1 and hash verified: sha256:007c7b7446eebcb18caeffe9898a3356ba1795f54df40ad39cfcc7382874a10a"),
}))
},
cleanFunc: func(g *WithT, build *chart.Build) {
g.Expect(os.Remove(build.Path)).To(Succeed())
g.Expect(os.Remove(build.ProvFilePath)).To(Succeed())
},
},
{
name: "Error on unavailable source",
beforeFunc: func(obj *sourcev1.HelmChart) {
Expand Down Expand Up @@ -605,6 +663,9 @@ func TestHelmChartReconciler_reconcileSource(t *testing.T) {
if tt.source != nil {
clientBuilder.WithRuntimeObjects(tt.source)
}
if tt.keyringSecret != nil {
clientBuilder.WithRuntimeObjects(tt.keyringSecret)
}

r := &HelmChartReconciler{
Client: clientBuilder.Build(),
Expand Down Expand Up @@ -1129,7 +1190,7 @@ func TestHelmChartReconciler_reconcileArtifact(t *testing.T) {
},
{
name: "Copying artifact to storage from build makes Ready=True",
build: mockChartBuild("helmchart", "0.1.0", "testdata/charts/helmchart-0.1.0.tgz", ""),
build: mockChartBuild("helmchart", "0.1.0", "testdata/charts/helmchart-0.1.0.tgz"),
beforeFunc: func(obj *sourcev1.HelmChart) {
conditions.MarkTrue(obj, sourcev1.ArtifactOutdatedCondition, "Foo", "")
},
Expand All @@ -1145,24 +1206,6 @@ func TestHelmChartReconciler_reconcileArtifact(t *testing.T) {
*conditions.TrueCondition(meta.ReadyCondition, sourcev1.ChartPullSucceededReason, "pulled 'helmchart' chart with version '0.1.0'"),
},
},
{
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) {
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", "helmchart-0.1.0.tgz.prov")
t.Expect(provArtifact.Path).ToNot(BeEmpty())
},
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"),
},
},
{
name: "Up-to-date chart build does not persist artifact to storage",
build: &chart.Build{
Expand Down Expand Up @@ -1208,7 +1251,7 @@ func TestHelmChartReconciler_reconcileArtifact(t *testing.T) {
},
{
name: "Removes ArtifactOutdatedCondition after creating new artifact",
build: mockChartBuild("helmchart", "0.1.0", "testdata/charts/helmchart-0.1.0.tgz", ""),
build: mockChartBuild("helmchart", "0.1.0", "testdata/charts/helmchart-0.1.0.tgz"),
beforeFunc: func(obj *sourcev1.HelmChart) {
conditions.MarkTrue(obj, sourcev1.ArtifactOutdatedCondition, "Foo", "")
},
Expand All @@ -1226,7 +1269,7 @@ func TestHelmChartReconciler_reconcileArtifact(t *testing.T) {
},
{
name: "Creates latest symlink to the created artifact",
build: mockChartBuild("helmchart", "0.1.0", "testdata/charts/helmchart-0.1.0.tgz", ""),
build: mockChartBuild("helmchart", "0.1.0", "testdata/charts/helmchart-0.1.0.tgz"),
afterFunc: func(t *WithT, obj *sourcev1.HelmChart) {
t.Expect(obj.GetArtifact()).ToNot(BeNil())

Expand Down Expand Up @@ -1726,10 +1769,8 @@ func TestHelmChartReconciler_reconcileSubRecs(t *testing.T) {
}
}

func mockChartBuild(name, version, path, provFilePath string) *chart.Build {
func mockChartBuild(name, version, path string) *chart.Build {
var copyP string
var copyPP string
var verSig *chart.VerificationSignature
if path != "" {
f, err := os.Open(path)
if err == nil {
Expand All @@ -1743,29 +1784,9 @@ func mockChartBuild(name, version, path, provFilePath string) *chart.Build {
}
}
}
if provFilePath != "" {
f, err := os.Open(provFilePath)
if err == nil {
defer f.Close()
ff, err := os.CreateTemp("", "chart-mock-*.tgz.prov")
if err == nil {
defer ff.Close()
if _, err = io.Copy(ff, f); err == nil {
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,
VerificationSignature: verSig,
Name: name,
Version: version,
Path: copyP,
}
}
6 changes: 3 additions & 3 deletions internal/helm/chart/builder.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -105,8 +105,8 @@ type BuildOptions struct {
// 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 can be configured with the bytes of a public kering in legacy
// PGP format used for verifying a chart's signature using a provenance file.
Keyring []byte
}

Expand Down Expand Up @@ -134,7 +134,7 @@ type Build struct {
// chart is not verified.
ProvFilePath string
// VerificationSignature is populated when a chart's signature
// is susccessfully verified using it's provenance file.
// is successfully verified using it's provenance file.
VerificationSignature *VerificationSignature
// ValuesFiles is the list of files used to compose the chart's
// default "values.yaml".
Expand Down
14 changes: 9 additions & 5 deletions internal/helm/chart/builder_local.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -137,9 +137,11 @@ func (b *localChartBuilder) Build(ctx context.Context, ref Reference, p string,
// package the chart ourselves, and instead stored it as is.
if !requiresPackaging {
provFilePath = provenanceFilePath(opts.CachedChart)
if ver, err := verifyProvFile(opts.CachedChart, provFilePath); err != nil {
ver, err := verifyProvFile(opts.CachedChart, provFilePath)
if err != nil {
return nil, err
} else {
}
if ver != nil {
result.VerificationSignature = buildVerificationSig(ver)
result.ProvFilePath = provFilePath
}
Expand All @@ -163,9 +165,11 @@ 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 ver, err := verifyProvFile(localRef.Path, provFilePath); err != nil {
return result, err
} else {
ver, err := verifyProvFile(localRef.Path, provFilePath)
if err != nil {
return nil, err
}
if ver != nil {
result.ProvFilePath = provFilePath
result.VerificationSignature = buildVerificationSig(ver)
}
Expand Down
Loading

0 comments on commit f347630

Please sign in to comment.