fix(dep): add logic to make use of the enabled field for dependencies

addresses #7908

But as per my comment in that issue, https://github.com/helm/helm/issues/7908#issuecomment-629275316
it's possible/likely that this field wasn't intended for use.
And the implementation of unmarshaling twice comes off as a bit hacky to me. So maybe instead of this PR, it's better to just remove
the enabled field from the documentation.

-------
SIDE NOTE:
I'm not sure how to regenerate frobnitz_with_bom.tgz for the test to pass.

Signed-off-by: Jeff Knurek <knurek.stuff@gmail.com>
pull/8138/head
Jeff Knurek 5 years ago
parent 3779d95966
commit d74806b951

@ -83,6 +83,17 @@ func LoadFiles(files []*BufferedFile) (*chart.Chart, error) {
if err := yaml.Unmarshal(f.Data, c.Metadata); err != nil { if err := yaml.Unmarshal(f.Data, c.Metadata); err != nil {
return c, errors.Wrap(err, "cannot load Chart.yaml") return c, errors.Wrap(err, "cannot load Chart.yaml")
} }
// when unmarshaling dependencies, the value of Enabled will default to false
// in order to make use of this flag, first need to change all values to true, then unmarshal again
if len(c.Metadata.Dependencies) > 0 {
for _, dep := range c.Metadata.Dependencies {
dep.Enabled = true
}
if err := yaml.Unmarshal(f.Data, c.Metadata); err != nil {
return c, errors.Wrap(err, "reloading Chart.yaml failed")
}
}
// NOTE(bacongobbler): while the chart specification says that APIVersion must be set, // NOTE(bacongobbler): while the chart specification says that APIVersion must be set,
// Helm 2 accepted charts that did not provide an APIVersion in their chart metadata. // Helm 2 accepted charts that did not provide an APIVersion in their chart metadata.
// Because of that, if APIVersion is unset, we should assume we're loading a v1 chart. // Because of that, if APIVersion is unset, we should assume we're loading a v1 chart.

@ -450,8 +450,8 @@ func verifyDependencies(t *testing.T, c *chart.Chart) {
t.Errorf("Expected 2 dependencies, got %d", len(c.Metadata.Dependencies)) t.Errorf("Expected 2 dependencies, got %d", len(c.Metadata.Dependencies))
} }
tests := []*chart.Dependency{ tests := []*chart.Dependency{
{Name: "alpine", Version: "0.1.0", Repository: "https://example.com/charts"}, {Name: "alpine", Version: "0.1.0", Repository: "https://example.com/charts", Enabled: true},
{Name: "mariner", Version: "4.3.2", Repository: "https://example.com/charts"}, {Name: "mariner", Version: "4.3.2", Repository: "https://example.com/charts", Enabled: false},
} }
for i, tt := range tests { for i, tt := range tests {
d := c.Metadata.Dependencies[i] d := c.Metadata.Dependencies[i]
@ -464,6 +464,9 @@ func verifyDependencies(t *testing.T, c *chart.Chart) {
if d.Repository != tt.Repository { if d.Repository != tt.Repository {
t.Errorf("Expected dependency named %q to have repository %q, got %q", tt.Name, tt.Repository, d.Repository) t.Errorf("Expected dependency named %q to have repository %q, got %q", tt.Name, tt.Repository, d.Repository)
} }
if d.Enabled != tt.Enabled {
t.Errorf("Expected dependency named %q to have enabled %t, got %t", tt.Name, tt.Enabled, d.Enabled)
}
} }
} }

Binary file not shown.

Binary file not shown.

@ -2,6 +2,8 @@ dependencies:
- name: alpine - name: alpine
version: "0.1.0" version: "0.1.0"
repository: https://example.com/charts repository: https://example.com/charts
enabled: true
- name: mariner - name: mariner
version: "4.3.2" version: "4.3.2"
repository: https://example.com/charts repository: https://example.com/charts
enabled: false

@ -2,6 +2,8 @@ dependencies:
- name: alpine - name: alpine
version: "0.1.0" version: "0.1.0"
repository: https://example.com/charts repository: https://example.com/charts
enabled: true
- name: mariner - name: mariner
version: "4.3.2" version: "4.3.2"
repository: https://example.com/charts repository: https://example.com/charts
enabled: false

@ -22,6 +22,8 @@ dependencies:
- name: alpine - name: alpine
version: "0.1.0" version: "0.1.0"
repository: https://example.com/charts repository: https://example.com/charts
enabled: true
- name: mariner - name: mariner
version: "4.3.2" version: "4.3.2"
repository: https://example.com/charts repository: https://example.com/charts
enabled: false

@ -22,6 +22,8 @@ dependencies:
- name: alpine - name: alpine
version: "0.1.0" version: "0.1.0"
repository: https://example.com/charts repository: https://example.com/charts
enabled: true
- name: mariner - name: mariner
version: "4.3.2" version: "4.3.2"
repository: https://example.com/charts repository: https://example.com/charts
enabled: false

@ -22,6 +22,8 @@ dependencies:
- name: alpine - name: alpine
version: "0.1.0" version: "0.1.0"
repository: https://example.com/charts repository: https://example.com/charts
enabled: true
- name: mariner - name: mariner
version: "4.3.2" version: "4.3.2"
repository: https://example.com/charts repository: https://example.com/charts
enabled: false

@ -137,6 +137,9 @@ Loop:
} }
for _, req := range c.Metadata.Dependencies { for _, req := range c.Metadata.Dependencies {
if req.Enabled == false {
continue
}
if chartDependency := getAliasDependency(c.Dependencies(), req); chartDependency != nil { if chartDependency := getAliasDependency(c.Dependencies(), req); chartDependency != nil {
chartDependencies = append(chartDependencies, chartDependency) chartDependencies = append(chartDependencies, chartDependency)
} }

@ -70,47 +70,55 @@ func TestDependencyEnabled(t *testing.T) {
}{{ }{{
"tags with no effect", "tags with no effect",
M{"tags": M{"nothinguseful": false}}, M{"tags": M{"nothinguseful": false}},
[]string{"parentchart", "parentchart.subchart1", "parentchart.subchart1.subcharta", "parentchart.subchart1.subchartb"}, []string{"parentchart", "parentchart.subchart1", "parentchart.subchart1.subcharta", "parentchart.subchart1.subchartb", "parentchart.subchart2alwaysEnabled"},
}, { }, {
"tags disabling a group", "tags disabling a group",
M{"tags": M{"front-end": false}}, M{"tags": M{"front-end": false}},
[]string{"parentchart"}, []string{"parentchart", "parentchart.subchart2alwaysEnabled"},
}, { }, {
"tags disabling a group and enabling a different group", "tags disabling a group and enabling a different group",
M{"tags": M{"front-end": false, "back-end": true}}, M{"tags": M{"front-end": false, "back-end": true}},
[]string{"parentchart", "parentchart.subchart2", "parentchart.subchart2.subchartb", "parentchart.subchart2.subchartc"}, []string{"parentchart", "parentchart.subchart2", "parentchart.subchart2.subchartb", "parentchart.subchart2.subchartc", "parentchart.subchart2alwaysEnabled", "parentchart.subchart2alwaysEnabled.subchartb", "parentchart.subchart2alwaysEnabled.subchartc"},
}, { }, {
"tags disabling only children, children still enabled since tag front-end=true in values.yaml", "tags disabling only children, children still enabled since tag front-end=true in values.yaml",
M{"tags": M{"subcharta": false, "subchartb": false}}, M{"tags": M{"subcharta": false, "subchartb": false}},
[]string{"parentchart", "parentchart.subchart1", "parentchart.subchart1.subcharta", "parentchart.subchart1.subchartb"}, []string{"parentchart", "parentchart.subchart1", "parentchart.subchart1.subcharta", "parentchart.subchart1.subchartb", "parentchart.subchart2alwaysEnabled"},
}, { }, {
"tags disabling all parents/children with additional tag re-enabling a parent", "tags disabling all parents/children with additional tag re-enabling a parent",
M{"tags": M{"front-end": false, "subchart1": true, "back-end": false}}, M{"tags": M{"front-end": false, "subchart1": true, "back-end": false}},
[]string{"parentchart", "parentchart.subchart1"}, []string{"parentchart", "parentchart.subchart1", "parentchart.subchart2alwaysEnabled"},
}, { }, {
"conditions enabling the parent charts, but back-end (b, c) is still disabled via values.yaml", "conditions enabling the parent charts, but back-end (b, c) is still disabled via values.yaml",
M{"subchart1": M{"enabled": true}, "subchart2": M{"enabled": true}}, M{"subchart1": M{"enabled": true}, "subchart2": M{"enabled": true}},
[]string{"parentchart", "parentchart.subchart1", "parentchart.subchart1.subcharta", "parentchart.subchart1.subchartb", "parentchart.subchart2"}, []string{"parentchart", "parentchart.subchart1", "parentchart.subchart1.subcharta", "parentchart.subchart1.subchartb", "parentchart.subchart2", "parentchart.subchart2alwaysEnabled"},
}, { }, {
"conditions disabling the parent charts, effectively disabling children", "conditions disabling the parent charts, effectively disabling children",
M{"subchart1": M{"enabled": false}, "subchart2": M{"enabled": false}}, M{"subchart1": M{"enabled": false}, "subchart2": M{"enabled": false}},
[]string{"parentchart"}, []string{"parentchart", "parentchart.subchart2alwaysEnabled"},
}, { }, {
"conditions a child using the second condition path of child's condition", "conditions a child using the second condition path of child's condition",
M{"subchart1": M{"subcharta": M{"enabled": false}}}, M{"subchart1": M{"subcharta": M{"enabled": false}}},
[]string{"parentchart", "parentchart.subchart1", "parentchart.subchart1.subchartb"}, []string{"parentchart", "parentchart.subchart1", "parentchart.subchart1.subchartb", "parentchart.subchart2alwaysEnabled"},
}, { }, {
"tags enabling a parent/child group with condition disabling one child", "tags enabling a parent/child group with condition disabling one child",
M{"subchart2": M{"subchartc": M{"enabled": false}}, "tags": M{"back-end": true}}, M{"subchart2": M{"subchartc": M{"enabled": false}}, "tags": M{"back-end": true}},
[]string{"parentchart", "parentchart.subchart1", "parentchart.subchart1.subcharta", "parentchart.subchart1.subchartb", "parentchart.subchart2", "parentchart.subchart2.subchartb"}, []string{"parentchart", "parentchart.subchart1", "parentchart.subchart1.subcharta", "parentchart.subchart1.subchartb", "parentchart.subchart2", "parentchart.subchart2.subchartb", "parentchart.subchart2alwaysEnabled", "parentchart.subchart2alwaysEnabled.subchartb"},
}, { }, {
"tags will not enable a child if parent is explicitly disabled with condition", "tags will not enable a child if parent is explicitly disabled with condition",
M{"subchart1": M{"enabled": false}, "tags": M{"front-end": true}}, M{"subchart1": M{"enabled": false}, "tags": M{"front-end": true}},
[]string{"parentchart"}, []string{"parentchart", "parentchart.subchart2alwaysEnabled"},
}, { }, {
"subcharts with alias also respect conditions", "subcharts with alias also respect conditions",
M{"subchart1": M{"enabled": false}, "subchart2alias": M{"enabled": true, "subchartb": M{"enabled": true}}}, M{"subchart1": M{"enabled": false}, "subchart2alias": M{"enabled": true, "subchartb": M{"enabled": true}}},
[]string{"parentchart", "parentchart.subchart2alias", "parentchart.subchart2alias.subchartb"}, []string{"parentchart", "parentchart.subchart2alias", "parentchart.subchart2alias.subchartb", "parentchart.subchart2alwaysEnabled"},
}, {
"conditions don't enable a disabled chart",
M{"subchart1": M{"enabled": false}, "subchart2diabled": M{"enabled": true}},
[]string{"parentchart", "parentchart.subchart2alwaysEnabled"},
}, {
"conditions can disable an enabled chart",
M{"subchart1": M{"enabled": false}, "subchart2alwaysEnabled": M{"enabled": false}},
[]string{"parentchart"},
}} }}
for _, tc := range tests { for _, tc := range tests {

@ -39,3 +39,17 @@ dependencies:
repository: http://localhost:10191 repository: http://localhost:10191
version: 0.1.0 version: 0.1.0
condition: subchart2alias.enabled condition: subchart2alias.enabled
- name: subchart2
alias: subchart2diabled
repository: http://localhost:10191
version: 0.1.0
enabled: false
condition: subchart2diabled.enabled
- name: subchart2
alias: subchart2alwaysEnabled
repository: http://localhost:10191
version: 0.1.0
enabled: true
condition: subchart2alwaysEnabled.enabled

Loading…
Cancel
Save