Skip Dependency Update for Ad Hoc Dependencies

[qkflies]

Discussed in #155

^{Originally posted by qkflies June 10, 2022}
I was going to open an issue for this, but apparently, it's not possible to open issues right now? (figured it out)

I've been using a local directory as an ad-hoc dependency for my helmfile templating use case. This has worked in internet connected environments (helm dependency update could run without failing, even if it didn't fetch anything), but in non internet connected environments, the build fails.

Looking at the pertinent code in chartify.go, it does not appear that there is any way to override this. Is this intended behavior? Would strongly prefer an option to override it.

[qkflies]

Taking another look at the explanatory comment, it seems to assume that any ad-hoc dependency chart (1) is remote and/or (2) has dependencies that are remote. For the use case where the ad-hoc dependency(s) are purely local, I'm not certain if a more viable path would be to detect whether the ad-hoc dependency(s) are local, or a flag forcibly skipping helm dependency up . I would think it would be the latter (local charts could still have remote dependencies), but don't really have a preference one way or the other.

Just would like to be able to run helmfile template commands in non internet connected environments without stripping all ad hoc dependencies.

[qkflies]

Oh, I forgot to mention #50 seems like it could possibly be the same thing, not a whole lot of detail on the report though

[mumoshu]

@qkflies Hey! Thanks for reporting. Before diving into a possible solution, can we confirm one thing- How can you ensure that running helmfile template without dep updates is safe in your case?

A local chart with its dependencies listed under it's Chart.yaml may or may not miss dependency downloads(under the local chart's charts/ dir). Helmfile(and chartfy)'s stance is that it runs helm dep build anyway so that it can ensure those dependencies are there and up-to-date anyway. Without doing so, there might be no way to ensure dependencies are there? 🤔

[mumoshu]

BTW, have you already tried running helmfile template with --skip-deps, or set skipDeps: true under helmDefaults or each release?

[qkflies]

BTW, have you already tried running helmfile template with --skip-deps, or set skipDeps: true under helmDefaults or each release?

Hi @mumoshu, we did try these options. Unfortunately, the section of code from Chartify linked above still triggers when we use these options, because we are utilizing the adhocChartDependencies to point to a local chart (that is, len(u.AdhocChartDependencies) == 0 is never satisfied).

Our use case always templates (rather than deploys), and assumes a fully realized helm chart packaged locally for use in an airgapped / non internet connected environment (i.e., running helm dep update will always fail, so dependencies must already exist). We had yet to actually try running it completely offline until last Friday, where we discovered this behavior of Chartify.

A local chart with its dependencies listed under it's Chart.yaml may or may not miss dependency downloads(under the local chart's charts/ dir). Helmfile(and chartfy)'s stance is that it runs helm dep build anyway so that it can ensure those dependencies are there and up-to-date anyway. Without doing so, there might be no way to ensure dependencies are there? 🤔

Yeah, I was crunching through the same logic tree... it makes sense what it does, for most use cases. This just happens to be the edge use case where this behavior is completely breaking.

If there was a way to specify skip all dependency updates as a separate flag (e.g., --force-skip-deps), I think that probably has the least amount of breaking changes? Not sure if I'm missing something else to consider.

[cadeff01]

Without doing so, there might be no way to ensure dependencies are there? 🤔

The problem with this logic is when they are and you are air gapped. This throws an error which causes helmfile to error and make adhoc dependencies impossible to use. We do a helm dependency build and update of the chart before moving to air gapped and have this issue. skipDeps: True works for everything in helmfile we've used so far except this one feature.

[mumoshu]

that is, len(u.AdhocChartDependencies) == 0 is never satisfied

@qkflies Ahhhh okay that makes sense. I don't remember the rationale behind that logic, but now it looks like we should just remove the redundant len(u.AdhocChartDependencies) == 0 check. Otherwise, SkipDeps doesn't fully do what it's supposed to to do...

However, that might not be the only thing we need.

@bhester01 @qkflies As of today, I thought Helmfile nor chartify had no way to "vendor", "predownload" or "bundle" all the ad-hoc dependencies before running helmfile template in an airgapped environment.

How are you going to do that? Do you have any external tooling to achieve that, or are you expecting Helmfile to eventually provide a way to bundle the whole project structure plus ad-hoc dependencies so that it can be later used by airgapped helmfile template?

[cadeff01]

@mumoshu it doesn't have any built in way for sure. We pull the chart, unpack it, run the updates in an internet connected environment, then ship them to the air gap. Even for charts without dependencies this required deps update breaks ad hoc dependency add. Currently I adhoc add another local chart that I have in the repo to combine with a chart I pull from a local registry.

The only reason we can't do the same for helmfile is needing to have environment specific values for each of our air gapped deployments and allowing templating in the air gap if local changes are required.

[mumoshu]

@bhester01 Thanks! Gotcha. So you might better open another dedicated issue to add support for airgapped deployments to Helmfile.

If you're going to provide your own way to bundle your helmfile.yaml and charts, the only missing piece would be what's being fixed via this issue- make skipDeps work for ad-hoc deps.

BTW, how do you manage environment secrets/ release secrets? Does your airgapped environment still has access to some secret management system like Vault, so you want the airgapped helmfile-template to read secrets from Vault and use it to render the K8s manifests there? Worth noting in the dedicated issue.

Currently, chartify works by rendering the target chart into K8s manifests and re-packing the manifests into a chart. That implies the output(which is a chart itself) of chartify contains unencrpted secret data in it. If an imaginary feature of helmfile bundled it as-is, the bundle should contain the unecrypted secret data as well. Is that acceptable for your use-case?

[cadeff01]

@mumoshu airgap deployments work fine with helmfile with skipDeps: true. The issue is just the adhoc dependencies so yes if chartify would acknowledge that flag it would fix the issue.

Yes we use a secrets management system and environment configuration information so our helmfile is as generic as possible and can be used to deploy to a large number of environments.

Things like sealed secrets and just in time secrets prevent this issue as we aren't using helm secrets. They may be unencrpted as far as helm is concerned but they are still encrypted values that are expanded in the cluster or pull just in time from the secrets management system.

[cadeff01]

An interesting note about the way chartify renders things into k8s manifests and maybe something for another issue but if there is a kustomize file in a chart (which there never should be but istio has one) chartify will use it. See the istio/istiod chart for a really awful example that caused us a lot of pain trying to figure out why we kept getting default values when using helmfile template but proper values when using helm template.

[qkflies]

that is, len(u.AdhocChartDependencies) == 0 is never satisfied

@qkflies Ahhhh okay that makes sense. I don't remember the rationale behind that logic, but now it looks like we should just remove the redundant len(u.AdhocChartDependencies) == 0 check. Otherwise, SkipDeps doesn't fully do what it's supposed to to do...

@mumoshu So, I almost went ahead and opened a pull request to do this, but looking at it I was reminded that the following explanatory comment for this check is like so:

if isLocal {
	// Note on `len(u.AdhocChartDependencies) == 0`:
	// This special handling is required because adding adhoc chart dependencies
	// means that you MUST run `helm dep up` and `helm dep build` to download the dependencies into the ./charts directory.
	// Otherwise you end up getting:
	//   Error: found in Chart.yaml, but missing in charts/ directory: $DEP_CHART_1, $DEP_CHART_2, ...`
	// ...which effectively making this useless when used in e.g. helmfile
	if u.SkipDeps && len(u.AdhocChartDependencies) == 0 {
		r.Logf("Skipping `helm dependency up` on release %s's chart due to that you've set SkipDeps=true.\n"+
			"This may result in outdated chart dependencies.", release)
...

So, would it still make sense to yank the check entirely? That would change current behavior, which was implemented 14 months ago.

[cadeff01]

I actually think I might have made a very poor assumption. We are doing a dependency build and update on the chart without the adhoc added. The adhoc is just a folder with manifests in it but without helm update it won't pull that into the chart. This has more to do with helm not having an option to support redirecting to a local repo/registry or being able to add local content without doing the deps update. This request is very likely moot, there is no way for adhoc dependencies to work in air gapped environments with helm if the chart has existing dependencies that point to repos on the internet as far as I can tell so that breaks adhoc dependencies regardless of chartify.

Or at least without modifying the chart.yaml, so maybe some use for this request but it is far more narrow than I had hoped it would be.

[stale]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.