mitigate issue with chart validation in Helm 3.14

[bb-Ricardo]

Implements mitigation suggested in #1515

just a copy of the updated validation logic in Helm 3.14.0+

[bb-Ricardo]

The validation of the index is also not ideal. Have tests lying around which I were not able to commit and hope to push next week. But the same issue is present in the same bit of code in Helm. As I said, hope to finish this next week.

[bb-Ricardo]

Sorry it took longer than expected.

Added some tests and also comments of where the code came from.

[bb-Ricardo]

Added additional tests to verify index filtering is working as expected.

Also opened issue in for Helm https://github.com/helm/helm/issues/13176

[stefanprodan]

@bb-Ricardo can you please rebase with upstream main and force push

[bb-Ricardo]

@stefanprodan, rebased on fluxed/source-controller main branch. please have a look

[bb-Ricardo]

I can see failing test, lets see if I can reproduce and fix them

[bb-Ricardo]

Hi @stefanprodan,

After looking at it for a while I now assume that the tests are kind of broken.

Due to the fixed validation the Helm charts are removed from the index as they failed validation and now are missing in the test.

The issue is subtle. The last chart in the index was always kept in the index, no matter if invalid or not. Now they are probably removed and the test fail as they try to test against an invalid chart.

Two options I guess:

• remove the test (not desired)
• fix the test Helm chart, which probably causes other tests to fail.

Can you have a look? What do you think?

[darkowlzz]

I had a look at the failing test and it seems the underlying error that results in the chart version to be removed comes from the index value used in that particular test. In https://github.com/fluxcd/source-controller/blob/58b4e6d7198d8db48943f7787521b65db078d0ab/internal/helm/chart/builder_remote_test.go#L98-L101, no name is set. In all the other test indices used in the other tests, we set a name field which makes the index entries valid. Without it, the Validate() method returns validation: chart.metadata.name is required which is not a skippable error. I added the following and it fixed the test

diff --git a/internal/helm/chart/builder_remote_test.go b/internal/helm/chart/builder_remote_test.go
index d43966d..ebe31ae 100644
--- a/internal/helm/chart/builder_remote_test.go
+++ b/internal/helm/chart/builder_remote_test.go
@@ -99,6 +99,7 @@ entries:
         - https://example.com/grafana.tgz
       description: string
       version: 6.17.4
+      name: grafana
 `)
 
        mockGetter := &mockIndexChartGetter{

This was not an issue before as you found out, the original entries were not mutated, returning the index content as read from the input.

[bb-Ricardo]

@darkowlzz,

thank you. so simple and actually obvious, feeling a bit stupid now.

however, I added the name to the chart in the index and these test then passed successfully. I also rebased the branch again on top of upstream/main.

please have a look

[bb-Ricardo]

Hi,

I added suggested changes, squashed the changes into one commit and pushed it again.

[fluxcdbot]

Successfully created backport PR for release/v1.3.x:

• #1554