Repository navigation
Conversation
cr index --rebuild builds the index from the chart packages of all published GitHub releases instead of adding the supplied packages to the existing index. Each chart version is created at the time its release was published, also for new versions without --rebuild, so the created field matches the release date and rebuilding the same releases is idempotent. Fixes helm#133 Signed-off-by: Srinivas Chippagiri <cvas22@gmail.com>
cpanato
left a comment
cpanato
left a comment
There was a problem hiding this comment.
Please address the inline finding before merging. A focused unit test for duplicate releases with identical package bytes and --replace-existing --packages-with-index should cover this case; an external GitHub E2E test is not necessary.
| return false, err | ||
| } | ||
| candidate := replacement.Entries[c.Name()][0] | ||
| if !created.IsZero() { |
There was a problem hiding this comment.
Please include Created in the replacement equality check below. During --rebuild --replace-existing --packages-with-index, when the same chart version and package bytes appear in multiple releases, the newer candidate has the same digest and relative URL as the existing entry. The early return then keeps the older Created timestamp, so documented newest-release-wins behavior is not honored. Add a focused regression test for duplicate releases with identical package bytes.
There was a problem hiding this comment.
Thanks, you're right. When the creation time comes from a release, the equality check now also compares Created, so a newer release of the same package replaces the entry and the newest release wins. Without a release time (the normal, non-rebuild path) the check is unchanged, so repeated runs still don't rewrite identical entries. I added TestReleaser_maybeAddToIndexFileReplaceExistingSamePackage, which adds the same package for an older and a newer release with --replace-existing and --packages-with-index, and fails without the change. On my Windows machine some other tests in the package, such as TestReleaser_CreateReleases, fail with and without this change, so I'm relying on CI for those.
|
Could you let us know whether AI tools were used to prepare this PR? If so, please disclose that in the PR description and briefly note the extent of use. |
…hen rebuilding Signed-off-by: Srinivas Chippagiri <cvas22@gmail.com>
|
@cpanato Updated the description. Thank you. |
Fixes #133
This adds
cr index --rebuildand makes each chart version'screatedtime the time its GitHub release was published, as I proposed in #133.--rebuild: instead of adding the packages in--package-pathto the existing index,cr indexlists all published releases of the repository (drafts are skipped), downloads their.tgzassets, and builds a new index from them. Assets that aren't chart packages are skipped with a message. Releases are added oldest first, so if a version is in more than one release, the oldest one wins, like publishing them one by one would (the newest with--replace-existing). Versions in the existing index without a release are dropped.created: the entry'screatedis the release'spublished_at, with and without--rebuild. Rebuilding the same releases therefore gives the same entries. When they match the existing index,cr indexreports that the index did not change and doesn't write or commit it.--packages-with-index,--use-api-asset-url), and--push,--prand--sign-commitswork the same way.The GitHub client gets
ListReleases(paginated) andDownloadReleaseAsset(uses the API asset endpoint, so it works with--tokenfor private repos).Releasenow carriesPublishedAt, andAssetcarries the assetID.Notes
maybeAddToIndexFilenow takes the directory to read the package from and the creation time, so the rebuild can reuse it for downloaded packages. Existing tests were updated for the new signature.TestReleaser_UpdateIndexFileRebuildcovers release order, non-chart assets, digests, URLs, idempotent reruns and dropped versions.TestReleaser_UpdateIndexFileRebuildKeepsFirstReleasecovers a version in two releases,TestReleaser_UpdateIndexFileCreatedFromReleasecoverscreatedwithout--rebuild, andTestClient_ListReleases/TestClient_DownloadReleaseAssetcover the client against anhttptestserver.TestSignedIndexCommit,TestReleaser_CreateReleases,TestReleaser_CreateReleasesSkipExistingPagesandTestReplaceReleaseAssetsPreflightfail there onmaintoo (path separators and OS error messages), so I'm relying on CI for those.