fix: address review on repoURL provenance stamping

- Stamp chart.Metadata.RepoURL in Install/Upgrade actions instead of
    pkg/cmd, so library consumers also get provenance recorded.
  - Don't mislabel local installs as path; fill RepoURL only for
    resolved repos, OCI refs, and absolute http(s) URLs.
  - Unexport ChartDownloader.RepositoryURL; expose via accessor to
    preserve ABI for downstream unkeyed-struct users.
  - Fix render test template to use .Chart.RepoURL.

Signed-off-by: Aleksei Krugliak <aleksei.krugliak@altenar.com>
pull/31792/head
Aleksei Krugliak 4 months ago
parent 31132a16fe
commit 97e54c8f11

@ -352,6 +352,16 @@ func (i *Install) RunWithContext(ctx context.Context, ch ci.Charter, vals map[st
return nil, fmt.Errorf("user supplied labels contains system reserved label name. System labels: %+v", driver.GetSystemLabels())
}
// Stamp provenance information into the chart metadata if available from
// the ChartPathOptions. Do not overwrite existing metadata.RepoURL set by
// the chart itself. This ensures library consumers who call actions get
// provenance filled in when LocateChart or a downloader discovered it.
if chrt.Metadata != nil && chrt.Metadata.RepoURL == "" {
if i.ChartPathOptions.RepoURL != "" {
chrt.Metadata.RepoURL = i.ChartPathOptions.RepoURL
}
}
rel := i.createRelease(chrt, vals, i.Labels)
var manifestDoc *bytes.Buffer
@ -857,6 +867,20 @@ func urlEqual(u1, u2 *url.URL) bool {
return u1.Scheme == u2.Scheme && u1.Hostname() == u2.Hostname() && portOrDefault(u1) == portOrDefault(u2)
}
// isRemoteChartRef reports whether ref points to a remote chart (an OCI
// reference or an absolute http(s) URL), as opposed to a local path or a
// repo-by-name reference.
func isRemoteChartRef(ref string) bool {
if registry.IsOCI(ref) {
return true
}
u, err := url.Parse(ref)
if err != nil {
return false
}
return u.IsAbs() && (u.Scheme == "http" || u.Scheme == "https")
}
// LocateChart looks for a chart directory in known places, and returns either the full path or an error.
//
// This does not ensure that the chart is well-formed; only that the requested filename exists.
@ -964,7 +988,16 @@ func (c *ChartPathOptions) LocateChart(name string, settings *cli.EnvSettings) (
if err != nil {
return "", err
}
c.RepoURL = dl.RepositoryURL
// Record provenance information about where the chart was resolved from.
// Prefer the URL discovered by the downloader; fall back to the original
// reference when it is an OCI ref or an absolute URL. Leave RepoURL empty
// for local paths and unresolvable references so callers can distinguish.
if u := dl.RepositoryURL(); u != "" {
c.RepoURL = u
} else if c.RepoURL == "" && isRemoteChartRef(name) {
c.RepoURL = name
}
lname, err := filepath.Abs(filename)
if err != nil {

@ -0,0 +1,62 @@
package action
import (
"path/filepath"
"testing"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
"helm.sh/helm/v4/pkg/chart/v2/loader"
chartutil "helm.sh/helm/v4/pkg/chart/v2/util"
)
func TestRenderRepoURL2_ValidAndInvalid(t *testing.T) {
cfg := actionConfigFixture(t)
tests := []struct {
name string
chartPath string
expectPart string
}{
{
name: "valid repoURL",
chartPath: "testdata/charts/chart-with-repourl-valid",
expectPart: `repoURL: "https://example.com/charts"`,
},
{
name: "invalid repoURL",
chartPath: "testdata/charts/chart-with-repourl-invalid",
expectPart: `repoURL: "ht!tp://not-a-valid-url"`,
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
ch, err := loader.Load(tt.chartPath)
require.NoError(t, err)
if ch.Metadata == nil {
md, err := chartutil.LoadChartfile(filepath.Join(tt.chartPath, "Chart.yaml"))
require.NoError(t, err)
ch.Metadata = md
}
_, buf, _, err := cfg.renderResources(
ch,
map[string]interface{}{},
"test-release",
"",
false,
false,
false,
nil,
false,
false,
false,
)
require.NoError(t, err)
require.NotNil(t, buf)
assert.Contains(t, buf.String(), tt.expectPart)
})
}
}

@ -0,0 +1,5 @@
apiVersion: v2
name: chart-with-repourl-invalid
version: 0.1.0
description: Test chart containing an invalid repoURL in metadata
repoURL: "ht!tp://not-a-valid-url"

@ -0,0 +1,6 @@
apiVersion: v1
kind: ConfigMap
metadata:
name: chart-with-repourl-invalid-cm
data:
repoURL: {{ .Chart.RepoURL | quote }}

@ -0,0 +1,5 @@
apiVersion: v2
name: chart-with-repourl-valid
version: 0.1.0
description: Test chart containing a valid repoURL in metadata
repoURL: https://example.com/charts

@ -0,0 +1,6 @@
apiVersion: v1
kind: ConfigMap
metadata:
name: chart-with-repourl-valid-cm
data:
repoURL: {{ .Chart.RepoURL | quote }}

@ -1,4 +1,4 @@
apiVersion: v1
name: multiplecharts-lint-chart-2
version: "1"
icon: ""
icon: ""

@ -312,6 +312,15 @@ func (u *Upgrade) prepareUpgrade(name string, chart *chartv2.Chart, vals map[str
u.cfg.Logger().Debug("determined release apply method", slog.Bool("server_side_apply", serverSideApply), slog.String("previous_release_apply_method", lastRelease.ApplyMethod))
// Stamp provenance information into the chart metadata if available from
// the ChartPathOptions. Only set it when chart metadata doesn't already
// contain a RepoURL so we don't overwrite embedded provenance.
if chart.Metadata != nil && chart.Metadata.RepoURL == "" {
if u.ChartPathOptions.RepoURL != "" {
chart.Metadata.RepoURL = u.ChartPathOptions.RepoURL
}
}
// Store an upgraded release.
upgradedRelease := &release.Release{
Name: name,

@ -52,6 +52,8 @@ type Metadata struct {
Home string `json:"home,omitempty"`
// Source is the URL to the source code of this chart
Sources []string `json:"sources,omitempty"`
// The URL to the chart's repository (provenance information)
RepoURL string `json:"repoURL,omitempty"`
// A version string of the chart. Required.
Version string `json:"version,omitempty"`
// A one-sentence description of the chart
@ -93,6 +95,7 @@ func (md *Metadata) Validate() error {
md.Name = sanitizeString(md.Name)
md.Description = sanitizeString(md.Description)
md.Home = sanitizeString(md.Home)
md.RepoURL = sanitizeString(md.RepoURL)
md.Icon = sanitizeString(md.Icon)
md.Condition = sanitizeString(md.Condition)
md.Tags = sanitizeString(md.Tags)

@ -262,12 +262,6 @@ func runInstall(args []string, client *action.Install, valueOpts *values.Options
return nil, err
}
if client.ChartPathOptions.RepoURL != "" {
chartRequested.Metadata.RepoURL = client.ChartPathOptions.RepoURL
} else {
chartRequested.Metadata.RepoURL = "path"
}
ac, err := chart.NewAccessor(chartRequested)
if err != nil {
return nil, err

@ -197,11 +197,6 @@ func newUpgradeCmd(cfg *action.Configuration, out io.Writer) *cobra.Command {
if err != nil {
return err
}
if client.ChartPathOptions.RepoURL != "" {
ch.Metadata.RepoURL = client.ChartPathOptions.RepoURL
} else {
ch.Metadata.RepoURL = "path"
}
ac, err := ci.NewAccessor(ch)
if err != nil {

@ -84,8 +84,16 @@ type ChartDownloader struct {
ContentCache string
// Cache specifies the cache implementation to use.
Cache Cache
RepositoryURL string
Cache Cache
// repositoryURL is the resolved Helm repository URL, set by
// ResolveChartVersion when the chart is looked up via a configured repo.
repositoryURL string
}
// RepositoryURL returns the resolved Helm repository URL, if any.
func (c *ChartDownloader) RepositoryURL() string {
return c.repositoryURL
}
// DownloadTo retrieves a chart. Depending on the settings, it may also download a provenance file.
@ -432,7 +440,7 @@ func (c *ChartDownloader) ResolveChartVersion(ref, version string) (string, *url
if r != nil && r.Config != nil {
if r.Config.URL != "" {
c.RepositoryURL = r.Config.URL
c.repositoryURL = r.Config.URL
}
if r.Config.CertFile != "" || r.Config.KeyFile != "" || r.Config.CAFile != "" {
c.Options = append(c.Options, getter.WithTLSClientConfig(r.Config.CertFile, r.Config.KeyFile, r.Config.CAFile))

Loading…
Cancel
Save