Add support for creating repository indexes in JSON format #12245

hiddeco · 2023-07-20T22:42:31Z

What this PR does / why we need it:

This adds support for creating an index.yaml in JSON format using helm repo index [...] --json.

Fixes #10542
Supersedes #12151

Special notes for your reviewer:

Given that it does not create unnecessary HTTP requests to discover index.json and/or index.yaml files hosted by a repository server, nor requires rigorous changes to the caching logic, and is fully backwards compatible due to YAML being a superset of JSON. This appears to be the least invasive way to add support for the JSON format.

Benchmark using Bitnami's infamous "full index" archive:

$  benchstat old.txt new.txt                                  
goos: linux
goarch: amd64
pkg: helm.sh/helm/v3/pkg/repo
cpu: Intel(R) Core(TM) i7-10750H CPU @ 2.60GHz
                      │   old.txt    │               new.txt               │
                      │    sec/op    │   sec/op     vs base                │
LoadIndexFile/YAML-12     2.692 ± 1%    2.770 ± 3%        ~ (p=0.242 n=20)
LoadIndexFile/JSON-12   2318.3m ± 0%   582.6m ± 1%  -74.87% (p=0.000 n=20)
geomean                   2.498         1.270       -49.15%

                      │    old.txt    │               new.txt                │
                      │     B/op      │     B/op      vs base                │
LoadIndexFile/YAML-12    836.0Mi ± 0%   836.0Mi ± 0%        ~ (p=0.355 n=20)
LoadIndexFile/JSON-12   671.99Mi ± 0%   93.84Mi ± 0%  -86.03% (p=0.000 n=20)
geomean                  749.5Mi        280.1Mi       -62.63%

                      │   old.txt    │               new.txt               │
                      │  allocs/op   │  allocs/op   vs base                │
LoadIndexFile/YAML-12    12.03M ± 0%   12.03M ± 0%        ~ (p=0.683 n=20)
LoadIndexFile/JSON-12   10.015M ± 0%   1.078M ± 0%  -89.23% (p=0.000 n=20)
geomean                  10.98M        3.601M       -67.19%

If applicable:

this PR contains documentation
this PR contains unit tests
this PR has been tested for backwards compatibility

When an index is in a JSON format, the `sigs.k8s.io/yaml` package uses an inefficient approach to unmarshaling the data, as it does an unnecessary roundtrip on the data to transform the YAML to valid JSON. To prevent this from happening, detect if the bytes which we attempt to load contain valid JSON, and unmarshal them directly using `json.Unmarshal` instead. Signed-off-by: Hidde Beydals <hidde@hhh.computer>

This adds support for generating the repository index file in JSON format using the `--json` flag. The index itself is still written to `index.yaml`, which is fully backwards compatible as YAML is a superset of JSON. For big indexes (think multiple megabytes), this approach is however more efficient in combination with the changes to the load logic, as it prevents a YAML -> JSON roundtrip during decoding. Signed-off-by: Hidde Beydals <hidde@hhh.computer>

pjbgf

This looks pretty good, it will make a massive difference processing large helm indexes.

sabre1041 · 2023-07-24T04:00:48Z

This is a really good enhancement and verified that the enhancement does function correctly. However, the use of index.yaml for both JSON and YAML content is confusing as to the actual format of content included in this file.

It might be interesting to consider two potential files that could contain repository metadata: index.yaml and index.json (where index.yaml would take precedence if both files were found)

hiddeco · 2023-07-24T08:22:53Z

It might be interesting to consider two potential files that could contain repository metadata: index.yaml and index.json (where index.yaml would take precedence if both files were found)

In an iteration before creating this PR, I actually had an approach like this where ChartRepository#DownloadIndexFile would loop over possible URLs (first JSON, then YAML), while breaking the loop on the first non-error response (or returning a collection of errors if none was found).

Besides this approach causing potentially lots of HTTP 404s for the server when a JSON isn't served, after taking a closer look at the caching logic (more precisely the various CacheIndexFile calls throughout the code base, and required changes to certain error types to be factually correct). Getting this done right appeared more painful than leveraging YAML being a superset of JSON.

Another upside of the approach taken is that it's backwards compatible without having to serve (and thus store) two different file types. A Helm repository administrator can simply decide to start serving JSON, and any version of Helm will happily be able to download and work with the new index format (albeit only the Hem version which would include this change with memory consumption improvements).

joejulian

... the use of index.yaml for both JSON and YAML content is confusing ...

I disagree. Since json is a subset of yaml, it seems perfectly valid to me.

The rest of hiddeco's rational is also very compelling.

I tested this on my use cases and it works for me.

sabre1041 · 2023-08-02T03:40:21Z

@hiddeco @joejulian My concern with mixing JSON and YAML content is centered around two primary areas:

Understanding content types within index.yaml file. Yes YAML is a subset of JSON, but if there was an assumption that the contents included in these files were one format or the other, unexpected parsing errors could occur
Backwards compatibility. Do we break it by implementing such a change?

joejulian · 2023-08-02T03:50:27Z

Other way around. Yaml is a superset of json. Anything in json is 100% compatible with yaml. Yaml has features that are not in json.

So any json content in a yaml file is guaranteed compatible.

hiddeco · 2023-08-02T08:41:01Z

YAML is a subset of JSON, but if there was an assumption that the contents included in these files were one format or the other, unexpected parsing errors could occur

The actual parser Helm uses converts YAML to JSON during unmarshalling. In addition, from the YAML 1.2 spec (2009, revised in 2021):

[...] Its primary focus was making YAML a strict superset of JSON. It also removed [...]

Backwards compatibility. Do we break it by implementing such a change?

As noted in my previous comment, the exact opposite is true. As introducing an index.json would mean you either:

Have to continue to publish both an index.yaml and index.json file as a Helm repository administrator, as otherwise older versions of Helm are unable to work with your repository. It's also worth to note this makes publishing itself more complicated, as you now have to ensure both files are in sync.
Stop publishing an index.yaml, which would yield your Helm repository useless for older versions of Helm.

sabre1041

@hiddeco @joejulian Reevaluated code changes, comments and LGTM

thernstig · 2023-09-11T18:10:18Z

@hiddeco @sabre1041 this is amazing! Should it not be mentioned in the docs?

hiddeco · 2023-09-11T19:41:35Z

They are partly automatically documented: https://github.com/helm/helm-www#updating-the-helm-cli-reference-docs

mattfarina · 2023-09-27T20:48:14Z

Note, while writing the release notes I discovered this doesn't fully work. I filed a bug at #12422.

helm makes this a thing in helm/helm#12245

ArtemShevelyukhin · 2025-05-27T14:09:20Z

@hiddeco sorry, I don't understand, how this PR should help with #9931

I have helm repository in artifactory and large (~50 Mb) index.yaml file and my "helm repo update" command consumes a lot of memory (~900 Mb). And as far as I know artifactory doesn't support json format for index.yaml file

So, could you explain does this PR help with "helm repo update" or not ?

hiddeco · 2025-06-06T11:39:12Z

It will not help if you are not in control of the published format of the repository index (which appears to be the case based on what you describe).

pull-request-size bot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Jul 20, 2023

hiddeco force-pushed the json-index branch from ca283f0 to 2544aa2 Compare July 21, 2023 06:31

hiddeco mentioned this pull request Jul 21, 2023

Store Helm repository indexes in JSON format fluxcd/source-controller#1172

Closed

pjbgf approved these changes Jul 21, 2023

View reviewed changes

joejulian approved these changes Jul 27, 2023

View reviewed changes

joejulian added the Has One Approval This PR has one approval. It still needs a second approval to be merged. label Jul 27, 2023

sabre1041 approved these changes Sep 10, 2023

View reviewed changes

joejulian added this to the 3.13.0 milestone Sep 11, 2023

joejulian merged commit e7bb860 into helm:main Sep 11, 2023

hiddeco deleted the json-index branch September 11, 2023 17:16

mattfarina mentioned this pull request Sep 27, 2023

index.json reading doesn't work #12422

Closed

This was referenced Dec 7, 2023

Add support for JSON index to chartmuseum helm/chartmuseum#738

Closed

Add support for creating repository indexes in JSON format helm/chartmuseum#739

Merged

xdavidwu added a commit to xdavidwu/sparkles that referenced this pull request Aug 2, 2024

HelmRepository: try to parse index in json first

594e93a

helm makes this a thing in helm/helm#12245

Add support for creating repository indexes in JSON format #12245

Add support for creating repository indexes in JSON format #12245

Uh oh!

Conversation

hiddeco commented Jul 20, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

pjbgf left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sabre1041 commented Jul 24, 2023

Uh oh!

hiddeco commented Jul 24, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

joejulian left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sabre1041 commented Aug 2, 2023

Uh oh!

joejulian commented Aug 2, 2023

Uh oh!

hiddeco commented Aug 2, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

sabre1041 left a comment

Choose a reason for hiding this comment

Uh oh!

thernstig commented Sep 11, 2023

Uh oh!

hiddeco commented Sep 11, 2023

Uh oh!

mattfarina commented Sep 27, 2023

Uh oh!

ArtemShevelyukhin commented May 27, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

hiddeco commented Jun 6, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

hiddeco commented Jul 20, 2023 •

edited

Loading

pjbgf left a comment •

edited

Loading

hiddeco commented Jul 24, 2023 •

edited

Loading

joejulian left a comment •

edited

Loading

hiddeco commented Aug 2, 2023 •

edited

Loading

ArtemShevelyukhin commented May 27, 2025 •

edited

Loading