Skip to content

Conversation

relu
Copy link
Member

@relu relu commented Oct 30, 2020

It looks like the use of chartutil.ProcessDependencies in the HelmChart
Controller was not correct, this method seems to be used in Helm only
during install/upgrade. The intention was to load the dependencies but
this seems to not be needed as it's already done through the loaders
(loader.Load).

The use of this method caused a regression where Chart.yaml files would
be overwritten and registered subcharts that had aliases would be
renamed using the alias name. While this is an expected behaviour of
chartutil.ProcessDependencies it is not what the controller should do
to the chart during (re)packaging.

relu added 2 commits October 30, 2020 19:16
It looks like the use of chartutil.ProcessDependencies in the HelmChart
Controller was not correct, this method seems to be used in Helm only
during install/upgrade. The intention was to load the dependencies but
this seems to not be needed as it's already done through the loaders
(loader.Load).

The use of this method caused a regression where Chart.yaml files would
be overwritten and registered subcharts that had aliases would be
renamed using the alias name. While this is an expected behaviour of
chartutil.ProcessDependencies it is not what the controller should do
to the chart during (re)packaging.

Signed-off-by: Aurel Canciu <[email protected]>
Cleanup some bits especially in the local chart suite.

Signed-off-by: Aurel Canciu <[email protected]>
@hiddeco hiddeco added area/helm Helm related issues and pull requests bug Something isn't working labels Oct 30, 2020
Copy link
Member

@hiddeco hiddeco left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thank you @relu 💯

@hiddeco hiddeco merged commit d7e3130 into fluxcd:main Oct 30, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/helm Helm related issues and pull requests bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants