-
Notifications
You must be signed in to change notification settings - Fork 117
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Allow more flexible chart options #284
Conversation
version
optional on helm.ChartOpts
Allows the `chart` input argument to be used in more of the same ways allowed as argument to `helm install`. Makes `repo` optional to allow chart to be provided in forms that require fully qualified references. Actually expose the `fetchOpts` argument so that `--repo` (and other customerizations to fetch) can be passed. Adds some additional test coverage. All of the following now work: // #1 repo: "stable", chart: "nginx-lego", version: "0.3.1", // #2 repo: "stable", chart: "nginx-lego", // #3 chart: "stable/nginx-lego", // #4 chart: "stable/nginx-lego", version: "0.3.1" // #5 chart: "nginx-lego", version: "0.3.1", fetchOpts: { repo: "https://kubernetes-charts.storage.googleapis.com" }, // #6 chart: "nginx-lego", fetchOpts: { repo: "https://kubernetes-charts.storage.googleapis.com" }, // #7 chart: "https://kubernetes-charts.storage.googleapis.com/nginx-lego-0.3.1.tgz", Fixes #229. Fixes #257.
3088a11
to
01bdd26
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is the sort of patch where you think: why didn't I do it this way to begin with?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actually, I just tried this:
export const istio = new k8s.helm.v2.Chart(
appName,
{
repo: "https://istio.io/charts",
chart: "istio",
namespace: namespace.metadata.apply(m => m.name),
version: "1.0.1",
// fetchOpts: { repo: },
// for all options check https://github.com/istio/istio/tree/master/install/kubernetes/helm/istio
values: { kiali: { enabled: true } }
},
{ dependsOn: [namespace], providers: { kubernetes: k8sProvider } }
);
and also this:
export const istio = new k8s.helm.v2.Chart(
appName,
{
chart: "istio",
namespace: namespace.metadata.apply(m => m.name),
version: "1.0.1",
fetchOpts: { repo: "https://istio.io/charts" },
// for all options check https://github.com/istio/istio/tree/master/install/kubernetes/helm/istio
values: { kiali: { enabled: true } }
},
{ dependsOn: [namespace], providers: { kubernetes: k8sProvider } }
);
both of these result in this:
error: Error: Command failed: helm fetch https\://istio.io/charts/istio --untar --version 1.0.1 --destination /var/folders/w5/n_5_kc696xxb8h9rd8d3px2r0000gp/T/tmp-41891FR25WX35Q094
Error: Failed to fetch https://istio.io/charts/istio : 404 Not Found
Am I misunderstanding the API?
Is that really where that chart lives? I get the same from
|
Looks like there is a bug in |
Opened helm/helm#4954 to track the issue in I'm not sure it makes sense to workaround this in our layer - the Were we to have done something here where we allowed this - I could imagine us adding the workaround to it - but we haven't added support for this form at all (because it isn't really something
Given that, I'll go ahead and merge this in as is for now. |
Fixes #255.
Fixes #229.
Fixes #257.
Align more closely with
helm
options for chart references to support all scenarios supported by Helm including (a) getting latest by passing no version (b) using explicitly repo URLs (c) providing fully qualified URLs to charts.All of the following now work: