changelog: restructure S3 layout to artifact-root#3562
Open
cotti wants to merge 6 commits into
Open
Conversation
Move changelog/bundle S3 keys from product-first to artifact-root: bundles are
product-scoped under bundle/{product}/{file}; changelog entries are stored once
per authoring repo under changelog/{repo}/{file}. This decouples repository
concerns (entries) from product concerns (bundles) and removes the developer-
foreknowledge requirement for entry distribution.
- upload: entries -> changelog/{repo}/{file} (repo via --repo > bundle.repo > git
remote); bundles -> bundle/{product}/{file}
- registries: bundle/{product}/registry.json and changelog/{repo}/registry.json;
scope-aware grouping and RegistryKey.IsRegistry validation
- CDN consumers + bundling: repo-scoped entry sourcing, decoupled gate,
ResolveCdnBundleUrl -> bundle/{product}/{file}
- scrubber Lambda: detect bundles by the bundle/ key prefix
- CLI: add --repo/--owner to `changelog upload`; regenerate cli-schema.json
- tests + docs updated; new cloud-shaped profile fixture
Refs elastic/docs-eng-team#413
Co-authored-by: Cursor <cursoragent@cursor.com>
2 tasks
cotti
changed the title
Contributor
|
Warning Review limit reached
Next review available in: 33 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Enterprise Run ID: 📒 Files selected for processing (7)
📝 WalkthroughWalkthroughThe PR updates changelog upload and bundle flows to use artifact-root S3 keys, adds repo/owner/branch scoping, and switches CDN changelog entry sourcing to authoring-repo pools. Related registry validation, configuration loading, CLI schema/docs, scrubber behavior, and tests were updated to match the new key layouts and URL paths. Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches✨ Simplify code
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/development/changelog-bundle-registry.md`:
- Around line 111-112: The producer details section is inconsistent after the
repo-scope update: the bullet that still says “for each product” needs to match
the new changelog-entry path model. Update the affected wording in
changelog-bundle-registry.md so the bullet in the producer details section
refers to repo-scoped entry registries instead of product-scoped processing,
keeping it consistent with the bundle/product and entry/repo grouping described
nearby.
In `@src/services/Elastic.Changelog/Bundling/ChangelogBundlingService.cs`:
- Around line 787-797: Normalize merged bundle repo values before they are used
to build CDN keys in ChangelogBundlingService, not just owner/repo inputs.
Update NormalizeRepo so it also maps combined defaults like elasticsearch+kibana
to the repo segment expected by the repo-scoped CDN layout, and ensure
BundleChangelogs and PlanBundleAsync both use the normalized value when
composing changelog/…/registry.json paths. Keep the behavior for null, empty,
and simple repo names unchanged.
In `@src/services/Elastic.Changelog/Uploading/RegistryKey.cs`:
- Around line 18-21: The RegistryKey validation currently allows dots in both
bundle and changelog registry segments, but bundle product scopes must stay
limited to the bundle upload pattern. Update the matching logic in RegistryKey
so IsRegistry only accepts dot-containing names for the changelog/{repo} form,
while bundle/{product}/registry.json remains restricted to the existing bundle
character class. Keep the regex or segment checks aligned with the scrubber
behavior so unchanged JSON is only passed through for valid bundle or changelog
registry keys.
In `@src/tooling/docs-builder/Commands/ChangelogCommand.cs`:
- Around line 1659-1664: The repo resolution in ChangelogCommand is using the
process current directory, which can pick up the wrong remote when --config or
--directory targets another checkout. Update the logic around
GitRemoteConfigurationReader.TryReadOriginUrl, Paths.FindGitRoot, and
resolvedRepo to derive the git root from the upload source (configPath or upload
directory) first, and only fall back to Directory.GetCurrentDirectory() when no
source-specific root is available. Ensure the repo lookup still flows through
GitHubRemoteParser.TryParseGitHubComOwnerRepo and the existing
repoCli/bundleConfig fallback order.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 5adba39d-1fdc-491f-ada6-5aefa9463e1b
📒 Files selected for processing (25)
docs/cli-schema.jsondocs/cli/changelog/cmd-upload.mddocs/contribute/configure-changelogs-ref.mddocs/development/changelog-bundle-registry.mdsrc/Elastic.Documentation.Configuration/ReleaseNotes/CdnChangelogEntryFetcher.cssrc/Elastic.Documentation.Configuration/ReleaseNotes/CdnChangelogFetcher.cssrc/Elastic.Documentation.Configuration/ReleaseNotes/ChangelogRegistry.cssrc/Elastic.Markdown/Myst/Directives/Changelog/ChangelogBlock.cssrc/infra/docs-lambda-changelog-scrubber/Program.cssrc/infra/docs-lambda-changelog-scrubber/README.mdsrc/services/Elastic.Changelog/Bundling/ChangelogBundlingService.cssrc/services/Elastic.Changelog/Uploading/ChangelogUploadService.cssrc/services/Elastic.Changelog/Uploading/Registry.cssrc/services/Elastic.Changelog/Uploading/RegistryBuilder.cssrc/services/Elastic.Changelog/Uploading/RegistryKey.cssrc/tooling/docs-builder/Commands/ChangelogCommand.cstests/Elastic.Changelog.Tests/Changelogs/BundleCdnSourcingTests.cstests/Elastic.Changelog.Tests/Changelogs/BundlePlanTests.cstests/Elastic.Changelog.Tests/Changelogs/CloudProfileFixtureTests.cstests/Elastic.Changelog.Tests/Changelogs/LinkAllowlistSanitizerTests.cstests/Elastic.Changelog.Tests/Uploading/ChangelogUploadServiceTests.cstests/Elastic.Changelog.Tests/Uploading/RegistryBuilderTests.cstests/Elastic.Changelog.Tests/Uploading/RegistryKeyTests.cstests/Elastic.Documentation.Configuration.Tests/ReleaseNotes/CdnChangelogEntryFetcherTests.cstests/Elastic.Documentation.Configuration.Tests/ReleaseNotes/CdnChangelogFetcherTests.cs
- RegistryKey: scope dot characters to changelog/{repo} segments; bundle product
segments stay [a-zA-Z0-9_-]+ (matches producer validation), so the scrubber no
longer passes through bundle/foo.bar/registry.json
- CLI upload: anchor the git-remote repo fallback to the --config/--directory
source rather than the process cwd, so an out-of-tree config resolves the right origin
- docs: fix the producer-details bullet to describe repo-scoped entry registries
- condense private-method XML doc summaries to a single line
Skipped CodeRabbit's NormalizeRepo '+' suggestion: the config loader already
rejects '+' in bundle.repo, so merged values never reach key construction.
Refs elastic/docs-eng-team#413
Co-authored-by: Cursor <cursoragent@cursor.com>
Per the docs-builder boolean-parameter rule, pass useLocalChangelogs/ explicitDirectory (ShouldSourceFromCdn) and allowDots (IsValidSegment) as named arguments at their call sites. Refs elastic/docs-eng-team#413 Co-authored-by: Cursor <cursoragent@cursor.com>
lcawl
reviewed
Jun 30, 2026
lcawl
reviewed
Jun 30, 2026
lcawl
reviewed
Jun 30, 2026
lcawl
reviewed
Jun 30, 2026
Extends the artifact-root layout so changelog entries are stored at
changelog/{org}/{repo}/{branch}/{file} (and registry.json), keyed by the
authoring org, repo, and branch. This supports changelogs from GitHub orgs
other than elastic (e.g. acquired companies) and from branches other than
main. The branch is stored verbatim, so a branch's '/' (feature/foo) and
'.' (8.x) become real key segments. Bundles are unchanged.
- Producer: ChangelogUploadService validates org/repo/branch and builds the
nested key; Owner/Branch added to the upload arguments.
- Registry: RegistryBuilder groups by the {org}/{repo}/{branch} prefix;
RegistryKey.IsRegistry accepts >=3 changelog segments (bundle stays one).
- Consumer: CdnChangelogEntryFetcher takes org/repo/branch (slash-aware);
ChangelogBundlingService resolves org (bundle.owner > elastic) and branch
(bundle.branch > main) when sourcing from the CDN.
- Config/CLI: add bundle.branch and --branch to upload/bundle; producer
branch falls back to the current checkout's branch.
- cli-schema.json regenerated; tests and docs updated.
lcawl
reviewed
Jun 30, 2026
lcawl
reviewed
Jun 30, 2026
lcawl
reviewed
Jun 30, 2026
lcawl
reviewed
Jun 30, 2026
lcawl
reviewed
Jun 30, 2026
lcawl
reviewed
Jun 30, 2026
lcawl
reviewed
Jun 30, 2026
lcawl
reviewed
Jun 30, 2026
lcawl
reviewed
Jun 30, 2026
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@src/Elastic.Documentation.Configuration/ReleaseNotes/CdnChangelogEntryFetcher.cs`:
- Around line 260-266: Validate the branch path segments before the CDN URI is
built in PoolSegments and the CombineSegments call path. Since branch is config
input, reject empty segments and dot segments like "." and ".." before yielding
parts from branch.Split('/'), so new Uri(...) cannot normalize them into a
different changelog pool than intended. Keep the fix localized to
CdnChangelogEntryFetcher and its PoolSegments helper.
In `@src/services/Elastic.Changelog/Bundling/ChangelogBundlingService.cs`:
- Around line 229-232: The repo owner prefix is being lost in the changelog
bundling flow, so non-default pools resolve to the wrong CDN path. Update the
logic around NormalizeRepo and the authoringOwner/authoringBranch setup in
ChangelogBundlingService so that if input.Owner is empty, the owner is derived
from the owner/repo prefix in input.Repo before falling back to DefaultOwner.
Keep ShouldSourceFromCdn using the normalized repo name, but ensure the resolved
owner used for CDN path construction matches the original repo owner segment
when present.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 8aebc6ba-b0b2-404d-8976-1e1bb227b946
📒 Files selected for processing (22)
config/changelog.example.ymldocs/cli-schema.jsonsrc/Elastic.Documentation.Configuration/Changelog/BundleConfiguration.cssrc/Elastic.Documentation.Configuration/Changelog/ChangelogConfigurationLoader.cssrc/Elastic.Documentation.Configuration/Changelog/ChangelogConfigurationYaml.cssrc/Elastic.Documentation.Configuration/ReleaseNotes/CdnChangelogEntryFetcher.cssrc/Elastic.Documentation.Configuration/ReleaseNotes/ChangelogRegistry.cssrc/infra/docs-lambda-changelog-scrubber/Program.cssrc/infra/docs-lambda-changelog-scrubber/README.mdsrc/services/Elastic.Changelog/Bundling/ChangelogBundlingService.cssrc/services/Elastic.Changelog/Uploading/ChangelogUploadService.cssrc/services/Elastic.Changelog/Uploading/Registry.cssrc/services/Elastic.Changelog/Uploading/RegistryBuilder.cssrc/services/Elastic.Changelog/Uploading/RegistryKey.cssrc/tooling/docs-builder/Commands/ChangelogCommand.cstests/Elastic.Changelog.Tests/Changelogs/BundleCdnSourcingTests.cstests/Elastic.Changelog.Tests/Changelogs/CloudProfileFixtureTests.cstests/Elastic.Changelog.Tests/Changelogs/LinkAllowlistSanitizerTests.cstests/Elastic.Changelog.Tests/Uploading/ChangelogUploadServiceTests.cstests/Elastic.Changelog.Tests/Uploading/RegistryBuilderTests.cstests/Elastic.Changelog.Tests/Uploading/RegistryKeyTests.cstests/Elastic.Documentation.Configuration.Tests/ReleaseNotes/CdnChangelogEntryFetcherTests.cs
✅ Files skipped from review due to trivial changes (5)
- src/infra/docs-lambda-changelog-scrubber/Program.cs
- tests/Elastic.Changelog.Tests/Changelogs/LinkAllowlistSanitizerTests.cs
- src/Elastic.Documentation.Configuration/ReleaseNotes/ChangelogRegistry.cs
- src/services/Elastic.Changelog/Uploading/Registry.cs
- src/infra/docs-lambda-changelog-scrubber/README.md
🚧 Files skipped from review as they are similar to previous changes (5)
- tests/Elastic.Changelog.Tests/Uploading/RegistryKeyTests.cs
- tests/Elastic.Changelog.Tests/Changelogs/CloudProfileFixtureTests.cs
- src/services/Elastic.Changelog/Uploading/RegistryBuilder.cs
- tests/Elastic.Documentation.Configuration.Tests/ReleaseNotes/CdnChangelogEntryFetcherTests.cs
- tests/Elastic.Changelog.Tests/Changelogs/BundleCdnSourcingTests.cs
Addresses review feedback on the changelog docs that still described the
repo-only entry layout. Updates cmd-upload, configure-changelogs-ref, and the
changelog-bundle-registry architecture page to changelog/{org}/{repo}/{branch}/...
(entries and the per-pool registry.json), documents owner/branch resolution and
the new bundle.branch config, and notes uploads can run from any branch.
Resolves the open question from review: the entry registry lives at the branch
level (changelog/{org}/{repo}/{branch}/registry.json), not repo-level.
Contributor
Author
|
Thanks @lcawl! Addressed your doc comments in 562bcab (on top of the new On your recurring open question — where the registry lives: we went with the per-branch form, so the registry is at All three reference docs ( |
Addresses CodeRabbit findings on the consumer-side org/repo/branch handling: - CdnChangelogEntryFetcher now validates the org, repo, and each branch segment before building the CDN URI, rejecting empty/"."/".." segments so a malformed branch from config/CLI cannot normalize "../" into a different pool. - ChangelogBundlingService derives the org from a combined owner/repo value (e.g. acme/widget -> acme) when no explicit owner is set, instead of dropping the prefix and defaulting to "elastic" — keeping the CDN pool path correct. Skipped the suggestion to strip "+" in NormalizeRepo: bundle.repo with "+" merged-repo syntax is already rejected at config load (ParseBundleConfiguration). Adds tests for the unsafe-branch rejection and owner-from-combined-repo path.
Contributor
Author
|
Addressed the latest CodeRabbit review in a8aeb9f:
Skipped (not valid against current code):
Added tests for both fixes (unsafe-branch rejection; owner-from-combined-repo path). |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
bundle/{product}/{file}; changelog entries are stored once per authoring org/repo/branch atchangelog/{org}/{repo}/{branch}/{file}. This decouples repository concerns (entries) from product concerns (bundles) and removes the developer-foreknowledge requirement for entry distribution.elastic(e.g. acquired companies that keep their org) and from branches other thanmainstay isolated. The branch is stored verbatim, so a branch's/(feature/foo) and.(8.x) become real key segments (variable key depth).changelog upload): entries keyed by org/repo/branch (--owner/--repo/--branch, falling back tobundle.owner/bundle.repo/git remote and the current checkout's branch), bundles keyed by product; scope-awareregistry.jsonandRegistryKey.IsRegistry(changelog manifests require ≥3{org}/{repo}/{branch}segments; bundles stay single-segment).elasticand branch tomain(overridable viabundle.owner/bundle.branch/--branch), andResolveCdnBundleUrl->bundle/{product}/{file}.bundle/key prefix (the old/bundle/substring check silently breaks under the new layout); registry pass-through accepts the deeperchangelog/{org}/{repo}/{branch}/registry.json.--owner/--repo/--branchtochangelog uploadand--branchtochangelog bundle;docs/cli-schema.jsonregenerated.elastic/cloud-shaped profile fixture.Part of the changelog folder restructure. Refs elastic/docs-eng-team#413. Companion PRs in
elastic/docs-actionsandelastic/docs-infra.Important
Org CI consumes docs-builder via
edge, so this propagates to all consumers on release — deploy the scrubber Lambda in lockstep. End-to-end CDN smoke (bundle-fetch/{changelog} cdn:) only validates after the first publish under the new layout.Test plan
dotnet formatclean./build.sh build --skip-dirty-check(lint + compile) cleandotnet publish -c Release) — no trim/AOT warningsElastic.Changelog.Tests(786) andElastic.Documentation.Configuration.Tests(417) greendocs/cli-schema.jsonstable vs. regenerated (CI schema check)Made with Cursor