diff --git a/config/changelog.example.yml b/config/changelog.example.yml index e4327e719..8c62b4e4c 100644 --- a/config/changelog.example.yml +++ b/config/changelog.example.yml @@ -236,6 +236,9 @@ bundle: # repo: elasticsearch # Optional: default GitHub owner applied to all profiles that do not specify their own. # owner: elastic + # Optional: branch whose CDN changelog pool (changelog/{org}/{repo}/{branch}/...) is sourced from when + # bundling entries from the CDN. Defaults to "main" when unset. Can be overridden per profile. + # branch: main # Optional: control auto-population of release-date for all profiles by default. # When true (default), auto-populate release dates. Profiles can override this setting. # release_dates: true diff --git a/docs/cli-schema.json b/docs/cli-schema.json index 1a366a48c..a1e68c60f 100644 --- a/docs/cli-schema.json +++ b/docs/cli-schema.json @@ -2779,6 +2779,13 @@ "required": false, "summary": "GitHub repository owner for PR/issue numbers or --release-version. Falls back to bundle.owner or \u0022elastic\u0022. This option is not supported in profile-based commands. The equivalent configuration options are bundle.owner or bundle.profiles.\u003Cname\u003E.owner." }, + { + "role": "flag", + "name": "branch", + "type": "string", + "required": false, + "summary": "Branch whose CDN changelog entry pool (changelog/{org}/{repo}/{branch}/...) is sourced from. Falls back to bundle.branch or \u0022main\u0022. This option is not supported in profile-based commands. The equivalent configuration options are bundle.branch or bundle.profiles.\u003Cname\u003E.branch." + }, { "role": "flag", "name": "plan", @@ -3923,7 +3930,7 @@ ], "name": "upload", "summary": "Upload changelog entries or bundle artifacts to S3 or Elasticsearch.", - "notes": "Uses content-hash\u2013based incremental transfer \u2014 only changed files are uploaded.", + "notes": "Uses content-hash\u2013based incremental transfer \u2014 only changed files are uploaded.\n\n\nChangelog entries are uploaded once under changelog/{org}/{repo}/{branch}/{file}, keyed by the\nauthoring owner (--owner \u003E bundle.owner \u003E git remote), repository (--repo\n\u003E bundle.repo \u003E git remote), and branch (--branch \u003E the current checkout\u0027s\nbranch). The branch is stored verbatim, so a branch\u0027s / become real key separators. Bundles\nare uploaded under bundle/{product}/{file}, product-scoped from the bundle YAML, and do not\nrequire an owner/repo/branch.", "usage": "docs-builder changelog upload --artifact-type \u003Cstring\u003E --target \u003Cstring\u003E [options]", "examples": [], "parameters": [ @@ -3982,6 +3989,27 @@ } ] }, + { + "role": "flag", + "name": "repo", + "type": "string", + "required": false, + "summary": "GitHub repository name, the second segment of changelog entry keys (changelog/{org}/{repo}/{branch}/...). Falls back to bundle.repo in changelog.yml, then the git remote origin. Required for changelog uploads; ignored for bundle uploads." + }, + { + "role": "flag", + "name": "owner", + "type": "string", + "required": false, + "summary": "GitHub owner (org), the first segment of changelog entry keys (changelog/{org}/{repo}/{branch}/...). Falls back to bundle.owner in changelog.yml, then the git remote origin. Required for changelog uploads; ignored for bundle uploads." + }, + { + "role": "flag", + "name": "branch", + "type": "string", + "required": false, + "summary": "Branch, the third segment of changelog entry keys (changelog/{org}/{repo}/{branch}/...), stored verbatim. Falls back to the current checkout\u0027s branch. Required for changelog uploads; ignored for bundle uploads." + }, { "role": "flag", "name": "log-level", diff --git a/docs/cli/changelog/cmd-upload.md b/docs/cli/changelog/cmd-upload.md index 73af2d091..26f3f2771 100644 --- a/docs/cli/changelog/cmd-upload.md +++ b/docs/cli/changelog/cmd-upload.md @@ -1,6 +1,6 @@ ## Description -Upload changelog entries or bundle artifacts to S3 or Elasticsearch. The command discovers `.yaml` and `.yml` files in a local directory, maps each file to one or more product-scoped S3 keys, and uploads only files whose content hash changed since the last run. +Upload changelog entries or bundle artifacts to S3 or Elasticsearch. The command discovers `.yaml` and `.yml` files in a local directory and uploads only files whose content hash changed since the last run. Changelog entries are uploaded once under `changelog/{org}/{repo}/{branch}/{file}`, keyed by the authoring owner, repository, and branch; bundles are uploaded under `bundle/{product}/{file}`, product-scoped from the bundle YAML. To create bundles first, use [](/cli/changelog/bundle.md). For the end-to-end workflow, see [](/contribute/bundle-changelogs.md). @@ -42,10 +42,8 @@ Your IAM policy must allow these S3 actions on the target bucket: You can scope object-level permissions to the key prefixes the command writes: -- `{product}/bundle/*` -- `{product}/changelog/*` -- `{product}/registry.json` -- `{product}/changelog/registry.json` +- `bundle/*` (bundle YAML and `bundle/{product}/registry.json`) +- `changelog/*` (entry YAML and `changelog/{org}/{repo}/{branch}/registry.json`) #### Local development @@ -79,10 +77,10 @@ Use `--artifact-type` to choose what to upload: | `bundle` | Consolidated bundle YAML files | `bundle.output_directory` from `changelog.yml`, or `docs/releases` | | `changelog` | Individual changelog entry YAML files | `bundle.directory` from `changelog.yml`, or `docs/changelog` | -Each file is uploaded once per product listed in its YAML: +Keying differs by artifact type: -- Changelog entries use the `products[].product` field. -- Bundles use the `products[].product` field (product ID). +- **Changelog entries** are uploaded **once** under the authoring owner/repo/branch, regardless of how many products they list (or none). The owner is resolved from `--owner`, then `bundle.owner` in `changelog.yml`, then the git remote origin; the repo from `--repo`, then `bundle.repo`, then the git remote origin; the branch from `--branch`, then the current checkout's branch. The branch is stored verbatim, so a branch name containing `/` (for example `feature/foo`) becomes additional key segments. +- **Bundles** are uploaded once per product listed in the bundle's `products[].product` field (a bundle that declares multiple products is written under each product prefix). ## Upload targets @@ -98,19 +96,21 @@ Use `--target` to choose the destination: For each discovered file, the command writes to: ```text -s3://{bucket}/{product}/changelog/{filename} # --artifact-type changelog -s3://{bucket}/{product}/bundle/{filename} # --artifact-type bundle +s3://{bucket}/changelog/{org}/{repo}/{branch}/{filename} # --artifact-type changelog +s3://{bucket}/bundle/{product}/{filename} # --artifact-type bundle ``` -A changelog or bundle that applies to multiple products is uploaded to multiple keys — one per product. +Changelog entries are written once under the authoring org/repo/branch. A bundle that applies to multiple products is uploaded to multiple keys — one per product. -After a successful upload, the command refreshes per-product `registry.json` manifests at: +After a successful upload, the command refreshes the relevant `registry.json` manifest: ```text -s3://{bucket}/{product}/registry.json # bundle uploads -s3://{bucket}/{product}/changelog/registry.json # changelog uploads +s3://{bucket}/changelog/{org}/{repo}/{branch}/registry.json # changelog uploads +s3://{bucket}/bundle/{product}/registry.json # bundle uploads ``` +When several repositories publish bundles for the same shared product (for example `cloud-serverless`), use a `{repo}-{dateOrVersion}.yaml` bundle filename convention so they don't overwrite each other under `bundle/{product}/`. + The registry refresh is best-effort: upload failures block the run, but a stale manifest does not fail an otherwise successful upload. :::{note} @@ -142,13 +142,16 @@ docs-builder changelog upload \ ### Upload changelog entries to S3 -Upload individual changelog YAML files from the default changelog directory (`docs/changelog`): +Upload individual changelog YAML files from the default changelog directory (`docs/changelog`). Entries are written to `changelog/{org}/{repo}/{branch}/...`; pass `--owner`, `--repo`, and `--branch` when the authoring owner/repo can't be inferred from `bundle.owner`/`bundle.repo` or the git remote, or to override the current checkout's branch: ```sh docs-builder changelog upload \ --artifact-type changelog \ --target s3 \ - --s3-bucket-name my-changelog-bundles + --s3-bucket-name my-changelog-bundles \ + --owner elastic \ + --repo my-repo \ + --branch main ``` ### Override the source directory diff --git a/docs/contribute/configure-changelogs-ref.md b/docs/contribute/configure-changelogs-ref.md index edc321364..e2c6015d4 100644 --- a/docs/contribute/configure-changelogs-ref.md +++ b/docs/contribute/configure-changelogs-ref.md @@ -46,12 +46,13 @@ These settings are relevant to one or all of the `changelog bundle`, `changelog | Setting | Description | | ------------------------- | ----------- | +| `bundle.branch` | Branch whose CDN changelog pool (`changelog/{org}/{repo}/{branch}/...`) entries are sourced from when bundling (default: `main`). Refer to [Entry sourcing](#bundle-entry-sourcing). | | `bundle.directory` | Input directory containing changelog YAML files (default: `docs/changelog`). | | `bundle.link_allow_repos` | List of `owner/repo` pairs whose PR/issue links are preserved. When set (including empty `[]`), links to unlisted repos become `# PRIVATE:` sentinels. Requires `bundle.resolve: true` | | `bundle.output_directory` | Output directory for bundled files (default: `docs/releases`). | -| `bundle.owner` | Default GitHub repository owner (for example, `elastic`). | +| `bundle.owner` | Default GitHub repository owner (for example, `elastic`). Also the org segment of uploaded changelog-entry keys (`changelog/{org}/{repo}/{branch}/...`) and CDN entry sourcing. | | `bundle.release_dates` | When `true`, bundles include a `release-date` field (default: true). | -| `bundle.repo` | Default GitHub repository name (for example, `elasticsearch`). Used by the `{changelog}` directive to generate correct PR and issue links. Only needed when the product ID doesn't match the GitHub repository name. | +| `bundle.repo` | Default GitHub repository name (for example, `elasticsearch`). Used by the `{changelog}` directive to generate correct PR and issue links, and to scope uploaded changelog-entry keys (`changelog/{org}/{repo}/{branch}/...`) and CDN entry sourcing. Only needed when the product ID doesn't match the GitHub repository name (or to override the git remote). | | `bundle.resolve` | When `true`, changelog contents are copied into bundle (default: `true`). | | `bundle.use_local_changelogs` | When `true`, always source entries from the local folder and never from the CDN (default: `false`). Refer to [Entry sourcing](#bundle-entry-sourcing). | @@ -66,20 +67,16 @@ When `bundle.link_allow_repos` is omitted, no link filtering occurs. ### Entry sourcing [bundle-entry-sourcing] -`changelog bundle` reads the individual changelog entries it aggregates either from the local folder or from the public CDN. CDN sourcing is **opt-in per product** (declared-gate): an entry is pulled from the CDN only when its product is declared under `release_notes` in the docset's `docset.yml`. +`changelog bundle` reads the individual changelog entries it aggregates either from the local folder or from the public CDN. Entries are stored on the CDN per **authoring org/repo/branch** (`changelog/{org}/{repo}/{branch}/...`), not per product, so CDN sourcing keys off the resolvable authoring org/repo/branch rather than the bundle's target products. -```yaml -# docset.yml -release_notes: - - product: elasticsearch -``` +The authoring repo is resolved with the same precedence as `changelog upload`: `--repo` > `bundle.repo` in `changelog.yml` > the git remote origin. The owner is resolved from `--owner` > `bundle.owner` (default `elastic`), and the branch from `--branch` > `bundle.branch` (default `main`). Sourcing is decided per run: -- **Local folder (default).** Used when no product in scope is declared under `release_notes`, when `bundle.use_local_changelogs: true`, when `--directory` is passed, or when the filter resolves no concrete product (for example, `--all` or PR/issue-only filters). The folder must contain the changelog files. -- **CDN.** Used only when every product in scope is declared under `release_notes` and none of the local-sourcing conditions above apply. The product must also exist in [products.yml](https://github.com/elastic/docs-builder/blob/main/config/products.yml) with the `release-notes` feature enabled. +- **Local folder.** Used when `bundle.use_local_changelogs: true`, when `--directory` is passed, or when the authoring repo cannot be resolved. The folder must contain the changelog files. +- **CDN (default when a repo resolves).** Used when the authoring repo resolves, local sourcing is not forced, and a CDN base URL is configured (`DOCS_BUILDER_CHANGELOG_CDN`, defaulting to the public distribution). The command fetches `changelog/{org}/{repo}/{branch}/registry.json` and the entries it lists, then applies the bundle's own product/PR/issue filters to the downloaded set. -The product ID under `release_notes` matches the product format described in [](/cli/changelog/bundle.md#product-format). This is the same declaration the `{changelog}` directive's `:cdn:` mode consumes, so a repository that opts into CDN-sourced bundling and CDN-rendered release notes declares each product once. +Because entries are org/repo/branch-scoped, one repository can produce a bundle for a shared product (for example, `cloud-serverless`) while sourcing its own entries from `changelog/{org}/{repo}/{branch}/`, without that product appearing in the repository's `docset.yml`. The `{changelog}` directive's `:cdn:` mode still consumes product-scoped *bundles*, so a repository that also renders its own release notes declares each product under `release_notes` as before. ### Bundle descriptions [bundle-descriptions] diff --git a/docs/development/changelog-bundle-registry.md b/docs/development/changelog-bundle-registry.md index 26b6b67c4..2f3072715 100644 --- a/docs/development/changelog-bundle-registry.md +++ b/docs/development/changelog-bundle-registry.md @@ -34,8 +34,8 @@ copies, no cross-repo file syncing. │ Client CI │ --artifact-type │ Private bundles │ ───────────────────▶ │ Changelog scrubber │ │ (docs-actions)│ bundle ───────────▶ │ S3 bucket │ │ Lambda │ └──────────────┘ │ │ └─────────┬─────────┘ - │ │ {product}/bundle/*.yaml │ scrub + copy - │ also refreshes │ {product}/registry.json │ (pass-through for + │ │ bundle/{product}/*.yaml │ scrub + copy + │ also refreshes │ bundle/{product}/registry.json │ (pass-through for └──────────────────────────────▶ │ │ registry.json) └────────────────────┘ ▼ ┌───────────────────┐ @@ -46,24 +46,28 @@ copies, no cross-repo file syncing. 1. **Producer** — `changelog upload --artifact-type bundle --target s3` (invoked by the docs-actions changelog upload workflow) uploads each bundle to - `{product}/bundle/{file}` in the **private** bucket, then refreshes - `{product}/registry.json` for every product the run touched. + `bundle/{product}/{file}` in the **private** bucket, then refreshes + `bundle/{product}/registry.json` for every product the run touched. The companion + `--artifact-type changelog` upload writes individual entries to `changelog/{org}/{repo}/{branch}/{file}` + (one copy, keyed by the authoring org/repo/branch) and refreshes + `changelog/{org}/{repo}/{branch}/registry.json`. 2. **Scrubber Lambda** — triggered by `s3:ObjectCreated` on the private bucket, it scrubs - private repository references out of bundle YAML and writes the sanitized copy to the - **public** bucket. The `registry.json` object is copied through **verbatim**. + private repository references out of bundle and entry YAML and writes the sanitized copy to + the **public** bucket. The `registry.json` object is copied through **verbatim**. 3. **Consumer** — for each product declared under `release_notes` in `docset.yml`, docs-builder - reads `{product}/registry.json` from the CDN at build startup and fetches each listed bundle; - the `{changelog}` directive in `cdn:` mode then renders from the prefetched result. + reads `bundle/{product}/registry.json` from the CDN at build startup and fetches each listed + bundle; the `{changelog}` directive in `cdn:` mode then renders from the prefetched result. ### Why a registry instead of an S3 listing The public surface is a CDN (CloudFront) in front of S3. CloudFront does not expose bucket -listing, so the consumer cannot enumerate `{product}/bundle/`. The registry is a stable, +listing, so the consumer cannot enumerate `bundle/{product}/`. The registry is a stable, cacheable manifest at a predictable key that lists exactly which bundles exist for a product. ## `registry.json` format -Stored at `{product}/registry.json`. Serialized with `snake_case` keys. +Stored at `bundle/{product}/registry.json` (bundle index) or `changelog/{org}/{repo}/{branch}/registry.json` +(changelog-entry index). Serialized with `snake_case` keys. ```json { @@ -80,9 +84,9 @@ Stored at `{product}/registry.json`. Serialized with `snake_case` keys. | Field | Meaning | |---|---| | `schema_version` | Bumped when consumers must change their parser. | -| `product` | Product identifier; matches the first S3 key segment. | +| `product` | Grouping identifier — the product for a bundle index (`bundle/{product}/…`) or the `{org}/{repo}/{branch}` prefix for a changelog-entry index (`changelog/{org}/{repo}/{branch}/…`). | | `generated_at` | UTC timestamp of the last regeneration. | -| `bundles[].file` | Bundle file name, resolved at `{product}/bundle/{file}`. | +| `bundles[].file` | Bundle file name, resolved at `bundle/{product}/{file}` (or entry file at `changelog/{org}/{repo}/{branch}/{file}` for the entry index). | | `bundles[].target` | Target version/date from the bundle's declaration of **this** product (may be null). | | `bundles[].etag` | See the ETag caveat below. | @@ -105,9 +109,11 @@ reads of the same bucket). The refresh runs inside `ChangelogUploadService` after a successful **bundle** upload (it is skipped for `--artifact-type changelog`). `RegistryBuilder`: -- Groups the run's upload targets by product (from the `{product}/bundle/{file}` key). -- For each product, derives one `registry` entry per bundle (file name, that product's - target, locally-computed S3 ETag). +- Groups the run's upload targets by group key — product for bundles + (`bundle/{product}/{file}`), the `{org}/{repo}/{branch}` prefix for entries + (`changelog/{org}/{repo}/{branch}/{file}`). +- For each group, derives one `registry` entry per file (file name, locally-computed S3 ETag, + and — for bundle indexes only — that product's target; entry indexes record no target). - Reads the existing manifest from S3, merges by file name (re-uploads replace their entry; others are preserved), and writes the merged manifest back. @@ -150,8 +156,10 @@ for the producer**: covering the registry `CopyObject` pass-through and the `ObjectRemoved` delete. - A CloudFront cache policy tuned for the manifest already exists (default TTL 1h, min 60s). -The scrubber only passes through keys accepted by `RegistryKey.IsRegistry` (a single -`{product}/registry.json` segment), so arbitrary JSON cannot reach the public surface. +The scrubber only passes through keys accepted by `RegistryKey.IsRegistry` +(`bundle/{product}/registry.json` with a single product segment, or +`changelog/{org}/{repo}/{branch}/registry.json` with at least three segments), so arbitrary JSON +cannot reach the public surface. **No new docs-actions workflow logic is required** for the producer either: the refresh is a side-effect of the existing `changelog upload` step; docs-actions only needs a docs-builder @@ -166,21 +174,26 @@ build that includes this feature. Consumers must therefore treat a missing bundle as non-fatal (skip + warn), not an error. -## `changelog bundle` entry sourcing (declared-gate) +## `changelog bundle` entry sourcing (org/repo/branch gate) The `changelog bundle` command aggregates individual changelog **entries**. It can read those -entries from the local folder or fetch a product's published entries from the CDN -(`{product}/changelog/registry.json` → `{product}/changelog/{file}`, via `CdnChangelogEntryFetcher`). - -CDN entry sourcing is **opt-in per product** (a *declared-gate*): a product's entries are pulled from -the CDN only when that product is declared under `release_notes` in the repo's `docset.yml` — the same -declaration the directive consumes. The decision is made per run by `ChangelogBundlingService`: - -- **Local folder** when `bundle.use_local_changelogs: true`, when `--directory` is passed, when no - concrete product is in scope (for example `--all` or PR/issue-only filters), or when any in-scope - product is **not** declared under `release_notes`. -- **CDN** only when every in-scope product is declared. The declared set is read with - `DocumentationSetFile.LoadMetadata` from the known docset locations (repo root or `docs/`). +entries from the local folder or fetch the **authoring pool's** published entries from the CDN +(`changelog/{org}/{repo}/{branch}/registry.json` → `changelog/{org}/{repo}/{branch}/{file}`, via +`CdnChangelogEntryFetcher`). + +Under the artifact-root layout, entries are org/repo/branch-scoped — not product-scoped — so CDN +entry sourcing keys off the resolvable authoring pool (repo with the same precedence as upload: +`--repo` > `bundle.repo` > git remote; owner from `--owner` > `bundle.owner`, default `elastic`; +branch from `--branch` > `bundle.branch`, default `main`), **not** off the bundle's target products. +This is what lets one repo (for example `kibana`) produce a bundle for a shared product (for example +`cloud-serverless`) while sourcing its own entries from `changelog/elastic/kibana/main/`, without that +product appearing in the repo's own `docset.yml`. The decision is made per run by +`ChangelogBundlingService`: + +- **Local folder** when `bundle.use_local_changelogs: true`, when `--directory` is passed, or when + the authoring repo cannot be resolved. +- **CDN** when the authoring repo resolves, local sourcing is not forced, and a CDN base is + configured (`DOCS_BUILDER_CHANGELOG_CDN`, defaulting to the public distribution). The same gate drives the `--plan` `needs_network` output, so a planning step and the actual bundle run agree on whether the Docker bundle needs network access. The registry-fetch is fail-fast and an @@ -239,8 +252,8 @@ notes use `NoopReleaseNotesResolver`. ### Fetch flow -1. `GET {cdnBase}/{product}/registry.json`. -2. Parse it; for each `bundles[].file`, `GET {cdnBase}/{product}/bundle/{file}`. +1. `GET {cdnBase}/bundle/{product}/registry.json`. +2. Parse it; for each `bundles[].file`, `GET {cdnBase}/bundle/{product}/{file}`. 3. Feed the downloaded YAML into the existing `BundleLoader` → `MergeBundlesByTarget` → render pipeline. **Rendering is unchanged**; only the source of the bundle bytes differs. @@ -270,7 +283,7 @@ logic still applies via `assembler.yml`, exactly as for local bundles. prefetched bundles at render time. - **Security.** The base URL is trusted configuration; the product and registry-supplied bundle file names are validated to single path segments so neither can traverse outside - `{product}/bundle/`. + `bundle/{product}/`. ### Follow-ups (not yet implemented) [implementation-notes] diff --git a/src/Elastic.Documentation.Configuration/Changelog/BundleConfiguration.cs b/src/Elastic.Documentation.Configuration/Changelog/BundleConfiguration.cs index f838ffb8c..6e0d121c4 100644 --- a/src/Elastic.Documentation.Configuration/Changelog/BundleConfiguration.cs +++ b/src/Elastic.Documentation.Configuration/Changelog/BundleConfiguration.cs @@ -52,6 +52,13 @@ public record BundleConfiguration /// public string? Owner { get; init; } + /// + /// Branch whose CDN changelog pool (changelog/{org}/{repo}/{branch}/…) entries are sourced from + /// when bundling from the CDN. Applied to all profiles that do not specify their own. Defaults to + /// main when unset. + /// + public string? Branch { get; init; } + /// /// When set (including an empty list), PR/issue references whose resolved owner/repo is not listed /// are rewritten to # PRIVATE: sentinels at bundle time. When absent, no link filtering is applied. @@ -118,6 +125,12 @@ public record BundleProfile /// public string? Owner { get; init; } + /// + /// Branch whose CDN changelog pool entries this profile sources from. Overrides + /// when set. + /// + public string? Branch { get; init; } + /// /// Feature IDs to mark as hidden in the bundle output. /// When the bundle is rendered, entries with matching feature-id values will be commented out. diff --git a/src/Elastic.Documentation.Configuration/Changelog/ChangelogConfigurationLoader.cs b/src/Elastic.Documentation.Configuration/Changelog/ChangelogConfigurationLoader.cs index 2106bf52d..732505501 100644 --- a/src/Elastic.Documentation.Configuration/Changelog/ChangelogConfigurationLoader.cs +++ b/src/Elastic.Documentation.Configuration/Changelog/ChangelogConfigurationLoader.cs @@ -519,6 +519,7 @@ private static PivotConfiguration ConvertPivot(PivotConfigurationYaml yamlPivot) Description = kvp.Value.Description, Repo = kvp.Value.Repo, Owner = kvp.Value.Owner, + Branch = kvp.Value.Branch, HideFeatures = kvp.Value.HideFeatures?.Values, ReleaseDates = kvp.Value.ReleaseDates, Source = kvp.Value.Source @@ -534,6 +535,7 @@ private static PivotConfiguration ConvertPivot(PivotConfigurationYaml yamlPivot) Description = yaml.Description, Repo = yaml.Repo, Owner = yaml.Owner, + Branch = yaml.Branch, ReleaseDates = yaml.ReleaseDates, LinkAllowRepos = linkAllowRepos, Profiles = profiles diff --git a/src/Elastic.Documentation.Configuration/Changelog/ChangelogConfigurationYaml.cs b/src/Elastic.Documentation.Configuration/Changelog/ChangelogConfigurationYaml.cs index 41f628acc..0e80be7af 100644 --- a/src/Elastic.Documentation.Configuration/Changelog/ChangelogConfigurationYaml.cs +++ b/src/Elastic.Documentation.Configuration/Changelog/ChangelogConfigurationYaml.cs @@ -301,6 +301,11 @@ internal sealed record BundleConfigurationYaml /// public string? Owner { get; set; } + /// + /// Branch whose CDN changelog pool entries are sourced from when bundling. Defaults to main. + /// + public string? Branch { get; set; } + /// /// When true, auto-populate release date in bundle output. Defaults to true when omitted. /// @@ -356,6 +361,11 @@ internal sealed record BundleProfileYaml /// public string? Owner { get; set; } + /// + /// Branch whose CDN changelog pool entries this profile sources from. Overrides bundle.branch. + /// + public string? Branch { get; set; } + /// /// Feature IDs to mark as hidden in the bundle output (string or list). /// diff --git a/src/Elastic.Documentation.Configuration/ReleaseNotes/CdnChangelogEntryFetcher.cs b/src/Elastic.Documentation.Configuration/ReleaseNotes/CdnChangelogEntryFetcher.cs index 30fe0fd8e..a11dc6281 100644 --- a/src/Elastic.Documentation.Configuration/ReleaseNotes/CdnChangelogEntryFetcher.cs +++ b/src/Elastic.Documentation.Configuration/ReleaseNotes/CdnChangelogEntryFetcher.cs @@ -14,11 +14,11 @@ namespace Elastic.Documentation.Configuration.ReleaseNotes; public readonly record struct CdnChangelogEntry(string FileName, string Content); /// -/// Fetches the individual (scrubbed) changelog entries for a single product from the public CDN, for -/// the changelog bundle command when sourcing entries from S3 rather than a local folder. It -/// reads {base}/{product}/changelog/registry.json to enumerate entries and downloads each -/// {base}/{product}/changelog/{file} as raw YAML; the bundle command then applies its usual -/// filter (products / prs / issues) to the downloaded set. +/// Fetches the individual (scrubbed) changelog entries for a single authoring org/repo/branch pool from +/// the public CDN, for the changelog bundle command when sourcing entries from S3 rather than a +/// local folder. It reads {base}/changelog/{org}/{repo}/{branch}/registry.json to enumerate entries +/// and downloads each {base}/changelog/{org}/{repo}/{branch}/{file} as raw YAML; the bundle command +/// then applies its usual filter (products / prs / issues) to the downloaded set. /// /// /// @@ -92,19 +92,34 @@ public CdnChangelogEntryFetcher( } /// - /// Downloads the changelog entries for from the CDN at - /// . Returns an empty list after emitting an error when the registry cannot - /// be read or when a registry-listed entry cannot be fetched within the retry budget. Entries are - /// returned in registry order; the caller owns filtering and de-duplication. + /// Downloads the changelog entries for the authoring // + /// pool from the CDN at . Returns an empty list after emitting an error when + /// the registry cannot be read or when a registry-listed entry cannot be fetched within the retry budget. + /// Entries are returned in registry order; the caller owns filtering and de-duplication. /// public async Task> FetchAsync( Uri baseUri, - string product, + string org, + string repo, + string branch, Action emitError, Action emitWarning, Cancel ctx) { - var registryUri = Combine(baseUri, product, "changelog", "registry.json"); + var poolLabel = $"{org}/{repo}/{branch}"; + + // Defense-in-depth: org/repo come from config and branch from config or CLI on the consumer side. + // Reject empty or traversal segments before building the URI so it cannot normalize a "../" into a + // different changelog pool than intended. + if (!IsValidPool(org, repo, branch)) + { + emitError( + $"Invalid changelog pool '{poolLabel}': the org, repo, and each branch segment must be non-empty, contain no path separators, and not be '.' or '..'."); + return []; + } + + var poolSegments = PoolSegments(org, repo, branch).ToArray(); + var registryUri = CombineSegments(baseUri, [.. poolSegments, "registry.json"]); ChangelogRegistry? registry; try @@ -113,20 +128,20 @@ public async Task> FetchAsync( } catch (Exception ex) when (ex is not OperationCanceledException) { - emitError($"Could not fetch changelog entry registry for product '{product}' from {registryUri}: {ex.Message}"); + emitError($"Could not fetch changelog entry registry for '{poolLabel}' from {registryUri}: {ex.Message}"); return []; } if (registry is null) { - emitError($"Changelog entry registry for product '{product}' at {registryUri} was empty or unparseable."); + emitError($"Changelog entry registry for '{poolLabel}' at {registryUri} was empty or unparseable."); return []; } if (registry.SchemaVersion > SupportedSchemaVersion) { emitError( - $"Changelog entry registry for product '{product}' uses schema version {registry.SchemaVersion}, but this build only understands version {SupportedSchemaVersion}. Update docs-builder."); + $"Changelog entry registry for '{poolLabel}' uses schema version {registry.SchemaVersion}, but this build only understands version {SupportedSchemaVersion}. Update docs-builder."); return []; } @@ -138,12 +153,12 @@ public async Task> FetchAsync( var fileName = entry.File; if (string.IsNullOrWhiteSpace(fileName) || !IsSafeFileName(fileName)) { - emitWarning($"Changelog entry registry for '{product}' lists an invalid file name '{fileName}'; skipping."); + emitWarning($"Changelog entry registry for '{poolLabel}' lists an invalid file name '{fileName}'; skipping."); continue; } - var entryUri = Combine(baseUri, product, "changelog", fileName); - var (fetched, content, lastError) = await TryFetchEntryAsync(entryUri, fileName, product, ctx).ConfigureAwait(false); + var entryUri = CombineSegments(baseUri, [.. poolSegments, fileName]); + var (fetched, content, lastError) = await TryFetchEntryAsync(entryUri, fileName, poolLabel, ctx).ConfigureAwait(false); if (fetched) { entries.Add(new CdnChangelogEntry(fileName, content)); @@ -154,12 +169,12 @@ public async Task> FetchAsync( // scrubbed to the public one within milliseconds. Still missing after the retry budget means // a genuine propagation/scrub failure — fail rather than ship a bundle missing this entry. emitError( - $"Changelog entry '{fileName}' for product '{product}' is listed in the registry but could not be fetched from {entryUri} after {_maxAttempts} attempt(s): {lastError}. " + + $"Changelog entry '{fileName}' for '{poolLabel}' is listed in the registry but could not be fetched from {entryUri} after {_maxAttempts} attempt(s): {lastError}. " + "The scrubbed copy may not have propagated to the CDN yet; retry shortly, and if it persists check the changelog scrubber pipeline."); return []; } - _logger.LogInformation("Fetched {Count} changelog entry(ies) for {Product} from {BaseUri}", entries.Count, product, baseUri); + _logger.LogInformation("Fetched {Count} changelog entry(ies) for {Pool} from {BaseUri}", entries.Count, poolLabel, baseUri); return entries; } @@ -168,7 +183,7 @@ public async Task> FetchAsync( /// up to times with exponential backoff. Retry requests are cache-busted /// so a CloudFront-cached 404 cannot pin the result for the whole window. /// - private async Task<(bool Fetched, string Content, string? LastError)> TryFetchEntryAsync(Uri uri, string fileName, string product, Cancel ctx) + private async Task<(bool Fetched, string Content, string? LastError)> TryFetchEntryAsync(Uri uri, string fileName, string poolLabel, Cancel ctx) { string? lastError = null; @@ -179,7 +194,7 @@ public async Task> FetchAsync( { var content = await FetchTextAsync(uri, attempt, ctx).ConfigureAwait(false); if (attempt > 1) - _logger.LogInformation("Fetched changelog entry '{File}' for {Product} on attempt {Attempt}/{Max}", fileName, product, attempt, _maxAttempts); + _logger.LogInformation("Fetched changelog entry '{File}' for {Pool} on attempt {Attempt}/{Max}", fileName, poolLabel, attempt, _maxAttempts); return (true, content, null); } catch (Exception ex) when (ex is not OperationCanceledException) @@ -190,8 +205,8 @@ public async Task> FetchAsync( var delay = RetryDelay(attempt); _logger.LogDebug( - "Changelog entry '{File}' for {Product} not yet available (attempt {Attempt}/{Max}: {Error}); retrying in {Delay}", - fileName, product, attempt, _maxAttempts, ex.Message, delay); + "Changelog entry '{File}' for {Pool} not yet available (attempt {Attempt}/{Max}: {Error}); retrying in {Delay}", + fileName, poolLabel, attempt, _maxAttempts, ex.Message, delay); await _sleep(delay, ctx).ConfigureAwait(false); } } @@ -241,16 +256,54 @@ private static Uri WithCacheBuster(Uri uri) return new Uri($"{uri.AbsoluteUri}{separator}_={DateTimeOffset.UtcNow.Ticks:x}"); } - private static Uri Combine(Uri baseUri, params string[] segments) + private static Uri CombineSegments(Uri baseUri, IReadOnlyList segments) { var basePath = baseUri.AbsoluteUri.TrimEnd('/'); var suffix = string.Join('/', segments.Select(Uri.EscapeDataString)); return new Uri($"{basePath}/{suffix}"); } + /// + /// Expands the pool into individually-escapable path segments — changelog, org, repo, then each + /// /-delimited part of the branch — so a branch's slashes stay real CDN key separators rather + /// than being percent-encoded into a single segment. + /// + private static IEnumerable PoolSegments(string org, string repo, string branch) + { + yield return "changelog"; + yield return org; + yield return repo; + foreach (var part in branch.Split('/')) + yield return part; + } + + /// + /// Validates the consumer-supplied pool coordinates (org, repo, and each /-delimited branch + /// segment) so a malformed branch cannot redirect the fetch to a different pool via URI normalization. + /// + private static bool IsValidPool(string org, string repo, string branch) + { + if (!IsSafePoolSegment(org) || !IsSafePoolSegment(repo)) + return false; + + foreach (var part in branch.Split('/')) + { + if (!IsSafePoolSegment(part)) + return false; + } + + return true; + } + + private static bool IsSafePoolSegment(string segment) => + !string.IsNullOrEmpty(segment) + && segment is not ("." or "..") + && !segment.Contains('/', StringComparison.Ordinal) + && !segment.Contains('\\', StringComparison.Ordinal); + /// /// Guards against path traversal or nested keys sneaking in via the registry: an entry file name - /// must be a single path segment (the producer always writes {product}/changelog/{file}). + /// must be a single path segment (the producer always writes changelog/{org}/{repo}/{branch}/{file}). /// private static bool IsSafeFileName(string fileName) => !fileName.Contains('/', StringComparison.Ordinal) diff --git a/src/Elastic.Documentation.Configuration/ReleaseNotes/CdnChangelogFetcher.cs b/src/Elastic.Documentation.Configuration/ReleaseNotes/CdnChangelogFetcher.cs index 21d33b3e1..f19ddbf23 100644 --- a/src/Elastic.Documentation.Configuration/ReleaseNotes/CdnChangelogFetcher.cs +++ b/src/Elastic.Documentation.Configuration/ReleaseNotes/CdnChangelogFetcher.cs @@ -12,8 +12,8 @@ namespace Elastic.Documentation.Configuration.ReleaseNotes; /// /// Fetches changelog bundles for a single product from the public CDN. It reads -/// {base}/{product}/registry.json to enumerate bundles, downloads each -/// {base}/{product}/bundle/{file}, and parses them via +/// {base}/bundle/{product}/registry.json to enumerate bundles, downloads each +/// {base}/bundle/{product}/{file}, and parses them via /// . /// /// @@ -93,7 +93,7 @@ public async Task> FetchAsync( Action emitWarning, Cancel ctx) { - var registryUri = Combine(baseUri, product, "registry.json"); + var registryUri = Combine(baseUri, "bundle", product, "registry.json"); ChangelogRegistry? registry; try @@ -164,7 +164,7 @@ public async Task> FetchAsync( continue; } - var bundleUri = Combine(baseUri, product, "bundle", fileName); + var bundleUri = Combine(baseUri, "bundle", product, fileName); try { contents.Add((fileName, await FetchTextAsync(bundleUri, ctx).ConfigureAwait(false))); diff --git a/src/Elastic.Documentation.Configuration/ReleaseNotes/ChangelogRegistry.cs b/src/Elastic.Documentation.Configuration/ReleaseNotes/ChangelogRegistry.cs index cccbf489b..7943918c6 100644 --- a/src/Elastic.Documentation.Configuration/ReleaseNotes/ChangelogRegistry.cs +++ b/src/Elastic.Documentation.Configuration/ReleaseNotes/ChangelogRegistry.cs @@ -7,18 +7,18 @@ namespace Elastic.Documentation.Configuration.ReleaseNotes; /// -/// Consumer-side view of the per-product {product}/registry.json manifest published -/// alongside scrubbed changelog bundles. Mirrors the producer's shape (see the changelog upload -/// service) but is intentionally lenient: only the fields the changelog directive needs to -/// enumerate bundles are declared, and nothing is required so a partially-written or -/// future-versioned manifest still deserializes. +/// Consumer-side view of the bundle/{product}/registry.json manifest (or the +/// changelog/{org}/{repo}/{branch}/registry.json entry index) published alongside scrubbed changelog content. +/// Mirrors the producer's shape (see the changelog upload service) but is intentionally lenient: only +/// the fields the changelog directive needs to enumerate bundles are declared, and nothing is +/// required so a partially-written or future-versioned manifest still deserializes. /// public sealed record ChangelogRegistry { /// Manifest schema version. A higher major than the consumer understands is rejected upstream. public int SchemaVersion { get; init; } = 1; - /// Product identifier the manifest belongs to (matches the first S3 key segment). + /// Grouping identifier the manifest belongs to (the second S3 key segment: product for a bundle index, repo for a changelog-entry index). public string? Product { get; init; } /// Bundles known for this product, newest first as written by the producer. @@ -28,7 +28,7 @@ public sealed record ChangelogRegistry /// One entry in . public sealed record ChangelogRegistryBundle { - /// Bundle file name, resolved at {product}/bundle/{file} on the CDN. + /// Bundle file name, resolved at bundle/{product}/{file} on the CDN (or entry file at changelog/{org}/{repo}/{branch}/{file} for the entry index). public string? File { get; init; } /// Target version or release date declared by the bundle (e.g. 9.3.0). diff --git a/src/Elastic.Markdown/Myst/Directives/Changelog/ChangelogBlock.cs b/src/Elastic.Markdown/Myst/Directives/Changelog/ChangelogBlock.cs index a19d700e7..0ce73912d 100644 --- a/src/Elastic.Markdown/Myst/Directives/Changelog/ChangelogBlock.cs +++ b/src/Elastic.Markdown/Myst/Directives/Changelog/ChangelogBlock.cs @@ -100,7 +100,7 @@ public class ChangelogBlock(DirectiveBlockParser parser, ParserContext context) /// /// Product to source bundles for from the public CDN (the :cdn: option). When set, the - /// directive fetches {cdnBase}/{product}/registry.json and the bundles it lists + /// directive fetches {cdnBase}/bundle/{product}/registry.json and the bundles it lists /// instead of reading a local folder; any folder argument is ignored. /// public string? CdnProduct { get; private set; } diff --git a/src/infra/docs-lambda-changelog-scrubber/Program.cs b/src/infra/docs-lambda-changelog-scrubber/Program.cs index 9ce65c731..8cd1a4ea6 100644 --- a/src/infra/docs-lambda-changelog-scrubber/Program.cs +++ b/src/infra/docs-lambda-changelog-scrubber/Program.cs @@ -181,7 +181,10 @@ async Task CopyPassThrough(IAmazonS3 s3Client, string sourceBucket, string key, async Task ScrubContent(string key, string content, ILambdaContext context) { - var isBundlePath = key.Contains("/bundle/", StringComparison.OrdinalIgnoreCase); + // Artifact-root layout: bundles live under "bundle/{product}/...", entries under + // "changelog/{org}/{repo}/{branch}/...". Match the bundle prefix (not a "/bundle/" substring, which no + // longer appears in the new keys) so bundles are not misclassified as changelog entries. + var isBundlePath = key.StartsWith("bundle/", StringComparison.OrdinalIgnoreCase); if (isBundlePath) return await ScrubBundle(content, context); diff --git a/src/infra/docs-lambda-changelog-scrubber/README.md b/src/infra/docs-lambda-changelog-scrubber/README.md index 42551ee0d..6797772a8 100644 --- a/src/infra/docs-lambda-changelog-scrubber/README.md +++ b/src/infra/docs-lambda-changelog-scrubber/README.md @@ -31,12 +31,12 @@ The `bootstrap` binary should be available under: ## Event handling - **`s3:ObjectCreated:*`** on `.yaml`/`.yml` files: read from private bucket, scrub private references, write to public bucket -- **`s3:ObjectCreated:*`** on `.json` files: only per-product registry manifests (keys matching `RegistryKey.IsRegistry`, i.e. `{product}/registry.json`) are passed through as-is; any other `.json` key is skipped +- **`s3:ObjectCreated:*`** on `.json` files: only registry manifests (keys matching `RegistryKey.IsRegistry`, i.e. `bundle/{product}/registry.json` or `changelog/{org}/{repo}/{branch}/registry.json`) are passed through as-is; any other `.json` key is skipped - **`s3:ObjectRemoved:*`**: delete the same key from the public bucket - Other keys are silently skipped ## Scrubbing logic -- **Bundle files** (`{product}/bundle/*.yaml`): `LinkAllowlistSanitizer.TryApplyBundle` scrubs `prs`/`issues` lists -- **Changelog entries** (`{product}/changelog/*.yaml`): `LinkAllowlistSanitizer.TryApplyChangelogEntry` scrubs `prs`, `issues`, `description`, `impact`, `action` +- **Bundle files** (`bundle/{product}/*.yaml`, detected by the `bundle/` key prefix): `LinkAllowlistSanitizer.TryApplyBundle` scrubs `prs`/`issues` lists +- **Changelog entries** (`changelog/{org}/{repo}/{branch}/*.yaml`): `LinkAllowlistSanitizer.TryApplyChangelogEntry` scrubs `prs`, `issues`, `description`, `impact`, `action` - The allowlist is built once at cold start from the embedded `assembler.yml` via `BuildAllowReposFromAssembler` diff --git a/src/services/Elastic.Changelog/Bundling/ChangelogBundlingService.cs b/src/services/Elastic.Changelog/Bundling/ChangelogBundlingService.cs index 329703a43..e31d7f278 100644 --- a/src/services/Elastic.Changelog/Bundling/ChangelogBundlingService.cs +++ b/src/services/Elastic.Changelog/Bundling/ChangelogBundlingService.cs @@ -2,8 +2,6 @@ // Elasticsearch B.V licenses this file to you under the Apache 2.0 License. // See the LICENSE file in the project root for more information -using System.Collections.Frozen; -using System.IO.Abstractions; using System.Security.Cryptography; using System.Text; using System.Text.RegularExpressions; @@ -15,7 +13,6 @@ using Elastic.Documentation.Configuration.Assembler; using Elastic.Documentation.Configuration.Changelog; using Elastic.Documentation.Configuration.ReleaseNotes; -using Elastic.Documentation.Configuration.Toc; using Elastic.Documentation.Diagnostics; using Elastic.Documentation.Extensions; using Elastic.Documentation.ReleaseNotes; @@ -48,6 +45,12 @@ public record BundleChangelogsArguments public string? Owner { get; init; } public string? Repo { get; init; } + /// + /// Branch whose CDN changelog pool (changelog/{org}/{repo}/{branch}/…) entries are sourced from. + /// null = use config bundle.branch, then the default branch (main). + /// + public string? Branch { get; init; } + /// /// Profile name to use (from bundle.profiles in config) /// @@ -122,7 +125,7 @@ public record BundlePlanResult public string? OutputPath { get; init; } /// - /// Public CDN URL of the (scrubbed) bundle once uploaded: {base}/{product}/bundle/{file}. + /// Public CDN URL of the (scrubbed) bundle once uploaded: {base}/bundle/{product}/{file}. /// Null when no concrete product can be resolved to scope the URL (e.g. option-mode PR/issue-only /// filters). Consumed by the bundle-PR action to poll for and download the scrubbed copy. /// @@ -148,13 +151,15 @@ public partial class ChangelogBundlingService( ? new ChangelogConfigurationLoader(logFactory, configurationContext, fileSystem ?? FileSystemFactory.RealRead) : null; + // Defaults applied when sourcing CDN entries and the org/branch are not otherwise resolvable. + private const string DefaultOwner = "elastic"; + private const string DefaultBranch = "main"; + /// /// UTF-8 encoding without BOM for writing YAML files. /// private static readonly UTF8Encoding Utf8NoBom = new(encoderShouldEmitUTF8Identifier: false); - private static readonly string[] DocsetFileNames = ["docset.yml", "_docset.yml"]; - [GeneratedRegex(@"(\s+)version:", RegexOptions.Multiline)] internal static partial Regex VersionToTargetRegex(); @@ -214,15 +219,17 @@ public async Task BundleChangelogs(IDiagnosticsCollector collector, Bundle // Apply config defaults if available input = ApplyConfigDefaults(input, config); - // Decide where the individual changelog entries come from. CDN sourcing is opt-in via the - // repo's docset.yml release_notes (declared-gate): a product's entries are pulled from the - // public CDN only when it is declared there. Otherwise — or when the user forces local - // sourcing (bundle.use_local_changelogs / --directory) or no concrete product is in scope — - // fall back to the local folder. This keeps the run in lockstep with PlanBundleAsync's - // needs_network decision. + // Decide where the individual changelog entries come from. Under Option AD entries are scoped to + // an org/repo/branch pool (changelog/{org}/{repo}/{branch}/...), so CDN sourcing keys off the + // resolvable authoring repo (bundle.repo / --repo), with org and branch defaulting when unset — + // not the bundle's target products. Fall back to the local folder when the user forces it + // (bundle.use_local_changelogs / --directory), the repo is unresolvable, or no CDN base is + // configured. This stays in lockstep with PlanBundleAsync's needs_network decision. var useLocalChangelogs = config?.Bundle?.UseLocalChangelogs ?? false; - var cdnProducts = ResolveCdnProducts(input); - var useCdn = ShouldSourceFromCdn(cdnProducts, useLocalChangelogs, explicitDirectory); + var authoringRepo = NormalizeRepo(input.Repo); + var authoringOwner = ResolveAuthoringOwner(input.Owner, input.Repo); + var authoringBranch = string.IsNullOrWhiteSpace(input.Branch) ? DefaultBranch : input.Branch; + var useCdn = ShouldSourceFromCdn(authoringRepo, useLocalChangelogs: useLocalChangelogs, explicitDirectory: explicitDirectory); // Validate input. In CDN mode the local input directory is not read, so its existence // is not required. @@ -270,7 +277,7 @@ public async Task BundleChangelogs(IDiagnosticsCollector collector, Bundle ChangelogMatchResult matchResult; if (useCdn) { - var contents = await FetchCdnEntriesAsync(collector, cdnProducts, ctx); + var contents = await FetchCdnEntriesAsync(collector, authoringOwner, authoringRepo, authoringBranch, ctx); if (contents == null) return false; matchResult = entryMatcher.MatchChangelogContents(collector, contents, filterCriteria, ctx); @@ -471,6 +478,7 @@ public async Task BundleChangelogs(IDiagnosticsCollector collector, Bundle IReadOnlyList? outputProducts = null; string? repo = null; string? owner = null; + string? branch = null; string[]? mergedHideFeatures = null; string? profileDescription = null; var profileSuppressReleaseDate = false; @@ -511,9 +519,10 @@ public async Task BundleChangelogs(IDiagnosticsCollector collector, Bundle outputProducts = parsedOutputProducts; } - // Profile-level repo/owner takes precedence; fall back to bundle-level defaults + // Profile-level repo/owner/branch takes precedence; fall back to bundle-level defaults repo = profile.Repo ?? config.Bundle.Repo; owner = profile.Owner ?? config.Bundle.Owner; + branch = profile.Branch ?? config.Bundle.Branch; mergedHideFeatures = profile.HideFeatures?.Count > 0 ? [.. profile.HideFeatures] : null; profileSuppressReleaseDate = !(profile.ReleaseDates ?? config.Bundle.ReleaseDates ?? true); @@ -559,6 +568,7 @@ public async Task BundleChangelogs(IDiagnosticsCollector collector, Bundle OutputProducts = outputProducts, Repo = repo, Owner = owner, + Branch = branch, HideFeatures = mergedHideFeatures, Description = profileDescription, SuppressReleaseDate = profileSuppressReleaseDate @@ -581,9 +591,10 @@ private BundleChangelogsArguments ApplyConfigDefaults(BundleChangelogsArguments // Apply resolve: CLI takes precedence over config. Only use config when CLI did not specify. var resolve = input.Resolve ?? config.Bundle.Resolve; - // Apply repo/owner: CLI takes precedence; fall back to bundle-level config defaults. + // Apply repo/owner/branch: CLI takes precedence; fall back to bundle-level config defaults. var repo = input.Repo ?? config.Bundle.Repo; var owner = input.Owner ?? config.Bundle.Owner; + var branch = input.Branch ?? config.Bundle.Branch; // Apply description: CLI takes precedence; fall back to bundle-level config default var description = input.Description ?? config.Bundle.Description; @@ -601,6 +612,7 @@ private BundleChangelogsArguments ApplyConfigDefaults(BundleChangelogsArguments Resolve = resolve, Repo = repo, Owner = owner, + Branch = branch, Description = description, SuppressReleaseDate = suppressReleaseDate, LinkAllowRepos = config.Bundle.LinkAllowRepos @@ -649,14 +661,13 @@ private BundleChangelogsArguments ApplyConfigDefaults(BundleChangelogsArguments } } - // CDN entry sourcing needs network access for the Docker bundle run. Mirror the run-mode - // declared-gate: active only when every in-scope product is declared in docset.yml release_notes - // and the user has not forced local sourcing. Products are resolved from the profile patterns - // (not yet materialized in plan mode) unioned with any explicit --input/--output-products. + // CDN entry sourcing needs network access for the Docker bundle run. Mirror the run-mode gate: + // active when the authoring repo resolves (profile/config bundle.repo), the user has not forced + // local sourcing, and a CDN base is configured. var useLocalChangelogs = config?.Bundle?.UseLocalChangelogs ?? false; var explicitDirectory = !string.IsNullOrWhiteSpace(input.Directory); - var cdnProducts = ResolveCdnProducts(input, profileDef); - if (ShouldSourceFromCdn(cdnProducts, useLocalChangelogs, explicitDirectory)) + var authoringRepo = NormalizeRepo(input.Repo ?? profileDef?.Repo ?? config?.Bundle?.Repo); + if (ShouldSourceFromCdn(authoringRepo, useLocalChangelogs: useLocalChangelogs, explicitDirectory: explicitDirectory)) needsNetwork = true; // Resolve output path — mirrors the logic in ProcessProfile + ApplyConfigDefaults. @@ -685,10 +696,7 @@ private BundleChangelogsArguments ApplyConfigDefaults(BundleChangelogsArguments }; } - /// - /// Public CDN URL of the scrubbed bundle ({base}/{product}/bundle/{file}), polled by the - /// bundle-PR action. Null when the product, output file name, or CDN base cannot be resolved. - /// + /// Public CDN URL of the scrubbed bundle ({base}/bundle/{product}/{file}); null when product, output file name, or CDN base cannot be resolved. private string? ResolveCdnBundleUrl(BundleProfile? profileDef, BundleChangelogsArguments input, string? outputPath) { if (string.IsNullOrWhiteSpace(outputPath)) @@ -706,7 +714,7 @@ private BundleChangelogsArguments ApplyConfigDefaults(BundleChangelogsArguments return null; var basePath = baseUri.AbsoluteUri.TrimEnd('/'); - return $"{basePath}/{Uri.EscapeDataString(product)}/bundle/{Uri.EscapeDataString(fileName)}"; + return $"{basePath}/bundle/{Uri.EscapeDataString(product)}/{Uri.EscapeDataString(fileName)}"; } /// @@ -738,25 +746,28 @@ private BundleChangelogsArguments ApplyConfigDefaults(BundleChangelogsArguments return null; } - /// - /// Downloads the in-scope changelog entries from the public CDN, scoped to the bundle's products. - /// Returns null (after emitting an error) on any fatal fetch failure; an entry not yet public is - /// skipped with a warning. - /// + /// Downloads the authoring // pool's changelog entries from the CDN (changelog/{org}/{repo}/{branch}/...); returns null after emitting an error on any fatal fetch failure. private async Task?> FetchCdnEntriesAsync( IDiagnosticsCollector collector, - IReadOnlyList products, + string? org, + string? repo, + string? branch, Cancel ctx) { - if (products.Count == 0) + if (string.IsNullOrWhiteSpace(repo)) { collector.EmitError(string.Empty, - "Sourcing changelog entries from the CDN requires a resolvable product declared under " + - "release_notes in docset.yml. Declare the product there, or set bundle.use_local_changelogs: " + - "true in changelog.yml / pass --directory to bundle local changelog files."); + "Sourcing changelog entries from the CDN requires a resolvable authoring repository. " + + "Set bundle.repo in changelog.yml (or pass --repo), or set bundle.use_local_changelogs: true " + + "in changelog.yml / pass --directory to bundle local changelog files."); return null; } + // org/branch always resolve to a default at the call site, but guard anyway so the fetcher never + // receives a blank pool segment. + var resolvedOrg = string.IsNullOrWhiteSpace(org) ? DefaultOwner : org; + var resolvedBranch = string.IsNullOrWhiteSpace(branch) ? DefaultBranch : branch; + var baseUri = ChangelogCdn.ResolveBaseUri(); if (baseUri is null) { @@ -765,20 +776,15 @@ private BundleChangelogsArguments ApplyConfigDefaults(BundleChangelogsArguments return null; } - var byName = new Dictionary(StringComparer.Ordinal); var fatalFailure = false; - foreach (var product in products) - { - var entries = await _entryFetcher.FetchAsync( - baseUri, - product, - msg => { fatalFailure = true; collector.EmitError(string.Empty, msg); }, - msg => collector.EmitWarning(string.Empty, msg), - ctx); - - foreach (var entry in entries) - byName[entry.FileName] = entry.Content; - } + var entries = await _entryFetcher.FetchAsync( + baseUri, + resolvedOrg, + repo, + resolvedBranch, + msg => { fatalFailure = true; collector.EmitError(string.Empty, msg); }, + msg => collector.EmitWarning(string.Empty, msg), + ctx); // The fetcher emits an error (via the callback above) for any fatal condition — a registry that // cannot be read, or a registry-listed entry still missing after its retry budget. Either would @@ -786,108 +792,51 @@ private BundleChangelogsArguments ApplyConfigDefaults(BundleChangelogsArguments if (fatalFailure) return null; - _logger.LogInformation("Sourced {Count} changelog entr(ies) from the CDN for product(s) {Products}", - byName.Count, string.Join(", ", products)); - - return byName.Select(kv => (kv.Key, kv.Value)).ToList(); - } + var byName = new Dictionary(StringComparer.Ordinal); + foreach (var entry in entries) + byName[entry.FileName] = entry.Content; - /// - /// The distinct, concrete product IDs that scope a CDN-sourced bundle. In run mode the input is already - /// materialized, so the ids come from input.InputProducts/OutputProducts. In plan mode pass - /// so the ids can also be read from the profile's (not yet materialized) - /// products/output_products patterns. Wildcards are excluded. - /// - private static IReadOnlyList ResolveCdnProducts(BundleChangelogsArguments input, BundleProfile? profileDef = null) - { - var ids = new List(); - AppendConcreteProductsFromPattern(ids, profileDef?.OutputProducts); - AppendConcreteProductsFromPattern(ids, profileDef?.Products); - AppendConcreteProducts(ids, input.InputProducts); - AppendConcreteProducts(ids, input.OutputProducts); - return ids.Distinct(StringComparer.Ordinal).ToList(); - } + _logger.LogInformation("Sourced {Count} changelog entr(ies) from the CDN for {Pool}", + byName.Count, $"{resolvedOrg}/{repo}/{resolvedBranch}"); - private static void AppendConcreteProducts(List ids, IReadOnlyList? products) - { - if (products == null) - return; - foreach (var product in products) - { - if (!string.IsNullOrWhiteSpace(product.Product) && product.Product != "*") - ids.Add(product.Product); - } + return byName.Select(kv => (kv.Key, kv.Value)).ToList(); } - // Profile product patterns are comma-separated groups; each group is whitespace-delimited - // "product target lifecycle", so the product id is the first token of each group. - private static void AppendConcreteProductsFromPattern(List ids, string? pattern) + /// Reduces a configured repo value to the single CDN-key path segment (owner/repo -> repo); null/empty unchanged. + private static string? NormalizeRepo(string? repo) { - if (string.IsNullOrWhiteSpace(pattern)) - return; - foreach (var group in pattern.Split(',', StringSplitOptions.RemoveEmptyEntries | StringSplitOptions.TrimEntries)) - { - var id = group.Split((char[]?)null, StringSplitOptions.RemoveEmptyEntries).FirstOrDefault(); - if (!string.IsNullOrWhiteSpace(id) && id != "*") - ids.Add(id); - } + if (string.IsNullOrWhiteSpace(repo)) + return repo; + var slash = repo.LastIndexOf('/'); + return slash >= 0 && slash < repo.Length - 1 ? repo[(slash + 1)..] : repo; } /// - /// Declared-gate for CDN entry sourcing: the public CDN is used only when every in-scope product is - /// declared under release_notes in the repo's docset.yml. Returns false (local sourcing) when - /// the user forced local sourcing (bundle.use_local_changelogs or an explicit --directory), - /// no concrete product is in scope, or any in-scope product is undeclared. + /// Resolves the org segment for the CDN entry pool: the explicit when set, + /// otherwise the owner/ prefix of a combined value (e.g. acme/widget + /// -> acme) so it is not lost, falling back to . /// - private bool ShouldSourceFromCdn(IReadOnlyList cdnProducts, bool useLocalChangelogs, bool explicitDirectory) + private static string ResolveAuthoringOwner(string? owner, string? repo) { - if (useLocalChangelogs || explicitDirectory || cdnProducts.Count == 0) - return false; - var declared = LoadDeclaredReleaseNotesProducts(); - return cdnProducts.All(declared.Contains); - } + if (!string.IsNullOrWhiteSpace(owner)) + return owner; - /// - /// Product ids declared under release_notes in the repo's docset.yml, normalized (underscores → - /// hyphens). Returns an empty set when no docset.yml is found in the known locations or it declares none. - /// - private IReadOnlySet LoadDeclaredReleaseNotesProducts() - { - var configFile = FindDocsetFile(); - if (configFile is null) - return FrozenSet.Empty; - try - { - var docSet = DocumentationSetFile.LoadMetadata(configFile); - return docSet.ReleaseNotes - .Select(r => r.Product?.Trim().Replace('_', '-')) - .Where(p => !string.IsNullOrEmpty(p)) - .Select(p => p!) - .ToFrozenSet(StringComparer.Ordinal); - } - catch (Exception ex) + if (!string.IsNullOrWhiteSpace(repo)) { - _logger.LogDebug("Could not read release_notes from docset.yml for CDN gating: {Error}", ex.Message); - return FrozenSet.Empty; + var slash = repo.IndexOf('/', StringComparison.Ordinal); + if (slash > 0) + return repo[..slash]; } + + return DefaultOwner; } - // Mirrors Paths.GetDocsetPathFromKnownLocations (repo root or docs/, docset.yml or _docset.yml) without - // taking a dependency on the tooling layer from this service. - private IFileInfo? FindDocsetFile() + /// Gate for repo-scoped CDN entry sourcing: true when the authoring repo resolves, local sourcing is not forced (bundle.use_local_changelogs/--directory), and a CDN base is configured. + private static bool ShouldSourceFromCdn(string? authoringRepo, bool useLocalChangelogs, bool explicitDirectory) { - var cwd = _fileSystem.Directory.GetCurrentDirectory(); - string[] folders = [cwd, _fileSystem.Path.Join(cwd, "docs")]; - foreach (var folder in folders) - { - foreach (var name in DocsetFileNames) - { - var candidate = _fileSystem.Path.Join(folder, name); - if (_fileSystem.File.Exists(candidate)) - return _fileSystem.FileInfo.New(candidate); - } - } - return null; + if (useLocalChangelogs || explicitDirectory || string.IsNullOrWhiteSpace(authoringRepo)) + return false; + return ChangelogCdn.ResolveBaseUri() is not null; } private bool ValidateInput(IDiagnosticsCollector collector, BundleChangelogsArguments input, bool requireDirectoryExists) diff --git a/src/services/Elastic.Changelog/Uploading/ChangelogUploadService.cs b/src/services/Elastic.Changelog/Uploading/ChangelogUploadService.cs index f3f2f70af..dd0e37180 100644 --- a/src/services/Elastic.Changelog/Uploading/ChangelogUploadService.cs +++ b/src/services/Elastic.Changelog/Uploading/ChangelogUploadService.cs @@ -29,6 +29,30 @@ public record ChangelogUploadArguments public required string S3BucketName { get; init; } public string? Config { get; init; } public string? Directory { get; init; } + + /// + /// Authoring repository identifier used to scope changelog-entry keys + /// (changelog/{org}/{repo}/{branch}/{file}). Required for + /// uploads; unused for bundle uploads (which are product-scoped from the bundle YAML). Resolved by the + /// CLI via the precedence --repo > bundle.repo > git remote origin. + /// + public string? Repo { get; init; } + + /// + /// GitHub owner (org), the first segment of changelog-entry keys + /// (changelog/{org}/{repo}/{branch}/{file}). Required for + /// uploads; unused for bundle uploads. Resolved by the CLI via the precedence + /// --owner > bundle.owner > git remote origin. + /// + public string? Owner { get; init; } + + /// + /// Branch segment of changelog-entry keys (changelog/{org}/{repo}/{branch}/{file}), stored + /// verbatim so any / in the branch become real key separators (e.g. feature/foo). + /// Required for uploads; unused for bundle uploads. Resolved by the + /// CLI via the precedence --branch > the current git branch. + /// + public string? Branch { get; init; } } public partial class ChangelogUploadService( @@ -47,8 +71,15 @@ public partial class ChangelogUploadService( [GeneratedRegex(@"^[a-zA-Z0-9_-]+$")] private static partial Regex ProductNameRegex(); - private static readonly YamlDotNet.Serialization.IDeserializer EntryDeserializer = - ReleaseNotesSerialization.GetEntryDeserializer(); + // Authoring repo identifier used as a single S3 path segment (changelog/{org}/{repo}/{branch}/{file}). + // Same character class as product names but also allows "."; "." / ".." are rejected to prevent traversal. + [GeneratedRegex(@"^[a-zA-Z0-9._-]+$")] + private static partial Regex RepoNameRegex(); + + // GitHub owner (org) login used as the first changelog key segment (changelog/{org}/...). GitHub logins + // are ASCII alphanumerics with single hyphens; "." is intentionally excluded. + [GeneratedRegex(@"^[a-zA-Z0-9-]+$")] + private static partial Regex OrgNameRegex(); public async Task Upload(IDiagnosticsCollector collector, ChangelogUploadArguments args, Cancel ctx) { @@ -73,7 +104,12 @@ public async Task Upload(IDiagnosticsCollector collector, ChangelogUploadA var targets = args.ArtifactType == ArtifactType.Bundle ? DiscoverBundleUploadTargets(collector, directory) - : DiscoverUploadTargets(collector, directory); + : DiscoverUploadTargets(collector, directory, args.Owner, args.Repo, args.Branch); + + // Entry uploads abort (rather than no-op) when the repo cannot be resolved: the keys would be + // unscoped and a silent skip would look like "nothing to upload". + if (collector.Errors > 0) + return false; if (targets.Count == 0) { @@ -132,8 +168,35 @@ private async Task RefreshRegistries( } } - internal IReadOnlyList DiscoverUploadTargets(IDiagnosticsCollector collector, string changelogDir) + internal IReadOnlyList DiscoverUploadTargets(IDiagnosticsCollector collector, string changelogDir, string? org, string? repo, string? branch) { + // Option AD: entries live once, under the authoring org/repo/branch pool — independent of which + // products later consume them. Org, repo, and branch must all resolve (CLI flags > bundle config > + // git); a missing/invalid value is fatal because every entry key derives from them. + if (string.IsNullOrWhiteSpace(org) || !OrgNameRegex().IsMatch(org)) + { + collector.EmitError(string.Empty, + $"A valid GitHub owner is required to upload changelog entries (resolved: \"{org ?? ""}\"). " + + "Set --owner, bundle.owner in changelog.yml, or run inside a checkout with a github.com origin remote."); + return []; + } + + if (string.IsNullOrWhiteSpace(repo) || !RepoNameRegex().IsMatch(repo) || repo is "." or "..") + { + collector.EmitError(string.Empty, + $"A valid repository identifier is required to upload changelog entries (resolved: \"{repo ?? ""}\"). " + + "Set --repo, bundle.repo in changelog.yml, or run inside a checkout with a github.com origin remote."); + return []; + } + + if (string.IsNullOrWhiteSpace(branch) || !IsValidBranch(branch)) + { + collector.EmitError(string.Empty, + $"A valid branch is required to upload changelog entries (resolved: \"{branch ?? ""}\"). " + + "Set --branch or run inside a checkout with a current branch."); + return []; + } + var rootDir = _fileSystem.DirectoryInfo.New(changelogDir); var yamlFiles = _fileSystem.Directory.GetFiles(changelogDir, "*.yaml", SearchOption.TopDirectoryOnly) @@ -151,52 +214,25 @@ internal IReadOnlyList DiscoverUploadTargets(IDiagnosticsCollector continue; } - var products = ReadProductsFromFragment(filePath); - if (products.Count == 0) - { - _logger.LogDebug("No products found in {File}, skipping", filePath); - continue; - } - var fileName = _fileSystem.Path.GetFileName(filePath); - - foreach (var product in products) - { - if (!ProductNameRegex().IsMatch(product)) - { - collector.EmitWarning(filePath, $"Skipping invalid product name \"{product}\" (must match [a-zA-Z0-9_-]+)"); - continue; - } - - var s3Key = $"{product}/changelog/{fileName}"; - targets.Add(new UploadTarget(filePath, s3Key)); - } + var s3Key = $"changelog/{org}/{repo}/{branch}/{fileName}"; + targets.Add(new UploadTarget(filePath, s3Key)); } return targets; } - private List ReadProductsFromFragment(string filePath) + // Validates a branch used verbatim as one-or-more changelog key segments + // (changelog/{org}/{repo}/{branch}/…). Each "/"-delimited part uses the repo-name class; empty parts + // and "." / ".." are rejected so slashes stay meaningful without enabling traversal. + private static bool IsValidBranch(string branch) { - try - { - var content = _fileSystem.File.ReadAllText(filePath); - var normalized = ReleaseNotesSerialization.NormalizeYaml(content); - var entry = EntryDeserializer.Deserialize(normalized); - if (entry?.Products == null) - return []; - - return entry.Products - .Select(p => p?.Product) - .Where(p => !string.IsNullOrWhiteSpace(p)) - .Select(p => p!) - .ToList(); - } - catch (Exception ex) + foreach (var part in branch.Split('/')) { - _logger.LogWarning(ex, "Could not read products from {File}", filePath); - return []; + if (part.Length == 0 || part is "." or ".." || !RepoNameRegex().IsMatch(part)) + return false; } + return true; } internal IReadOnlyList DiscoverBundleUploadTargets(IDiagnosticsCollector collector, string bundleDir) @@ -235,7 +271,7 @@ internal IReadOnlyList DiscoverBundleUploadTargets(IDiagnosticsCol continue; } - var s3Key = $"{product}/bundle/{fileName}"; + var s3Key = $"bundle/{product}/{fileName}"; targets.Add(new UploadTarget(filePath, s3Key)); } } diff --git a/src/services/Elastic.Changelog/Uploading/Registry.cs b/src/services/Elastic.Changelog/Uploading/Registry.cs index e597ed908..376810e1d 100644 --- a/src/services/Elastic.Changelog/Uploading/Registry.cs +++ b/src/services/Elastic.Changelog/Uploading/Registry.cs @@ -12,8 +12,9 @@ namespace Elastic.Changelog.Uploading; /// bundle files without an S3 listing call. /// /// -/// Stored at {product}/registry.json in the changelog bundles bucket. -/// The scrubber Lambda mirrors it verbatim to the public bucket (pass-through). +/// Stored at bundle/{product}/registry.json (bundle index) or +/// changelog/{org}/{repo}/{branch}/registry.json (changelog-entry index) in the changelog bundles +/// bucket. The scrubber Lambda mirrors it verbatim to the public bucket (pass-through). /// public sealed record Registry { @@ -23,7 +24,9 @@ public sealed record Registry public int SchemaVersion { get; init; } = 1; /// - /// Product identifier (matches the first segment of the S3 key). + /// Grouping identifier: the product for a bundle index (bundle/{product}/…) or the + /// {org}/{repo}/{branch} prefix for a changelog-entry index + /// (changelog/{org}/{repo}/{branch}/…). Informational only; consumers locate the manifest by key. /// public required string Product { get; init; } @@ -45,8 +48,9 @@ public sealed record Registry public sealed record RegistryBundle { /// - /// Bundle file name (e.g. 9.3.0.yaml or 2025-11.yaml), - /// resolved at {product}/bundle/{file}. + /// Bundle file name (e.g. 9.3.0.yaml or cloud-2025-11.yaml), + /// resolved at bundle/{product}/{file}. For changelog-entry indexes this is the entry + /// file name resolved at changelog/{org}/{repo}/{branch}/{file}. /// public required string File { get; init; } diff --git a/src/services/Elastic.Changelog/Uploading/RegistryBuilder.cs b/src/services/Elastic.Changelog/Uploading/RegistryBuilder.cs index d402c8615..30d3ae651 100644 --- a/src/services/Elastic.Changelog/Uploading/RegistryBuilder.cs +++ b/src/services/Elastic.Changelog/Uploading/RegistryBuilder.cs @@ -20,20 +20,21 @@ namespace Elastic.Changelog.Uploading; /// internal enum RegistryScope { - /// The bundle index at {product}/registry.json, listing scrubbed bundle files. + /// The bundle index at bundle/{product}/registry.json, listing scrubbed bundle files. Bundle, - /// The changelog-entry index at {product}/changelog/registry.json, listing individual entry files. + /// The changelog-entry index at changelog/{org}/{repo}/{branch}/registry.json, listing individual entry files. Changelog } /// -/// Refreshes a per-product registry.json manifest in the private bucket after an upload run. +/// Refreshes a registry.json manifest in the private bucket after an upload run. /// Depending on this is either the bundle index -/// ({product}/registry.json) or the changelog-entry index -/// ({product}/changelog/registry.json). Each product touched in the run gets its manifest -/// merged with what is already known on S3 (read back, merged by file name, written with an -/// optimistic concurrency guard so parallel uploads for the same product cannot clobber each other). +/// (bundle/{product}/registry.json) or the changelog-entry index +/// (changelog/{org}/{repo}/{branch}/registry.json). Each grouping (product, or org/repo/branch) +/// touched in the run gets its manifest merged with what is already known on S3 (read back, merged by +/// file name, written with an optimistic concurrency guard so parallel uploads for the same group cannot +/// clobber each other). /// internal sealed class RegistryBuilder( ILoggerFactory logFactory, @@ -70,10 +71,11 @@ public async Task RefreshAsync( Cancel ctx, RegistryScope scope = RegistryScope.Bundle) { - // Each upload target carries a "{product}/{bundle|changelog}/{file}" S3 key. Group by product - // so we can produce one manifest per affected product. + // Each upload target carries an artifact-root S3 key — "bundle/{product}/{file}" or + // "changelog/{org}/{repo}/{branch}/{file}". Group by the scope's key (product for bundles, the + // {org}/{repo}/{branch} prefix for entries) so we produce one manifest per affected group. var byProduct = uploadTargets - .Select(t => (Target: t, Product: ExtractProduct(t.S3Key))) + .Select(t => (Target: t, Product: ExtractGroupKey(t.S3Key, scope))) .Where(x => x.Product is not null) .GroupBy(x => x.Product!, StringComparer.Ordinal); @@ -110,20 +112,51 @@ public async Task RefreshAsync( return new RefreshResult(updated, unchanged, failed); } - /// Extracts the leading product segment from a {product}/bundle/{file} or {product}/changelog/{file} S3 key, or null. - private static string? ExtractProduct(string s3Key) + /// Extracts the grouping key (product for bundle/{product}/…, {org}/{repo}/{branch} for changelog/{org}/{repo}/{branch}/…) from an artifact-root S3 key, or null. + private static string? ExtractGroupKey(string s3Key, RegistryScope scope) { - var firstSlash = s3Key.IndexOf('/'); - if (firstSlash <= 0) + var prefix = scope == RegistryScope.Changelog ? "changelog/" : "bundle/"; + if (!s3Key.StartsWith(prefix, StringComparison.Ordinal)) return null; - return s3Key.AsSpan(0, firstSlash).ToString(); + + var rest = s3Key.AsSpan(prefix.Length); + + // Bundles group by the single product segment (bundle/{product}/{file}). Changelog entries group by + // the whole {org}/{repo}/{branch} prefix (changelog/{org}/{repo}/{branch...}/{file}), which can carry + // extra slashes when the branch itself contains them — so take everything before the final segment. + if (scope == RegistryScope.Bundle) + { + var slash = rest.IndexOf('/'); + return slash <= 0 ? null : rest[..slash].ToString(); + } + + var lastSlash = rest.LastIndexOf('/'); + if (lastSlash <= 0) + return null; + var group = rest[..lastSlash]; + // Require at least org/repo/branch (3 segments) ahead of the file name. + return CountSegments(group) >= 3 ? group.ToString() : null; + } + + /// Counts the non-empty-delimited segments in a /-joined path span. + private static int CountSegments(ReadOnlySpan path) + { + if (path.IsEmpty) + return 0; + var count = 1; + foreach (var c in path) + { + if (c == '/') + count++; + } + return count; } - /// The S3 key of the per-product manifest for the given . - private static string RegistryKeyFor(string product, RegistryScope scope) => scope switch + /// The S3 key of the manifest for the given and grouping segment. + private static string RegistryKeyFor(string group, RegistryScope scope) => scope switch { - RegistryScope.Changelog => $"{product}/changelog/registry.json", - _ => $"{product}/registry.json" + RegistryScope.Changelog => $"changelog/{group}/registry.json", + _ => $"bundle/{group}/registry.json" }; /// Builds manifest entries for this run's bundles, recording the per- target for the bundle index. diff --git a/src/services/Elastic.Changelog/Uploading/RegistryKey.cs b/src/services/Elastic.Changelog/Uploading/RegistryKey.cs index e25519b49..4b8e9d14d 100644 --- a/src/services/Elastic.Changelog/Uploading/RegistryKey.cs +++ b/src/services/Elastic.Changelog/Uploading/RegistryKey.cs @@ -5,49 +5,74 @@ namespace Elastic.Changelog.Uploading; /// -/// Helpers for validating S3 object keys related to the per-product +/// Helpers for validating S3 object keys related to the per-group /// manifests (the bundle index and the changelog-entry index). /// public static class RegistryKey { - private const string BundleSuffix = "/registry.json"; - private const string ChangelogSuffix = "/changelog/registry.json"; + private const string RegistrySuffix = "/registry.json"; + private const string BundlePrefix = "bundle/"; + private const string ChangelogPrefix = "changelog/"; /// - /// Returns true when is a per-product manifest of either form - /// {product}/registry.json (the bundle index) or - /// {product}/changelog/registry.json (the changelog-entry index), where - /// {product} matches the same character class enforced by - /// ChangelogUploadService.ProductNameRegex ([a-zA-Z0-9_-]+). + /// Returns true when is a manifest of either artifact-root form + /// bundle/{product}/registry.json (the bundle index, exactly one product segment) or + /// changelog/{org}/{repo}/{branch}/registry.json (the changelog-entry index, at least three + /// segments because the branch is stored verbatim and may itself contain /). The bundle product + /// segment matches the producer's product class ([a-zA-Z0-9_-]+); the changelog segments + /// additionally allow . (GitHub repo names and branches such as 8.x can contain dots). /// /// /// Used by the changelog scrubber Lambda to decide whether to pass an incoming - /// *.json object through to the public bucket. Anything else (e.g. nested - /// under a bundle/ prefix, or a multi-segment product) is rejected, which keeps - /// arbitrary JSON out of the public surface. + /// *.json object through to the public bucket. Anything else (a bare + /// {x}/registry.json, a changelog manifest shallower than org/repo/branch, or an unknown + /// top-level prefix) is rejected, which keeps arbitrary JSON out of the public surface. /// public static bool IsRegistry(string key) { if (string.IsNullOrEmpty(key)) return false; - if (key.EndsWith(ChangelogSuffix, StringComparison.Ordinal)) - return IsValidProduct(key.AsSpan(0, key.Length - ChangelogSuffix.Length)); + // Bundle index: exactly one product segment, no dots. Changelog-entry index: the + // {org}/{repo}/{branch} prefix — at least three segments, dots allowed (repo/branch may contain them). + return IsScopedRegistry(key, BundlePrefix, minSegments: 1, maxSegments: 1, allowDots: false) + || IsScopedRegistry(key, ChangelogPrefix, minSegments: 3, maxSegments: int.MaxValue, allowDots: true); + } + + private static bool IsScopedRegistry(string key, string prefix, int minSegments, int maxSegments, bool allowDots) + { + if (!key.StartsWith(prefix, StringComparison.Ordinal) || !key.EndsWith(RegistrySuffix, StringComparison.Ordinal)) + return false; + + var middleLength = key.Length - prefix.Length - RegistrySuffix.Length; + if (middleLength <= 0) + return false; + + var middle = key.AsSpan(prefix.Length, middleLength); + var segments = 0; + var start = 0; + for (var i = 0; i <= middle.Length; i++) + { + if (i != middle.Length && middle[i] != '/') + continue; - if (key.EndsWith(BundleSuffix, StringComparison.Ordinal)) - return IsValidProduct(key.AsSpan(0, key.Length - BundleSuffix.Length)); + segments++; + if (segments > maxSegments || !IsValidSegment(middle[start..i], allowDots)) + return false; + start = i + 1; + } - return false; + return segments >= minSegments; } - private static bool IsValidProduct(ReadOnlySpan product) + private static bool IsValidSegment(ReadOnlySpan segment, bool allowDots) { - if (product.IsEmpty || product.Contains('/')) + if (segment.IsEmpty || segment is "." or "..") return false; - foreach (var c in product) + foreach (var c in segment) { - if (!(char.IsAsciiLetterOrDigit(c) || c == '_' || c == '-')) + if (!(char.IsAsciiLetterOrDigit(c) || c == '_' || c == '-' || (allowDots && c == '.'))) return false; } return true; diff --git a/src/tooling/docs-builder/Commands/ChangelogCommand.cs b/src/tooling/docs-builder/Commands/ChangelogCommand.cs index f821c52fa..ff97b17ad 100644 --- a/src/tooling/docs-builder/Commands/ChangelogCommand.cs +++ b/src/tooling/docs-builder/Commands/ChangelogCommand.cs @@ -566,6 +566,7 @@ async static (s, collector, state, ctx) => await s.CreateChangelog(collector, st /// Output path for the bundled changelog (directory or .yml/.yaml file). Uses config bundle.output_directory or defaults to 'changelog-bundle.yaml' in the input directory. This option is not supported in profile-based commands. The equivalent configuration option is bundle.profiles.<name>.output. /// Explicitly set the products array in the output file in format "product target lifecycle, ...". This option is not supported in profile-based commands. The equivalent configuration option is bundle.profiles.<name>.output_products. /// GitHub repository owner for PR/issue numbers or --release-version. Falls back to bundle.owner or "elastic". This option is not supported in profile-based commands. The equivalent configuration options are bundle.owner or bundle.profiles.<name>.owner. + /// Branch whose CDN changelog entry pool (changelog/{org}/{repo}/{branch}/...) is sourced from. Falls back to bundle.branch or "main". This option is not supported in profile-based commands. The equivalent configuration options are bundle.branch or bundle.profiles.<name>.branch. /// Filter by pull request URLs (comma-separated), or a path to a newline-delimited file containing fully-qualified GitHub PR URLs. Can be specified multiple times. This option is not supported in profile-based commands. Pass a promotion report as the second or third positional argument instead, or set source: github_release on the profile. /// GitHub repository name for PR/issue numbers or --release-version. Falls back to bundle.repo or the product ID. This option is not supported in profile-based commands. The equivalent configuration options are bundle.repo or bundle.profiles.<name>.repo. /// URL or file path to a promotion report; extracts PR URLs as the filter. This option is not supported in profile-based commands. Pass the report as the second or third positional argument instead. @@ -590,6 +591,7 @@ public async Task Bundle( [ArgumentParser(typeof(ProductInfoParser))] ProductArgumentList? outputProducts = null, string[]? issues = null, string? owner = null, + string? branch = null, bool plan = false, string[]? prs = null, string? releaseVersion = null, @@ -901,6 +903,7 @@ public async Task Bundle( Issues = allIssues.Count > 0 ? allIssues.ToArray() : null, Owner = owner, Repo = repo, + Branch = branch, Profile = profile, ProfileArgument = profileArg, ProfileReport = isProfileMode ? profileReport : null, @@ -1585,12 +1588,25 @@ private string ApplyChangelogInitBundleRepoSeed(string content, string? ownerCli } /// Upload changelog entries or bundle artifacts to S3 or Elasticsearch. - /// Uses content-hash–based incremental transfer — only changed files are uploaded. + /// + /// Uses content-hash–based incremental transfer — only changed files are uploaded. + /// + /// Changelog entries are uploaded once under changelog/{org}/{repo}/{branch}/{file}, keyed by the + /// authoring owner (--owner > bundle.owner > git remote), repository (--repo + /// > bundle.repo > git remote), and branch (--branch > the current checkout's + /// branch). The branch is stored verbatim, so a branch's / become real key separators. Bundles + /// are uploaded under bundle/{product}/{file}, product-scoped from the bundle YAML, and do not + /// require an owner/repo/branch. + /// + /// /// Artifact type to upload: 'changelog' (individual entries) or 'bundle' (consolidated bundles). /// Upload destination: 's3' or 'elasticsearch'. /// S3 bucket name (required when target is 's3'). /// Path to changelog.yml configuration file. Defaults to docs/changelog.yml. /// Override changelog directory instead of reading it from config. + /// GitHub repository name, the second segment of changelog entry keys (changelog/{org}/{repo}/{branch}/...). Falls back to bundle.repo in changelog.yml, then the git remote origin. Required for changelog uploads; ignored for bundle uploads. + /// GitHub owner (org), the first segment of changelog entry keys (changelog/{org}/{repo}/{branch}/...). Falls back to bundle.owner in changelog.yml, then the git remote origin. Required for changelog uploads; ignored for bundle uploads. + /// Branch, the third segment of changelog entry keys (changelog/{org}/{repo}/{branch}/...), stored verbatim. Falls back to the current checkout's branch. Required for changelog uploads; ignored for bundle uploads. [NoOptionsInjection] public async Task Upload( string artifactType, @@ -1598,6 +1614,9 @@ public async Task Upload( string s3BucketName = "", [Existing, ExpandUserProfile, RejectSymbolicLinks, FileExtensions(Extensions = "yml,yaml")] FileInfo? config = null, [ExpandUserProfile, RejectSymbolicLinks] DirectoryInfo? directory = null, + string? repo = null, + string? owner = null, + string? branch = null, CancellationToken ct = default ) { @@ -1623,6 +1642,11 @@ public async Task Upload( var resolvedDirectory = directory != null ? directory?.FullName : null; var resolvedConfig = config != null ? config?.FullName : null; + // Resolve the authoring owner/repo/branch for entry keys: CLI flags > bundle.{owner,repo} + // (changelog.yml) > git. The repo is reduced to a single path segment (owner/repo -> repo) for the + // changelog/{org}/{repo}/{branch}/ key. + var (resolvedRepo, resolvedOwner, resolvedBranch) = await ResolveUploadRepoOwnerBranch(repo, owner, branch, resolvedConfig, resolvedDirectory, ctx); + await using var serviceInvoker = new ServiceInvoker(collector); var service = new ChangelogUploadService(logFactory, configurationContext); var args = new ChangelogUploadArguments @@ -1631,7 +1655,10 @@ public async Task Upload( Target = parsedTarget, S3BucketName = s3BucketName, Config = resolvedConfig, - Directory = resolvedDirectory + Directory = resolvedDirectory, + Repo = resolvedRepo, + Owner = resolvedOwner, + Branch = resolvedBranch }; serviceInvoker.AddCommand(service, args, static async (s, c, state, ct) => await s.Upload(c, state, ct) @@ -1639,6 +1666,54 @@ static async (s, c, state, ct) => await s.Upload(c, state, ct) return await serviceInvoker.InvokeAsync(ctx); } + /// Resolves the authoring repo/owner/branch for uploads (CLI flags > bundle.{repo,owner} > git), reducing the repo to a single path segment. + private async Task<(string? Repo, string? Owner, string? Branch)> ResolveUploadRepoOwnerBranch(string? repoCli, string? ownerCli, string? branchCli, string? configPath, string? uploadDirectory, CancellationToken ctx) + { + var bundleConfig = await new ChangelogConfigurationLoader(logFactory, configurationContext, _fileSystem) + .LoadChangelogConfiguration(collector, configPath, ctx); + + // Anchor the git fallbacks to the upload source (config file or changelog directory), not the + // process cwd, so an out-of-tree --config/--directory resolves the right origin and branch. Both + // values are absolute (FileInfo/DirectoryInfo FullName) when present. + string? anchor = null; + if (!string.IsNullOrWhiteSpace(configPath)) + { + var configDir = _fileSystem.Path.GetDirectoryName(configPath); + if (!string.IsNullOrWhiteSpace(configDir) && _fileSystem.Directory.Exists(configDir)) + anchor = configDir; + } + if (anchor is null && !string.IsNullOrWhiteSpace(uploadDirectory) && _fileSystem.Directory.Exists(uploadDirectory)) + anchor = uploadDirectory; + anchor ??= Directory.GetCurrentDirectory(); + + string? gitOwner = null; + string? gitRepo = null; + var repoRoot = Paths.FindGitRoot(_fileSystem.DirectoryInfo.New(anchor))?.FullName ?? anchor; + if (GitRemoteConfigurationReader.TryReadOriginUrl(_fileSystem, repoRoot, out var originUrl)) + _ = GitHubRemoteParser.TryParseGitHubComOwnerRepo(originUrl, out gitOwner, out gitRepo); + + var resolvedRepo = !string.IsNullOrWhiteSpace(repoCli) ? repoCli : (bundleConfig?.Bundle?.Repo ?? gitRepo); + var resolvedOwner = ownerCli ?? bundleConfig?.Bundle?.Owner ?? gitOwner; + + // The producer branch is the branch being published: --branch, else the current checkout's branch. + // bundle.branch is intentionally not consulted here — it selects which pool to read when bundling. + var resolvedBranch = branchCli; + if (string.IsNullOrWhiteSpace(resolvedBranch)) + { + var checkout = GitCheckoutInformationFactory.Create(_fileSystem.DirectoryInfo.New(anchor), _fileSystem); + resolvedBranch = checkout.Branch; + } + + if (!string.IsNullOrWhiteSpace(resolvedRepo)) + { + var slash = resolvedRepo.LastIndexOf('/'); + if (slash >= 0 && slash < resolvedRepo.Length - 1) + resolvedRepo = resolvedRepo[(slash + 1)..]; + } + + return (resolvedRepo, resolvedOwner, resolvedBranch); + } + /// /// Normalizes a file path by expanding tilde (~) to the user's home directory /// and converting relative paths to absolute paths. diff --git a/tests/Elastic.Changelog.Tests/Changelogs/BundleCdnSourcingTests.cs b/tests/Elastic.Changelog.Tests/Changelogs/BundleCdnSourcingTests.cs index 090053b1a..ce48121a9 100644 --- a/tests/Elastic.Changelog.Tests/Changelogs/BundleCdnSourcingTests.cs +++ b/tests/Elastic.Changelog.Tests/Changelogs/BundleCdnSourcingTests.cs @@ -50,7 +50,7 @@ public class BundleCdnSourcingTests(ITestOutputHelper output) : ChangelogTestBas private static StubHandler RegistryHandler() => new(req => { var path = req.RequestUri!.AbsolutePath; - if (path.EndsWith("/changelog/registry.json", StringComparison.Ordinal)) + if (path.EndsWith("/registry.json", StringComparison.Ordinal)) return Json(RegistryJson); if (path.EndsWith("1-alpha.yaml", StringComparison.Ordinal)) return Yaml(EntryAlpha); @@ -69,16 +69,19 @@ private string OutputPath() => FileSystem.Path.Join(Paths.WorkingDirectoryRoot.FullName, Guid.NewGuid().ToString(), "bundle.yaml"); [Fact] - public async Task OptionMode_ProductFilter_SourcesAllMatchingEntriesFromCdn() + public async Task OptionMode_RepoResolvable_SourcesAllEntriesFromRepoPoolOnCdn() { - DeclareReleaseNotesProducts("elasticsearch"); - var service = new ChangelogBundlingService(LoggerFactory, null, FileSystem, null, Fetcher()); + // Under the artifact-root layout the CDN entry pool is keyed by the authoring repo, not the + // target product. A resolvable repo (here via --repo) is what enables CDN sourcing. + var handler = RegistryHandler(); + var service = new ChangelogBundlingService(LoggerFactory, null, FileSystem, null, Fetcher(Output, handler)); var output = OutputPath(); var input = new BundleChangelogsArguments { InputProducts = [new ProductArgument { Product = "elasticsearch", Target = "*", Lifecycle = "*" }], Output = output, + Repo = "elasticsearch", Resolve = true }; @@ -87,6 +90,9 @@ public async Task OptionMode_ProductFilter_SourcesAllMatchingEntriesFromCdn() result.Should().BeTrue($"Errors: {string.Join("; ", Collector.Diagnostics.Where(d => d.Severity == Severity.Error).Select(d => d.Message))}"); Collector.Errors.Should().Be(0); + // Entries are sourced from the authoring pool, with org/branch defaulting: changelog/{org}/{repo}/{branch}/... + handler.RequestedPaths.Should().Contain("/changelog/elastic/elasticsearch/main/registry.json"); + var bundle = await FileSystem.File.ReadAllTextAsync(output, TestContext.Current.CancellationToken); bundle.Should().Contain("Alpha"); bundle.Should().Contain("Bravo"); @@ -94,11 +100,60 @@ public async Task OptionMode_ProductFilter_SourcesAllMatchingEntriesFromCdn() } [Fact] - public async Task OptionMode_NoResolvableProduct_FallsBackToLocal() + public async Task OptionMode_OwnerAndBranchOverride_SourcesFromThatPoolOnCdn() + { + // Explicit owner/branch select a specific pool; the branch is stored verbatim (dots kept). + var handler = RegistryHandler(); + var service = new ChangelogBundlingService(LoggerFactory, null, FileSystem, null, Fetcher(Output, handler)); + var output = OutputPath(); + + var input = new BundleChangelogsArguments + { + InputProducts = [new ProductArgument { Product = "elasticsearch", Target = "*", Lifecycle = "*" }], + Output = output, + Owner = "acme-corp", + Repo = "elasticsearch", + Branch = "8.x", + Resolve = true + }; + + var result = await service.BundleChangelogs(Collector, input, TestContext.Current.CancellationToken); + + result.Should().BeTrue($"Errors: {string.Join("; ", Collector.Diagnostics.Where(d => d.Severity == Severity.Error).Select(d => d.Message))}"); + Collector.Errors.Should().Be(0); + handler.RequestedPaths.Should().Contain("/changelog/acme-corp/elasticsearch/8.x/registry.json"); + } + + [Fact] + public async Task OptionMode_OwnerFromCombinedRepo_SourcesFromThatPool() + { + // When --repo is given in owner/repo form and no explicit owner is set, the owner segment must be + // taken from the repo prefix (not defaulted to elastic), so the CDN pool path stays correct. + var handler = RegistryHandler(); + var service = new ChangelogBundlingService(LoggerFactory, null, FileSystem, null, Fetcher(Output, handler)); + var output = OutputPath(); + + var input = new BundleChangelogsArguments + { + InputProducts = [new ProductArgument { Product = "elasticsearch", Target = "*", Lifecycle = "*" }], + Output = output, + Repo = "acme-corp/widget", + Resolve = true + }; + + var result = await service.BundleChangelogs(Collector, input, TestContext.Current.CancellationToken); + + result.Should().BeTrue($"Errors: {string.Join("; ", Collector.Diagnostics.Where(d => d.Severity == Severity.Error).Select(d => d.Message))}"); + Collector.Errors.Should().Be(0); + handler.RequestedPaths.Should().Contain("/changelog/acme-corp/widget/main/registry.json"); + } + + [Fact] + public async Task OptionMode_NoResolvableRepo_FallsBackToLocal() { - // An option-mode --all filter resolves no product to scope the CDN fetch. With no --directory and - // no use_local_changelogs, the bundler should fall back to local folder sourcing rather than hit - // the CDN — preserving the pre-CDN behaviour for PR/issue-only and --all flows. + // With no --repo, no bundle.repo in config, and no git-remote resolution at the service layer, the + // authoring repo is unresolvable. With no --directory and no use_local_changelogs, the bundler + // still falls back to local folder sourcing rather than hitting the CDN. var localDir = FileSystem.Path.Join(Paths.WorkingDirectoryRoot.FullName, Guid.NewGuid().ToString(), "changelog"); FileSystem.Directory.CreateDirectory(localDir); await FileSystem.File.WriteAllTextAsync( @@ -129,11 +184,10 @@ await FileSystem.File.WriteAllTextAsync( } [Fact] - public async Task OptionMode_UndeclaredProduct_FallsBackToLocal() + public async Task OptionMode_UseLocalChangelogs_ForcesLocalEvenWithResolvableRepo() { - // The declared-gate: a product not listed under docset.yml release_notes is never sourced from the - // CDN, even with a resolvable product filter and no --directory. The bundler falls back to local - // folder sourcing instead. Here elasticsearch is intentionally NOT declared. + // use_local_changelogs is the explicit opt-out: even when the authoring repo resolves (bundle.repo), + // entries are read from the local folder and the CDN is never touched. var localDir = FileSystem.Path.Join(Paths.WorkingDirectoryRoot.FullName, Guid.NewGuid().ToString(), "changelog"); FileSystem.Directory.CreateDirectory(localDir); await FileSystem.File.WriteAllTextAsync( @@ -143,6 +197,8 @@ await FileSystem.File.WriteAllTextAsync( $""" bundle: directory: {localDir} + repo: elasticsearch + use_local_changelogs: true """; var configPath = FileSystem.Path.Join(Paths.WorkingDirectoryRoot.FullName, Guid.NewGuid().ToString(), "changelog.yml"); FileSystem.Directory.CreateDirectory(FileSystem.Path.GetDirectoryName(configPath)!); @@ -163,7 +219,7 @@ await FileSystem.File.WriteAllTextAsync( var result = await service.BundleChangelogs(Collector, input, TestContext.Current.CancellationToken); result.Should().BeTrue($"Errors: {string.Join("; ", Collector.Diagnostics.Where(d => d.Severity == Severity.Error).Select(d => d.Message))}"); - handler.RequestedPaths.Should().BeEmpty("an undeclared product must not reach the CDN"); + handler.RequestedPaths.Should().BeEmpty("use_local_changelogs must not reach the CDN"); var bundle = await FileSystem.File.ReadAllTextAsync(output, TestContext.Current.CancellationToken); bundle.Should().Contain("name: 1-local.yaml"); @@ -172,7 +228,6 @@ await FileSystem.File.WriteAllTextAsync( [Fact] public async Task RegistryFailure_FailsBundle() { - DeclareReleaseNotesProducts("elasticsearch"); var fetcher = new CdnChangelogEntryFetcher(new TestLoggerFactory(Output), new StubHandler(_ => new HttpResponseMessage(HttpStatusCode.NotFound)), sleep: (_, _) => Task.CompletedTask); var service = new ChangelogBundlingService(LoggerFactory, null, FileSystem, null, fetcher); @@ -181,6 +236,7 @@ public async Task RegistryFailure_FailsBundle() { InputProducts = [new ProductArgument { Product = "elasticsearch", Target = "*", Lifecycle = "*" }], Output = OutputPath(), + Repo = "elasticsearch", Resolve = true }; @@ -198,20 +254,20 @@ public async Task EntryMissingAfterRetries_FailsBundle() var handler = new StubHandler(req => { var path = req.RequestUri!.AbsolutePath; - if (path.EndsWith("/changelog/registry.json", StringComparison.Ordinal)) + if (path.EndsWith("/registry.json", StringComparison.Ordinal)) return Json(RegistryJson); if (path.EndsWith("1-alpha.yaml", StringComparison.Ordinal)) return Yaml(EntryAlpha); return new HttpResponseMessage(HttpStatusCode.NotFound); // 2-bravo.yaml never propagates }); var fetcher = new CdnChangelogEntryFetcher(new TestLoggerFactory(Output), handler, maxAttempts: 2, sleep: (_, _) => Task.CompletedTask); - DeclareReleaseNotesProducts("elasticsearch"); var service = new ChangelogBundlingService(LoggerFactory, null, FileSystem, null, fetcher); var input = new BundleChangelogsArguments { InputProducts = [new ProductArgument { Product = "elasticsearch", Target = "*", Lifecycle = "*" }], Output = OutputPath(), + Repo = "elasticsearch", Resolve = true }; @@ -224,9 +280,8 @@ public async Task EntryMissingAfterRetries_FailsBundle() [Fact] public async Task ProfileGitHubRelease_ScopesByOutputProductsAndFiltersByReleasePrs() { - // A github_release profile resolves the product from output_products (to scope the CDN fetch) - // and the PR filter from the release body. Only the entry referenced by the release survives. - DeclareReleaseNotesProducts("elasticsearch"); + // A github_release profile resolves the authoring repo from the profile (to scope the CDN entry + // pool) and the PR filter from the release body. Only the entry referenced by the release survives. var releaseService = A.Fake(); var outputDir = FileSystem.Path.Join(Paths.WorkingDirectoryRoot.FullName, Guid.NewGuid().ToString()); FileSystem.Directory.CreateDirectory(outputDir); diff --git a/tests/Elastic.Changelog.Tests/Changelogs/BundlePlanTests.cs b/tests/Elastic.Changelog.Tests/Changelogs/BundlePlanTests.cs index ff3b56625..75070688a 100644 --- a/tests/Elastic.Changelog.Tests/Changelogs/BundlePlanTests.cs +++ b/tests/Elastic.Changelog.Tests/Changelogs/BundlePlanTests.cs @@ -53,16 +53,16 @@ public async Task Plan_ReleaseVersion_ReturnsNeedsNetwork() } [Fact] - public async Task Plan_ProfileMode_ResolvesOutputPath() + public async Task Plan_ProfileMode_RepoResolvable_ReturnsNeedsNetwork() { - // A profile whose product is declared under release_notes sources entries from the CDN, so the plan + // A profile whose authoring repo resolves (bundle.repo) sources entries from the CDN, so the plan // reports needs_network (but not a GitHub token, since this is not a github_release profile). - DeclareReleaseNotesProducts("elasticsearch"); // language=yaml var configContent = """ bundle: output_directory: docs/releases + repo: elasticsearch profiles: my-profile: products: "elasticsearch {version} {lifecycle}" @@ -83,16 +83,16 @@ public async Task Plan_ProfileMode_ResolvesOutputPath() result.NeedsNetwork.Should().BeTrue(); result.NeedsGithubToken.Should().BeFalse(); result.OutputPath.Should().EndWith(FileSystem.Path.Join("docs", "releases", "elasticsearch-9.2.0.yaml").OptionalWindowsReplace()); - // The bundle-PR action polls this URL for the scrubbed copy: {base}/{product}/bundle/{file}. - result.CdnUrl.Should().Be("https://d10xozp44eyz7q.cloudfront.net/elasticsearch/bundle/elasticsearch-9.2.0.yaml"); + // The bundle-PR action polls this URL for the scrubbed copy: {base}/bundle/{product}/{file}. + result.CdnUrl.Should().Be("https://d10xozp44eyz7q.cloudfront.net/bundle/elasticsearch/elasticsearch-9.2.0.yaml"); } [Fact] - public async Task Plan_ProfileMode_UndeclaredProduct_ReturnsNoNetwork() + public async Task Plan_ProfileMode_NoRepo_ReturnsNoNetwork() { - // Declared-gate at plan time: with the product absent from docset.yml release_notes, the bundle - // run will source locally, so the plan reports no network. The CDN URL is still resolved because - // it only describes where a (possibly local) bundle would be published, independent of the gate. + // Repo gate at plan time: with no bundle.repo (and no --repo), the authoring repo is unresolvable, + // so the bundle run sources locally and the plan reports no network. The CDN URL is still resolved + // because it only describes where a (possibly local) bundle would be published, independent of the gate. // language=yaml var configContent = """ @@ -116,7 +116,7 @@ public async Task Plan_ProfileMode_UndeclaredProduct_ReturnsNoNetwork() result.Should().NotBeNull(); result.NeedsNetwork.Should().BeFalse(); - result.CdnUrl.Should().Be("https://d10xozp44eyz7q.cloudfront.net/elasticsearch/bundle/elasticsearch-9.2.0.yaml"); + result.CdnUrl.Should().Be("https://d10xozp44eyz7q.cloudfront.net/bundle/elasticsearch/elasticsearch-9.2.0.yaml"); } [Fact] @@ -146,7 +146,7 @@ public async Task Plan_ProfileMode_OutputProductsScopeCdnUrl() var result = await Service.PlanBundleAsync(Collector, input, hasReleaseVersion: false, TestContext.Current.CancellationToken); result.Should().NotBeNull(); - result.CdnUrl.Should().Be("https://d10xozp44eyz7q.cloudfront.net/cloud-serverless/bundle/serverless-2026-03.yaml"); + result.CdnUrl.Should().Be("https://d10xozp44eyz7q.cloudfront.net/bundle/cloud-serverless/serverless-2026-03.yaml"); } [Fact] diff --git a/tests/Elastic.Changelog.Tests/Changelogs/CloudProfileFixtureTests.cs b/tests/Elastic.Changelog.Tests/Changelogs/CloudProfileFixtureTests.cs new file mode 100644 index 000000000..93997c9bf --- /dev/null +++ b/tests/Elastic.Changelog.Tests/Changelogs/CloudProfileFixtureTests.cs @@ -0,0 +1,197 @@ +// Licensed to Elasticsearch B.V under one or more agreements. +// Elasticsearch B.V licenses this file to you under the Apache 2.0 License. +// See the LICENSE file in the project root for more information + +using System.Net; +using AwesomeAssertions; +using Elastic.Changelog.Bundling; +using Elastic.Documentation.Configuration; +using Elastic.Documentation.Configuration.ReleaseNotes; +using Elastic.Documentation.Diagnostics; + +namespace Elastic.Changelog.Tests.Changelogs; + +/// +/// End-to-end fixture modelled on the non-trivial elastic/cloud changelog configuration +/// (multi-product, a monthly profile, bundle.repo with repo != product, link_allow_repos, +/// resolve: true, release_dates: false, and rules.bundle.exclude_types), but using +/// anonymized repo and product names only — never the real org/repo/product IDs. +/// +/// It verifies the artifact-root (Option AD) behaviour: entries are sourced once from the authoring +/// pool (changelog/{org}/{repo}/{branch}/...), and the profile bundle that results is sound — resolved, +/// link-scrubbed against the allowlist, with the docs entry excluded and no auto-populated release date. +/// +/// The authoring repo is an anonymized test name (widget) that deliberately differs from the +/// product IDs it publishes for — exactly the repo != product "moniker mismatch" that motivates Option +/// AD. Product IDs are drawn from the test harness's products.yml allowlist. +/// +public class CloudProfileFixtureTests(ITestOutputHelper output) : ChangelogTestBase(output) +{ + private const string AuthoringRepo = "widget"; + + // A feature entry whose only PR points at an allowlisted public repo (kept on scrub) plus a PR to a + // non-allowlisted repo (scrubbed away). + // language=yaml + private const string FeatureEntry = """ + title: Faster hosted search + type: feature + products: + - product: cloud-hosted + target: 2026-05-15 + lifecycle: ga + prs: + - https://github.com/elastic/elasticsearch/pull/100 + - https://github.com/elastic/widget-internal/pull/7 + """; + + // A docs entry that matches the product/target filter but must be dropped by rules.bundle.exclude_types. + // language=yaml + private const string DocsEntry = """ + title: Tidy up the hosted docs + type: docs + products: + - product: cloud-hosted + target: 2026-05-20 + lifecycle: ga + prs: + - https://github.com/elastic/elasticsearch/pull/200 + """; + + // A feature for a different product; excluded by the profile's cloud-hosted product filter. + // language=yaml + private const string OtherProductEntry = """ + title: Serverless-only change + type: feature + products: + - product: cloud-serverless + target: 2026-05-10 + lifecycle: ga + prs: + - https://github.com/elastic/kibana/pull/300 + """; + + // language=json + private const string RegistryJson = + """{ "schema_version": 1, "product": "widget", "bundles": [ { "file": "1-feature.yaml" }, { "file": "2-docs.yaml" }, { "file": "3-other.yaml" } ] }"""; + + private StubHandler RepoPoolHandler() => new(req => + { + var path = req.RequestUri!.AbsolutePath; + if (path.EndsWith("/registry.json", StringComparison.Ordinal)) + return Json(RegistryJson); + if (path.EndsWith("1-feature.yaml", StringComparison.Ordinal)) + return Yaml(FeatureEntry); + if (path.EndsWith("2-docs.yaml", StringComparison.Ordinal)) + return Yaml(DocsEntry); + if (path.EndsWith("3-other.yaml", StringComparison.Ordinal)) + return Yaml(OtherProductEntry); + return new HttpResponseMessage(HttpStatusCode.NotFound); + }); + + private CdnChangelogEntryFetcher Fetcher(StubHandler handler) => + new(new TestLoggerFactory(Output), handler, sleep: (_, _) => Task.CompletedTask); + + [Fact] + public async Task MonthlyProfile_SourcesFromRepoPool_ProducesSoundBundle() + { + var outputDir = FileSystem.Path.Join(Paths.WorkingDirectoryRoot.FullName, Guid.NewGuid().ToString()); + FileSystem.Directory.CreateDirectory(outputDir); + + // language=yaml + var configContent = + """ + products: + available: + - cloud-hosted + - cloud-serverless + pivot: + types: + feature: ">feature" + bug-fix: ">bug" + breaking-change: ">breaking" + docs: ">docs" + rules: + bundle: + exclude_types: "docs" + bundle: + output_directory: PLACEHOLDER + resolve: true + repo: widget + owner: elastic + release_dates: false + link_allow_repos: + - elastic/elasticsearch + - elastic/kibana + profiles: + wh-monthly: + products: "cloud-hosted {version}-* *" + output: "widget-{version}.yaml" + output_products: "cloud-hosted {version}" + """.Replace("PLACEHOLDER", outputDir); + + var configPath = FileSystem.Path.Join(Paths.WorkingDirectoryRoot.FullName, Guid.NewGuid().ToString(), "changelog.yml"); + FileSystem.Directory.CreateDirectory(FileSystem.Path.GetDirectoryName(configPath)!); + await FileSystem.File.WriteAllTextAsync(configPath, configContent, TestContext.Current.CancellationToken); + + var handler = RepoPoolHandler(); + var service = new ChangelogBundlingService(LoggerFactory, ConfigurationContext, FileSystem, null, Fetcher(handler)); + + var input = new BundleChangelogsArguments + { + Profile = "wh-monthly", + ProfileArgument = "2026-05", + Config = configPath + }; + + var result = await service.BundleChangelogs(Collector, input, TestContext.Current.CancellationToken); + + result.Should().BeTrue($"Errors: {string.Join("; ", Collector.Diagnostics.Where(d => d.Severity == Severity.Error).Select(d => d.Message))}"); + Collector.Errors.Should().Be(0); + + // Entries are sourced once from the authoring pool (changelog/{org}/{repo}/{branch}/...), not from + // any product-scoped path. Owner comes from bundle.owner; branch defaults to "main". + handler.RequestedPaths.Should().Contain($"/changelog/elastic/{AuthoringRepo}/main/registry.json"); + handler.RequestedPaths.Should().NotContain(p => p.Contains("/cloud-hosted/changelog/", StringComparison.Ordinal)); + + var outputFiles = FileSystem.Directory.GetFiles(outputDir, "*.yaml"); + outputFiles.Should().ContainSingle("the monthly profile writes a single bundle file"); + FileSystem.Path.GetFileName(outputFiles[0]).Should().Be("widget-2026-05.yaml"); + + var bundle = await FileSystem.File.ReadAllTextAsync(outputFiles[0], TestContext.Current.CancellationToken); + + // Sound bundle: the matching feature is present and resolved; the docs entry and the other-product + // entry are excluded. + bundle.Should().Contain("Faster hosted search"); + bundle.Should().NotContain("Tidy up the hosted docs", "rules.bundle.exclude_types drops docs entries"); + bundle.Should().NotContain("Serverless-only change", "the cloud-hosted product filter excludes other products"); + + // Link allowlist: the allowlisted public PR is kept verbatim; the non-allowlisted repo reference is + // rewritten to a "# PRIVATE:" sentinel rather than left as a live link. + bundle.Should().Contain("- https://github.com/elastic/elasticsearch/pull/100"); + bundle.Should().Contain("# PRIVATE: https://github.com/elastic/widget-internal/pull/7", + "non-allowlisted PR links must be scrubbed to a PRIVATE sentinel in bundle output"); + + // release_dates: false → no auto-populated release date. + bundle.Should().NotContain("release_date"); + } + + private static HttpResponseMessage Json(string body) => + new(HttpStatusCode.OK) { Content = new StringContent(body, System.Text.Encoding.UTF8, "application/json") }; + + private static HttpResponseMessage Yaml(string body) => + new(HttpStatusCode.OK) { Content = new StringContent(body, System.Text.Encoding.UTF8, "text/yaml") }; + + private sealed class StubHandler(Func responder) : HttpMessageHandler + { + public List RequestedPaths { get; } = []; + + protected override HttpResponseMessage Send(HttpRequestMessage request, CancellationToken cancellationToken) + { + RequestedPaths.Add(request.RequestUri!.AbsolutePath); + return responder(request); + } + + protected override Task SendAsync(HttpRequestMessage request, CancellationToken cancellationToken) => + Task.FromResult(Send(request, cancellationToken)); + } +} diff --git a/tests/Elastic.Changelog.Tests/Changelogs/LinkAllowlistSanitizerTests.cs b/tests/Elastic.Changelog.Tests/Changelogs/LinkAllowlistSanitizerTests.cs index d93d7e5e0..929f697e2 100644 --- a/tests/Elastic.Changelog.Tests/Changelogs/LinkAllowlistSanitizerTests.cs +++ b/tests/Elastic.Changelog.Tests/Changelogs/LinkAllowlistSanitizerTests.cs @@ -534,7 +534,7 @@ public void TryApplyChangelogEntry_NullFields_PreservesNulls() public void TryApplyChangelogEntry_BarePrNumberWithoutDefaultRepo_KeptWithWarning() { // The scrubber Lambda calls TryApplyChangelogEntry with defaultRepo=null because per-entry - // YAMLs uploaded under {product}/changelog/*.yaml carry no embedded repo context. A bare + // YAMLs uploaded under changelog/{org}/{repo}/{branch}/*.yaml carry no embedded repo context. A bare // numeric PR ref ("155500") must be tolerated rather than failing the whole entry — the // reference carries no repo identity so it cannot leak a private link, and downstream // rendering supplies the owner/repo from runtime context. diff --git a/tests/Elastic.Changelog.Tests/Uploading/ChangelogUploadServiceTests.cs b/tests/Elastic.Changelog.Tests/Uploading/ChangelogUploadServiceTests.cs index 8887ef24e..30424e9c7 100644 --- a/tests/Elastic.Changelog.Tests/Uploading/ChangelogUploadServiceTests.cs +++ b/tests/Elastic.Changelog.Tests/Uploading/ChangelogUploadServiceTests.cs @@ -48,7 +48,7 @@ private string AddChangelog(string fileName, string yaml) } [Fact] - public void DiscoverUploadTargets_SingleProduct_MapsToCorrectS3Key() + public void DiscoverUploadTargets_SingleEntry_MapsToPoolScopedKey() { // language=yaml var path = AddChangelog("entry.yaml", """ @@ -61,17 +61,61 @@ public void DiscoverUploadTargets_SingleProduct_MapsToCorrectS3Key() - "100" """); - var targets = _service.DiscoverUploadTargets(_collector, _changelogDir); + var targets = _service.DiscoverUploadTargets(_collector, _changelogDir, "elastic", "elasticsearch", "main"); targets.Should().HaveCount(1); targets[0].LocalPath.Should().Be(path); - targets[0].S3Key.Should().Be("elasticsearch/changelog/entry.yaml"); + targets[0].S3Key.Should().Be("changelog/elastic/elasticsearch/main/entry.yaml"); + _collector.Errors.Should().Be(0); + } + + [Theory] + // Branch stored verbatim: dots (release trains) and slashes (feature branches) become real key segments. + [InlineData("8.x", "changelog/elastic/elasticsearch/8.x/entry.yaml")] + [InlineData("9.0", "changelog/elastic/elasticsearch/9.0/entry.yaml")] + [InlineData("feature/foo", "changelog/elastic/elasticsearch/feature/foo/entry.yaml")] + [InlineData("release/8.x", "changelog/elastic/elasticsearch/release/8.x/entry.yaml")] + public void DiscoverUploadTargets_BranchWithDotsOrSlashes_MapsVerbatim(string branch, string expectedKey) + { + // language=yaml + AddChangelog("entry.yaml", """ + title: New feature + type: feature + products: + - product: elasticsearch + """); + + var targets = _service.DiscoverUploadTargets(_collector, _changelogDir, "elastic", "elasticsearch", branch); + + targets.Should().ContainSingle(); + targets[0].S3Key.Should().Be(expectedKey); _collector.Errors.Should().Be(0); } [Fact] - public void DiscoverUploadTargets_MultipleProducts_CreatesTargetPerProduct() + public void DiscoverUploadTargets_ExternalOrg_MapsToPoolScopedKey() { + // An acquired company keeping its own GitHub org still gets a faithful pool. + // language=yaml + AddChangelog("entry.yaml", """ + title: New feature + type: feature + products: + - product: widgets + """); + + var targets = _service.DiscoverUploadTargets(_collector, _changelogDir, "acme-corp", "widgets", "main"); + + targets.Should().ContainSingle(); + targets[0].S3Key.Should().Be("changelog/acme-corp/widgets/main/entry.yaml"); + _collector.Errors.Should().Be(0); + } + + [Fact] + public void DiscoverUploadTargets_EntryWithMultipleProducts_StillSingleRepoKey() + { + // Option AD: entries are stored once per authoring repo, regardless of how many products they + // list (or will later be consumed by). No per-product fan-out. // language=yaml AddChangelog("fix.yaml", """ title: Cross-product fix @@ -85,82 +129,158 @@ public void DiscoverUploadTargets_MultipleProducts_CreatesTargetPerProduct() - "200" """); - var targets = _service.DiscoverUploadTargets(_collector, _changelogDir); + var targets = _service.DiscoverUploadTargets(_collector, _changelogDir, "elastic", "kibana", "main"); - targets.Should().HaveCount(2); - targets.Should().Contain(t => t.S3Key == "elasticsearch/changelog/fix.yaml"); - targets.Should().Contain(t => t.S3Key == "kibana/changelog/fix.yaml"); + targets.Should().ContainSingle(); + targets[0].S3Key.Should().Be("changelog/elastic/kibana/main/fix.yaml"); } [Fact] - public void DiscoverUploadTargets_InvalidProductName_SkipsWithWarning() + public void DiscoverUploadTargets_EntryWithNoProducts_StillUploaded() { + // Author foreknowledge of consuming products is no longer required: an entry with no products is + // still uploaded under the repo pool. // language=yaml - AddChangelog("bad.yaml", """ - title: Bad product + AddChangelog("noproducts.yaml", """ + title: No products type: feature - products: - - product: "../traversal" prs: - - "300" + - "400" """); - var targets = _service.DiscoverUploadTargets(_collector, _changelogDir); + var targets = _service.DiscoverUploadTargets(_collector, _changelogDir, "elastic", "elasticsearch", "main"); - targets.Should().BeEmpty(); - _collector.Warnings.Should().BeGreaterThan(0); + targets.Should().ContainSingle(); + targets[0].S3Key.Should().Be("changelog/elastic/elasticsearch/main/noproducts.yaml"); + _collector.Errors.Should().Be(0); } [Fact] - public void DiscoverUploadTargets_NoProducts_ReturnsEmpty() + public void DiscoverUploadTargets_MissingRepo_EmitsErrorAndReturnsEmpty() { // language=yaml - AddChangelog("noproducts.yaml", """ - title: No products + AddChangelog("entry.yaml", """ + title: New feature type: feature - prs: - - "400" + products: + - product: elasticsearch """); - var targets = _service.DiscoverUploadTargets(_collector, _changelogDir); + var targets = _service.DiscoverUploadTargets(_collector, _changelogDir, "elastic", repo: null, "main"); targets.Should().BeEmpty(); - _collector.Errors.Should().Be(0); + _collector.Errors.Should().BeGreaterThan(0); } [Fact] - public void DiscoverUploadTargets_EmptyDirectory_ReturnsEmpty() + public void DiscoverUploadTargets_MissingOwner_EmitsErrorAndReturnsEmpty() { - var targets = _service.DiscoverUploadTargets(_collector, _changelogDir); + // language=yaml + AddChangelog("entry.yaml", """ + title: New feature + type: feature + products: + - product: elasticsearch + """); + + var targets = _service.DiscoverUploadTargets(_collector, _changelogDir, org: null, "elasticsearch", "main"); targets.Should().BeEmpty(); + _collector.Errors.Should().BeGreaterThan(0); } [Fact] - public void DiscoverUploadTargets_MixedValidAndInvalidProducts_FiltersCorrectly() + public void DiscoverUploadTargets_MissingBranch_EmitsErrorAndReturnsEmpty() { // language=yaml - AddChangelog("mixed.yaml", """ - title: Mixed products + AddChangelog("entry.yaml", """ + title: New feature type: feature products: - product: elasticsearch - - product: "bad product with spaces" - - product: kibana - prs: - - "500" """); - var targets = _service.DiscoverUploadTargets(_collector, _changelogDir); + var targets = _service.DiscoverUploadTargets(_collector, _changelogDir, "elastic", "elasticsearch", branch: null); - targets.Should().HaveCount(2); - targets.Should().Contain(t => t.S3Key == "elasticsearch/changelog/mixed.yaml"); - targets.Should().Contain(t => t.S3Key == "kibana/changelog/mixed.yaml"); - _collector.Warnings.Should().BeGreaterThan(0); + targets.Should().BeEmpty(); + _collector.Errors.Should().BeGreaterThan(0); + } + + [Theory] + [InlineData("bad repo")] + [InlineData("..")] + [InlineData(".")] + [InlineData("owner/repo")] + public void DiscoverUploadTargets_InvalidRepo_EmitsError(string repo) + { + // language=yaml + AddChangelog("entry.yaml", """ + title: New feature + type: feature + products: + - product: elasticsearch + """); + + var targets = _service.DiscoverUploadTargets(_collector, _changelogDir, "elastic", repo, "main"); + + targets.Should().BeEmpty(); + _collector.Errors.Should().BeGreaterThan(0); + } + + [Theory] + // Org is a GitHub login: no dots, no slashes, no spaces. + [InlineData("bad org")] + [InlineData("acme.corp")] + [InlineData("acme/corp")] + public void DiscoverUploadTargets_InvalidOrg_EmitsError(string org) + { + // language=yaml + AddChangelog("entry.yaml", """ + title: New feature + type: feature + products: + - product: elasticsearch + """); + + var targets = _service.DiscoverUploadTargets(_collector, _changelogDir, org, "elasticsearch", "main"); + + targets.Should().BeEmpty(); + _collector.Errors.Should().BeGreaterThan(0); + } + + [Theory] + // Branch segments reject traversal, empty parts, and out-of-class characters. + [InlineData("bad branch")] + [InlineData("..")] + [InlineData("feature/..")] + [InlineData("feature//foo")] + public void DiscoverUploadTargets_InvalidBranch_EmitsError(string branch) + { + // language=yaml + AddChangelog("entry.yaml", """ + title: New feature + type: feature + products: + - product: elasticsearch + """); + + var targets = _service.DiscoverUploadTargets(_collector, _changelogDir, "elastic", "elasticsearch", branch); + + targets.Should().BeEmpty(); + _collector.Errors.Should().BeGreaterThan(0); } [Fact] - public void DiscoverUploadTargets_MultipleFiles_DiscoversBoth() + public void DiscoverUploadTargets_EmptyDirectory_ReturnsEmpty() + { + var targets = _service.DiscoverUploadTargets(_collector, _changelogDir, "elastic", "elasticsearch", "main"); + + targets.Should().BeEmpty(); + _collector.Errors.Should().Be(0); + } + + [Fact] + public void DiscoverUploadTargets_MultipleFiles_DiscoversAllUnderRepo() { // language=yaml AddChangelog("first.yaml", """ @@ -181,32 +301,34 @@ public void DiscoverUploadTargets_MultipleFiles_DiscoversBoth() - "2" """); - var targets = _service.DiscoverUploadTargets(_collector, _changelogDir); + var targets = _service.DiscoverUploadTargets(_collector, _changelogDir, "elastic", "elasticsearch", "main"); targets.Should().HaveCount(2); - targets.Should().Contain(t => t.S3Key == "elasticsearch/changelog/first.yaml"); - targets.Should().Contain(t => t.S3Key == "kibana/changelog/second.yaml"); + targets.Should().Contain(t => t.S3Key == "changelog/elastic/elasticsearch/main/first.yaml"); + targets.Should().Contain(t => t.S3Key == "changelog/elastic/elasticsearch/main/second.yaml"); } - [Fact] - public void DiscoverUploadTargets_ProductWithHyphensAndUnderscores_Accepted() + [Theory] + [InlineData("elasticsearch-net")] + [InlineData("cloud_hosted")] + [InlineData("apm-agent-dotnet")] + [InlineData("docs.elastic")] + public void DiscoverUploadTargets_RepoWithHyphensDotsUnderscores_Accepted(string repo) { // language=yaml - AddChangelog("hyphen.yaml", """ + AddChangelog("entry.yaml", """ title: Hyphenated type: feature products: - - product: elastic-agent - - product: cloud_hosted + - product: elasticsearch prs: - "600" """); - var targets = _service.DiscoverUploadTargets(_collector, _changelogDir); + var targets = _service.DiscoverUploadTargets(_collector, _changelogDir, "elastic", repo, "main"); - targets.Should().HaveCount(2); - targets.Should().Contain(t => t.S3Key == "elastic-agent/changelog/hyphen.yaml"); - targets.Should().Contain(t => t.S3Key == "cloud_hosted/changelog/hyphen.yaml"); + targets.Should().ContainSingle(); + targets[0].S3Key.Should().Be($"changelog/elastic/{repo}/main/entry.yaml"); _collector.Errors.Should().Be(0); _collector.Warnings.Should().Be(0); } @@ -236,7 +358,10 @@ public async Task Upload_WithValidChangelogs_UploadsToS3() ArtifactType = ArtifactType.Changelog, Target = UploadTargetKind.S3, S3BucketName = "test-bucket", - Directory = _changelogDir + Directory = _changelogDir, + Owner = "elastic", + Repo = "elasticsearch", + Branch = "main" }; var ct = TestContext.Current.CancellationToken; var result = await _service.Upload(_collector, args, ct); @@ -245,11 +370,45 @@ public async Task Upload_WithValidChangelogs_UploadsToS3() _collector.Errors.Should().Be(0); A.CallTo(() => _s3Client.PutObjectAsync( - A.That.Matches(r => r.Key == "elasticsearch/changelog/entry.yaml" && r.BucketName == "test-bucket"), + A.That.Matches(r => r.Key == "changelog/elastic/elasticsearch/main/entry.yaml" && r.BucketName == "test-bucket"), A._ )).MustHaveHappenedOnceExactly(); } + [Fact] + public async Task Upload_ChangelogWithoutRepo_FailsWithoutS3Calls() + { + // language=yaml + AddChangelog("entry.yaml", """ + title: New feature + type: feature + products: + - product: elasticsearch + target: 9.2.0 + prs: + - "100" + """); + + var args = new ChangelogUploadArguments + { + ArtifactType = ArtifactType.Changelog, + Target = UploadTargetKind.S3, + S3BucketName = "test-bucket", + Directory = _changelogDir, + Owner = "elastic", + Branch = "main" + // Repo intentionally unset. + }; + var ct = TestContext.Current.CancellationToken; + var result = await _service.Upload(_collector, args, ct); + + result.Should().BeFalse(); + _collector.Errors.Should().BeGreaterThan(0); + + A.CallTo(() => _s3Client.PutObjectAsync(A._, A._)) + .MustNotHaveHappened(); + } + [Fact] public async Task Upload_EmptyDirectory_ReturnsTrue() { @@ -258,7 +417,10 @@ public async Task Upload_EmptyDirectory_ReturnsTrue() ArtifactType = ArtifactType.Changelog, Target = UploadTargetKind.S3, S3BucketName = "test-bucket", - Directory = _changelogDir + Directory = _changelogDir, + Owner = "elastic", + Repo = "elasticsearch", + Branch = "main" }; var ct = TestContext.Current.CancellationToken; var result = await _service.Upload(_collector, args, ct); @@ -294,7 +456,10 @@ public async Task Upload_WithFailedUpload_ReturnsFalseAndEmitsError() ArtifactType = ArtifactType.Changelog, Target = UploadTargetKind.S3, S3BucketName = "test-bucket", - Directory = _changelogDir + Directory = _changelogDir, + Owner = "elastic", + Repo = "elasticsearch", + Branch = "main" }; var ct = TestContext.Current.CancellationToken; var result = await _service.Upload(_collector, args, ct); @@ -375,13 +540,13 @@ public async Task Upload_BundleArtifactType_UploadsToS3() _collector.Errors.Should().Be(0); A.CallTo(() => _s3Client.PutObjectAsync( - A.That.Matches(r => r.Key == "elasticsearch/bundle/elasticsearch-9.2.0.yaml" && r.BucketName == "test-bucket"), + A.That.Matches(r => r.Key == "bundle/elasticsearch/elasticsearch-9.2.0.yaml" && r.BucketName == "test-bucket"), A._ )).MustHaveHappenedOnceExactly(); } [Fact] - public void DiscoverBundleUploadTargets_MapsToCorrectS3Key() + public void DiscoverBundleUploadTargets_MapsToArtifactRootKey() { var bundleDir = _mockFileSystem.Path.Join(Paths.WorkingDirectoryRoot.FullName, Guid.NewGuid().ToString(), "releases"); _mockFileSystem.Directory.CreateDirectory(bundleDir); @@ -405,7 +570,7 @@ public void DiscoverBundleUploadTargets_MapsToCorrectS3Key() var targets = _service.DiscoverBundleUploadTargets(_collector, bundleDir); targets.Should().HaveCount(1); - targets[0].S3Key.Should().Be("elasticsearch/bundle/elasticsearch-9.2.0.yaml"); + targets[0].S3Key.Should().Be("bundle/elasticsearch/elasticsearch-9.2.0.yaml"); _collector.Errors.Should().Be(0); } @@ -436,8 +601,8 @@ public void DiscoverBundleUploadTargets_MultipleProducts_CreatesTargetPerProduct var targets = _service.DiscoverBundleUploadTargets(_collector, bundleDir); targets.Should().HaveCount(2); - targets.Should().Contain(t => t.S3Key == "elasticsearch/bundle/stack-9.2.0.yaml"); - targets.Should().Contain(t => t.S3Key == "kibana/bundle/stack-9.2.0.yaml"); + targets.Should().Contain(t => t.S3Key == "bundle/elasticsearch/stack-9.2.0.yaml"); + targets.Should().Contain(t => t.S3Key == "bundle/kibana/stack-9.2.0.yaml"); } [Fact] @@ -492,18 +657,18 @@ public async Task Upload_BundleArtifactType_UploadsRegistryAlongsideBundle() _collector.Errors.Should().Be(0); A.CallTo(() => _s3Client.PutObjectAsync( - A.That.Matches(r => r.Key == "elasticsearch/bundle/9.3.0.yaml"), + A.That.Matches(r => r.Key == "bundle/elasticsearch/9.3.0.yaml"), A._ )).MustHaveHappenedOnceExactly(); A.CallTo(() => _s3Client.PutObjectAsync( - A.That.Matches(r => r.Key == "elasticsearch/registry.json"), + A.That.Matches(r => r.Key == "bundle/elasticsearch/registry.json"), A._ )).MustHaveHappenedOnceExactly(); } [Fact] - public async Task Upload_ChangelogArtifactType_DoesNotUploadRegistry() + public async Task Upload_ChangelogArtifactType_RefreshesRepoScopedRegistry() { // language=yaml AddChangelog("entry.yaml", """ @@ -518,6 +683,8 @@ public async Task Upload_ChangelogArtifactType_DoesNotUploadRegistry() A.CallTo(() => _s3Client.GetObjectMetadataAsync(A._, A._)) .Throws(new AmazonS3Exception("Not Found") { StatusCode = HttpStatusCode.NotFound }); + A.CallTo(() => _s3Client.GetObjectAsync(A._, A._)) + .Throws(new AmazonS3Exception("Not Found") { StatusCode = HttpStatusCode.NotFound }); A.CallTo(() => _s3Client.PutObjectAsync(A._, A._)) .Returns(new PutObjectResponse()); @@ -526,15 +693,24 @@ public async Task Upload_ChangelogArtifactType_DoesNotUploadRegistry() ArtifactType = ArtifactType.Changelog, Target = UploadTargetKind.S3, S3BucketName = "test-bucket", - Directory = _changelogDir + Directory = _changelogDir, + Owner = "elastic", + Repo = "elasticsearch", + Branch = "main" }; var ct = TestContext.Current.CancellationToken; var result = await _service.Upload(_collector, args, ct); result.Should().BeTrue(); + // Changelog uploads refresh the pool-scoped entry index, not a bundle index. + A.CallTo(() => _s3Client.PutObjectAsync( + A.That.Matches(r => r.Key == "changelog/elastic/elasticsearch/main/registry.json"), + A._ + )).MustHaveHappenedOnceExactly(); + A.CallTo(() => _s3Client.PutObjectAsync( - A.That.Matches(r => r.Key.EndsWith("/registry.json", StringComparison.Ordinal)), + A.That.Matches(r => r.Key.StartsWith("bundle/", StringComparison.Ordinal)), A._ )).MustNotHaveHappened(); } diff --git a/tests/Elastic.Changelog.Tests/Uploading/RegistryBuilderTests.cs b/tests/Elastic.Changelog.Tests/Uploading/RegistryBuilderTests.cs index d773c17d9..39e774432 100644 --- a/tests/Elastic.Changelog.Tests/Uploading/RegistryBuilderTests.cs +++ b/tests/Elastic.Changelog.Tests/Uploading/RegistryBuilderTests.cs @@ -84,7 +84,7 @@ private void StubExistingManifestNotFound() => private void StubExistingManifest(string product, Registry manifest, string etag = "\"existing-etag\"") => A.CallTo(() => _s3Client.GetObjectAsync( - A.That.Matches(r => r.Key == $"{product}/registry.json"), + A.That.Matches(r => r.Key == $"bundle/{product}/registry.json"), A._)) .ReturnsLazily(() => MakeManifestResponse(manifest, etag)); @@ -105,7 +105,7 @@ private static Registry Deserialize(string? json) => public async Task Refresh_NoExistingManifest_CreatesManifestWithIfNoneMatch() { var path = AddBundle("9.3.0.yaml", "elasticsearch", "9.3.0"); - var targets = new List { new(path, "elasticsearch/bundle/9.3.0.yaml") }; + var targets = new List { new(path, "bundle/elasticsearch/9.3.0.yaml") }; StubExistingManifestNotFound(); var result = await _builder.RefreshAsync(_collector, targets, TestContext.Current.CancellationToken); @@ -113,7 +113,7 @@ public async Task Refresh_NoExistingManifest_CreatesManifestWithIfNoneMatch() result.Updated.Should().Be(1); _puts.Should().ContainSingle(); var put = _puts[0]; - put.Key.Should().Be("elasticsearch/registry.json"); + put.Key.Should().Be("bundle/elasticsearch/registry.json"); put.IfNoneMatch.Should().Be("*"); put.IfMatch.Should().BeNull(); @@ -145,8 +145,8 @@ public async Task Refresh_ExistingManifest_MergesByFileNameAndUsesIfMatch() var ten = AddBundle("9.4.0.yaml", "elasticsearch", "9.4.0"); var targets = new List { - new(newer, "elasticsearch/bundle/9.3.0.yaml"), - new(ten, "elasticsearch/bundle/9.4.0.yaml") + new(newer, "bundle/elasticsearch/9.3.0.yaml"), + new(ten, "bundle/elasticsearch/9.4.0.yaml") }; var result = await _builder.RefreshAsync(_collector, targets, TestContext.Current.CancellationToken); @@ -177,8 +177,8 @@ public async Task Refresh_SortsManifestByVersionNotLexicographically() var v99 = AddBundle("9.9.0.yaml", "elasticsearch", "9.9.0"); var targets = new List { - new(v99, "elasticsearch/bundle/9.9.0.yaml"), - new(v910, "elasticsearch/bundle/9.10.0.yaml") + new(v99, "bundle/elasticsearch/9.9.0.yaml"), + new(v910, "bundle/elasticsearch/9.10.0.yaml") }; _ = await _builder.RefreshAsync(_collector, targets, TestContext.Current.CancellationToken); @@ -194,8 +194,8 @@ public async Task Refresh_MultipleProducts_WritesOneManifestPerProduct() var kb = AddBundle("kb-9.3.0.yaml", "kibana", "9.3.0"); var targets = new List { - new(es, "elasticsearch/bundle/9.3.0.yaml"), - new(kb, "kibana/bundle/kb-9.3.0.yaml") + new(es, "bundle/elasticsearch/9.3.0.yaml"), + new(kb, "bundle/kibana/kb-9.3.0.yaml") }; StubExistingManifestNotFound(); @@ -203,8 +203,8 @@ public async Task Refresh_MultipleProducts_WritesOneManifestPerProduct() result.Updated.Should().Be(2); _puts.Should().HaveCount(2); - _puts.Should().Contain(p => p.Key == "elasticsearch/registry.json"); - _puts.Should().Contain(p => p.Key == "kibana/registry.json"); + _puts.Should().Contain(p => p.Key == "bundle/elasticsearch/registry.json"); + _puts.Should().Contain(p => p.Key == "bundle/kibana/registry.json"); } [Fact] @@ -232,17 +232,17 @@ public async Task Refresh_MultiProductBundle_RecordsTargetPerProduct() """)); var targets = new List { - new(path, "elasticsearch/bundle/multi.yaml"), - new(path, "kibana/bundle/multi.yaml") + new(path, "bundle/elasticsearch/multi.yaml"), + new(path, "bundle/kibana/multi.yaml") }; StubExistingManifestNotFound(); _ = await _builder.RefreshAsync(_collector, targets, TestContext.Current.CancellationToken); - var es = Deserialize(_puts.Single(p => p.Key == "elasticsearch/registry.json").ContentBody); + var es = Deserialize(_puts.Single(p => p.Key == "bundle/elasticsearch/registry.json").ContentBody); es.Bundles[0].Target.Should().Be("9.3.0"); - var kb = Deserialize(_puts.Single(p => p.Key == "kibana/registry.json").ContentBody); + var kb = Deserialize(_puts.Single(p => p.Key == "bundle/kibana/registry.json").ContentBody); kb.Bundles[0].Target.Should().Be("9.4.0"); } @@ -257,7 +257,7 @@ public async Task Refresh_ExistingManifestUnreadable_OverwritesUsingLiveETag() }); var path = AddBundle("9.3.0.yaml", "elasticsearch", "9.3.0"); - var targets = new List { new(path, "elasticsearch/bundle/9.3.0.yaml") }; + var targets = new List { new(path, "bundle/elasticsearch/9.3.0.yaml") }; var result = await _builder.RefreshAsync(_collector, targets, TestContext.Current.CancellationToken); @@ -277,7 +277,7 @@ public async Task Refresh_BundleWithoutTarget_RecordsEntryWithoutTarget() _mockFileSystem.AddFile(path, new MockFileData(""" entries: [] """)); - var targets = new List { new(path, "elasticsearch/bundle/no-target.yaml") }; + var targets = new List { new(path, "bundle/elasticsearch/no-target.yaml") }; StubExistingManifestNotFound(); var result = await _builder.RefreshAsync(_collector, targets, TestContext.Current.CancellationToken); @@ -305,7 +305,7 @@ public async Task Refresh_UnchangedManifest_SkipsWrite() }; StubExistingManifest("elasticsearch", existing, "\"manifest-v1\""); - var targets = new List { new(path, "elasticsearch/bundle/9.3.0.yaml") }; + var targets = new List { new(path, "bundle/elasticsearch/9.3.0.yaml") }; var result = await _builder.RefreshAsync(_collector, targets, TestContext.Current.CancellationToken); result.Unchanged.Should().Be(1); @@ -344,7 +344,7 @@ public async Task Refresh_ConcurrentWrite_RetriesAfterPreconditionFailed() .Once(); var path = AddBundle("9.4.0.yaml", "elasticsearch", "9.4.0"); - var targets = new List { new(path, "elasticsearch/bundle/9.4.0.yaml") }; + var targets = new List { new(path, "bundle/elasticsearch/9.4.0.yaml") }; var result = await _builder.RefreshAsync(_collector, targets, TestContext.Current.CancellationToken); @@ -379,7 +379,7 @@ public async Task Refresh_PersistentConcurrentWrite_EmitsWarningAndReportsFailur .Throws(new AmazonS3Exception("Precondition Failed") { StatusCode = HttpStatusCode.PreconditionFailed }); var path = AddBundle("9.4.0.yaml", "elasticsearch", "9.4.0"); - var targets = new List { new(path, "elasticsearch/bundle/9.4.0.yaml") }; + var targets = new List { new(path, "bundle/elasticsearch/9.4.0.yaml") }; var result = await _builder.RefreshAsync(_collector, targets, TestContext.Current.CancellationToken); @@ -408,7 +408,7 @@ public async Task Refresh_ChangelogScope_WritesEntryRegistryWithoutTarget() - product: elasticsearch target: 9.3.0 """)); - var targets = new List { new(path, "elasticsearch/changelog/1-feature.yaml") }; + var targets = new List { new(path, "changelog/elastic/elasticsearch/main/1-feature.yaml") }; A.CallTo(() => _s3Client.GetObjectAsync(A._, A._)) .Throws(new AmazonS3Exception("Not Found") { StatusCode = HttpStatusCode.NotFound }); @@ -416,10 +416,11 @@ public async Task Refresh_ChangelogScope_WritesEntryRegistryWithoutTarget() result.Updated.Should().Be(1); _puts.Should().ContainSingle(); - _puts[0].Key.Should().Be("elasticsearch/changelog/registry.json"); + _puts[0].Key.Should().Be("changelog/elastic/elasticsearch/main/registry.json"); var manifest = Deserialize(_puts[0].ContentBody); - manifest.Product.Should().Be("elasticsearch"); + // The grouping identifier for the entry index is the {org}/{repo}/{branch} prefix. + manifest.Product.Should().Be("elastic/elasticsearch/main"); manifest.Bundles.Should().ContainSingle(); manifest.Bundles[0].File.Should().Be("1-feature.yaml"); // The changelog-entry index only enumerates files; per-entry target is not recorded. @@ -427,6 +428,34 @@ public async Task Refresh_ChangelogScope_WritesEntryRegistryWithoutTarget() manifest.Bundles[0].ETag.Should().NotBeNullOrEmpty(); } + [Fact] + public async Task Refresh_ChangelogScope_BranchWithSlashes_GroupsByFullPoolPrefix() + { + var path = _mockFileSystem.Path.Join(_bundleDir, "2-feature.yaml"); + // language=yaml + _mockFileSystem.AddFile(path, new MockFileData(""" + title: Sample on a feature branch + type: enhancement + products: + - product: elasticsearch + """)); + // The branch "feature/foo" contributes two key segments; the registry must live at the pool root. + var targets = new List { new(path, "changelog/elastic/elasticsearch/feature/foo/2-feature.yaml") }; + A.CallTo(() => _s3Client.GetObjectAsync(A._, A._)) + .Throws(new AmazonS3Exception("Not Found") { StatusCode = HttpStatusCode.NotFound }); + + var result = await _builder.RefreshAsync(_collector, targets, TestContext.Current.CancellationToken, RegistryScope.Changelog); + + result.Updated.Should().Be(1); + _puts.Should().ContainSingle(); + _puts[0].Key.Should().Be("changelog/elastic/elasticsearch/feature/foo/registry.json"); + + var manifest = Deserialize(_puts[0].ContentBody); + manifest.Product.Should().Be("elastic/elasticsearch/feature/foo"); + manifest.Bundles.Should().ContainSingle(); + manifest.Bundles[0].File.Should().Be("2-feature.yaml"); + } + private sealed class FakeTimeProvider(DateTimeOffset now) : TimeProvider { public override DateTimeOffset GetUtcNow() => now; diff --git a/tests/Elastic.Changelog.Tests/Uploading/RegistryKeyTests.cs b/tests/Elastic.Changelog.Tests/Uploading/RegistryKeyTests.cs index bd0091a22..2e253c90d 100644 --- a/tests/Elastic.Changelog.Tests/Uploading/RegistryKeyTests.cs +++ b/tests/Elastic.Changelog.Tests/Uploading/RegistryKeyTests.cs @@ -10,33 +10,62 @@ namespace Elastic.Changelog.Tests.Uploading; public class RegistryKeyTests { [Theory] - // Bundle index: {product}/registry.json - [InlineData("elasticsearch/registry.json")] - [InlineData("kibana/registry.json")] - [InlineData("elastic-agent/registry.json")] - [InlineData("cloud_hosted/registry.json")] - [InlineData("a/registry.json")] - // Changelog-entry index: {product}/changelog/registry.json - [InlineData("elasticsearch/changelog/registry.json")] - [InlineData("kibana/changelog/registry.json")] - [InlineData("cloud_hosted/changelog/registry.json")] - public void IsRegistry_ValidProductKeys_ReturnsTrue(string key) => + // Bundle index (artifact-root): bundle/{product}/registry.json — exactly one product segment. + [InlineData("bundle/elasticsearch/registry.json")] + [InlineData("bundle/kibana/registry.json")] + [InlineData("bundle/elastic-agent/registry.json")] + [InlineData("bundle/cloud_hosted/registry.json")] + [InlineData("bundle/cloud-serverless/registry.json")] + [InlineData("bundle/a/registry.json")] + // Changelog-entry index (artifact-root): changelog/{org}/{repo}/{branch}/registry.json. + [InlineData("changelog/elastic/elasticsearch/main/registry.json")] + [InlineData("changelog/elastic/kibana/master/registry.json")] + // External org (e.g. an acquired company keeping its own GitHub org). + [InlineData("changelog/acme-corp/widgets/main/registry.json")] + // Repo and branch segments may contain dots (e.g. apm-agent-dotnet, branch 8.x). + [InlineData("changelog/elastic/apm.agent/main/registry.json")] + [InlineData("changelog/elastic/elasticsearch/8.x/registry.json")] + [InlineData("changelog/elastic/kibana/9.0/registry.json")] + // Branch stored verbatim: a branch's own '/' become additional, valid key segments. + [InlineData("changelog/elastic/kibana/feature/foo/registry.json")] + [InlineData("changelog/elastic/kibana/release/8.x/registry.json")] + public void IsRegistry_ValidArtifactRootKeys_ReturnsTrue(string key) => RegistryKey.IsRegistry(key).Should().BeTrue(); [Theory] [InlineData("")] [InlineData("registry.json")] [InlineData("/registry.json")] + // Old product-first and single-segment changelog layouts are no longer valid manifest keys. + [InlineData("elasticsearch/registry.json")] + [InlineData("elasticsearch/changelog/registry.json")] + [InlineData("changelog/elasticsearch/registry.json")] + // Changelog manifests shallower than org/repo/branch (3 segments) are rejected. + [InlineData("changelog/elastic/elasticsearch/registry.json")] + // Missing/empty middle segment. + [InlineData("bundle/registry.json")] + [InlineData("changelog/registry.json")] + [InlineData("bundle//registry.json")] + [InlineData("changelog/elastic//main/registry.json")] + // Unknown top-level prefix. + [InlineData("entries/elastic/elasticsearch/main/registry.json")] [InlineData("elasticsearch/bundle/registry.json")] - [InlineData("elasticsearch/registry.yaml")] - [InlineData("elasticsearch/changelog/registry.yaml")] - [InlineData("elasticsearch/changelog/bundle/registry.json")] - [InlineData("../registry.json")] - [InlineData("elastic search/registry.json")] - [InlineData("elastic.search/registry.json")] - [InlineData("elastic/search/registry.json")] - [InlineData("elastic search/changelog/registry.json")] - [InlineData("elastic/search/changelog/registry.json")] + // Dots are allowed only for changelog segments, never for bundle product segments + // (producers validate products as [a-zA-Z0-9_-]+). + [InlineData("bundle/foo.bar/registry.json")] + // Wrong extension. + [InlineData("bundle/elasticsearch/registry.yaml")] + [InlineData("changelog/elastic/elasticsearch/main/registry.yaml")] + // Deeper nesting is rejected for bundles (must stay single-segment). + [InlineData("bundle/elastic/search/registry.json")] + // Traversal anywhere in the middle segments. + [InlineData("bundle/../registry.json")] + [InlineData("changelog/../registry.json")] + [InlineData("changelog/elastic/../main/registry.json")] + [InlineData("changelog/elastic/elasticsearch/../registry.json")] + // Spaces (and other out-of-class characters) are rejected. + [InlineData("bundle/elastic search/registry.json")] + [InlineData("changelog/elastic/elastic search/main/registry.json")] public void IsRegistry_InvalidKeys_ReturnsFalse(string key) => RegistryKey.IsRegistry(key).Should().BeFalse(); } diff --git a/tests/Elastic.Documentation.Configuration.Tests/ReleaseNotes/CdnChangelogEntryFetcherTests.cs b/tests/Elastic.Documentation.Configuration.Tests/ReleaseNotes/CdnChangelogEntryFetcherTests.cs index 9839e7a35..7d2f18278 100644 --- a/tests/Elastic.Documentation.Configuration.Tests/ReleaseNotes/CdnChangelogEntryFetcherTests.cs +++ b/tests/Elastic.Documentation.Configuration.Tests/ReleaseNotes/CdnChangelogEntryFetcherTests.cs @@ -37,19 +37,60 @@ private static (List Errors, List Warnings, Action EmitE public async Task FetchAsync_HappyPath_ReturnsAllEntriesFromRegistry() { var handler = new StubHandler(req => - req.RequestUri!.AbsolutePath.EndsWith("/changelog/registry.json", StringComparison.Ordinal) + req.RequestUri!.AbsolutePath.EndsWith("/registry.json", StringComparison.Ordinal) ? Json(/*lang=json,strict*/ """{ "schema_version": 1, "product": "elasticsearch", "bundles": [ { "file": "1-a.yaml" }, { "file": "2-b.yaml" } ] }""") : Yaml(SampleEntry)); var (errors, warnings, emitError, emitWarning) = Diagnostics(); using var fetcher = CreateFetcher(handler); - var entries = await fetcher.FetchAsync(BaseUri, "elasticsearch", emitError, emitWarning, TestContext.Current.CancellationToken); + var entries = await fetcher.FetchAsync(BaseUri, "elastic", "elasticsearch", "main", emitError, emitWarning, TestContext.Current.CancellationToken); errors.Should().BeEmpty(); warnings.Should().BeEmpty(); entries.Select(e => e.FileName).Should().BeEquivalentTo("1-a.yaml", "2-b.yaml"); entries.Should().OnlyContain(e => e.Content.Contains("Sample enhancement")); - handler.RequestedPaths.Should().Contain(p => p.EndsWith("/elasticsearch/changelog/1-a.yaml", StringComparison.Ordinal)); + // Artifact-root layout: entries and their registry live under changelog/{org}/{repo}/{branch}/... + handler.RequestedPaths.Should().Contain("/changelog/elastic/elasticsearch/main/registry.json"); + handler.RequestedPaths.Should().Contain(p => p.EndsWith("/changelog/elastic/elasticsearch/main/1-a.yaml", StringComparison.Ordinal)); + } + + [Fact] + public async Task FetchAsync_BranchWithSlashes_KeepsBranchSeparatorsInPath() + { + var handler = new StubHandler(req => + req.RequestUri!.AbsolutePath.EndsWith("/registry.json", StringComparison.Ordinal) + ? Json(/*lang=json,strict*/ """{ "schema_version": 1, "product": "elastic/elasticsearch/feature/foo", "bundles": [ { "file": "1-a.yaml" } ] }""") + : Yaml(SampleEntry)); + var (errors, warnings, emitError, emitWarning) = Diagnostics(); + + using var fetcher = CreateFetcher(handler); + var entries = await fetcher.FetchAsync(BaseUri, "elastic", "elasticsearch", "feature/foo", emitError, emitWarning, TestContext.Current.CancellationToken); + + errors.Should().BeEmpty(); + warnings.Should().BeEmpty(); + entries.Select(e => e.FileName).Should().BeEquivalentTo("1-a.yaml"); + // The branch's '/' stays a real path separator (not percent-encoded into one segment). + handler.RequestedPaths.Should().Contain("/changelog/elastic/elasticsearch/feature/foo/registry.json"); + handler.RequestedPaths.Should().Contain(p => p.EndsWith("/changelog/elastic/elasticsearch/feature/foo/1-a.yaml", StringComparison.Ordinal)); + } + + [Theory] + [InlineData("..")] + [InlineData("feature/..")] + [InlineData("")] + public async Task FetchAsync_UnsafeBranch_EmitsErrorAndDoesNotHitCdn(string branch) + { + // A traversal/empty branch segment must be rejected before any request, so URI normalization + // can't redirect the fetch to a different pool. + var handler = new StubHandler(_ => new HttpResponseMessage(HttpStatusCode.OK)); + var (errors, _, emitError, emitWarning) = Diagnostics(); + + using var fetcher = CreateFetcher(handler); + var entries = await fetcher.FetchAsync(BaseUri, "elastic", "elasticsearch", branch, emitError, emitWarning, TestContext.Current.CancellationToken); + + entries.Should().BeEmpty(); + errors.Should().ContainSingle().Which.Should().Contain("Invalid changelog pool"); + handler.RequestedPaths.Should().BeEmpty("validation must happen before any CDN request"); } [Fact] @@ -59,7 +100,7 @@ public async Task FetchAsync_RegistryNotFound_EmitsErrorAndReturnsEmpty() var (errors, _, emitError, emitWarning) = Diagnostics(); using var fetcher = CreateFetcher(handler); - var entries = await fetcher.FetchAsync(BaseUri, "elasticsearch", emitError, emitWarning, TestContext.Current.CancellationToken); + var entries = await fetcher.FetchAsync(BaseUri, "elastic", "elasticsearch", "main", emitError, emitWarning, TestContext.Current.CancellationToken); entries.Should().BeEmpty(); errors.Should().ContainSingle().Which.Should().Contain("registry"); @@ -72,7 +113,7 @@ public async Task FetchAsync_EntryMissingAfterRetries_EmitsErrorAndReturnsEmpty( // (not skipped) so the bundle fails rather than silently dropping a release entry. var handler = new StubHandler(req => { - if (req.RequestUri!.AbsolutePath.EndsWith("/changelog/registry.json", StringComparison.Ordinal)) + if (req.RequestUri!.AbsolutePath.EndsWith("/registry.json", StringComparison.Ordinal)) return Json(/*lang=json,strict*/ """{ "schema_version": 1, "product": "elasticsearch", "bundles": [ { "file": "1-a.yaml" }, { "file": "2-missing.yaml" } ] }"""); return req.RequestUri!.AbsolutePath.EndsWith("/2-missing.yaml", StringComparison.Ordinal) ? new HttpResponseMessage(HttpStatusCode.NotFound) @@ -81,7 +122,7 @@ public async Task FetchAsync_EntryMissingAfterRetries_EmitsErrorAndReturnsEmpty( var (errors, _, emitError, emitWarning) = Diagnostics(); using var fetcher = CreateFetcher(handler, maxAttempts: 3); - var entries = await fetcher.FetchAsync(BaseUri, "elasticsearch", emitError, emitWarning, TestContext.Current.CancellationToken); + var entries = await fetcher.FetchAsync(BaseUri, "elastic", "elasticsearch", "main", emitError, emitWarning, TestContext.Current.CancellationToken); entries.Should().BeEmpty(); errors.Should().ContainSingle().Which.Should().Contain("2-missing.yaml"); @@ -97,7 +138,7 @@ public async Task FetchAsync_EntryRecoversAfterRetry_ReturnsEntry() var handler = new StubHandler(req => { var path = req.RequestUri!.AbsolutePath; - if (path.EndsWith("/changelog/registry.json", StringComparison.Ordinal)) + if (path.EndsWith("/registry.json", StringComparison.Ordinal)) return Json(/*lang=json,strict*/ """{ "schema_version": 1, "product": "elasticsearch", "bundles": [ { "file": "1-a.yaml" } ] }"""); if (path.EndsWith("/1-a.yaml", StringComparison.Ordinal)) return Interlocked.Increment(ref entryAttempts) == 1 @@ -108,7 +149,7 @@ public async Task FetchAsync_EntryRecoversAfterRetry_ReturnsEntry() var (errors, warnings, emitError, emitWarning) = Diagnostics(); using var fetcher = CreateFetcher(handler); - var entries = await fetcher.FetchAsync(BaseUri, "elasticsearch", emitError, emitWarning, TestContext.Current.CancellationToken); + var entries = await fetcher.FetchAsync(BaseUri, "elastic", "elasticsearch", "main", emitError, emitWarning, TestContext.Current.CancellationToken); errors.Should().BeEmpty(); warnings.Should().BeEmpty(); @@ -124,7 +165,7 @@ public async Task FetchAsync_SchemaVersionTooNew_EmitsError() var (errors, _, emitError, emitWarning) = Diagnostics(); using var fetcher = CreateFetcher(handler); - var entries = await fetcher.FetchAsync(BaseUri, "elasticsearch", emitError, emitWarning, TestContext.Current.CancellationToken); + var entries = await fetcher.FetchAsync(BaseUri, "elastic", "elasticsearch", "main", emitError, emitWarning, TestContext.Current.CancellationToken); entries.Should().BeEmpty(); errors.Should().ContainSingle().Which.Should().Contain("schema version"); @@ -134,13 +175,13 @@ public async Task FetchAsync_SchemaVersionTooNew_EmitsError() public async Task FetchAsync_UnsafeFileName_EmitsWarningAndSkips() { var handler = new StubHandler(req => - req.RequestUri!.AbsolutePath.EndsWith("/changelog/registry.json", StringComparison.Ordinal) + req.RequestUri!.AbsolutePath.EndsWith("/registry.json", StringComparison.Ordinal) ? Json(/*lang=json,strict*/ """{ "schema_version": 1, "product": "elasticsearch", "bundles": [ { "file": "../escape.yaml" }, { "file": "ok.yaml" } ] }""") : Yaml(SampleEntry)); var (errors, warnings, emitError, emitWarning) = Diagnostics(); using var fetcher = CreateFetcher(handler); - var entries = await fetcher.FetchAsync(BaseUri, "elasticsearch", emitError, emitWarning, TestContext.Current.CancellationToken); + var entries = await fetcher.FetchAsync(BaseUri, "elastic", "elasticsearch", "main", emitError, emitWarning, TestContext.Current.CancellationToken); errors.Should().BeEmpty(); entries.Select(e => e.FileName).Should().BeEquivalentTo("ok.yaml"); diff --git a/tests/Elastic.Documentation.Configuration.Tests/ReleaseNotes/CdnChangelogFetcherTests.cs b/tests/Elastic.Documentation.Configuration.Tests/ReleaseNotes/CdnChangelogFetcherTests.cs index 8de403a05..1a071f96b 100644 --- a/tests/Elastic.Documentation.Configuration.Tests/ReleaseNotes/CdnChangelogFetcherTests.cs +++ b/tests/Elastic.Documentation.Configuration.Tests/ReleaseNotes/CdnChangelogFetcherTests.cs @@ -53,6 +53,10 @@ public async Task FetchAsync_HappyPath_ReturnsBundlesFromRegistry() bundles.Should().ContainSingle(); bundles[0].Version.Should().Be("9.3.0"); bundles[0].Entries.Should().ContainSingle().Which.Title.Should().Be("Sample enhancement"); + + // Artifact-root layout: bundles and their registry live under bundle/{product}/... + handler.RequestedPaths.Should().Contain("/bundle/elasticsearch/registry.json"); + handler.RequestedPaths.Should().Contain("/bundle/elasticsearch/9.3.0.yaml"); } [Fact]