Helm error message is not helpful

[pwillis-eiq]

Summary

When using Helm, if you declare a dependency but do not specify a version constraint in that dependency declaration, you get an error which doesn't tell you what is really wrong. Instead it says that a dependency has an "improper" constraint, which isn't true (it doesn't have any constraint at all, because of the missing key). After an hour of googling and experimentation, you finally learn that, actually, it was just a missing key.

Suggested fix

Change error message from:

Error: dependency "some-sub-chart" has an invalid version/constraint format: improper constraint:
helm.go:84: [debug] improper constraint:
dependency "some-sub-chart" has an invalid version/constraint format

To:

Error: dependency "some-sub-chart" has an invalid version/constraint format: missing "version" key in dependency definition
helm.go:84: [debug] improper constraint:
dependency "some-sub-chart" has an invalid version/constraint format: missing "version" key in dependency definition

Reproducing the bug

Make a chart like this:

# Chart.yaml
apiVersion: v2
name: foobar
version: 1.0.0
dependencies:
- name: some-sub-chart

And a sub-chart:

# charts/some-sub-chart/Charts.yaml
apiVersion: v2
name: some-sub-chart
version: 1.0.0

Try to check dependencies in the parent:

$ helm dependency update --skip-refresh --debug
Error: dependency "some-sub-chart" has an invalid version/constraint format: improper constraint:
helm.go:84: [debug] improper constraint:
dependency "some-sub-chart" has an invalid version/constraint format
helm.sh/helm/v3/internal/resolver.(*Resolver).Resolve
	helm.sh/helm/v3/internal/resolver/resolver.go:63
helm.sh/helm/v3/pkg/downloader.(*Manager).resolve
	helm.sh/helm/v3/pkg/downloader/manager.go:236
helm.sh/helm/v3/pkg/downloader.(*Manager).Update
	helm.sh/helm/v3/pkg/downloader/manager.go:195
main.newDependencyUpdateCmd.func1
	helm.sh/helm/v3/cmd/helm/dependency_update.go:74
github.com/spf13/cobra.(*Command).execute
	github.com/spf13/cobra@v1.5.0/command.go:872
github.com/spf13/cobra.(*Command).ExecuteC
	github.com/spf13/cobra@v1.5.0/command.go:990
github.com/spf13/cobra.(*Command).Execute
	github.com/spf13/cobra@v1.5.0/command.go:918
main.main
	helm.sh/helm/v3/cmd/helm/helm.go:83
runtime.main
	runtime/proc.go:250
runtime.goexit
	runtime/asm_arm64.s:1172

Then modify the parent chart:

# Chart.yaml
apiVersion: v2
name: foobar
version: 1.0.0
dependencies:
- name: some-sub-chart
  version: 1.0.0

Run again from parent:

$ helm dependency update --skip-refresh --debug
Saving 1 charts
Dependency some-sub-chart did not declare a repository. Assuming it exists in the charts directory
Deleting outdated charts

Debugging

Output of helm version:

$ helm version
version.BuildInfo{Version:"v3.10.2", GitCommit:"50f003e5ee8704ec937a756c646870227d7c8b58", GitTreeState:"clean", GoVersion:"go1.19.3"}

Output of kubectl version:

$ kubectl version
Client Version: version.Info{Major:"1", Minor:"21", GitVersion:"v1.21.14", GitCommit:"0f77da5bd4809927e15d1658fb4aa8f13ad890a5", GitTreeState:"clean", BuildDate:"2022-06-15T14:17:29Z", GoVersion:"go1.16.15", Compiler:"gc", Platform:"darwin/arm64"}
Server Version: version.Info{Major:"1", Minor:"21", GitVersion:"v1.21.14-gke.4300", GitCommit:"348bdc1040d273677ca07c0862de867332eeb3a1", GitTreeState:"clean", BuildDate:"2022-08-17T09:22:54Z", GoVersion:"go1.16.15b7", Compiler:"gc", Platform:"linux/amd64"}

Cloud Provider/Platform (AKS, GKE, Minikube etc.): GKE

[gjenkins8]

Thanks for the suggestion. A PR to make an improvement here would be welcome please!

[pwillis-eiq]

(Incidentally I found another issue where somebody else came across this: #10095)

Hi @gjenkins8 , thanks for the reply. I don't really know Go, but I'm willing to give a PR a shot...

I took a look at the code, and there are at least 3 sections that parse dependencies:

• https://github.com/helm/helm/blob/main/cmd/helm/search_repo.go#L146-L175
• https://github.com/helm/helm/blob/main/internal/resolver/resolver.go#L54-L108
• https://github.com/helm/helm/blob/main/pkg/downloader/manager.go#L239-L312

It appears that there is some inconsistency in parsing here, and this would probably benefit from DRYing up the parsing to a single function/class/whatever Go uses in this case.

it might also be useful to remove 'omitempty' (https://github.com/helm/helm/blob/main/pkg/chart/dependency.go#L20-L50) from the json parsing for testing if the key is there (or empty), but I'm not sure yet, I haven't worked with this library before.

What would your thoughts be on an ideal implementation/fix, @gjenkins8 ?

[joejulian]

You'd need to check here for d.Version being nil. If it is, return your improved error message.

[pwillis-eiq]

Thanks @joejulian !

In this issue (#10095) OP is suggesting that version should be optional, for self-managed subcharts / charts with no repo. Based on your knowledge of helm, would that behavior be preferred? The difference would be, either helm always requires a version constraint, or the version constraint is optional (meaning any version is fine).

[joejulian]

The version constraint is optional. If you have a dependent chart that resides in the charts/ path or uses the file:// protocol, no version is needed or used.

[github-actions]

This issue has been marked as stale because it has been open for 90 days with no activity. This thread will be automatically closed in 30 days if no further activity occurs.

[crabique]

The version constraint is optional. If you have a dependent chart that resides in the charts/ path or uses the file:// protocol, no version is needed or used.

This is not true for dependancy charts, the value for the version key could be "*", which makes it use any availavle version, but the version key itself is not optional. The operation fails with a fatal error for an invalid (or empty) version constraint:

helm/internal/resolver/resolver.go

Lines 61 to 64 in 3a94215

	constraint, err := semver.NewConstraint(d.Version)
	if err != nil {
	return nil, errors.Wrapf(err, "dependency %q has an invalid version/constraint format", d.Name)
	}

PR #10096 is an effort to make it actually optional, unfortunately it's waiting on a review since 2021, while all related issues got disappeared by the stale bot.