diff --git a/pkg/state/chart_dependency.go b/pkg/state/chart_dependency.go index 7f88081c..242da4de 100644 --- a/pkg/state/chart_dependency.go +++ b/pkg/state/chart_dependency.go @@ -183,10 +183,10 @@ func resolveDependencies(st *HelmState, depMan *chartDependencyManager, unresolv return st, nil } - repoToURL := map[string]string{} + repoToSpec := map[string]RepositorySpec{} for _, r := range st.Repositories { - repoToURL[r.Name] = r.URL + repoToSpec[r.Name] = r } updated := *st @@ -196,14 +196,26 @@ func resolveDependencies(st *HelmState, depMan *chartDependencyManager, unresolv continue } - _, ok = repoToURL[repo] + repoSpec, ok := repoToSpec[repo] // Skip this chart from dependency management, as there's no matching `repository` in the helmfile state, // which may imply that this is a local chart within a directory, like `charts/myapp` if !ok { continue } - ver, err := resolved.Get(chart, r.Version) + // For OCI charts, the dependency name in the lock file is the chart basename + // (last path segment), not the full path. Apply the same transformation as + // getUnresolvedDependenciess to ensure the lookup matches (see issue #954). + lookupChart := chart + if repoSpec.OCI { + lookupChart = ociDependencyChartName(chart) + } + + ver, err := resolved.Get(lookupChart, r.Version) + if err != nil && repoSpec.OCI && lookupChart != chart { + // Backward compatibility: old lock files may use the full path as name + ver, err = resolved.Get(chart, r.Version) + } if err != nil { return nil, err } @@ -238,6 +250,35 @@ func chartDependenciesAlias(namespace, releaseName string) string { return fmt.Sprintf("%s-%s", namespace, releaseName) } +// ociDependencyChartName returns the last path segment of a chart name, which is +// used as the dependency name in Chart.yaml for OCI charts. For example: +// +// "path_with_underscores/example" → "example" +// "example" → "example" +// +// This avoids issues with slashes, underscores, and other special characters in +// the chart path when helm processes OCI references during dependency update. +// See issue #954. +func ociDependencyChartName(chart string) string { + if idx := strings.LastIndex(chart, "/"); idx >= 0 { + return chart[idx+1:] + } + return chart +} + +// ociDependencyRepoURL builds the repository URL for an OCI chart dependency by +// combining the base registry URL with any path prefix from the chart name. +// For example: +// +// chart="path_with_underscores/example", baseURL="oci://registry.example.com" +// → "oci://registry.example.com/path_with_underscores" +func ociDependencyRepoURL(chart, ociBaseURL string) string { + if idx := strings.LastIndex(chart, "/"); idx >= 0 { + return strings.TrimSuffix(ociBaseURL, "/") + "/" + chart[:idx] + } + return ociBaseURL +} + func getUnresolvedDependenciess(st *HelmState) (string, *UnresolvedDependencies) { repoToURL := map[string]RepositorySpec{} @@ -263,7 +304,13 @@ func getUnresolvedDependenciess(st *HelmState) (string, *UnresolvedDependencies) url := repoSpec.URL if repoSpec.OCI { - url = fmt.Sprintf("oci://%s", url) + ociBaseURL := fmt.Sprintf("oci://%s", url) + // For OCI charts with multi-segment paths (e.g., "path/chart"), put the + // path prefix into the repository URL and use just the chart basename as + // the dependency name. This avoids issues with underscores and other + // special characters in OCI references during helm dependency update. + url = ociDependencyRepoURL(chart, ociBaseURL) + chart = ociDependencyChartName(chart) } unresolved.Add(chart, url, r.Version, chartDependenciesAlias(r.Namespace, r.Name)) diff --git a/pkg/state/chart_dependency_test.go b/pkg/state/chart_dependency_test.go index 70585b1c..ed558d6d 100644 --- a/pkg/state/chart_dependency_test.go +++ b/pkg/state/chart_dependency_test.go @@ -77,7 +77,57 @@ func TestGetUnresolvedDependenciess(t *testing.T) { expectDeps *UnresolvedDependencies }{ { - name: "oci chart", + name: "oci chart with path prefix and underscores (issue #954)", + helmState: &HelmState{ + FilePath: "helmfile.yaml", + ReleaseSetSpec: ReleaseSetSpec{ + Releases: []ReleaseSpec{ + { + Name: "example", + Chart: "myrepo/path_with_underscores/example", + Version: "1.0.0", + Namespace: "myns", + }, + { + Name: "another", + Chart: "myrepo/another_path/chart", + Version: "2.0.0", + Namespace: "myns", + }, + }, + Repositories: []RepositorySpec{ + { + Name: "myrepo", + URL: "harbor.custom.com", + OCI: true, + }, + }, + }, + }, + expectfile: "helmfile", + expectDeps: &UnresolvedDependencies{ + deps: map[string][]unresolvedChartDependency{ + "example": { + { + ChartName: "example", + Repository: "oci://harbor.custom.com/path_with_underscores", + VersionConstraint: "1.0.0", + Alias: "myns-example", + }, + }, + "chart": { + { + ChartName: "chart", + Repository: "oci://harbor.custom.com/another_path", + VersionConstraint: "2.0.0", + Alias: "myns-another", + }, + }, + }, + }, + }, + { + name: "oci chart without path prefix (unchanged behavior)", helmState: &HelmState{ FilePath: "helmfile.yaml", ReleaseSetSpec: ReleaseSetSpec{ @@ -226,3 +276,66 @@ func TestChartDependenciesAlias(t *testing.T) { } } } + +func TestOciDependencyChartName(t *testing.T) { + tests := []struct { + name string + chart string + want string + }{ + {name: "simple chart name", chart: "example", want: "example"}, + {name: "path with underscores", chart: "path_with_underscores/example", want: "example"}, + {name: "deep nested path", chart: "deep/nested/chart", want: "chart"}, + {name: "single segment with underscore", chart: "my_chart", want: "my_chart"}, + {name: "path with hyphens", chart: "path-with-hyphens/example", want: "example"}, + {name: "empty string", chart: "", want: ""}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got := ociDependencyChartName(tt.chart) + require.Equal(t, tt.want, got) + }) + } +} + +func TestOciDependencyRepoURL(t *testing.T) { + tests := []struct { + name string + chart string + ociBaseURL string + want string + }{ + { + name: "simple chart (no path prefix)", + chart: "example", + ociBaseURL: "oci://registry.example.com", + want: "oci://registry.example.com", + }, + { + name: "chart with path prefix and underscores", + chart: "path_with_underscores/example", + ociBaseURL: "oci://harbor.custom.com", + want: "oci://harbor.custom.com/path_with_underscores", + }, + { + name: "deeply nested path", + chart: "deep/nested/chart", + ociBaseURL: "oci://registry.example.com", + want: "oci://registry.example.com/deep/nested", + }, + { + name: "base URL with trailing slash", + chart: "path/example", + ociBaseURL: "oci://registry.example.com/", + want: "oci://registry.example.com/path", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got := ociDependencyRepoURL(tt.chart, tt.ociBaseURL) + require.Equal(t, tt.want, got) + }) + } +} diff --git a/pkg/state/state_test.go b/pkg/state/state_test.go index be883d32..9aea14ad 100644 --- a/pkg/state/state_test.go +++ b/pkg/state/state_test.go @@ -3139,6 +3139,133 @@ generated: 2019-05-16T15:42:45.50486+09:00 } } +func TestHelmState_UpdateDeps_OCIUnderscores(t *testing.T) { + helm := &exectest.Helm{ + UpdateDepsCallbacks: map[string]func(string) error{}, + } + + var generatedDir string + var generatedChartYaml string + tempDir := func(dir, prefix string) (string, error) { + var err error + generatedDir, err = os.MkdirTemp(dir, prefix) + if err != nil { + return "", err + } + helm.UpdateDepsCallbacks[generatedDir] = func(string) error { + // Read the Chart.yaml that helmfile generated to verify its content + chartYamlBytes, readErr := os.ReadFile(filepath.Join(generatedDir, "Chart.yaml")) + if readErr == nil { + generatedChartYaml = string(chartYamlBytes) + } + // Simulate helm writing Chart.lock with the basename as dependency name + content := []byte(`dependencies: +- name: example + repository: oci://harbor.custom.com/path_with_underscores + version: 1.0.0 +digest: sha256:abc123def456 +generated: 2023-08-01T23:04:02Z +`) + return os.WriteFile(filepath.Join(generatedDir, "Chart.lock"), content, 0644) + } + return generatedDir, nil + } + + logger := helmexec.NewLogger(io.Discard, "debug") + basePath := filepath.ToSlash(t.TempDir()) + state := &HelmState{ + basePath: basePath, + FilePath: filepath.Join(basePath, "helmfile.yaml"), + ReleaseSetSpec: ReleaseSetSpec{ + Releases: []ReleaseSpec{ + { + Name: "example", + Chart: "myrepo/path_with_underscores/example", + Version: "1.0.0", + Namespace: "myns", + }, + }, + Repositories: []RepositorySpec{ + { + Name: "myrepo", + URL: "harbor.custom.com", + OCI: true, + }, + }, + }, + tempDir: tempDir, + logger: logger, + } + + fs := testhelper.NewTestFs(map[string]string{}) + fs.Cwd = basePath + state = injectFs(state, fs) + errs := state.UpdateDeps(helm, false) + if len(errs) != 0 { + t.Fatalf("HelmState.UpdateDeps() - unexpected %d errors: %v", len(errs), errs) + } + + // Verify the generated Chart.yaml has basename as name and path prefix in repository + assert.Contains(t, generatedChartYaml, "name: example") + assert.Contains(t, generatedChartYaml, "repository: oci://harbor.custom.com/path_with_underscores") + // It should NOT contain the full path as the name + assert.NotContains(t, generatedChartYaml, "name: path_with_underscores/example") + + // Verify the lock file was written with the correct resolved version + resolved, err := state.ResolveDeps() + assert.NoError(t, err) + assert.Equal(t, "1.0.0", resolved.Releases[0].Version) +} + +func TestHelmState_ResolveDeps_OCIUnderscores_BackwardCompat(t *testing.T) { + logger := helmexec.NewLogger(io.Discard, "debug") + basePath := filepath.ToSlash(t.TempDir()) + + // Old-format lock file using the full path as the dependency name + oldLockContent := `version: 0.155.0 +dependencies: +- name: path_with_underscores/example + repository: oci://harbor.custom.com + version: 1.0.0 +digest: sha256:abc123 +generated: 2023-08-01T23:04:02Z +` + lockPath := filepath.Join(basePath, "helmfile.lock") + err := os.WriteFile(lockPath, []byte(oldLockContent), 0644) + require.NoError(t, err) + + state := &HelmState{ + basePath: basePath, + FilePath: filepath.Join(basePath, "helmfile.yaml"), + ReleaseSetSpec: ReleaseSetSpec{ + Releases: []ReleaseSpec{ + { + Name: "example", + Chart: "myrepo/path_with_underscores/example", + Version: "1.0.0", + }, + }, + Repositories: []RepositorySpec{ + { + Name: "myrepo", + URL: "harbor.custom.com", + OCI: true, + }, + }, + }, + logger: logger, + } + + fs := testhelper.NewTestFs(map[string]string{}) + fs.Cwd = basePath + state = injectFs(state, fs) + + resolved, err := state.ResolveDeps() + require.NoError(t, err) + assert.Equal(t, "1.0.0", resolved.Releases[0].Version, + "old lock file with full-path name should still resolve via backward-compat fallback") +} + func TestHelmState_ResolveDeps_NoLockFile(t *testing.T) { logger := helmexec.NewLogger(io.Discard, "debug") state := &HelmState{