diff --git a/api/v1beta2/helmchart_types.go b/api/v1beta2/helmchart_types.go index aca993fd..96321a09 100644 --- a/api/v1beta2/helmchart_types.go +++ b/api/v1beta2/helmchart_types.go @@ -90,7 +90,7 @@ type HelmChartSpec struct { // Verify contains the secret name containing the trusted public keys // used to verify the signature and specifies which provider to use to check // whether OCI image is authentic. - // This field is only supported for OCI sources. + // This field is only supported when using HelmRepository source with spec.type 'oci'. // Chart dependencies, which are not bundled in the umbrella chart artifact, are not verified. // +optional Verify *OCIRepositoryVerification `json:"verify,omitempty"` diff --git a/config/crd/bases/source.toolkit.fluxcd.io_helmcharts.yaml b/config/crd/bases/source.toolkit.fluxcd.io_helmcharts.yaml index c6cdeefe..c1ac4b6e 100644 --- a/config/crd/bases/source.toolkit.fluxcd.io_helmcharts.yaml +++ b/config/crd/bases/source.toolkit.fluxcd.io_helmcharts.yaml @@ -407,8 +407,9 @@ spec: description: Verify contains the secret name containing the trusted public keys used to verify the signature and specifies which provider to use to check whether OCI image is authentic. This field is only - supported for OCI sources. Chart dependencies, which are not bundled - in the umbrella chart artifact, are not verified. + supported when using HelmRepository source with spec.type 'oci'. + Chart dependencies, which are not bundled in the umbrella chart + artifact, are not verified. properties: provider: default: cosign diff --git a/controllers/helmchart_controller.go b/controllers/helmchart_controller.go index d74a7ae1..1300db9f 100644 --- a/controllers/helmchart_controller.go +++ b/controllers/helmchart_controller.go @@ -372,6 +372,12 @@ func (r *HelmChartReconciler) reconcileStorage(ctx context.Context, obj *sourcev } func (r *HelmChartReconciler) reconcileSource(ctx context.Context, obj *sourcev1.HelmChart, build *chart.Build) (_ sreconcile.Result, retErr error) { + // Remove any failed verification condition. + // The reason is that a failing verification should be recalculated. + if conditions.IsFalse(obj, sourcev1.SourceVerifiedCondition) { + conditions.Delete(obj, sourcev1.SourceVerifiedCondition) + } + // Retrieve the source s, err := r.getSource(ctx, obj) if err != nil { @@ -577,10 +583,10 @@ func (r *HelmChartReconciler) buildFromHelmRepository(ctx context.Context, obj * if obj.Spec.Verify.SecretRef == nil { provider = fmt.Sprintf("%s keyless", provider) } - e := serror.NewGeneric( - fmt.Errorf("failed to verify the signature using provider '%s': %w", provider, err), - sourcev1.VerificationError, - ) + e := &serror.Event{ + Err: fmt.Errorf("failed to verify the signature using provider '%s': %w", provider, err), + Reason: sourcev1.VerificationError, + } conditions.MarkFalse(obj, sourcev1.SourceVerifiedCondition, e.Reason, e.Err.Error()) return sreconcile.ResultEmpty, e } @@ -650,15 +656,8 @@ func (r *HelmChartReconciler) buildFromHelmRepository(ctx context.Context, obj * Force: obj.Generation != obj.Status.ObservedGeneration, // The remote builder will not attempt to download the chart if // an artifact exists with the same name and version and `Force` is false. - // It will try to verify the chart if: - // - we are on the first reconciliation - // - the HelmChart spec has changed (generation drift) - // - the previous reconciliation resulted in a failed artifact verification - // - there is no artifact in storage - Verify: obj.Spec.Verify != nil && (obj.Generation <= 0 || - conditions.GetObservedGeneration(obj, sourcev1.SourceVerifiedCondition) != obj.Generation || - conditions.IsFalse(obj, sourcev1.SourceVerifiedCondition) || - obj.GetArtifact() == nil), + // It will however try to verify the chart if `obj.Spec.Verify` is set, at every reconciliation. + Verify: obj.Spec.Verify != nil && obj.Spec.Verify.Provider != "", } if artifact := obj.GetArtifact(); artifact != nil { opts.CachedChart = r.Storage.LocalPath(*artifact) @@ -1293,9 +1292,13 @@ func observeChartBuild(obj *sourcev1.HelmChart, build *chart.Build, err error) { } switch buildErr.Reason { - case chart.ErrChartMetadataPatch, chart.ErrValuesFilesMerge, chart.ErrDependencyBuild, chart.ErrChartPackage, chart.ErrChartVerification: + case chart.ErrChartMetadataPatch, chart.ErrValuesFilesMerge, chart.ErrDependencyBuild, chart.ErrChartPackage: conditions.Delete(obj, sourcev1.FetchFailedCondition) conditions.MarkTrue(obj, sourcev1.BuildFailedCondition, buildErr.Reason.Reason, buildErr.Error()) + case chart.ErrChartVerification: + conditions.Delete(obj, sourcev1.FetchFailedCondition) + conditions.MarkTrue(obj, sourcev1.BuildFailedCondition, buildErr.Reason.Reason, buildErr.Error()) + conditions.MarkFalse(obj, sourcev1.SourceVerifiedCondition, sourcev1.VerificationError, buildErr.Error()) default: conditions.Delete(obj, sourcev1.BuildFailedCondition) conditions.MarkTrue(obj, sourcev1.FetchFailedCondition, buildErr.Reason.Reason, buildErr.Error()) diff --git a/controllers/helmchart_controller_test.go b/controllers/helmchart_controller_test.go index f9aaf2c8..6f6bb0dd 100644 --- a/controllers/helmchart_controller_test.go +++ b/controllers/helmchart_controller_test.go @@ -2262,11 +2262,11 @@ func TestHelmChartReconciler_reconcileSourceFromOCI_verifySignature(t *testing.T tests := []struct { name string + shouldSign bool + beforeFunc func(obj *sourcev1.HelmChart) want sreconcile.Result wantErr bool wantErrMsg string - shouldSign bool - beforeFunc func(obj *sourcev1.HelmChart) assertConditions []metav1.Condition cleanFunc func(g *WithT, build *chart.Build) }{ @@ -2280,11 +2280,12 @@ func TestHelmChartReconciler_reconcileSourceFromOCI_verifySignature(t *testing.T SecretRef: &meta.LocalObjectReference{Name: "cosign-key"}, } }, + want: sreconcile.ResultEmpty, wantErr: true, wantErrMsg: "chart verification error: failed to verify : no matching signatures:", - want: sreconcile.ResultEmpty, assertConditions: []metav1.Condition{ *conditions.TrueCondition(sourcev1.BuildFailedCondition, "ChartVerificationError", "chart verification error: failed to verify : no matching signatures:"), + *conditions.FalseCondition(sourcev1.SourceVerifiedCondition, sourcev1.VerificationError, "chart verification error: failed to verify : no matching signatures:"), }, }, { @@ -2296,14 +2297,16 @@ func TestHelmChartReconciler_reconcileSourceFromOCI_verifySignature(t *testing.T Provider: "cosign", } }, - wantErr: true, want: sreconcile.ResultEmpty, + wantErr: true, assertConditions: []metav1.Condition{ *conditions.TrueCondition(sourcev1.BuildFailedCondition, "ChartVerificationError", "chart verification error: failed to verify : no matching signatures:"), + *conditions.FalseCondition(sourcev1.SourceVerifiedCondition, sourcev1.VerificationError, "chart verification error: failed to verify : no matching signatures:"), }, }, { - name: "signed charts should pass verification", + name: "signed charts should pass verification", + shouldSign: true, beforeFunc: func(obj *sourcev1.HelmChart) { obj.Spec.Chart = metadata.Name obj.Spec.Version = metadata.Version @@ -2312,8 +2315,7 @@ func TestHelmChartReconciler_reconcileSourceFromOCI_verifySignature(t *testing.T SecretRef: &meta.LocalObjectReference{Name: "cosign-key"}, } }, - shouldSign: true, - want: sreconcile.ResultSuccess, + want: sreconcile.ResultSuccess, assertConditions: []metav1.Condition{ *conditions.TrueCondition(sourcev1.ArtifactOutdatedCondition, "NewChart", "pulled '' chart with version ''"), *conditions.TrueCondition(sourcev1.SourceVerifiedCondition, meta.SucceededReason, "verified signature of version "), diff --git a/docs/api/source.md b/docs/api/source.md index 060197be..819248f1 100644 --- a/docs/api/source.md +++ b/docs/api/source.md @@ -684,7 +684,7 @@ OCIRepositoryVerification

Verify contains the secret name containing the trusted public keys used to verify the signature and specifies which provider to use to check whether OCI image is authentic. -This field is only supported for OCI sources. +This field is only supported when using HelmRepository source with spec.type ‘oci’. Chart dependencies, which are not bundled in the umbrella chart artifact, are not verified.

@@ -2269,7 +2269,7 @@ OCIRepositoryVerification

Verify contains the secret name containing the trusted public keys used to verify the signature and specifies which provider to use to check whether OCI image is authentic. -This field is only supported for OCI sources. +This field is only supported when using HelmRepository source with spec.type ‘oci’. Chart dependencies, which are not bundled in the umbrella chart artifact, are not verified.

diff --git a/docs/spec/v1beta2/helmcharts.md b/docs/spec/v1beta2/helmcharts.md index d6e18987..990ff869 100644 --- a/docs/spec/v1beta2/helmcharts.md +++ b/docs/spec/v1beta2/helmcharts.md @@ -290,7 +290,7 @@ data: Note that the keys must have the `.pub` extension for Flux to make use of them. -Flux will loop over the public keys and use them verify a HelmChart's signature. +Flux will loop over the public keys and use them to verify a HelmChart's signature. This allows for older HelmCharts to be valid as long as the right key is in the secret. #### Keyless verification @@ -309,6 +309,7 @@ metadata: name: podinfo spec: interval: 5m + chart: podinfo reconcileStrategy: ChartVersion sourceRef: kind: HelmRepository diff --git a/docs/spec/v1beta2/ocirepositories.md b/docs/spec/v1beta2/ocirepositories.md index a6b22aa9..39d1decf 100644 --- a/docs/spec/v1beta2/ocirepositories.md +++ b/docs/spec/v1beta2/ocirepositories.md @@ -462,7 +462,7 @@ data: Note that the keys must have the `.pub` extension for Flux to make use of them. -Flux will loop over the public keys and use them verify an artifact's signature. +Flux will loop over the public keys and use them to verify an artifact's signature. This allows for older artifacts to be valid as long as the right key is in the secret. #### Keyless verification diff --git a/internal/helm/registry/auth.go b/internal/helm/registry/auth.go index debe87ea..d843d7d3 100644 --- a/internal/helm/registry/auth.go +++ b/internal/helm/registry/auth.go @@ -105,7 +105,7 @@ func KeychainAdaptHelper(keyChain authn.Keychain) func(string) (registry.LoginOp } // AuthAdaptHelper returns an ORAS credentials callback configured with the authorization data -// from the given authn authenticator This allows for example to make use of credential helpers from +// from the given authn authenticator. This allows for example to make use of credential helpers from // cloud providers. // Ref: https://github.com/google/go-containerregistry/tree/main/pkg/authn func AuthAdaptHelper(auth authn.Authenticator) (registry.LoginOption, error) { diff --git a/internal/helm/repository/chart_repository.go b/internal/helm/repository/chart_repository.go index 15e62432..596bc1a8 100644 --- a/internal/helm/repository/chart_repository.go +++ b/internal/helm/repository/chart_repository.go @@ -523,10 +523,8 @@ func (r *ChartRepository) RemoveCache() error { } // VerifyChart verifies the chart against a signature. -// If no signature is provided, a keyless verification is performed. // It returns an error on failure. func (r *ChartRepository) VerifyChart(_ context.Context, _ *repo.ChartVersion) error { - // no-op // this is a no-op because this is not implemented yet. return fmt.Errorf("not implemented") }