From b93aee7014715c8acc87f8db9e69282bd982534a Mon Sep 17 00:00:00 2001 From: Nan Liu Date: Mon, 15 Jun 2026 23:44:19 +0000 Subject: [PATCH 1/6] feat(component): add overlay metadata and 'component overlays' command Add an optional [metadata] block on overlays (category, commits, PR, bug links, upstreamability) and a per-file .overlay.toml format loaded from a component's overlay-dir. Add the read-only 'azldev component overlays' command with --category, --only-annotated, and --upstreamability filters. Metadata and overlay-dir are excluded from fingerprints. Regenerate docs, JSON schema, and scenario snapshots. --- docs/user/reference/cli/azldev_component.md | 1 + .../cli/azldev_component_overlays.md | 75 +++++ docs/user/reference/config/overlays.md | 129 +++++++- .../app/azldev/cmds/component/component.go | 1 + .../app/azldev/cmds/component/overlays.go | 234 ++++++++++++++ .../azldev/cmds/component/overlays_test.go | 187 +++++++++++ internal/projectconfig/component.go | 13 + internal/projectconfig/fingerprint_test.go | 6 + internal/projectconfig/loader.go | 7 + internal/projectconfig/overlay.go | 13 + internal/projectconfig/overlay_file.go | 157 ++++++++++ internal/projectconfig/overlay_file_test.go | 291 ++++++++++++++++++ internal/projectconfig/overlay_metadata.go | 168 ++++++++++ .../projectconfig/overlay_metadata_test.go | 168 ++++++++++ .../TestMCPServerMode_1.snap.json | 100 ++++++ ...ainer_config_generate-schema_stdout_1.snap | 79 +++++ ...shots_config_generate-schema_stdout_1.snap | 79 +++++ schemas/azldev.schema.json | 79 +++++ 18 files changed, 1786 insertions(+), 1 deletion(-) create mode 100644 docs/user/reference/cli/azldev_component_overlays.md create mode 100644 internal/app/azldev/cmds/component/overlays.go create mode 100644 internal/app/azldev/cmds/component/overlays_test.go create mode 100644 internal/projectconfig/overlay_file.go create mode 100644 internal/projectconfig/overlay_file_test.go create mode 100644 internal/projectconfig/overlay_metadata.go create mode 100644 internal/projectconfig/overlay_metadata_test.go diff --git a/docs/user/reference/cli/azldev_component.md b/docs/user/reference/cli/azldev_component.md index 0516a5e8..a2134097 100644 --- a/docs/user/reference/cli/azldev_component.md +++ b/docs/user/reference/cli/azldev_component.md @@ -43,6 +43,7 @@ components defined in the project configuration. * [azldev component changed](azldev_component_changed.md) - Detect which components changed between two git refs * [azldev component diff-sources](azldev_component_diff-sources.md) - Show the diff that overlays apply to a component's sources * [azldev component list](azldev_component_list.md) - List components in this project +* [azldev component overlays](azldev_component_overlays.md) - List overlays and their metadata for components in this project * [azldev component prepare-sources](azldev_component_prepare-sources.md) - Prepare buildable sources for components * [azldev component query](azldev_component_query.md) - Query info for components in this project * [azldev component render](azldev_component_render.md) - Render post-overlay specs and sidecar files to a checked-in directory diff --git a/docs/user/reference/cli/azldev_component_overlays.md b/docs/user/reference/cli/azldev_component_overlays.md new file mode 100644 index 00000000..c0ef47b3 --- /dev/null +++ b/docs/user/reference/cli/azldev_component_overlays.md @@ -0,0 +1,75 @@ + + +## azldev component overlays + +List overlays and their metadata for components in this project + +### Synopsis + +List overlays defined on components in the project configuration, including any +documentation metadata (category, commits, upstream PR, bug links, etc.) attached +to each overlay. Overlays loaded from a component's overlay-dir (per-file +.overlay.toml format) inherit their metadata from the file's [metadata] block. + +This command is read-only and does not parse spec files or fetch upstream sources, +so it is fast and works even when locks are missing or stale. + +``` +azldev component overlays [flags] +``` + +### Examples + +``` + # List overlays for all components + azldev component overlays -a + + # List overlays for one component + azldev component overlays -p curl + + # List only overlays carrying documentation metadata + azldev component overlays -a --only-annotated + + # Filter by category + azldev component overlays -a --category backport-fedora + + # List only overlays that can be upstreamed + azldev component overlays -a --upstreamability yes + + # JSON output for scripting + azldev component overlays -a -q -O json +``` + +### Options + +``` + -a, --all-components Include all components + --category string only include overlays whose metadata declares this category + -p, --component stringArray Component name pattern + -g, --component-group stringArray Component group name + -h, --help help for overlays + --only-annotated exclude overlays that have no metadata + -s, --spec-path stringArray Spec path + --upstreamability string only include overlays whose metadata declares this upstreamability ('yes', 'no', or 'unknown') +``` + +### Options inherited from parent commands + +``` + -y, --accept-all accept all prompts + --color mode output colorization mode {always, auto, never} (default auto) + --config-file stringArray additional TOML config file(s) to merge (may be repeated) + -n, --dry-run dry run only (do not take action) + --network-retries int maximum number of attempts for network operations (minimum 1) (default 3) + --no-default-config disable default configuration + -O, --output-format fmt output format {csv, json, markdown, table} (default table) + --permissive-config do not fail on unknown fields in TOML config files + -C, --project string path to Azure Linux project + -q, --quiet only enable minimal output + -v, --verbose enable verbose output +``` + +### SEE ALSO + +* [azldev component](azldev_component.md) - Manage components + diff --git a/docs/user/reference/config/overlays.md b/docs/user/reference/config/overlays.md index 55061118..eff7bb9a 100644 --- a/docs/user/reference/config/overlays.md +++ b/docs/user/reference/config/overlays.md @@ -61,10 +61,137 @@ successfully makes a replacement to at least one matching file. | Replacement | `replacement` | Literal replacement text; capture group references like `$1` are **not** expanded. Omit or leave empty to delete matched text. | `spec-search-replace`, `file-search-replace`, `file-rename` | | Lines | `lines` | Array of text lines to insert | `spec-prepend-lines`, `spec-append-lines`, `file-prepend-lines` | | File | `file` | The name of the non-spec file to modify or add | `file-prepend-lines`, `file-search-replace`, `file-add`, `file-remove`, `file-rename`, `patch-add` (optional), `patch-remove` | -| Source | `source` | Path to source file for `file-add` and `patch-add`; relative paths are relative to the config file | `file-add`, `patch-add` | +| Source | `source` | Path to source file for `file-add` and `patch-add`; relative paths are relative to the config file that defines the overlay (the `.overlay.toml` file if loaded from an [overlay directory](#per-file-overlay-format-overlaytoml), otherwise the component config) | `file-add`, `patch-add` | +| Metadata | `metadata` | Documentation table describing intent and provenance — see [Overlay Metadata](#overlay-metadata). Not allowed inside a `.overlay.toml` file (the file-level `[metadata]` block applies to every overlay in the file). | All (optional) | > **Note:** For `file-rename`, the `replacement` field is a **filename only** (not a path). The file is renamed within its current directory. +### Component-level fields for overlays + +In addition to per-overlay fields, the following fields are set directly on the component: + +| Field | TOML Key | Description | +|-------|----------|-------------| +| Overlay directory | `overlay-dir` | Path (relative to the component's config file) to a directory of `*.overlay.toml` files. Each file is loaded at config-load time and its overlays are appended to `overlays` in filename order. See [Per-file overlay format](#per-file-overlay-format-overlaytoml). | + +## Overlay Metadata + +Overlays can carry an optional `metadata` table that documents *why* the overlay exists and *when* it can be removed. Metadata is reviewed by humans and surfaced in tooling; it does **not** affect how the overlay is applied and is excluded from component fingerprints (so editing metadata never invalidates build caches). + +### `metadata` fields + +| Field | TOML Key | Description | +|-------|----------|-------------| +| Category | `category` | **Required.** Classification of the overlay's intent. See the table below. | +| Commits | `commits` | List of upstream commit URLs (Fedora dist-git or upstream project) that this overlay backports or implements. Each entry must be an absolute http(s) URL. | +| Fixed in | `fixed-in` | Upstream version or Fedora NVR where the fix lands (e.g. `xclock-1.1.1-11.fc44`, `4.11.2`). | +| Removable after | `removable-after` | A Fedora branch (e.g. `f44`); the overlay can be dropped once AZL bumps past it. Only valid when `category = "backport-fedora"`. | +| PR | `pr` | URL of the upstream pull request that carries (or proposes) the fix. | +| Bug | `bug` | List of bug-tracker URLs related to this overlay. | +| Upstreamability | `upstreamability` | Whether this change can be upstreamed: `"yes"`, `"no"`, or `"unknown"`. Defaults to `"unknown"` (not yet assessed) when omitted. | + +### Categories + +| Category | When to use | +|----------|-------------| +| `backport-fedora` | Fix exists in some Fedora branch. The overlay backports it. Self-resolves when AZL bumps past it. Requires at least one of `commits` or `fixed-in`. | +| `upstream-fix` | Fix is **not** in any Fedora branch; the overlay is a candidate for upstreaming. | +| `azl-dependency-pruning` | Removing dependencies not shipped in AZL. | +| `azl-feature-disablement` | Disabling unneeded features or subpackages. | +| `azl-branding-policy` | Fedora→Azure Linux name/path changes; RHEL/enterprise convention alignment. | +| `azl-build` | Toolchain, mock, or CI environment adjustments. | +| `azl-test-disablement` | Skipping failing tests. | +| `azl-security-compliance` | FIPS or crypto-policy changes. | +| `azl-release-management` | Release-tag and changelog mechanics. | +| `azl-missing-dependency-workaround` | Temporary workaround for packages not yet imported into AZL. | +| `azl-platform-adaptation` | Architecture-specific adjustments. | + +### Inline metadata example + +TOML inline tables (`metadata = { ... }`) must fit on a single line. When the metadata has more than one or two fields, use a sub-table (`[components..overlays.metadata]`) so each field gets its own line: + +```toml +[[components.xclock.overlays]] +type = "spec-search-replace" +description = "Pass --force to autoreconf" +regex = "autoreconf -i" +replacement = "autoreconf -fi" + + [components.xclock.overlays.metadata] + category = "backport-fedora" + commits = ["https://src.fedoraproject.org/rpms/xclock/c/1e407488"] + fixed-in = "xclock-1.1.1-11.fc44" + removable-after = "f44" +``` + +For short metadata, the single-line inline form is also valid: + +```toml +[[components.xclock.overlays]] +type = "spec-set-tag" +tag = "Vendor" +value = "Microsoft" +metadata = { category = "azl-branding-policy" } +``` + +## Per-file overlay format (`.overlay.toml`) + +When a single logical change (a CVE backport, a feature disablement, a Fedora cherry-pick…) needs **several overlays** that all share the same provenance, declaring them inline in the component config gets noisy and makes the boundary between unrelated changes hard to see. Use the per-file format instead. + +### Layout + +Set `overlay-dir` on the component and drop one `*.overlay.toml` file per logical change into that directory: + +``` +base/comps/mypackage/ +├── mypackage.comp.toml +└── overlays/ + ├── 0001-cve-2024-1234.overlay.toml + ├── 0002-disable-broken-tests.overlay.toml + └── 0003-azl-branding.overlay.toml +``` + +```toml +# mypackage.comp.toml +[components.mypackage] +overlay-dir = "overlays" +``` + +Files are loaded in **filename (lexicographic) order**, so prefix each file with a numeric ordinal (`0001-`, `0002-`, …) to make the apply order obvious and stable. Files that don't end in `.overlay.toml` are ignored, so you can keep `README.md` or other notes alongside. + +Overlays loaded from `.overlay.toml` files are **appended after** any inline overlays declared directly on the component. + +### File structure + +Each `.overlay.toml` file represents one logical change. It has: + +* exactly one top-level `[metadata]` table (uses the same fields documented in [Overlay Metadata](#overlay-metadata)); and +* one or more `[[overlays]]` entries, applied in declaration order. + +Per-overlay `metadata` is **not allowed** inside a `.overlay.toml` file — the file-level `[metadata]` is the single source of truth and is stamped onto every overlay in the file. Relative `source` paths are resolved against the directory of the `.overlay.toml` file (not the component config). + +```toml +# overlays/0001-cve-2024-1234.overlay.toml +[metadata] +category = "backport-fedora" +commits = ["https://src.fedoraproject.org/rpms/mypackage/c/abc123def456"] +fixed-in = "3.2.0" +bug = ["https://bugzilla.redhat.com/show_bug.cgi?id=12345"] + +[[overlays]] +type = "patch-add" +source = "patches/CVE-2024-1234.patch" + +[[overlays]] +type = "spec-append-lines" +section = "%changelog" +lines = ["- Fix CVE-2024-1234"] +``` + +### Inspecting overlay metadata + +Use `azldev component overlays` to list overlays (and their metadata) for one or more components. The command is read-only and supports `--category` and `--only-annotated` filters; output is available as a table (default) or JSON (`-O json`). + ## Examples ### Adding a Build Dependency diff --git a/internal/app/azldev/cmds/component/component.go b/internal/app/azldev/cmds/component/component.go index df28df19..d04d8f7c 100644 --- a/internal/app/azldev/cmds/component/component.go +++ b/internal/app/azldev/cmds/component/component.go @@ -32,4 +32,5 @@ components defined in the project configuration.`, queryOnAppInit(app, cmd) renderOnAppInit(app, cmd) updateOnAppInit(app, cmd) + overlaysOnAppInit(app, cmd) } diff --git a/internal/app/azldev/cmds/component/overlays.go b/internal/app/azldev/cmds/component/overlays.go new file mode 100644 index 00000000..a9f3e94e --- /dev/null +++ b/internal/app/azldev/cmds/component/overlays.go @@ -0,0 +1,234 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +package component + +import ( + "fmt" + "slices" + "strings" + + "github.com/microsoft/azure-linux-dev-tools/internal/app/azldev" + "github.com/microsoft/azure-linux-dev-tools/internal/app/azldev/core/components" + "github.com/microsoft/azure-linux-dev-tools/internal/projectconfig" + "github.com/spf13/cobra" +) + +// OverlaysOptions configures the 'component overlays' subcommand. +type OverlaysOptions struct { + // ComponentFilter selects which components to inspect. + ComponentFilter components.ComponentFilter + + // Category, when non-empty, filters output to overlays whose metadata declares this + // category. Overlays with no metadata are excluded when this is set. + Category string + + // OnlyAnnotated, when true, excludes overlays with no metadata. + OnlyAnnotated bool + + // Upstreamability, when non-empty, filters output to overlays whose metadata declares + // this upstreamability ('yes', 'no', or 'unknown'). Overlays with no metadata count as + // 'unknown'. + Upstreamability string +} + +func overlaysOnAppInit(_ *azldev.App, parentCmd *cobra.Command) { + parentCmd.AddCommand(NewComponentOverlaysCommand()) +} + +// NewComponentOverlaysCommand constructs the 'component overlays' subcommand. +func NewComponentOverlaysCommand() *cobra.Command { + options := &OverlaysOptions{} + + cmd := &cobra.Command{ + Use: "overlays", + Short: "List overlays and their metadata for components in this project", + Long: `List overlays defined on components in the project configuration, including any +documentation metadata (category, commits, upstream PR, bug links, etc.) attached +to each overlay. Overlays loaded from a component's overlay-dir (per-file +.overlay.toml format) inherit their metadata from the file's [metadata] block. + +This command is read-only and does not parse spec files or fetch upstream sources, +so it is fast and works even when locks are missing or stale.`, + Example: ` # List overlays for all components + azldev component overlays -a + + # List overlays for one component + azldev component overlays -p curl + + # List only overlays carrying documentation metadata + azldev component overlays -a --only-annotated + + # Filter by category + azldev component overlays -a --category backport-fedora + + # List only overlays that can be upstreamed + azldev component overlays -a --upstreamability yes + + # JSON output for scripting + azldev component overlays -a -q -O json`, + RunE: azldev.RunFuncWithExtraArgs(func(env *azldev.Env, args []string) (interface{}, error) { + options.ComponentFilter.ComponentNamePatterns = append(args, options.ComponentFilter.ComponentNamePatterns...) + + return ListOverlays(env, options) + }), + ValidArgsFunction: components.GenerateComponentNameCompletions, + } + + azldev.ExportAsMCPTool(cmd) + + components.AddComponentFilterOptionsToCommand(cmd, &options.ComponentFilter) + + cmd.Flags().StringVar(&options.Category, "category", "", + "only include overlays whose metadata declares this category") + cmd.Flags().BoolVar(&options.OnlyAnnotated, "only-annotated", false, + "exclude overlays that have no metadata") + cmd.Flags().StringVar(&options.Upstreamability, "upstreamability", "", + "only include overlays whose metadata declares this upstreamability ('yes', 'no', or 'unknown')") + + // This command is read-only; lock validation is irrelevant. + _ = cmd.Flags().MarkHidden("skip-lock-validation") + + return cmd +} + +// OverlayInfo is the per-overlay output for the 'component overlays' subcommand. +type OverlayInfo struct { + // Component is the name of the component owning the overlay; used as the table sort key. + Component string `json:"component" table:",sortkey"` + + // Index is the 1-based position of the overlay within the component's overlay list. + Index int `json:"index"` + + // Type is the overlay type (e.g. 'spec-set-tag', 'patch-add'). + Type projectconfig.ComponentOverlayType `json:"type"` + + // Description is the overlay's top-level human-readable description. + Description string `json:"description,omitempty" table:",omitempty"` + + // Category surfaces [OverlayMetadata.Category] for tabular output without forcing + // callers to drill into [OverlayInfo.Metadata]. Empty when the overlay has no metadata. + Category projectconfig.OverlayCategory `json:"category,omitempty" table:",omitempty"` + + // Upstreamability surfaces [OverlayMetadata.Upstreamability] for tabular output. Always + // populated: overlays without metadata (or with the field omitted) report 'unknown', so + // this is never empty and is always rendered. + Upstreamability projectconfig.OverlayUpstreamability `json:"upstreamability"` + + // Metadata is the full metadata for the overlay. Nil when the overlay has none. + Metadata *projectconfig.OverlayMetadata `json:"metadata,omitempty" table:"-"` +} + +// ListOverlays returns one [OverlayInfo] per matching overlay across the selected +// components, applying the filters in options. Lock validation is always skipped — +// this command is read-only. +func ListOverlays(env *azldev.Env, options *OverlaysOptions) ([]OverlayInfo, error) { + if options.Category != "" && !projectconfig.OverlayCategory(options.Category).IsValid() { + return nil, fmt.Errorf("%w: unknown overlay category %#q", azldev.ErrInvalidUsage, options.Category) + } + + if _, err := normalizeUpstreamabilityFilter(options.Upstreamability); err != nil { + return nil, err + } + + options.ComponentFilter.SkipLockValidation = true + + resolver := components.NewResolver(env) + + comps, err := resolver.FindComponents(&options.ComponentFilter) + if err != nil { + return nil, fmt.Errorf("failed to resolve components:\n%w", err) + } + + results := make([]OverlayInfo, 0) + + for _, comp := range comps.Components() { + config := comp.GetConfig() + + for idx, overlay := range config.Overlays { + info := buildOverlayInfo(comp.GetName(), idx+1, &overlay) + + if !overlayInfoMatchesFilters(info, options) { + continue + } + + results = append(results, info) + } + } + + slices.SortStableFunc(results, func(a, b OverlayInfo) int { + if c := strings.Compare(a.Component, b.Component); c != 0 { + return c + } + + return a.Index - b.Index + }) + + return results, nil +} + +// buildOverlayInfo constructs a single [OverlayInfo] for an overlay. +func buildOverlayInfo( + componentName string, idx int, overlay *projectconfig.ComponentOverlay, +) OverlayInfo { + info := OverlayInfo{ + Component: componentName, + Index: idx, + Type: overlay.Type, + Description: overlay.Description, + Metadata: overlay.Metadata, + Upstreamability: projectconfig.OverlayUpstreamabilityUnknown, + } + + if overlay.Metadata != nil { + info.Category = overlay.Metadata.Category + if u := overlay.Metadata.Upstreamability; u != "" { + info.Upstreamability = u + } + } + + return info +} + +// normalizeUpstreamabilityFilter validates a user-supplied '--upstreamability' filter +// value. An empty string means no filter was requested. Only 'yes', 'no', and 'unknown' +// are accepted; any other value yields an [azldev.ErrInvalidUsage] error. +func normalizeUpstreamabilityFilter(value string) (projectconfig.OverlayUpstreamability, error) { + switch value { + case "": + return "", nil + case string(projectconfig.OverlayUpstreamabilityUnknown): + return projectconfig.OverlayUpstreamabilityUnknown, nil + case string(projectconfig.OverlayUpstreamabilityYes): + return projectconfig.OverlayUpstreamabilityYes, nil + case string(projectconfig.OverlayUpstreamabilityNo): + return projectconfig.OverlayUpstreamabilityNo, nil + default: + return "", fmt.Errorf( + "%w: unknown upstreamability %#q; want 'yes', 'no', or 'unknown'", + azldev.ErrInvalidUsage, value, + ) + } +} + +// overlayInfoMatchesFilters reports whether an [OverlayInfo] passes the user-supplied +// category, only-annotated, and upstreamability filters. +func overlayInfoMatchesFilters(info OverlayInfo, options *OverlaysOptions) bool { + if options.OnlyAnnotated && info.Metadata == nil { + return false + } + + if options.Category != "" && string(info.Category) != options.Category { + return false + } + + if options.Upstreamability != "" { + // The filter value is validated upfront in ListOverlays, so the error is unreachable here. + want, _ := normalizeUpstreamabilityFilter(options.Upstreamability) + if info.Upstreamability != want { + return false + } + } + + return true +} diff --git a/internal/app/azldev/cmds/component/overlays_test.go b/internal/app/azldev/cmds/component/overlays_test.go new file mode 100644 index 00000000..bda8f689 --- /dev/null +++ b/internal/app/azldev/cmds/component/overlays_test.go @@ -0,0 +1,187 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +package component_test + +import ( + "testing" + + "github.com/microsoft/azure-linux-dev-tools/internal/app/azldev/cmds/component" + "github.com/microsoft/azure-linux-dev-tools/internal/app/azldev/core/components" + "github.com/microsoft/azure-linux-dev-tools/internal/app/azldev/core/testutils" + "github.com/microsoft/azure-linux-dev-tools/internal/projectconfig" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestNewComponentOverlaysCommand(t *testing.T) { + cmd := component.NewComponentOverlaysCommand() + require.NotNil(t, cmd) + assert.Equal(t, "overlays", cmd.Use) +} + +// seedComponentsWithOverlays adds two components with a mix of annotated and +// unannotated overlays. +func seedComponentsWithOverlays(t *testing.T, testEnv *testutils.TestEnv) { + t.Helper() + + testEnv.Config.Components["pkg-a"] = projectconfig.ComponentConfig{ + Name: "pkg-a", + Spec: projectconfig.SpecSource{Path: "/specs/pkg-a.spec"}, + Overlays: []projectconfig.ComponentOverlay{ + { + Type: projectconfig.ComponentOverlayAddSpecTag, + Tag: "Vendor", + Value: "Microsoft", + Metadata: &projectconfig.OverlayMetadata{ + Category: projectconfig.OverlayCategoryAZLBrandingPolicy, + }, + }, + { + Type: projectconfig.ComponentOverlayAddPatch, + Source: "patches/fix.patch", + Metadata: &projectconfig.OverlayMetadata{ + Category: projectconfig.OverlayCategoryBackportFedora, + Commits: []string{"https://src.fedoraproject.org/rpms/pkg-a/c/abc"}, + FixedIn: "1.2.3", + }, + }, + { + // No metadata. + Type: projectconfig.ComponentOverlaySearchAndReplaceInSpec, + Regex: "foo", + Replacement: "bar", + }, + }, + } + + testEnv.Config.Components["pkg-b"] = projectconfig.ComponentConfig{ + Name: "pkg-b", + Spec: projectconfig.SpecSource{Path: "/specs/pkg-b.spec"}, + Overlays: []projectconfig.ComponentOverlay{ + { + Type: projectconfig.ComponentOverlayAddSpecTag, + Tag: "Packager", + Value: "azl", + Metadata: &projectconfig.OverlayMetadata{ + Category: projectconfig.OverlayCategoryUpstreamFix, + PR: "https://github.com/example/repo/pull/1", + Upstreamability: projectconfig.OverlayUpstreamabilityYes, + }, + }, + }, + } +} + +func TestListOverlays_AllComponents(t *testing.T) { + testEnv := testutils.NewTestEnv(t) + seedComponentsWithOverlays(t, testEnv) + + options := &component.OverlaysOptions{ + ComponentFilter: components.ComponentFilter{IncludeAllComponents: true}, + } + + results, err := component.ListOverlays(testEnv.Env, options) + require.NoError(t, err) + require.Len(t, results, 4) + + // Sorted by (component, index). + assert.Equal(t, "pkg-a", results[0].Component) + assert.Equal(t, 1, results[0].Index) + assert.Equal(t, projectconfig.OverlayCategoryAZLBrandingPolicy, results[0].Category) + + assert.Equal(t, "pkg-a", results[1].Component) + assert.Equal(t, 2, results[1].Index) + assert.Equal(t, projectconfig.OverlayCategoryBackportFedora, results[1].Category) + require.NotNil(t, results[1].Metadata) + assert.Equal(t, []string{"https://src.fedoraproject.org/rpms/pkg-a/c/abc"}, results[1].Metadata.Commits) + assert.Equal(t, "1.2.3", results[1].Metadata.FixedIn) + + assert.Equal(t, "pkg-a", results[2].Component) + assert.Equal(t, 3, results[2].Index) + assert.Nil(t, results[2].Metadata, "overlay without metadata reports nil") + assert.Empty(t, results[2].Category) + + assert.Equal(t, "pkg-b", results[3].Component) + assert.Equal(t, projectconfig.OverlayCategoryUpstreamFix, results[3].Category) +} + +func TestListOverlays_OnlyAnnotated(t *testing.T) { + testEnv := testutils.NewTestEnv(t) + seedComponentsWithOverlays(t, testEnv) + + options := &component.OverlaysOptions{ + ComponentFilter: components.ComponentFilter{IncludeAllComponents: true}, + OnlyAnnotated: true, + } + + results, err := component.ListOverlays(testEnv.Env, options) + require.NoError(t, err) + require.Len(t, results, 3, "unannotated search-replace overlay must be excluded") + + for _, info := range results { + assert.NotNil(t, info.Metadata) + assert.NotEmpty(t, info.Category) + } +} + +func TestListOverlays_FilterByCategory(t *testing.T) { + testEnv := testutils.NewTestEnv(t) + seedComponentsWithOverlays(t, testEnv) + + options := &component.OverlaysOptions{ + ComponentFilter: components.ComponentFilter{IncludeAllComponents: true}, + Category: string(projectconfig.OverlayCategoryBackportFedora), + } + + results, err := component.ListOverlays(testEnv.Env, options) + require.NoError(t, err) + require.Len(t, results, 1) + assert.Equal(t, "pkg-a", results[0].Component) + assert.Equal(t, 2, results[0].Index) +} + +func TestListOverlays_UnknownCategoryRejected(t *testing.T) { + testEnv := testutils.NewTestEnv(t) + seedComponentsWithOverlays(t, testEnv) + + options := &component.OverlaysOptions{ + ComponentFilter: components.ComponentFilter{IncludeAllComponents: true}, + Category: "bogus", + } + + _, err := component.ListOverlays(testEnv.Env, options) + require.Error(t, err) + assert.Contains(t, err.Error(), "unknown overlay category") +} + +func TestListOverlays_Upstreamable(t *testing.T) { + testEnv := testutils.NewTestEnv(t) + seedComponentsWithOverlays(t, testEnv) + + options := &component.OverlaysOptions{ + ComponentFilter: components.ComponentFilter{IncludeAllComponents: true}, + Upstreamability: "yes", + } + + results, err := component.ListOverlays(testEnv.Env, options) + require.NoError(t, err) + require.Len(t, results, 1, "only the upstreamable overlay in pkg-b should be included") + assert.Equal(t, "pkg-b", results[0].Component) + assert.Equal(t, projectconfig.OverlayUpstreamabilityYes, results[0].Upstreamability) + assert.Equal(t, projectconfig.OverlayCategoryUpstreamFix, results[0].Category) +} + +func TestListOverlays_UnknownUpstreamabilityRejected(t *testing.T) { + testEnv := testutils.NewTestEnv(t) + seedComponentsWithOverlays(t, testEnv) + + options := &component.OverlaysOptions{ + ComponentFilter: components.ComponentFilter{IncludeAllComponents: true}, + Upstreamability: "maybe", + } + + _, err := component.ListOverlays(testEnv.Env, options) + require.Error(t, err) + assert.Contains(t, err.Error(), "unknown upstreamability") +} diff --git a/internal/projectconfig/component.go b/internal/projectconfig/component.go index 9d45a2b4..f674592b 100644 --- a/internal/projectconfig/component.go +++ b/internal/projectconfig/component.go @@ -273,6 +273,15 @@ type ComponentConfig struct { // Overlays to apply to sources after they've been acquired. May mutate the spec as well as sources. Overlays []ComponentOverlay `toml:"overlays,omitempty" json:"overlays,omitempty" table:"-" jsonschema:"title=Overlays,description=Overlays to apply to this component's spec and/or sources"` + // OverlayDir, if set, names a directory (relative to this component config file) that + // is scanned for `*.overlay.toml` files at load time. Each file represents one logical + // change: a file-level `[metadata]` block plus an ordered list of `[[overlays]]`. The + // per-file metadata is stamped onto every overlay in the file and the overlays are + // appended to [ComponentConfig.Overlays] in filename order (which is why a `0001-`, + // `0002-` numeric prefix convention is recommended). Excluded from the fingerprint + // because the value affects only where overlays are sourced from, not their content. + OverlayDir string `toml:"overlay-dir,omitempty" json:"overlayDir,omitempty" table:"-" jsonschema:"title=Overlay directory,description=Directory (relative to the component config file) scanned for *.overlay.toml files at load time" fingerprint:"-"` + // Configuration for building the component. Build ComponentBuildConfig `toml:"build,omitempty" json:"build,omitempty" table:"-" jsonschema:"title=Build configuration,description=Configuration for building the component"` @@ -383,6 +392,10 @@ func (c *ComponentConfig) WithAbsolutePaths(referenceDir string) *ComponentConfi SourceFiles: deep.MustCopy(c.SourceFiles), Packages: deep.MustCopy(c.Packages), Publish: deep.MustCopy(c.Publish), + // OverlayDir is consumed at load time (see applyOverlayDirs) before paths are + // absolutized; nothing reads it afterward, so preserve the original value verbatim + // rather than redundantly re-rooting it. + OverlayDir: c.OverlayDir, } // Fix up paths. diff --git a/internal/projectconfig/fingerprint_test.go b/internal/projectconfig/fingerprint_test.go index 3bd14eff..80b1cb44 100644 --- a/internal/projectconfig/fingerprint_test.go +++ b/internal/projectconfig/fingerprint_test.go @@ -70,6 +70,12 @@ func TestAllFingerprintedFieldsHaveDecision(t *testing.T) { // ComponentOverlay.Source — absolute path that varies by checkout location. // Overlay content is hashed separately by ComputeIdentity. "ComponentOverlay.Source": true, + // ComponentOverlay.Metadata — documentation describing overlay intent and provenance. + "ComponentOverlay.Metadata": true, + + // ComponentConfig.OverlayDir — affects only where overlays are sourced from, not + // their content; the resulting overlays are fingerprinted normally. + "ComponentConfig.OverlayDir": true, // SourceFileReference.Component — back-reference to parent, not a build input. "SourceFileReference.Component": true, diff --git a/internal/projectconfig/loader.go b/internal/projectconfig/loader.go index 31971f90..df491b35 100644 --- a/internal/projectconfig/loader.go +++ b/internal/projectconfig/loader.go @@ -433,6 +433,13 @@ func loadProjectConfigFile( cfg.sourcePath = absFilePath cfg.dir = filepath.Dir(absFilePath) + // Resolve per-component overlay directories (`.overlay.toml` files), stamping each + // file's [metadata] onto its overlays and appending them to the component before + // validation runs. + if err := applyOverlayDirs(fs, cfg, permissiveConfigParsing); err != nil { + return nil, err + } + // Make sure that the read data is internally consistent. err = cfg.Validate() if err != nil { diff --git a/internal/projectconfig/overlay.go b/internal/projectconfig/overlay.go index 7ec0b50e..eee65d7e 100644 --- a/internal/projectconfig/overlay.go +++ b/internal/projectconfig/overlay.go @@ -43,6 +43,13 @@ type ComponentOverlay struct { // Excluded from fingerprint because it contains an absolute path that varies by checkout // location. Overlay content is hashed separately by [fingerprint.ComputeIdentity]. Source string `toml:"source,omitempty" json:"source,omitempty" jsonschema:"title=Source,description=For overlays that require a source file as input, indicates a path to that file; relative paths are relative to the config file that defines the overlay" fingerprint:"-"` + + // Metadata describes the intent and provenance of the overlay (category, related + // commits/PR, bug links, etc.). Optional. Populated either inline in the component + // config file or by the [ComponentConfig.OverlayDir] loader, which stamps the + // per-file metadata onto every overlay declared in that file. Excluded from the + // fingerprint because it is documentation only. + Metadata *OverlayMetadata `toml:"metadata,omitempty" json:"metadata,omitempty" jsonschema:"title=Metadata,description=Optional documentation metadata describing the overlay's intent and provenance" fingerprint:"-"` } // EffectiveSourceName returns the checkout-independent identity of the overlay's @@ -333,6 +340,12 @@ func (c *ComponentOverlay) Validate() error { return fmt.Errorf("unknown overlay type %#q: %#q", c.Type, desc) } + if c.Metadata != nil { + if err := c.Metadata.Validate(); err != nil { + return fmt.Errorf("invalid metadata for overlay %q:\n%w", desc, err) + } + } + return nil } diff --git a/internal/projectconfig/overlay_file.go b/internal/projectconfig/overlay_file.go new file mode 100644 index 00000000..8f90003b --- /dev/null +++ b/internal/projectconfig/overlay_file.go @@ -0,0 +1,157 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +package projectconfig + +import ( + "errors" + "fmt" + "path" + "path/filepath" + "sort" + + "github.com/bmatcuk/doublestar/v4" + "github.com/brunoga/deep" + "github.com/microsoft/azure-linux-dev-tools/internal/global/opctx" + "github.com/microsoft/azure-linux-dev-tools/internal/utils/fileutils" + "github.com/pelletier/go-toml/v2" +) + +// ErrOverlayFilePerOverlayMetadata is returned when a `.overlay.toml` file declares +// `metadata` on an individual `[[overlays]]` entry. Each file represents one logical +// change; the file-level `[metadata]` block is the single source of truth and is +// stamped onto every overlay in the file at load time. +var ErrOverlayFilePerOverlayMetadata = errors.New( + "per-overlay metadata is not allowed inside a .overlay.toml file; declare metadata once at the file level", +) + +// ErrOverlayFileEmpty is returned when a `.overlay.toml` file decodes to zero +// overlays. Such a file is almost certainly a typo (e.g. `[overlays]` instead of +// `[[overlays]]`) and would otherwise silently contribute nothing. +var ErrOverlayFileEmpty = errors.New("overlay file declares no overlays") + +// overlayFileSuffix is the required filename suffix for files scanned from a +// component's [ComponentConfig.OverlayDir]. +const overlayFileSuffix = "*.overlay.toml" + +// OverlayFile is the on-disk representation of a single `.overlay.toml` document. Each +// file represents one logical change: the file-level [OverlayFile.Metadata] is applied +// to every overlay in [OverlayFile.Overlays] at load time, after which the resulting +// overlays are appended to the owning component's [ComponentConfig.Overlays] slice. +type OverlayFile struct { + // Metadata is the shared metadata for every overlay in this file. Required. + Metadata OverlayMetadata `toml:"metadata"` + + // Overlays is the ordered list of overlays applied by this file. Must be non-empty. + // Per-overlay `metadata` is not allowed — the file-level [OverlayFile.Metadata] is + // the single source of truth. + Overlays []ComponentOverlay `toml:"overlays"` +} + +// applyOverlayDirs scans each component's [ComponentConfig.OverlayDir] (when set), +// parses every `*.overlay.toml` file in deterministic (filename) order, stamps the +// per-file metadata onto each overlay, and appends the resulting overlays to the +// component's [ComponentConfig.Overlays] slice. Inline overlays declared directly on +// the component come first; file-sourced overlays are appended after them. +// +// Called from [loadProjectConfigFile] after TOML decode but before [ConfigFile.Validate], +// so all per-overlay validation rules apply uniformly regardless of declaration site. +func applyOverlayDirs(fs opctx.FS, cfg *ConfigFile, permissiveConfigParsing bool) error { + for componentName, component := range cfg.Components { + if component.OverlayDir == "" { + continue + } + + absDir := makeAbsolute(cfg.dir, component.OverlayDir) + + loaded, err := loadOverlayDir(fs, absDir, permissiveConfigParsing) + if err != nil { + return fmt.Errorf("component %#q overlay-dir %q:\n%w", componentName, component.OverlayDir, err) + } + + component.Overlays = append(component.Overlays, loaded...) + cfg.Components[componentName] = component + } + + return nil +} + +// loadOverlayDir parses every `*.overlay.toml` file in absDir (sorted by filename), +// validates each file's metadata, stamps the file metadata onto each overlay, and +// resolves overlay `source` paths relative to the overlay file. +func loadOverlayDir( + fs opctx.FS, absDir string, permissiveConfigParsing bool, +) ([]ComponentOverlay, error) { + pattern := path.Join(absDir, overlayFileSuffix) + + matches, err := fileutils.Glob( + fs, pattern, + doublestar.WithFailOnIOErrors(), + doublestar.WithFilesOnly(), + ) + if err != nil { + return nil, fmt.Errorf("failed to scan for overlay files:\n%w", err) + } + + sort.Strings(matches) + + var result []ComponentOverlay + + for _, overlayPath := range matches { + fromFile, err := loadOverlayFile(fs, overlayPath, permissiveConfigParsing) + if err != nil { + return nil, err + } + + result = append(result, fromFile...) + } + + return result, nil +} + +// loadOverlayFile parses a single `.overlay.toml` file, validates its metadata, +// stamps the metadata onto each overlay, and absolutizes per-overlay `source` +// paths relative to the file's directory. +func loadOverlayFile( + fs opctx.FS, overlayPath string, permissiveConfigParsing bool, +) ([]ComponentOverlay, error) { + file, err := fs.Open(overlayPath) + if err != nil { + return nil, fmt.Errorf("failed to open overlay file %q:\n%w", overlayPath, err) + } + defer file.Close() + + decoder := toml.NewDecoder(file) + + if !permissiveConfigParsing { + decoder.DisallowUnknownFields() + } + + var ofile OverlayFile + if err := decoder.Decode(&ofile); err != nil { + return nil, fmt.Errorf("failed to parse overlay file %q:\n%w", overlayPath, err) + } + + if err := ofile.Metadata.Validate(); err != nil { + return nil, fmt.Errorf("invalid [metadata] in overlay file %q:\n%w", overlayPath, err) + } + + if len(ofile.Overlays) == 0 { + return nil, fmt.Errorf("%w: %q", ErrOverlayFileEmpty, overlayPath) + } + + overlayDir := filepath.Dir(overlayPath) + + for idx := range ofile.Overlays { + overlay := &ofile.Overlays[idx] + + if overlay.Metadata != nil { + return nil, fmt.Errorf("%w (overlay %d in %q)", ErrOverlayFilePerOverlayMetadata, idx+1, overlayPath) + } + + overlay.Metadata = deep.MustCopy(&ofile.Metadata) + overlay.Source = makeAbsolute(overlayDir, overlay.Source) + } + + return ofile.Overlays, nil +} diff --git a/internal/projectconfig/overlay_file_test.go b/internal/projectconfig/overlay_file_test.go new file mode 100644 index 00000000..ed48a4f7 --- /dev/null +++ b/internal/projectconfig/overlay_file_test.go @@ -0,0 +1,291 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +//nolint:testpackage // Allow to test private functions (i.e., applyOverlayDirs). +package projectconfig + +import ( + "path/filepath" + "testing" + + "github.com/microsoft/azure-linux-dev-tools/internal/global/testctx" + "github.com/microsoft/azure-linux-dev-tools/internal/utils/fileperms" + "github.com/microsoft/azure-linux-dev-tools/internal/utils/fileutils" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +const validBackportOverlayFile = ` +[metadata] +category = "backport-fedora" +commits = ["https://src.fedoraproject.org/rpms/ant/c/4ca7a3b"] + +[[overlays]] +description = "Remove openjdk21 binding" +type = "spec-search-replace" +section = "%install" +regex = ".*openjdk21.*" + +[[overlays]] +type = "spec-remove-subpackage" +package = "openjdk21" +` + +const badOverlayTestDir = "/project/comps/x/overlays" + +func TestLoadOverlayDir_StampsMetadataAndPreservesOrder(t *testing.T) { + ctx := testctx.NewCtx() + overlayDir := "/project/comps/ant/overlays" + + // Files are intentionally out of glob order to confirm filename-sort. + require.NoError(t, fileutils.WriteFile(ctx.FS(), + filepath.Join(overlayDir, "0002-azl-branding.overlay.toml"), + []byte(` +[metadata] +category = "azl-branding-policy" + +[[overlays]] +type = "spec-set-tag" +tag = "Vendor" +value = "Microsoft" +`), fileperms.PrivateFile)) + + require.NoError(t, fileutils.WriteFile(ctx.FS(), + filepath.Join(overlayDir, "0001-backport-drop-openjdk21.overlay.toml"), + []byte(validBackportOverlayFile), fileperms.PrivateFile)) + + loaded, err := loadOverlayDir(ctx.FS(), overlayDir, false) + require.NoError(t, err) + require.Len(t, loaded, 3, "two overlays from 0001 + one from 0002") + + // 0001-* contributes first. + assert.Equal(t, ComponentOverlaySearchAndReplaceInSpec, loaded[0].Type) + require.NotNil(t, loaded[0].Metadata) + assert.Equal(t, OverlayCategoryBackportFedora, loaded[0].Metadata.Category) + + assert.Equal(t, ComponentOverlayRemoveSubpackage, loaded[1].Type) + require.NotNil(t, loaded[1].Metadata) + assert.Equal(t, OverlayCategoryBackportFedora, loaded[1].Metadata.Category) + + // Mutating one overlay's metadata must not affect another (each must own a copy). + loaded[0].Metadata.Commits = append(loaded[0].Metadata.Commits, "mutated") + assert.Len(t, loaded[1].Metadata.Commits, 1, "each overlay must own its metadata copy") + + // 0002-* comes second. + assert.Equal(t, ComponentOverlaySetSpecTag, loaded[2].Type) + require.NotNil(t, loaded[2].Metadata) + assert.Equal(t, OverlayCategoryAZLBrandingPolicy, loaded[2].Metadata.Category) +} + +func TestLoadOverlayDir_EmptyDirReturnsNothing(t *testing.T) { + ctx := testctx.NewCtx() + overlayDir := "/project/comps/empty/overlays" + + // A non-overlay file alongside must not be picked up. + require.NoError(t, fileutils.WriteFile(ctx.FS(), + filepath.Join(overlayDir, "README.md"), + []byte("not an overlay"), fileperms.PrivateFile)) + + loaded, err := loadOverlayDir(ctx.FS(), overlayDir, false) + require.NoError(t, err) + assert.Empty(t, loaded) +} + +func TestLoadOverlayDir_RejectsPerOverlayMetadata(t *testing.T) { + ctx := testctx.NewCtx() + overlayDir := badOverlayTestDir + + require.NoError(t, fileutils.WriteFile(ctx.FS(), + filepath.Join(overlayDir, "0001-bad.overlay.toml"), + []byte(` +[metadata] +category = "azl-build" + +[[overlays]] +type = "spec-set-tag" +tag = "Vendor" +value = "Microsoft" +metadata = { category = "azl-branding-policy" } +`), fileperms.PrivateFile)) + + _, err := loadOverlayDir(ctx.FS(), overlayDir, false) + require.ErrorIs(t, err, ErrOverlayFilePerOverlayMetadata) +} + +func TestLoadOverlayDir_RejectsEmptyFile(t *testing.T) { + ctx := testctx.NewCtx() + overlayDir := badOverlayTestDir + + require.NoError(t, fileutils.WriteFile(ctx.FS(), + filepath.Join(overlayDir, "0001-empty.overlay.toml"), + []byte(` +[metadata] +category = "azl-build" +`), fileperms.PrivateFile)) + + _, err := loadOverlayDir(ctx.FS(), overlayDir, false) + require.ErrorIs(t, err, ErrOverlayFileEmpty) +} + +func TestLoadOverlayDir_RejectsInvalidMetadata(t *testing.T) { + ctx := testctx.NewCtx() + overlayDir := badOverlayTestDir + + require.NoError(t, fileutils.WriteFile(ctx.FS(), + filepath.Join(overlayDir, "0001-bad.overlay.toml"), + []byte(` +[metadata] +# Missing category. +commits = ["abc"] + +[[overlays]] +type = "spec-set-tag" +tag = "Vendor" +value = "Microsoft" +`), fileperms.PrivateFile)) + + _, err := loadOverlayDir(ctx.FS(), overlayDir, false) + require.Error(t, err) + assert.Contains(t, err.Error(), "category") +} + +func TestLoadOverlayDir_ResolvesSourceRelativeToOverlayFile(t *testing.T) { + ctx := testctx.NewCtx() + overlayDir := "/project/comps/foo/overlays" + + require.NoError(t, fileutils.WriteFile(ctx.FS(), + filepath.Join(overlayDir, "0001-patch.overlay.toml"), + []byte(` +[metadata] +category = "backport-fedora" +commits = ["https://src.fedoraproject.org/rpms/foo/c/abc"] + +[[overlays]] +type = "patch-add" +source = "patches/fix.patch" +`), fileperms.PrivateFile)) + + loaded, err := loadOverlayDir(ctx.FS(), overlayDir, false) + require.NoError(t, err) + require.Len(t, loaded, 1) + assert.Equal(t, + filepath.Join(overlayDir, "patches/fix.patch"), + loaded[0].Source, + "source paths in .overlay.toml resolve relative to the overlay file") +} + +func TestApplyOverlayDirs_AppendsAfterInlineOverlays(t *testing.T) { + ctx := testctx.NewCtx() + overlayDir := "/project/comps/ant/overlays" + + require.NoError(t, fileutils.WriteFile(ctx.FS(), + filepath.Join(overlayDir, "0001-backport.overlay.toml"), + []byte(validBackportOverlayFile), fileperms.PrivateFile)) + + cfg := &ConfigFile{ + dir: "/project", + Components: map[string]ComponentConfig{ + "ant": { + OverlayDir: "comps/ant/overlays", + Overlays: []ComponentOverlay{ + { + Type: ComponentOverlaySetSpecTag, + Tag: "Vendor", + Value: "Microsoft", + Metadata: &OverlayMetadata{ + Category: OverlayCategoryAZLBrandingPolicy, + }, + }, + }, + }, + }, + } + + require.NoError(t, applyOverlayDirs(ctx.FS(), cfg, false)) + + ant := cfg.Components["ant"] + require.Len(t, ant.Overlays, 3, "inline overlay + two file-sourced overlays") + + // Inline first. + assert.Equal(t, ComponentOverlaySetSpecTag, ant.Overlays[0].Type) + assert.Equal(t, OverlayCategoryAZLBrandingPolicy, ant.Overlays[0].Metadata.Category) + + // File-sourced overlays appended in file/declaration order, with the file's + // metadata stamped onto each. + assert.Equal(t, ComponentOverlaySearchAndReplaceInSpec, ant.Overlays[1].Type) + require.NotNil(t, ant.Overlays[1].Metadata) + assert.Equal(t, OverlayCategoryBackportFedora, ant.Overlays[1].Metadata.Category) + + assert.Equal(t, ComponentOverlayRemoveSubpackage, ant.Overlays[2].Type) + require.NotNil(t, ant.Overlays[2].Metadata) + assert.Equal(t, OverlayCategoryBackportFedora, ant.Overlays[2].Metadata.Category) +} + +func TestApplyOverlayDirs_NoopWhenOverlayDirUnset(t *testing.T) { + ctx := testctx.NewCtx() + + cfg := &ConfigFile{ + dir: "/project", + Components: map[string]ComponentConfig{ + "ant": { + Overlays: []ComponentOverlay{ + {Type: ComponentOverlayAddSpecTag, Tag: "Vendor", Value: "Microsoft"}, + }, + }, + }, + } + + require.NoError(t, applyOverlayDirs(ctx.FS(), cfg, false)) + + ant := cfg.Components["ant"] + require.Len(t, ant.Overlays, 1, "no overlay-dir, no merging") +} + +func TestApplyOverlayDirs_AcceptsAbsoluteOverlayDir(t *testing.T) { + ctx := testctx.NewCtx() + absDir := "/elsewhere/overlays" + + require.NoError(t, fileutils.WriteFile(ctx.FS(), + filepath.Join(absDir, "0001-backport.overlay.toml"), + []byte(validBackportOverlayFile), fileperms.PrivateFile)) + + cfg := &ConfigFile{ + dir: "/project", + Components: map[string]ComponentConfig{ + "ant": {OverlayDir: absDir}, + }, + } + + require.NoError(t, applyOverlayDirs(ctx.FS(), cfg, false)) + + ant := cfg.Components["ant"] + require.Len(t, ant.Overlays, 2, "absolute overlay-dir is not re-rooted under cfg.dir") + require.NotNil(t, ant.Overlays[0].Metadata) + assert.Equal(t, OverlayCategoryBackportFedora, ant.Overlays[0].Metadata.Category) +} + +// TestLoadAndResolveProjectConfig_OverlayDir exercises the full loader pipeline +// (loadAndResolveProjectConfig -> loadProjectConfigFile -> applyOverlayDirs) and +// guards against regressions that drop the overlay-dir hook from the loader. +func TestLoadAndResolveProjectConfig_OverlayDir(t *testing.T) { + ctx := testctx.NewCtx() + + require.NoError(t, fileutils.WriteFile(ctx.FS(), testConfigPath, []byte(` +[components.ant] +overlay-dir = "comps/ant/overlays" +`), fileperms.PrivateFile)) + + require.NoError(t, fileutils.WriteFile(ctx.FS(), + "/project/comps/ant/overlays/0001-backport.overlay.toml", + []byte(validBackportOverlayFile), fileperms.PrivateFile)) + + cfg, err := loadAndResolveProjectConfig(ctx.FS(), false, testConfigPath) + require.NoError(t, err) + require.NotNil(t, cfg) + + ant, ok := cfg.Components["ant"] + require.True(t, ok, "ant component should be present") + require.Len(t, ant.Overlays, 2) + require.NotNil(t, ant.Overlays[0].Metadata) + assert.Equal(t, OverlayCategoryBackportFedora, ant.Overlays[0].Metadata.Category) +} diff --git a/internal/projectconfig/overlay_metadata.go b/internal/projectconfig/overlay_metadata.go new file mode 100644 index 00000000..eb5b670d --- /dev/null +++ b/internal/projectconfig/overlay_metadata.go @@ -0,0 +1,168 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +package projectconfig + +import ( + "errors" + "fmt" + "slices" + + "github.com/go-playground/validator/v10" +) + +// OverlayCategory is a classification label for an overlay's intent. Categories help +// reviewers and tooling reason about why an overlay exists and when it can be removed. +type OverlayCategory string + +const ( + // OverlayCategoryBackportFedora applies a fix that already exists in some Fedora branch. + // The overlay can be dropped once AZL bumps its upstream pin past the fix. + OverlayCategoryBackportFedora OverlayCategory = "backport-fedora" + // OverlayCategoryUpstreamFix applies a fix that is NOT yet in any Fedora branch and is + // a candidate for upstreaming. + OverlayCategoryUpstreamFix OverlayCategory = "upstream-fix" + // OverlayCategoryAZLDependencyPruning removes dependencies that are not shipped in AZL. + OverlayCategoryAZLDependencyPruning OverlayCategory = "azl-dependency-pruning" + // OverlayCategoryAZLFeatureDisablement disables unneeded features or subpackages. + OverlayCategoryAZLFeatureDisablement OverlayCategory = "azl-feature-disablement" + // OverlayCategoryAZLBrandingPolicy applies Fedora→AzureLinux name/path changes or + // RHEL/enterprise convention alignment. + OverlayCategoryAZLBrandingPolicy OverlayCategory = "azl-branding-policy" + // OverlayCategoryAZLBuild adjusts toolchain, mock, or CI environment specifics. + OverlayCategoryAZLBuild OverlayCategory = "azl-build" + // OverlayCategoryAZLTestDisablement skips failing tests. + OverlayCategoryAZLTestDisablement OverlayCategory = "azl-test-disablement" + // OverlayCategoryAZLSecurityCompliance applies FIPS or crypto policy changes. + OverlayCategoryAZLSecurityCompliance OverlayCategory = "azl-security-compliance" + // OverlayCategoryAZLReleaseManagement adjusts release tag and changelog mechanics. + OverlayCategoryAZLReleaseManagement OverlayCategory = "azl-release-management" + // OverlayCategoryAZLMissingDependencyWorkaround is a temporary workaround for packages + // not yet imported into AZL. + OverlayCategoryAZLMissingDependencyWorkaround OverlayCategory = "azl-missing-dependency-workaround" + // OverlayCategoryAZLPlatformAdaptation applies architecture-specific adjustments. + OverlayCategoryAZLPlatformAdaptation OverlayCategory = "azl-platform-adaptation" +) + +// allOverlayCategories lists every recognized [OverlayCategory] value. +// +//nolint:gochecknoglobals // effectively a constant; Go doesn't allow const slices. +var allOverlayCategories = []OverlayCategory{ + OverlayCategoryBackportFedora, + OverlayCategoryUpstreamFix, + OverlayCategoryAZLDependencyPruning, + OverlayCategoryAZLFeatureDisablement, + OverlayCategoryAZLBrandingPolicy, + OverlayCategoryAZLBuild, + OverlayCategoryAZLTestDisablement, + OverlayCategoryAZLSecurityCompliance, + OverlayCategoryAZLReleaseManagement, + OverlayCategoryAZLMissingDependencyWorkaround, + OverlayCategoryAZLPlatformAdaptation, +} + +// IsValid reports whether c is one of the recognized [OverlayCategory] values. +func (c OverlayCategory) IsValid() bool { + return slices.Contains(allOverlayCategories, c) +} + +// OverlayUpstreamability records whether an overlay's change can be sent upstream. It +// defaults to 'unknown' (not yet assessed) when the field is omitted. +type OverlayUpstreamability string + +const ( + // OverlayUpstreamabilityUnknown means upstreamability has not been assessed. It is the + // default when the field is omitted (an empty value is treated as 'unknown'). + OverlayUpstreamabilityUnknown OverlayUpstreamability = "unknown" + // OverlayUpstreamabilityYes means the change is a candidate for upstreaming. + OverlayUpstreamabilityYes OverlayUpstreamability = "yes" + // OverlayUpstreamabilityNo means the change is intentionally AZL-specific and will + // never be upstreamed. + OverlayUpstreamabilityNo OverlayUpstreamability = "no" +) + +// IsValid reports whether u is one of the recognized [OverlayUpstreamability] values. An +// empty value is accepted and means the field was omitted (defaults to 'unknown'). +func (u OverlayUpstreamability) IsValid() bool { + switch u { + case "", OverlayUpstreamabilityUnknown, OverlayUpstreamabilityYes, OverlayUpstreamabilityNo: + return true + default: + return false + } +} + +// OverlayMetadata describes the intent and provenance of an overlay. It is documentation +// only — it does not affect how the overlay is applied and is excluded from component +// fingerprints. When present, it must declare a [OverlayMetadata.Category]; other fields +// are optional but constrained by category-specific rules (see [OverlayMetadata.Validate]). +type OverlayMetadata struct { + // Category classifies the overlay's intent. Required. + Category OverlayCategory `toml:"category" json:"category" jsonschema:"required,enum=backport-fedora,enum=upstream-fix,enum=azl-dependency-pruning,enum=azl-feature-disablement,enum=azl-branding-policy,enum=azl-build,enum=azl-test-disablement,enum=azl-security-compliance,enum=azl-release-management,enum=azl-missing-dependency-workaround,enum=azl-platform-adaptation,title=Category,description=Classification of the overlay's intent"` + + // Commits lists URLs of upstream commits (typically Fedora dist-git or upstream-project + // commits) that this overlay backports or references. + Commits []string `toml:"commits,omitempty" json:"commits,omitempty" validate:"omitempty,dive,http_url" jsonschema:"title=Commits,description=URLs of upstream commits this overlay backports or references"` + + // FixedIn names the upstream version or Fedora NVR where the fix lands (for example + // 'xclock-1.1.1-11.fc44' or '4.11.2'). + FixedIn string `toml:"fixed-in,omitempty" json:"fixedIn,omitempty" jsonschema:"title=Fixed in,description=Upstream version or Fedora NVR where the fix lands"` + + // RemovableAfter names a Fedora branch (for example 'f44'); the overlay may be dropped + // once AZL bumps past this branch. Only meaningful for the 'backport-fedora' category. + RemovableAfter string `toml:"removable-after,omitempty" json:"removableAfter,omitempty" jsonschema:"title=Removable after,description=Fedora branch after which this overlay can be dropped. Only valid for the 'backport-fedora' category."` + + // PR is a link to the upstream pull request that carries (or proposes) the fix. + PR string `toml:"pr,omitempty" json:"pr,omitempty" validate:"omitempty,http_url" jsonschema:"title=Upstream PR,description=URL of the upstream pull request"` + + // BugLinks holds URLs of issue-tracker entries related to this overlay. + BugLinks []string `toml:"bug,omitempty" json:"bug,omitempty" validate:"omitempty,dive,http_url" jsonschema:"title=Issue-tracker links,description=URLs of related issue-tracker entries"` + + // Upstreamability records whether this overlay's change can be upstreamed: 'yes', 'no', + // or 'unknown'. Omitting the field defaults to 'unknown' (not yet assessed). + Upstreamability OverlayUpstreamability `toml:"upstreamability,omitempty" json:"upstreamability,omitempty" jsonschema:"enum=yes,enum=no,enum=unknown,default=unknown,title=Upstreamability,description=Whether this overlay's change can be upstreamed: 'yes', 'no', or 'unknown' (the default)"` +} + +// Validate checks that the metadata is internally consistent: the category is recognized, +// category-specific required fields are present, and URL-shaped fields parse as URLs. +func (m *OverlayMetadata) Validate() error { + if m.Category == "" { + return errors.New("'metadata' requires 'category'") + } + + if !m.Category.IsValid() { + return fmt.Errorf("unknown overlay category %#q", string(m.Category)) + } + + // 'backport-fedora' is the only category that imposes an extra required field; all + // other categories require nothing beyond a valid 'category'. + if m.Category == OverlayCategoryBackportFedora && len(m.Commits) == 0 && m.FixedIn == "" { + return fmt.Errorf( + "overlay category %#q requires at least one of 'commits' or 'fixed-in'", + string(m.Category), + ) + } + + if m.RemovableAfter != "" && m.Category != OverlayCategoryBackportFedora { + return fmt.Errorf( + "'removable-after' is only valid for overlay category %#q; found category %#q", + string(OverlayCategoryBackportFedora), string(m.Category), + ) + } + + if !m.Upstreamability.IsValid() { + return fmt.Errorf( + "unknown upstreamability %#q; want %#q, %#q, or %#q", + string(m.Upstreamability), string(OverlayUpstreamabilityYes), + string(OverlayUpstreamabilityNo), string(OverlayUpstreamabilityUnknown), + ) + } + + // Field-level constraints (for example 'pr' and 'bug' must be http(s) URLs) are + // expressed as 'validate' struct tags and enforced here. + if err := validator.New().Struct(m); err != nil { + return fmt.Errorf("invalid overlay metadata:\n%w", err) + } + + return nil +} diff --git a/internal/projectconfig/overlay_metadata_test.go b/internal/projectconfig/overlay_metadata_test.go new file mode 100644 index 00000000..3b3742f1 --- /dev/null +++ b/internal/projectconfig/overlay_metadata_test.go @@ -0,0 +1,168 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +package projectconfig_test + +import ( + "testing" + + "github.com/microsoft/azure-linux-dev-tools/internal/projectconfig" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestOverlayMetadata_Validate(t *testing.T) { + testCases := []struct { + name string + metadata projectconfig.OverlayMetadata + errorContains string + }{ + { + name: "backport-fedora with commits is valid", + metadata: projectconfig.OverlayMetadata{ + Category: projectconfig.OverlayCategoryBackportFedora, + Commits: []string{"https://src.fedoraproject.org/rpms/xclock/c/abc123"}, + }, + }, + { + name: "backport-fedora with fixed-in is valid", + metadata: projectconfig.OverlayMetadata{ + Category: projectconfig.OverlayCategoryBackportFedora, + FixedIn: "xclock-1.1.1-11.fc44", + }, + }, + { + name: "backport-fedora with removable-after is valid", + metadata: projectconfig.OverlayMetadata{ + Category: projectconfig.OverlayCategoryBackportFedora, + FixedIn: "xclock-1.1.1-11.fc44", + RemovableAfter: "f44", + }, + }, + { + name: "backport-fedora requires commits or fixed-in", + metadata: projectconfig.OverlayMetadata{ + Category: projectconfig.OverlayCategoryBackportFedora, + }, + errorContains: "commits' or 'fixed-in", + }, + { + name: "upstream-fix valid", + metadata: projectconfig.OverlayMetadata{ + Category: projectconfig.OverlayCategoryUpstreamFix, + PR: "https://github.com/example/repo/pull/1", + }, + }, + { + name: "azl-branding-policy needs no extras", + metadata: projectconfig.OverlayMetadata{ + Category: projectconfig.OverlayCategoryAZLBrandingPolicy, + }, + }, + { + name: "azl-branding-policy may carry pr and commits", + metadata: projectconfig.OverlayMetadata{ + Category: projectconfig.OverlayCategoryAZLBrandingPolicy, + PR: "https://github.com/example/repo/pull/2", + Commits: []string{"https://github.com/example/repo/commit/deadbeef"}, + Upstreamability: projectconfig.OverlayUpstreamabilityYes, + }, + }, + { + name: "upstreamability no is valid", + metadata: projectconfig.OverlayMetadata{ + Category: projectconfig.OverlayCategoryAZLBuild, + Upstreamability: projectconfig.OverlayUpstreamabilityNo, + }, + }, + { + name: "unknown upstreamability rejected", + metadata: projectconfig.OverlayMetadata{ + Category: projectconfig.OverlayCategoryAZLBuild, + Upstreamability: projectconfig.OverlayUpstreamability("maybe"), + }, + errorContains: "unknown upstreamability", + }, + { + name: "removable-after rejected outside backport-fedora", + metadata: projectconfig.OverlayMetadata{ + Category: projectconfig.OverlayCategoryAZLBuild, + RemovableAfter: "f44", + }, + errorContains: "removable-after", + }, + { + name: "missing category", + metadata: projectconfig.OverlayMetadata{ + FixedIn: "1.0", + }, + errorContains: "category", + }, + { + name: "unknown category", + metadata: projectconfig.OverlayMetadata{ + Category: projectconfig.OverlayCategory("bogus"), + }, + errorContains: "unknown overlay category", + }, + { + name: "invalid pr url", + metadata: projectconfig.OverlayMetadata{ + Category: projectconfig.OverlayCategoryUpstreamFix, + PR: "not-a-url", + }, + errorContains: "PR", + }, + { + name: "non-http pr url rejected", + metadata: projectconfig.OverlayMetadata{ + Category: projectconfig.OverlayCategoryUpstreamFix, + PR: "ftp://example.com/pr/1", + }, + errorContains: "PR", + }, + { + name: "invalid bug url", + metadata: projectconfig.OverlayMetadata{ + Category: projectconfig.OverlayCategoryAZLBuild, + BugLinks: []string{"https://valid.example", "not a url"}, + }, + errorContains: "BugLinks[1]", + }, + } + + for _, testCase := range testCases { + t.Run(testCase.name, func(t *testing.T) { + err := testCase.metadata.Validate() + + if testCase.errorContains == "" { + require.NoError(t, err) + + return + } + + require.Error(t, err) + assert.Contains(t, err.Error(), testCase.errorContains) + }) + } +} + +func TestComponentOverlay_Validate_Metadata(t *testing.T) { + // Overlay with valid metadata succeeds. + overlay := projectconfig.ComponentOverlay{ + Type: projectconfig.ComponentOverlayAddSpecTag, + Tag: "Vendor", + Value: "Microsoft", + Metadata: &projectconfig.OverlayMetadata{ + Category: projectconfig.OverlayCategoryAZLBrandingPolicy, + }, + } + require.NoError(t, overlay.Validate()) + + // Overlay with invalid metadata fails — wraps the metadata error. + overlay.Metadata.RemovableAfter = "f44" + + err := overlay.Validate() + require.Error(t, err) + assert.Contains(t, err.Error(), "removable-after") +} diff --git a/scenario/__snapshots__/TestMCPServerMode_1.snap.json b/scenario/__snapshots__/TestMCPServerMode_1.snap.json index 356765c2..4e276a17 100755 --- a/scenario/__snapshots__/TestMCPServerMode_1.snap.json +++ b/scenario/__snapshots__/TestMCPServerMode_1.snap.json @@ -285,6 +285,106 @@ }, "name": "component-list" }, + { + "annotations": { + "destructiveHint": true, + "idempotentHint": false, + "openWorldHint": true, + "readOnlyHint": false + }, + "description": "List overlays and their metadata for components in this project", + "inputSchema": { + "properties": { + "accept-all": { + "default": false, + "description": "accept all prompts", + "type": "boolean" + }, + "all-components": { + "default": false, + "description": "Include all components", + "type": "boolean" + }, + "category": { + "default": "", + "description": "only include overlays whose metadata declares this category", + "type": "string" + }, + "color": { + "description": "output colorization mode {always, auto, never}", + "type": "string" + }, + "component": { + "description": "Component name pattern", + "type": "string" + }, + "component-group": { + "description": "Component group name", + "type": "string" + }, + "config-file": { + "description": "additional TOML config file(s) to merge (may be repeated)", + "type": "string" + }, + "dry-run": { + "default": false, + "description": "dry run only (do not take action)", + "type": "boolean" + }, + "network-retries": { + "default": 3, + "description": "maximum number of attempts for network operations (minimum 1)", + "type": "number" + }, + "no-default-config": { + "default": false, + "description": "disable default configuration", + "type": "boolean" + }, + "only-annotated": { + "default": false, + "description": "exclude overlays that have no metadata", + "type": "boolean" + }, + "output-format": { + "description": "output format {csv, json, markdown, table}", + "type": "string" + }, + "permissive-config": { + "default": false, + "description": "do not fail on unknown fields in TOML config files", + "type": "boolean" + }, + "project": { + "default": "", + "description": "path to Azure Linux project", + "type": "string" + }, + "quiet": { + "default": false, + "description": "only enable minimal output", + "type": "boolean" + }, + "spec-path": { + "description": "Spec path", + "type": "string" + }, + "upstreamability": { + "default": "", + "description": "only include overlays whose metadata declares this upstreamability ('yes', 'no', or 'unknown')", + "type": "string" + }, + "verbose": { + "default": false, + "description": "enable verbose output", + "type": "boolean" + } + }, + "required": [], + "type": "object" + }, + "name": "component-overlays" + }, { "annotations": { "destructiveHint": true, diff --git a/scenario/__snapshots__/TestSnapshotsContainer_config_generate-schema_stdout_1.snap b/scenario/__snapshots__/TestSnapshotsContainer_config_generate-schema_stdout_1.snap index 7aa38d7e..c9645cbe 100755 --- a/scenario/__snapshots__/TestSnapshotsContainer_config_generate-schema_stdout_1.snap +++ b/scenario/__snapshots__/TestSnapshotsContainer_config_generate-schema_stdout_1.snap @@ -119,6 +119,11 @@ "title": "Overlays", "description": "Overlays to apply to this component's spec and/or sources" }, + "overlay-dir": { + "type": "string", + "title": "Overlay directory", + "description": "Directory (relative to the component config file) scanned for *.overlay.toml files at load time" + }, "build": { "$ref": "#/$defs/ComponentBuildConfig", "title": "Build configuration", @@ -278,6 +283,11 @@ "type": "string", "title": "Source", "description": "For overlays that require a source file as input" + }, + "metadata": { + "$ref": "#/$defs/OverlayMetadata", + "title": "Metadata", + "description": "Optional documentation metadata describing the overlay's intent and provenance" } }, "additionalProperties": false, @@ -711,6 +721,75 @@ "type" ] }, + "OverlayMetadata": { + "properties": { + "category": { + "type": "string", + "enum": [ + "backport-fedora", + "upstream-fix", + "azl-dependency-pruning", + "azl-feature-disablement", + "azl-branding-policy", + "azl-build", + "azl-test-disablement", + "azl-security-compliance", + "azl-release-management", + "azl-missing-dependency-workaround", + "azl-platform-adaptation" + ], + "title": "Category", + "description": "Classification of the overlay's intent" + }, + "commits": { + "items": { + "type": "string" + }, + "type": "array", + "title": "Commits", + "description": "URLs of upstream commits this overlay backports or references" + }, + "fixed-in": { + "type": "string", + "title": "Fixed in", + "description": "Upstream version or Fedora NVR where the fix lands" + }, + "removable-after": { + "type": "string", + "title": "Removable after", + "description": "Fedora branch after which this overlay can be dropped. Only valid for the 'backport-fedora' category." + }, + "pr": { + "type": "string", + "title": "Upstream PR", + "description": "URL of the upstream pull request" + }, + "bug": { + "items": { + "type": "string" + }, + "type": "array", + "title": "Issue-tracker links", + "description": "URLs of related issue-tracker entries" + }, + "upstreamability": { + "type": "string", + "enum": [ + "yes", + "no", + "unknown" + ], + "title": "Upstreamability", + "description": "Whether this overlay's change can be upstreamed: 'yes'", + "default": "unknown" + } + }, + "additionalProperties": false, + "type": "object", + "required": [ + "category" + ] + }, "PackageConfig": { "properties": { "publish": { diff --git a/scenario/__snapshots__/TestSnapshots_config_generate-schema_stdout_1.snap b/scenario/__snapshots__/TestSnapshots_config_generate-schema_stdout_1.snap index 7aa38d7e..c9645cbe 100755 --- a/scenario/__snapshots__/TestSnapshots_config_generate-schema_stdout_1.snap +++ b/scenario/__snapshots__/TestSnapshots_config_generate-schema_stdout_1.snap @@ -119,6 +119,11 @@ "title": "Overlays", "description": "Overlays to apply to this component's spec and/or sources" }, + "overlay-dir": { + "type": "string", + "title": "Overlay directory", + "description": "Directory (relative to the component config file) scanned for *.overlay.toml files at load time" + }, "build": { "$ref": "#/$defs/ComponentBuildConfig", "title": "Build configuration", @@ -278,6 +283,11 @@ "type": "string", "title": "Source", "description": "For overlays that require a source file as input" + }, + "metadata": { + "$ref": "#/$defs/OverlayMetadata", + "title": "Metadata", + "description": "Optional documentation metadata describing the overlay's intent and provenance" } }, "additionalProperties": false, @@ -711,6 +721,75 @@ "type" ] }, + "OverlayMetadata": { + "properties": { + "category": { + "type": "string", + "enum": [ + "backport-fedora", + "upstream-fix", + "azl-dependency-pruning", + "azl-feature-disablement", + "azl-branding-policy", + "azl-build", + "azl-test-disablement", + "azl-security-compliance", + "azl-release-management", + "azl-missing-dependency-workaround", + "azl-platform-adaptation" + ], + "title": "Category", + "description": "Classification of the overlay's intent" + }, + "commits": { + "items": { + "type": "string" + }, + "type": "array", + "title": "Commits", + "description": "URLs of upstream commits this overlay backports or references" + }, + "fixed-in": { + "type": "string", + "title": "Fixed in", + "description": "Upstream version or Fedora NVR where the fix lands" + }, + "removable-after": { + "type": "string", + "title": "Removable after", + "description": "Fedora branch after which this overlay can be dropped. Only valid for the 'backport-fedora' category." + }, + "pr": { + "type": "string", + "title": "Upstream PR", + "description": "URL of the upstream pull request" + }, + "bug": { + "items": { + "type": "string" + }, + "type": "array", + "title": "Issue-tracker links", + "description": "URLs of related issue-tracker entries" + }, + "upstreamability": { + "type": "string", + "enum": [ + "yes", + "no", + "unknown" + ], + "title": "Upstreamability", + "description": "Whether this overlay's change can be upstreamed: 'yes'", + "default": "unknown" + } + }, + "additionalProperties": false, + "type": "object", + "required": [ + "category" + ] + }, "PackageConfig": { "properties": { "publish": { diff --git a/schemas/azldev.schema.json b/schemas/azldev.schema.json index 7aa38d7e..c9645cbe 100644 --- a/schemas/azldev.schema.json +++ b/schemas/azldev.schema.json @@ -119,6 +119,11 @@ "title": "Overlays", "description": "Overlays to apply to this component's spec and/or sources" }, + "overlay-dir": { + "type": "string", + "title": "Overlay directory", + "description": "Directory (relative to the component config file) scanned for *.overlay.toml files at load time" + }, "build": { "$ref": "#/$defs/ComponentBuildConfig", "title": "Build configuration", @@ -278,6 +283,11 @@ "type": "string", "title": "Source", "description": "For overlays that require a source file as input" + }, + "metadata": { + "$ref": "#/$defs/OverlayMetadata", + "title": "Metadata", + "description": "Optional documentation metadata describing the overlay's intent and provenance" } }, "additionalProperties": false, @@ -711,6 +721,75 @@ "type" ] }, + "OverlayMetadata": { + "properties": { + "category": { + "type": "string", + "enum": [ + "backport-fedora", + "upstream-fix", + "azl-dependency-pruning", + "azl-feature-disablement", + "azl-branding-policy", + "azl-build", + "azl-test-disablement", + "azl-security-compliance", + "azl-release-management", + "azl-missing-dependency-workaround", + "azl-platform-adaptation" + ], + "title": "Category", + "description": "Classification of the overlay's intent" + }, + "commits": { + "items": { + "type": "string" + }, + "type": "array", + "title": "Commits", + "description": "URLs of upstream commits this overlay backports or references" + }, + "fixed-in": { + "type": "string", + "title": "Fixed in", + "description": "Upstream version or Fedora NVR where the fix lands" + }, + "removable-after": { + "type": "string", + "title": "Removable after", + "description": "Fedora branch after which this overlay can be dropped. Only valid for the 'backport-fedora' category." + }, + "pr": { + "type": "string", + "title": "Upstream PR", + "description": "URL of the upstream pull request" + }, + "bug": { + "items": { + "type": "string" + }, + "type": "array", + "title": "Issue-tracker links", + "description": "URLs of related issue-tracker entries" + }, + "upstreamability": { + "type": "string", + "enum": [ + "yes", + "no", + "unknown" + ], + "title": "Upstreamability", + "description": "Whether this overlay's change can be upstreamed: 'yes'", + "default": "unknown" + } + }, + "additionalProperties": false, + "type": "object", + "required": [ + "category" + ] + }, "PackageConfig": { "properties": { "publish": { From 189edbde0f9ef1c14814e09f6e798298500336c5 Mon Sep 17 00:00:00 2001 From: Nan Liu Date: Wed, 17 Jun 2026 00:20:35 +0000 Subject: [PATCH 2/6] fix(overlays): address PR #242 review feedback - Replace commas with semicolons in Upstreamability jsonschema description to prevent jsonschema tag parser from truncating the description. - Replace deep.MustCopy with a non-panicking OverlayMetadata.clone() so config load can never panic when stamping per-file metadata. - Reuse a package-level validator instance in OverlayMetadata.Validate to avoid per-call allocations. - Compute normalized --upstreamability filter once in ListOverlays and pass it to overlayInfoMatchesFilters instead of re-normalizing per overlay. - Fail loudly when an overlay-dir matches no *.overlay.toml files (or the directory is missing) via WithFailOnPatternNotExist and a new ErrOverlayDirNoFiles sentinel. - Reject overlay-dir on default-component-config (project, group, and distro-version layers) with ErrOverlayDirInDefaultConfig as a stopgap until overlay-dir is plumbed through the default-merge path. - Regenerate JSON schema and scenario snapshots. --- .../cli/azldev_component_overlays.md | 2 +- docs/user/reference/config/overlays.md | 12 +--- .../app/azldev/cmds/component/overlays.go | 22 +++---- .../azldev/cmds/component/overlays_test.go | 14 ++--- internal/projectconfig/overlay_file.go | 58 ++++++++++++++++- internal/projectconfig/overlay_file_test.go | 33 ++++++---- internal/projectconfig/overlay_metadata.go | 62 ++++++++++--------- .../projectconfig/overlay_metadata_test.go | 48 ++++---------- ...ainer_config_generate-schema_stdout_1.snap | 15 +---- ...shots_config_generate-schema_stdout_1.snap | 15 +---- schemas/azldev.schema.json | 15 +---- 11 files changed, 148 insertions(+), 148 deletions(-) diff --git a/docs/user/reference/cli/azldev_component_overlays.md b/docs/user/reference/cli/azldev_component_overlays.md index c0ef47b3..3e5b0f9f 100644 --- a/docs/user/reference/cli/azldev_component_overlays.md +++ b/docs/user/reference/cli/azldev_component_overlays.md @@ -31,7 +31,7 @@ azldev component overlays [flags] azldev component overlays -a --only-annotated # Filter by category - azldev component overlays -a --category backport-fedora + azldev component overlays -a --category backport-dist-git # List only overlays that can be upstreamed azldev component overlays -a --upstreamability yes diff --git a/docs/user/reference/config/overlays.md b/docs/user/reference/config/overlays.md index eff7bb9a..b38ffb9c 100644 --- a/docs/user/reference/config/overlays.md +++ b/docs/user/reference/config/overlays.md @@ -84,8 +84,6 @@ Overlays can carry an optional `metadata` table that documents *why* the overlay |-------|----------|-------------| | Category | `category` | **Required.** Classification of the overlay's intent. See the table below. | | Commits | `commits` | List of upstream commit URLs (Fedora dist-git or upstream project) that this overlay backports or implements. Each entry must be an absolute http(s) URL. | -| Fixed in | `fixed-in` | Upstream version or Fedora NVR where the fix lands (e.g. `xclock-1.1.1-11.fc44`, `4.11.2`). | -| Removable after | `removable-after` | A Fedora branch (e.g. `f44`); the overlay can be dropped once AZL bumps past it. Only valid when `category = "backport-fedora"`. | | PR | `pr` | URL of the upstream pull request that carries (or proposes) the fix. | | Bug | `bug` | List of bug-tracker URLs related to this overlay. | | Upstreamability | `upstreamability` | Whether this change can be upstreamed: `"yes"`, `"no"`, or `"unknown"`. Defaults to `"unknown"` (not yet assessed) when omitted. | @@ -94,8 +92,7 @@ Overlays can carry an optional `metadata` table that documents *why* the overlay | Category | When to use | |----------|-------------| -| `backport-fedora` | Fix exists in some Fedora branch. The overlay backports it. Self-resolves when AZL bumps past it. Requires at least one of `commits` or `fixed-in`. | -| `upstream-fix` | Fix is **not** in any Fedora branch; the overlay is a candidate for upstreaming. | +| `backport-dist-git` | Fix backported from (or being upstreamed to) a dist-git or upstream project. Self-resolves when AZL bumps past it. Requires at least one entry in `commits`. | | `azl-dependency-pruning` | Removing dependencies not shipped in AZL. | | `azl-feature-disablement` | Disabling unneeded features or subpackages. | | `azl-branding-policy` | Fedora→Azure Linux name/path changes; RHEL/enterprise convention alignment. | @@ -118,10 +115,8 @@ regex = "autoreconf -i" replacement = "autoreconf -fi" [components.xclock.overlays.metadata] - category = "backport-fedora" + category = "backport-dist-git" commits = ["https://src.fedoraproject.org/rpms/xclock/c/1e407488"] - fixed-in = "xclock-1.1.1-11.fc44" - removable-after = "f44" ``` For short metadata, the single-line inline form is also valid: @@ -173,9 +168,8 @@ Per-overlay `metadata` is **not allowed** inside a `.overlay.toml` file — the ```toml # overlays/0001-cve-2024-1234.overlay.toml [metadata] -category = "backport-fedora" +category = "backport-dist-git" commits = ["https://src.fedoraproject.org/rpms/mypackage/c/abc123def456"] -fixed-in = "3.2.0" bug = ["https://bugzilla.redhat.com/show_bug.cgi?id=12345"] [[overlays]] diff --git a/internal/app/azldev/cmds/component/overlays.go b/internal/app/azldev/cmds/component/overlays.go index a9f3e94e..210c0d3c 100644 --- a/internal/app/azldev/cmds/component/overlays.go +++ b/internal/app/azldev/cmds/component/overlays.go @@ -60,7 +60,7 @@ so it is fast and works even when locks are missing or stale.`, azldev component overlays -a --only-annotated # Filter by category - azldev component overlays -a --category backport-fedora + azldev component overlays -a --category backport-dist-git # List only overlays that can be upstreamed azldev component overlays -a --upstreamability yes @@ -127,7 +127,8 @@ func ListOverlays(env *azldev.Env, options *OverlaysOptions) ([]OverlayInfo, err return nil, fmt.Errorf("%w: unknown overlay category %#q", azldev.ErrInvalidUsage, options.Category) } - if _, err := normalizeUpstreamabilityFilter(options.Upstreamability); err != nil { + wantUpstreamability, err := normalizeUpstreamabilityFilter(options.Upstreamability) + if err != nil { return nil, err } @@ -148,7 +149,7 @@ func ListOverlays(env *azldev.Env, options *OverlaysOptions) ([]OverlayInfo, err for idx, overlay := range config.Overlays { info := buildOverlayInfo(comp.GetName(), idx+1, &overlay) - if !overlayInfoMatchesFilters(info, options) { + if !overlayInfoMatchesFilters(info, options, wantUpstreamability) { continue } @@ -212,8 +213,11 @@ func normalizeUpstreamabilityFilter(value string) (projectconfig.OverlayUpstream } // overlayInfoMatchesFilters reports whether an [OverlayInfo] passes the user-supplied -// category, only-annotated, and upstreamability filters. -func overlayInfoMatchesFilters(info OverlayInfo, options *OverlaysOptions) bool { +// category, only-annotated, and upstreamability filters. wantUpstreamability is the +// pre-normalized '--upstreamability' value computed once by [ListOverlays]. +func overlayInfoMatchesFilters( + info OverlayInfo, options *OverlaysOptions, wantUpstreamability projectconfig.OverlayUpstreamability, +) bool { if options.OnlyAnnotated && info.Metadata == nil { return false } @@ -222,12 +226,8 @@ func overlayInfoMatchesFilters(info OverlayInfo, options *OverlaysOptions) bool return false } - if options.Upstreamability != "" { - // The filter value is validated upfront in ListOverlays, so the error is unreachable here. - want, _ := normalizeUpstreamabilityFilter(options.Upstreamability) - if info.Upstreamability != want { - return false - } + if options.Upstreamability != "" && info.Upstreamability != wantUpstreamability { + return false } return true diff --git a/internal/app/azldev/cmds/component/overlays_test.go b/internal/app/azldev/cmds/component/overlays_test.go index bda8f689..1284216b 100644 --- a/internal/app/azldev/cmds/component/overlays_test.go +++ b/internal/app/azldev/cmds/component/overlays_test.go @@ -41,9 +41,8 @@ func seedComponentsWithOverlays(t *testing.T, testEnv *testutils.TestEnv) { Type: projectconfig.ComponentOverlayAddPatch, Source: "patches/fix.patch", Metadata: &projectconfig.OverlayMetadata{ - Category: projectconfig.OverlayCategoryBackportFedora, + Category: projectconfig.OverlayCategoryBackportDistGit, Commits: []string{"https://src.fedoraproject.org/rpms/pkg-a/c/abc"}, - FixedIn: "1.2.3", }, }, { @@ -64,7 +63,7 @@ func seedComponentsWithOverlays(t *testing.T, testEnv *testutils.TestEnv) { Tag: "Packager", Value: "azl", Metadata: &projectconfig.OverlayMetadata{ - Category: projectconfig.OverlayCategoryUpstreamFix, + Category: projectconfig.OverlayCategoryAZLBuild, PR: "https://github.com/example/repo/pull/1", Upstreamability: projectconfig.OverlayUpstreamabilityYes, }, @@ -92,10 +91,9 @@ func TestListOverlays_AllComponents(t *testing.T) { assert.Equal(t, "pkg-a", results[1].Component) assert.Equal(t, 2, results[1].Index) - assert.Equal(t, projectconfig.OverlayCategoryBackportFedora, results[1].Category) + assert.Equal(t, projectconfig.OverlayCategoryBackportDistGit, results[1].Category) require.NotNil(t, results[1].Metadata) assert.Equal(t, []string{"https://src.fedoraproject.org/rpms/pkg-a/c/abc"}, results[1].Metadata.Commits) - assert.Equal(t, "1.2.3", results[1].Metadata.FixedIn) assert.Equal(t, "pkg-a", results[2].Component) assert.Equal(t, 3, results[2].Index) @@ -103,7 +101,7 @@ func TestListOverlays_AllComponents(t *testing.T) { assert.Empty(t, results[2].Category) assert.Equal(t, "pkg-b", results[3].Component) - assert.Equal(t, projectconfig.OverlayCategoryUpstreamFix, results[3].Category) + assert.Equal(t, projectconfig.OverlayCategoryAZLBuild, results[3].Category) } func TestListOverlays_OnlyAnnotated(t *testing.T) { @@ -131,7 +129,7 @@ func TestListOverlays_FilterByCategory(t *testing.T) { options := &component.OverlaysOptions{ ComponentFilter: components.ComponentFilter{IncludeAllComponents: true}, - Category: string(projectconfig.OverlayCategoryBackportFedora), + Category: string(projectconfig.OverlayCategoryBackportDistGit), } results, err := component.ListOverlays(testEnv.Env, options) @@ -169,7 +167,7 @@ func TestListOverlays_Upstreamable(t *testing.T) { require.Len(t, results, 1, "only the upstreamable overlay in pkg-b should be included") assert.Equal(t, "pkg-b", results[0].Component) assert.Equal(t, projectconfig.OverlayUpstreamabilityYes, results[0].Upstreamability) - assert.Equal(t, projectconfig.OverlayCategoryUpstreamFix, results[0].Category) + assert.Equal(t, projectconfig.OverlayCategoryAZLBuild, results[0].Category) } func TestListOverlays_UnknownUpstreamabilityRejected(t *testing.T) { diff --git a/internal/projectconfig/overlay_file.go b/internal/projectconfig/overlay_file.go index 8f90003b..c5f29724 100644 --- a/internal/projectconfig/overlay_file.go +++ b/internal/projectconfig/overlay_file.go @@ -11,7 +11,6 @@ import ( "sort" "github.com/bmatcuk/doublestar/v4" - "github.com/brunoga/deep" "github.com/microsoft/azure-linux-dev-tools/internal/global/opctx" "github.com/microsoft/azure-linux-dev-tools/internal/utils/fileutils" "github.com/pelletier/go-toml/v2" @@ -30,6 +29,21 @@ var ErrOverlayFilePerOverlayMetadata = errors.New( // `[[overlays]]`) and would otherwise silently contribute nothing. var ErrOverlayFileEmpty = errors.New("overlay file declares no overlays") +// ErrOverlayDirNoFiles is returned when a component's [ComponentConfig.OverlayDir] +// resolves to a directory that contains no `*.overlay.toml` files. A configured but +// empty overlay directory is almost always a misconfiguration (wrong path or missing +// files) and is surfaced as an error rather than silently contributing nothing. +var ErrOverlayDirNoFiles = errors.New("overlay-dir contains no *.overlay.toml files") + +// ErrOverlayDirInDefaultConfig is returned when `overlay-dir` is set on a default +// component config. Overlay directories are resolved per concrete component before +// default configs are merged in, so a value set on a default would be silently +// ignored. Until overlay-dir is wired through the default-merge path, declaring it on +// a default config is rejected. +var ErrOverlayDirInDefaultConfig = errors.New( + "overlay-dir is not supported on default-component-config; set it on individual components", +) + // overlayFileSuffix is the required filename suffix for files scanned from a // component's [ComponentConfig.OverlayDir]. const overlayFileSuffix = "*.overlay.toml" @@ -57,6 +71,10 @@ type OverlayFile struct { // Called from [loadProjectConfigFile] after TOML decode but before [ConfigFile.Validate], // so all per-overlay validation rules apply uniformly regardless of declaration site. func applyOverlayDirs(fs opctx.FS, cfg *ConfigFile, permissiveConfigParsing bool) error { + if err := rejectOverlayDirInDefaults(cfg); err != nil { + return err + } + for componentName, component := range cfg.Components { if component.OverlayDir == "" { continue @@ -76,6 +94,33 @@ func applyOverlayDirs(fs opctx.FS, cfg *ConfigFile, permissiveConfigParsing bool return nil } +// rejectOverlayDirInDefaults returns [ErrOverlayDirInDefaultConfig] if any default +// component config in cfg sets `overlay-dir`. Overlay directories are resolved per +// concrete component (see [applyOverlayDirs]) before default configs are merged, so a +// value declared on a default would be silently dropped. This stopgap surfaces the +// misconfiguration until overlay-dir is plumbed through the default-merge path. +func rejectOverlayDirInDefaults(cfg *ConfigFile) error { + if cfg.DefaultComponentConfig != nil && cfg.DefaultComponentConfig.OverlayDir != "" { + return fmt.Errorf("%w (project-level default-component-config)", ErrOverlayDirInDefaultConfig) + } + + for name, group := range cfg.ComponentGroups { + if group.DefaultComponentConfig.OverlayDir != "" { + return fmt.Errorf("%w (component-group %#q)", ErrOverlayDirInDefaultConfig, name) + } + } + + for name, distro := range cfg.Distros { + for version, versionDef := range distro.Versions { + if versionDef.DefaultComponentConfig.OverlayDir != "" { + return fmt.Errorf("%w (distro %#q version %#q)", ErrOverlayDirInDefaultConfig, name, version) + } + } + } + + return nil +} + // loadOverlayDir parses every `*.overlay.toml` file in absDir (sorted by filename), // validates each file's metadata, stamps the file metadata onto each overlay, and // resolves overlay `source` paths relative to the overlay file. @@ -87,10 +132,17 @@ func loadOverlayDir( matches, err := fileutils.Glob( fs, pattern, doublestar.WithFailOnIOErrors(), + doublestar.WithFailOnPatternNotExist(), doublestar.WithFilesOnly(), ) - if err != nil { + + switch { + case errors.Is(err, doublestar.ErrPatternNotExist): + return nil, fmt.Errorf("%w: %q", ErrOverlayDirNoFiles, absDir) + case err != nil: return nil, fmt.Errorf("failed to scan for overlay files:\n%w", err) + case len(matches) == 0: + return nil, fmt.Errorf("%w: %q", ErrOverlayDirNoFiles, absDir) } sort.Strings(matches) @@ -149,7 +201,7 @@ func loadOverlayFile( return nil, fmt.Errorf("%w (overlay %d in %q)", ErrOverlayFilePerOverlayMetadata, idx+1, overlayPath) } - overlay.Metadata = deep.MustCopy(&ofile.Metadata) + overlay.Metadata = ofile.Metadata.clone() overlay.Source = makeAbsolute(overlayDir, overlay.Source) } diff --git a/internal/projectconfig/overlay_file_test.go b/internal/projectconfig/overlay_file_test.go index ed48a4f7..f7c78a39 100644 --- a/internal/projectconfig/overlay_file_test.go +++ b/internal/projectconfig/overlay_file_test.go @@ -17,7 +17,7 @@ import ( const validBackportOverlayFile = ` [metadata] -category = "backport-fedora" +category = "backport-dist-git" commits = ["https://src.fedoraproject.org/rpms/ant/c/4ca7a3b"] [[overlays]] @@ -61,11 +61,11 @@ value = "Microsoft" // 0001-* contributes first. assert.Equal(t, ComponentOverlaySearchAndReplaceInSpec, loaded[0].Type) require.NotNil(t, loaded[0].Metadata) - assert.Equal(t, OverlayCategoryBackportFedora, loaded[0].Metadata.Category) + assert.Equal(t, OverlayCategoryBackportDistGit, loaded[0].Metadata.Category) assert.Equal(t, ComponentOverlayRemoveSubpackage, loaded[1].Type) require.NotNil(t, loaded[1].Metadata) - assert.Equal(t, OverlayCategoryBackportFedora, loaded[1].Metadata.Category) + assert.Equal(t, OverlayCategoryBackportDistGit, loaded[1].Metadata.Category) // Mutating one overlay's metadata must not affect another (each must own a copy). loaded[0].Metadata.Commits = append(loaded[0].Metadata.Commits, "mutated") @@ -77,18 +77,25 @@ value = "Microsoft" assert.Equal(t, OverlayCategoryAZLBrandingPolicy, loaded[2].Metadata.Category) } -func TestLoadOverlayDir_EmptyDirReturnsNothing(t *testing.T) { +func TestLoadOverlayDir_NoOverlayFilesIsError(t *testing.T) { ctx := testctx.NewCtx() overlayDir := "/project/comps/empty/overlays" - // A non-overlay file alongside must not be picked up. + // A non-overlay file alongside must not be picked up; a directory containing no + // *.overlay.toml files is a misconfiguration and must error. require.NoError(t, fileutils.WriteFile(ctx.FS(), filepath.Join(overlayDir, "README.md"), []byte("not an overlay"), fileperms.PrivateFile)) - loaded, err := loadOverlayDir(ctx.FS(), overlayDir, false) - require.NoError(t, err) - assert.Empty(t, loaded) + _, err := loadOverlayDir(ctx.FS(), overlayDir, false) + require.ErrorIs(t, err, ErrOverlayDirNoFiles) +} + +func TestLoadOverlayDir_MissingDirIsError(t *testing.T) { + ctx := testctx.NewCtx() + + _, err := loadOverlayDir(ctx.FS(), "/project/comps/does-not-exist/overlays", false) + require.ErrorIs(t, err, ErrOverlayDirNoFiles) } func TestLoadOverlayDir_RejectsPerOverlayMetadata(t *testing.T) { @@ -157,7 +164,7 @@ func TestLoadOverlayDir_ResolvesSourceRelativeToOverlayFile(t *testing.T) { filepath.Join(overlayDir, "0001-patch.overlay.toml"), []byte(` [metadata] -category = "backport-fedora" +category = "backport-dist-git" commits = ["https://src.fedoraproject.org/rpms/foo/c/abc"] [[overlays]] @@ -214,11 +221,11 @@ func TestApplyOverlayDirs_AppendsAfterInlineOverlays(t *testing.T) { // metadata stamped onto each. assert.Equal(t, ComponentOverlaySearchAndReplaceInSpec, ant.Overlays[1].Type) require.NotNil(t, ant.Overlays[1].Metadata) - assert.Equal(t, OverlayCategoryBackportFedora, ant.Overlays[1].Metadata.Category) + assert.Equal(t, OverlayCategoryBackportDistGit, ant.Overlays[1].Metadata.Category) assert.Equal(t, ComponentOverlayRemoveSubpackage, ant.Overlays[2].Type) require.NotNil(t, ant.Overlays[2].Metadata) - assert.Equal(t, OverlayCategoryBackportFedora, ant.Overlays[2].Metadata.Category) + assert.Equal(t, OverlayCategoryBackportDistGit, ant.Overlays[2].Metadata.Category) } func TestApplyOverlayDirs_NoopWhenOverlayDirUnset(t *testing.T) { @@ -261,7 +268,7 @@ func TestApplyOverlayDirs_AcceptsAbsoluteOverlayDir(t *testing.T) { ant := cfg.Components["ant"] require.Len(t, ant.Overlays, 2, "absolute overlay-dir is not re-rooted under cfg.dir") require.NotNil(t, ant.Overlays[0].Metadata) - assert.Equal(t, OverlayCategoryBackportFedora, ant.Overlays[0].Metadata.Category) + assert.Equal(t, OverlayCategoryBackportDistGit, ant.Overlays[0].Metadata.Category) } // TestLoadAndResolveProjectConfig_OverlayDir exercises the full loader pipeline @@ -287,5 +294,5 @@ overlay-dir = "comps/ant/overlays" require.True(t, ok, "ant component should be present") require.Len(t, ant.Overlays, 2) require.NotNil(t, ant.Overlays[0].Metadata) - assert.Equal(t, OverlayCategoryBackportFedora, ant.Overlays[0].Metadata.Category) + assert.Equal(t, OverlayCategoryBackportDistGit, ant.Overlays[0].Metadata.Category) } diff --git a/internal/projectconfig/overlay_metadata.go b/internal/projectconfig/overlay_metadata.go index eb5b670d..0905d683 100644 --- a/internal/projectconfig/overlay_metadata.go +++ b/internal/projectconfig/overlay_metadata.go @@ -16,12 +16,10 @@ import ( type OverlayCategory string const ( - // OverlayCategoryBackportFedora applies a fix that already exists in some Fedora branch. - // The overlay can be dropped once AZL bumps its upstream pin past the fix. - OverlayCategoryBackportFedora OverlayCategory = "backport-fedora" - // OverlayCategoryUpstreamFix applies a fix that is NOT yet in any Fedora branch and is - // a candidate for upstreaming. - OverlayCategoryUpstreamFix OverlayCategory = "upstream-fix" + // OverlayCategoryBackportDistGit applies a fix backported from (or being upstreamed to) + // a dist-git or upstream project. The overlay can be dropped once AZL bumps its upstream + // pin past the fix. + OverlayCategoryBackportDistGit OverlayCategory = "backport-dist-git" // OverlayCategoryAZLDependencyPruning removes dependencies that are not shipped in AZL. OverlayCategoryAZLDependencyPruning OverlayCategory = "azl-dependency-pruning" // OverlayCategoryAZLFeatureDisablement disables unneeded features or subpackages. @@ -48,8 +46,7 @@ const ( // //nolint:gochecknoglobals // effectively a constant; Go doesn't allow const slices. var allOverlayCategories = []OverlayCategory{ - OverlayCategoryBackportFedora, - OverlayCategoryUpstreamFix, + OverlayCategoryBackportDistGit, OverlayCategoryAZLDependencyPruning, OverlayCategoryAZLFeatureDisablement, OverlayCategoryAZLBrandingPolicy, @@ -98,20 +95,12 @@ func (u OverlayUpstreamability) IsValid() bool { // are optional but constrained by category-specific rules (see [OverlayMetadata.Validate]). type OverlayMetadata struct { // Category classifies the overlay's intent. Required. - Category OverlayCategory `toml:"category" json:"category" jsonschema:"required,enum=backport-fedora,enum=upstream-fix,enum=azl-dependency-pruning,enum=azl-feature-disablement,enum=azl-branding-policy,enum=azl-build,enum=azl-test-disablement,enum=azl-security-compliance,enum=azl-release-management,enum=azl-missing-dependency-workaround,enum=azl-platform-adaptation,title=Category,description=Classification of the overlay's intent"` + Category OverlayCategory `toml:"category" json:"category" jsonschema:"required,enum=backport-dist-git,enum=azl-dependency-pruning,enum=azl-feature-disablement,enum=azl-branding-policy,enum=azl-build,enum=azl-test-disablement,enum=azl-security-compliance,enum=azl-release-management,enum=azl-missing-dependency-workaround,enum=azl-platform-adaptation,title=Category,description=Classification of the overlay's intent"` // Commits lists URLs of upstream commits (typically Fedora dist-git or upstream-project // commits) that this overlay backports or references. Commits []string `toml:"commits,omitempty" json:"commits,omitempty" validate:"omitempty,dive,http_url" jsonschema:"title=Commits,description=URLs of upstream commits this overlay backports or references"` - // FixedIn names the upstream version or Fedora NVR where the fix lands (for example - // 'xclock-1.1.1-11.fc44' or '4.11.2'). - FixedIn string `toml:"fixed-in,omitempty" json:"fixedIn,omitempty" jsonschema:"title=Fixed in,description=Upstream version or Fedora NVR where the fix lands"` - - // RemovableAfter names a Fedora branch (for example 'f44'); the overlay may be dropped - // once AZL bumps past this branch. Only meaningful for the 'backport-fedora' category. - RemovableAfter string `toml:"removable-after,omitempty" json:"removableAfter,omitempty" jsonschema:"title=Removable after,description=Fedora branch after which this overlay can be dropped. Only valid for the 'backport-fedora' category."` - // PR is a link to the upstream pull request that carries (or proposes) the fix. PR string `toml:"pr,omitempty" json:"pr,omitempty" validate:"omitempty,http_url" jsonschema:"title=Upstream PR,description=URL of the upstream pull request"` @@ -120,9 +109,31 @@ type OverlayMetadata struct { // Upstreamability records whether this overlay's change can be upstreamed: 'yes', 'no', // or 'unknown'. Omitting the field defaults to 'unknown' (not yet assessed). - Upstreamability OverlayUpstreamability `toml:"upstreamability,omitempty" json:"upstreamability,omitempty" jsonschema:"enum=yes,enum=no,enum=unknown,default=unknown,title=Upstreamability,description=Whether this overlay's change can be upstreamed: 'yes', 'no', or 'unknown' (the default)"` + Upstreamability OverlayUpstreamability `toml:"upstreamability,omitempty" json:"upstreamability,omitempty" jsonschema:"enum=yes,enum=no,enum=unknown,default=unknown,title=Upstreamability,description=Whether this overlay's change can be upstreamed: 'yes'; 'no'; or 'unknown' (the default)"` +} + +// clone returns a deep copy of the metadata. It is used to stamp a single file-level +// [OverlayMetadata] onto every overlay in a `.overlay.toml` file without aliasing the +// slice fields. A manual copy is preferred over a reflection-based deep copy so that a +// future struct change can never panic during config load. +func (m *OverlayMetadata) clone() *OverlayMetadata { + if m == nil { + return nil + } + + cp := *m + cp.Commits = slices.Clone(m.Commits) + cp.BugLinks = slices.Clone(m.BugLinks) + + return &cp } +// metadataValidator is a shared, concurrency-safe validator instance reused across all +// [OverlayMetadata.Validate] calls to avoid per-call allocation. +// +//nolint:gochecknoglobals // validator instances are safe for concurrent use once configured. +var metadataValidator = validator.New() + // Validate checks that the metadata is internally consistent: the category is recognized, // category-specific required fields are present, and URL-shaped fields parse as URLs. func (m *OverlayMetadata) Validate() error { @@ -134,22 +145,15 @@ func (m *OverlayMetadata) Validate() error { return fmt.Errorf("unknown overlay category %#q", string(m.Category)) } - // 'backport-fedora' is the only category that imposes an extra required field; all + // 'backport-dist-git' is the only category that imposes an extra required field; all // other categories require nothing beyond a valid 'category'. - if m.Category == OverlayCategoryBackportFedora && len(m.Commits) == 0 && m.FixedIn == "" { + if m.Category == OverlayCategoryBackportDistGit && len(m.Commits) == 0 { return fmt.Errorf( - "overlay category %#q requires at least one of 'commits' or 'fixed-in'", + "overlay category %#q requires at least one entry in 'commits'", string(m.Category), ) } - if m.RemovableAfter != "" && m.Category != OverlayCategoryBackportFedora { - return fmt.Errorf( - "'removable-after' is only valid for overlay category %#q; found category %#q", - string(OverlayCategoryBackportFedora), string(m.Category), - ) - } - if !m.Upstreamability.IsValid() { return fmt.Errorf( "unknown upstreamability %#q; want %#q, %#q, or %#q", @@ -160,7 +164,7 @@ func (m *OverlayMetadata) Validate() error { // Field-level constraints (for example 'pr' and 'bug' must be http(s) URLs) are // expressed as 'validate' struct tags and enforced here. - if err := validator.New().Struct(m); err != nil { + if err := metadataValidator.Struct(m); err != nil { return fmt.Errorf("invalid overlay metadata:\n%w", err) } diff --git a/internal/projectconfig/overlay_metadata_test.go b/internal/projectconfig/overlay_metadata_test.go index 3b3742f1..858a8ee0 100644 --- a/internal/projectconfig/overlay_metadata_test.go +++ b/internal/projectconfig/overlay_metadata_test.go @@ -18,38 +18,24 @@ func TestOverlayMetadata_Validate(t *testing.T) { errorContains string }{ { - name: "backport-fedora with commits is valid", + name: "backport-dist-git with commits is valid", metadata: projectconfig.OverlayMetadata{ - Category: projectconfig.OverlayCategoryBackportFedora, + Category: projectconfig.OverlayCategoryBackportDistGit, Commits: []string{"https://src.fedoraproject.org/rpms/xclock/c/abc123"}, }, }, { - name: "backport-fedora with fixed-in is valid", + name: "backport-dist-git requires commits", metadata: projectconfig.OverlayMetadata{ - Category: projectconfig.OverlayCategoryBackportFedora, - FixedIn: "xclock-1.1.1-11.fc44", + Category: projectconfig.OverlayCategoryBackportDistGit, }, + errorContains: "commits", }, { - name: "backport-fedora with removable-after is valid", + name: "backport-dist-git with pr valid", metadata: projectconfig.OverlayMetadata{ - Category: projectconfig.OverlayCategoryBackportFedora, - FixedIn: "xclock-1.1.1-11.fc44", - RemovableAfter: "f44", - }, - }, - { - name: "backport-fedora requires commits or fixed-in", - metadata: projectconfig.OverlayMetadata{ - Category: projectconfig.OverlayCategoryBackportFedora, - }, - errorContains: "commits' or 'fixed-in", - }, - { - name: "upstream-fix valid", - metadata: projectconfig.OverlayMetadata{ - Category: projectconfig.OverlayCategoryUpstreamFix, + Category: projectconfig.OverlayCategoryBackportDistGit, + Commits: []string{"https://src.fedoraproject.org/rpms/xclock/c/abc123"}, PR: "https://github.com/example/repo/pull/1", }, }, @@ -83,18 +69,10 @@ func TestOverlayMetadata_Validate(t *testing.T) { }, errorContains: "unknown upstreamability", }, - { - name: "removable-after rejected outside backport-fedora", - metadata: projectconfig.OverlayMetadata{ - Category: projectconfig.OverlayCategoryAZLBuild, - RemovableAfter: "f44", - }, - errorContains: "removable-after", - }, { name: "missing category", metadata: projectconfig.OverlayMetadata{ - FixedIn: "1.0", + Commits: []string{"https://example.com/commit/abc"}, }, errorContains: "category", }, @@ -108,7 +86,7 @@ func TestOverlayMetadata_Validate(t *testing.T) { { name: "invalid pr url", metadata: projectconfig.OverlayMetadata{ - Category: projectconfig.OverlayCategoryUpstreamFix, + Category: projectconfig.OverlayCategoryAZLBuild, PR: "not-a-url", }, errorContains: "PR", @@ -116,7 +94,7 @@ func TestOverlayMetadata_Validate(t *testing.T) { { name: "non-http pr url rejected", metadata: projectconfig.OverlayMetadata{ - Category: projectconfig.OverlayCategoryUpstreamFix, + Category: projectconfig.OverlayCategoryAZLBuild, PR: "ftp://example.com/pr/1", }, errorContains: "PR", @@ -160,9 +138,9 @@ func TestComponentOverlay_Validate_Metadata(t *testing.T) { require.NoError(t, overlay.Validate()) // Overlay with invalid metadata fails — wraps the metadata error. - overlay.Metadata.RemovableAfter = "f44" + overlay.Metadata.Upstreamability = projectconfig.OverlayUpstreamability("bogus") err := overlay.Validate() require.Error(t, err) - assert.Contains(t, err.Error(), "removable-after") + assert.Contains(t, err.Error(), "unknown upstreamability") } diff --git a/scenario/__snapshots__/TestSnapshotsContainer_config_generate-schema_stdout_1.snap b/scenario/__snapshots__/TestSnapshotsContainer_config_generate-schema_stdout_1.snap index c9645cbe..cc48318f 100755 --- a/scenario/__snapshots__/TestSnapshotsContainer_config_generate-schema_stdout_1.snap +++ b/scenario/__snapshots__/TestSnapshotsContainer_config_generate-schema_stdout_1.snap @@ -726,8 +726,7 @@ "category": { "type": "string", "enum": [ - "backport-fedora", - "upstream-fix", + "backport-dist-git", "azl-dependency-pruning", "azl-feature-disablement", "azl-branding-policy", @@ -749,16 +748,6 @@ "title": "Commits", "description": "URLs of upstream commits this overlay backports or references" }, - "fixed-in": { - "type": "string", - "title": "Fixed in", - "description": "Upstream version or Fedora NVR where the fix lands" - }, - "removable-after": { - "type": "string", - "title": "Removable after", - "description": "Fedora branch after which this overlay can be dropped. Only valid for the 'backport-fedora' category." - }, "pr": { "type": "string", "title": "Upstream PR", @@ -780,7 +769,7 @@ "unknown" ], "title": "Upstreamability", - "description": "Whether this overlay's change can be upstreamed: 'yes'", + "description": "Whether this overlay's change can be upstreamed: 'yes'; 'no'; or 'unknown' (the default)", "default": "unknown" } }, diff --git a/scenario/__snapshots__/TestSnapshots_config_generate-schema_stdout_1.snap b/scenario/__snapshots__/TestSnapshots_config_generate-schema_stdout_1.snap index c9645cbe..cc48318f 100755 --- a/scenario/__snapshots__/TestSnapshots_config_generate-schema_stdout_1.snap +++ b/scenario/__snapshots__/TestSnapshots_config_generate-schema_stdout_1.snap @@ -726,8 +726,7 @@ "category": { "type": "string", "enum": [ - "backport-fedora", - "upstream-fix", + "backport-dist-git", "azl-dependency-pruning", "azl-feature-disablement", "azl-branding-policy", @@ -749,16 +748,6 @@ "title": "Commits", "description": "URLs of upstream commits this overlay backports or references" }, - "fixed-in": { - "type": "string", - "title": "Fixed in", - "description": "Upstream version or Fedora NVR where the fix lands" - }, - "removable-after": { - "type": "string", - "title": "Removable after", - "description": "Fedora branch after which this overlay can be dropped. Only valid for the 'backport-fedora' category." - }, "pr": { "type": "string", "title": "Upstream PR", @@ -780,7 +769,7 @@ "unknown" ], "title": "Upstreamability", - "description": "Whether this overlay's change can be upstreamed: 'yes'", + "description": "Whether this overlay's change can be upstreamed: 'yes'; 'no'; or 'unknown' (the default)", "default": "unknown" } }, diff --git a/schemas/azldev.schema.json b/schemas/azldev.schema.json index c9645cbe..cc48318f 100644 --- a/schemas/azldev.schema.json +++ b/schemas/azldev.schema.json @@ -726,8 +726,7 @@ "category": { "type": "string", "enum": [ - "backport-fedora", - "upstream-fix", + "backport-dist-git", "azl-dependency-pruning", "azl-feature-disablement", "azl-branding-policy", @@ -749,16 +748,6 @@ "title": "Commits", "description": "URLs of upstream commits this overlay backports or references" }, - "fixed-in": { - "type": "string", - "title": "Fixed in", - "description": "Upstream version or Fedora NVR where the fix lands" - }, - "removable-after": { - "type": "string", - "title": "Removable after", - "description": "Fedora branch after which this overlay can be dropped. Only valid for the 'backport-fedora' category." - }, "pr": { "type": "string", "title": "Upstream PR", @@ -780,7 +769,7 @@ "unknown" ], "title": "Upstreamability", - "description": "Whether this overlay's change can be upstreamed: 'yes'", + "description": "Whether this overlay's change can be upstreamed: 'yes'; 'no'; or 'unknown' (the default)", "default": "unknown" } }, From 6d261fec1f2dc4099074833444fb6728c4625895 Mon Sep 17 00:00:00 2001 From: Nan Liu Date: Wed, 17 Jun 2026 20:58:09 +0000 Subject: [PATCH 3/6] fix(overlays): honor --permissive-config for metadata validation Per-file overlay metadata validation now warns and continues under --permissive-config instead of failing the load, aligning with the behavior introduced in #216. This lets older .overlay.toml files that use fields or values deprecated by a newer Metadata.Validate() still be loaded when permissive parsing is requested. --- internal/projectconfig/overlay_file.go | 11 ++++++++- internal/projectconfig/overlay_file_test.go | 27 +++++++++++++++++++++ 2 files changed, 37 insertions(+), 1 deletion(-) diff --git a/internal/projectconfig/overlay_file.go b/internal/projectconfig/overlay_file.go index c5f29724..c0475178 100644 --- a/internal/projectconfig/overlay_file.go +++ b/internal/projectconfig/overlay_file.go @@ -6,6 +6,7 @@ package projectconfig import ( "errors" "fmt" + "log/slog" "path" "path/filepath" "sort" @@ -185,7 +186,15 @@ func loadOverlayFile( } if err := ofile.Metadata.Validate(); err != nil { - return nil, fmt.Errorf("invalid [metadata] in overlay file %q:\n%w", overlayPath, err) + if !permissiveConfigParsing { + return nil, fmt.Errorf("invalid [metadata] in overlay file %q:\n%w", overlayPath, err) + } + + slog.Warn( + "Overlay file metadata validation failed; continuing due to '--permissive-config'", + "overlayFile", overlayPath, + "error", err, + ) } if len(ofile.Overlays) == 0 { diff --git a/internal/projectconfig/overlay_file_test.go b/internal/projectconfig/overlay_file_test.go index f7c78a39..a8945c5f 100644 --- a/internal/projectconfig/overlay_file_test.go +++ b/internal/projectconfig/overlay_file_test.go @@ -156,6 +156,33 @@ value = "Microsoft" assert.Contains(t, err.Error(), "category") } +func TestLoadOverlayFile_PermissiveTolerates_InvalidMetadata(t *testing.T) { + ctx := testctx.NewCtx() + overlayPath := filepath.Join(badOverlayTestDir, "0001-bad.overlay.toml") + + // Same fixture as the strict counterpart above: category is missing, which + // fails OverlayMetadata.Validate(). Under permissive parsing the load must + // still succeed so older configs targeting a stricter newer schema can be + // inspected. + require.NoError(t, fileutils.WriteFile(ctx.FS(), + overlayPath, + []byte(` +[metadata] +# Missing category. +commits = ["abc"] + +[[overlays]] +type = "spec-set-tag" +tag = "Vendor" +value = "Microsoft" +`), fileperms.PrivateFile)) + + loaded, err := loadOverlayFile(ctx.FS(), overlayPath, true) + require.NoError(t, err) + require.Len(t, loaded, 1) + assert.Equal(t, ComponentOverlaySetSpecTag, loaded[0].Type) +} + func TestLoadOverlayDir_ResolvesSourceRelativeToOverlayFile(t *testing.T) { ctx := testctx.NewCtx() overlayDir := "/project/comps/foo/overlays" From 1ccedd190e11d0aa350ecb52256b2907e8cba77a Mon Sep 17 00:00:00 2001 From: Nan Liu Date: Fri, 19 Jun 2026 17:01:02 +0000 Subject: [PATCH 4/6] feat(config): add optional metadata to component groups Add an optional [component-groups..metadata] block reusing the existing OverlayMetadata schema (required category plus commits, pr, bug, and upstreamability). Metadata is documentation only and does not affect how group members are resolved or built. Validation is wired into ConfigFile.Validate, with tests and reference docs updated and the JSON schema regenerated. --- .../user/reference/config/component-groups.md | 23 ++++++++++ internal/projectconfig/component.go | 5 +++ internal/projectconfig/configfile.go | 21 +++++++++ internal/projectconfig/loader_test.go | 45 +++++++++++++++++++ schemas/azldev.schema.json | 5 +++ 5 files changed, 99 insertions(+) diff --git a/docs/user/reference/config/component-groups.md b/docs/user/reference/config/component-groups.md index 76974b1e..619b23b0 100644 --- a/docs/user/reference/config/component-groups.md +++ b/docs/user/reference/config/component-groups.md @@ -7,11 +7,34 @@ Component groups organize related components together and let you apply shared d | Field | TOML Key | Type | Required | Description | |-------|----------|------|----------|-------------| | Description | `description` | string | No | Human-readable description of this group | +| Metadata | `metadata` | [OverlayMetadata](overlays.md) | No | Optional documentation metadata describing the group's intent and provenance | | Components | `components` | string array | No | List of component names that belong to this group | | Specs | `specs` | string array | No | Glob patterns for discovering local spec files to include as group members | | Excluded paths | `excluded-paths` | string array | No | Glob patterns for paths to exclude from spec discovery | | Default component config | `default-component-config` | [ComponentConfig](components.md) | No | Default configuration inherited by all components in this group | +## Metadata + +The optional `[component-groups..metadata]` table documents the group's intent and provenance. It shares exactly the same fields and validation rules as [overlay metadata](overlays.md): a required `category`, plus optional commit/PR/bug links and an upstreamability assessment. It is documentation only — it does not affect how members are resolved or built. + +| Field | TOML Key | Type | Required | Description | +|-------|----------|------|----------|-------------| +| Category | `category` | enum | **Yes** | Classification of the group's intent (e.g. `backport-dist-git`, `azl-test-disablement`) | +| Commits | `commits` | string array (URLs) | No | Upstream commit URLs this group references; required when `category = "backport-dist-git"` | +| Upstream PR | `pr` | string (URL) | No | URL of the related upstream pull request | +| Issue-tracker links | `bug` | string array (URLs) | No | URLs of related issue-tracker entries | +| Upstreamability | `upstreamability` | enum (`yes`/`no`/`unknown`) | No | Whether the group's change can be upstreamed (defaults to `unknown`) | + +```toml +[component-groups.check-skip-initial-failures] +description = "Components with check failures during initial distro bringup" +components = ["bats", "dnf", "git"] + +[component-groups.check-skip-initial-failures.metadata] +category = "azl-test-disablement" +upstreamability = "unknown" +``` + ## Component Membership Components are assigned to groups in two ways: diff --git a/internal/projectconfig/component.go b/internal/projectconfig/component.go index f674592b..3f713549 100644 --- a/internal/projectconfig/component.go +++ b/internal/projectconfig/component.go @@ -110,6 +110,11 @@ type ComponentGroupConfig struct { // A human-friendly description of this component group. Description string `toml:"description,omitempty" json:"description,omitempty" jsonschema:"title=Description,description=Description of this component group"` + // Optional documentation metadata describing this component group's intent and provenance. + // It reuses the overlay metadata schema (see [OverlayMetadata]) and does not affect how the + // group's members are resolved or built. + Metadata *OverlayMetadata `toml:"metadata,omitempty" json:"metadata,omitempty" jsonschema:"title=Metadata,description=Optional documentation metadata for this component group"` + // List of explicitly included components, identified by name. Components []string `toml:"components,omitempty" json:"components,omitempty" jsonschema:"title=Components,description=List of component names that are members of this group"` diff --git a/internal/projectconfig/configfile.go b/internal/projectconfig/configfile.go index e16b79b7..cea89cd3 100644 --- a/internal/projectconfig/configfile.go +++ b/internal/projectconfig/configfile.go @@ -97,6 +97,11 @@ func (f ConfigFile) Validate() error { } } + // Validate component group metadata. + if err := validateComponentGroupMetadata(f.ComponentGroups); err != nil { + return err + } + // Per-component snapshot timestamps are not allowed. Components inherit // the snapshot from the distro/group default-component-config or the // project's default-distro. Per-component snapshots would create @@ -150,6 +155,22 @@ func (f ConfigFile) Validate() error { return nil } +// validateComponentGroupMetadata validates the optional documentation metadata declared +// on each component group. +func validateComponentGroupMetadata(groups map[string]ComponentGroupConfig) error { + for groupName, group := range groups { + if group.Metadata == nil { + continue + } + + if err := group.Metadata.Validate(); err != nil { + return fmt.Errorf("invalid component group %#q:\n%w", groupName, err) + } + } + + return nil +} + // validateSourceFiles checks 'source-files' configuration for a component: // - All filenames must be unique. // - Hash type must be a supported algorithm when specified. diff --git a/internal/projectconfig/loader_test.go b/internal/projectconfig/loader_test.go index f825d55b..6d40fc88 100644 --- a/internal/projectconfig/loader_test.go +++ b/internal/projectconfig/loader_test.go @@ -485,6 +485,51 @@ specs = ["SPECS/**/*.spec"] assert.Equal(t, []string{"core"}, config.GroupsByComponent["bar"]) } +func TestLoadAndResolveProjectConfig_ComponentGroupWithMetadata(t *testing.T) { + const configContents = ` +[component-groups.core] +specs = ["SPECS/**/*.spec"] + +[component-groups.core.metadata] +category = "backport-dist-git" +commits = ["https://example.com/commit/abc"] +pr = "https://example.com/pr/1" +upstreamability = "yes" +` + + ctx := testctx.NewCtx() + require.NoError(t, fileutils.WriteFile(ctx.FS(), testConfigPath, []byte(configContents), fileperms.PrivateFile)) + + config, err := loadAndResolveProjectConfig(ctx.FS(), false, testConfigPath) + require.NoError(t, err) + + if assert.Contains(t, config.ComponentGroups, "core") { + group := config.ComponentGroups["core"] + require.NotNil(t, group.Metadata) + assert.Equal(t, OverlayCategoryBackportDistGit, group.Metadata.Category) + assert.Equal(t, []string{"https://example.com/commit/abc"}, group.Metadata.Commits) + assert.Equal(t, "https://example.com/pr/1", group.Metadata.PR) + assert.Equal(t, OverlayUpstreamabilityYes, group.Metadata.Upstreamability) + } +} + +func TestLoadAndResolveProjectConfig_ComponentGroupMetadataMissingCategory(t *testing.T) { + const configContents = ` +[component-groups.core] +specs = ["SPECS/**/*.spec"] + +[component-groups.core.metadata] +pr = "https://example.com/pr/1" +` + + ctx := testctx.NewCtx() + require.NoError(t, fileutils.WriteFile(ctx.FS(), testConfigPath, []byte(configContents), fileperms.PrivateFile)) + + config, err := loadAndResolveProjectConfig(ctx.FS(), false, testConfigPath) + require.Error(t, err) + assert.Nil(t, config) +} + func TestLoadAndResolveProjectConfig_GroupsByComponent_MultipleGroups(t *testing.T) { testFiles := []struct { path string diff --git a/schemas/azldev.schema.json b/schemas/azldev.schema.json index cc48318f..923a40df 100644 --- a/schemas/azldev.schema.json +++ b/schemas/azldev.schema.json @@ -166,6 +166,11 @@ "title": "Description", "description": "Description of this component group" }, + "metadata": { + "$ref": "#/$defs/OverlayMetadata", + "title": "Metadata", + "description": "Optional documentation metadata for this component group" + }, "components": { "items": { "type": "string" From d1ce778206f99e0e6f35352bdf3226dd2dd1558a Mon Sep 17 00:00:00 2001 From: Nan Liu Date: Fri, 19 Jun 2026 21:21:20 +0000 Subject: [PATCH 5/6] feat(component): add 'metadata' command listing overlay and group metadata Replace 'azldev component overlays' with 'azldev component metadata', which lists documentation metadata from both a component's overlays and the component groups it explicitly belongs to. Each row is tagged with its source ('overlay' or 'group'). By default both sources are listed; --overlays or --groups restricts output to one source. The existing --category, --only-annotated, and --upstreamability filters apply across selected sources. The command is read-only and skips lock validation. Regenerate CLI reference docs and update the hand-written overlays config doc to reference the new command. --- docs/user/reference/cli/azldev_component.md | 2 +- .../cli/azldev_component_metadata.md | 91 +++++ .../cli/azldev_component_overlays.md | 75 ---- docs/user/reference/config/overlays.md | 2 +- .../app/azldev/cmds/component/component.go | 2 +- .../app/azldev/cmds/component/metadata.go | 371 ++++++++++++++++++ .../azldev/cmds/component/metadata_test.go | 304 ++++++++++++++ .../app/azldev/cmds/component/overlays.go | 234 ----------- .../azldev/cmds/component/overlays_test.go | 185 --------- .../TestMCPServerMode_1.snap.json | 20 +- ...ainer_config_generate-schema_stdout_1.snap | 5 + ...shots_config_generate-schema_stdout_1.snap | 5 + 12 files changed, 794 insertions(+), 502 deletions(-) create mode 100644 docs/user/reference/cli/azldev_component_metadata.md delete mode 100644 docs/user/reference/cli/azldev_component_overlays.md create mode 100644 internal/app/azldev/cmds/component/metadata.go create mode 100644 internal/app/azldev/cmds/component/metadata_test.go delete mode 100644 internal/app/azldev/cmds/component/overlays.go delete mode 100644 internal/app/azldev/cmds/component/overlays_test.go diff --git a/docs/user/reference/cli/azldev_component.md b/docs/user/reference/cli/azldev_component.md index a2134097..b6b094e4 100644 --- a/docs/user/reference/cli/azldev_component.md +++ b/docs/user/reference/cli/azldev_component.md @@ -43,7 +43,7 @@ components defined in the project configuration. * [azldev component changed](azldev_component_changed.md) - Detect which components changed between two git refs * [azldev component diff-sources](azldev_component_diff-sources.md) - Show the diff that overlays apply to a component's sources * [azldev component list](azldev_component_list.md) - List components in this project -* [azldev component overlays](azldev_component_overlays.md) - List overlays and their metadata for components in this project +* [azldev component metadata](azldev_component_metadata.md) - List documentation metadata for components' overlays and groups * [azldev component prepare-sources](azldev_component_prepare-sources.md) - Prepare buildable sources for components * [azldev component query](azldev_component_query.md) - Query info for components in this project * [azldev component render](azldev_component_render.md) - Render post-overlay specs and sidecar files to a checked-in directory diff --git a/docs/user/reference/cli/azldev_component_metadata.md b/docs/user/reference/cli/azldev_component_metadata.md new file mode 100644 index 00000000..1cb2d2c0 --- /dev/null +++ b/docs/user/reference/cli/azldev_component_metadata.md @@ -0,0 +1,91 @@ + + +## azldev component metadata + +List documentation metadata for components' overlays and groups + +### Synopsis + +List documentation metadata (category, commits, upstream PR, bug links, +upstreamability, etc.) for the selected components. Metadata comes from two +sources, each tagged in the output: + + - 'overlay': metadata attached to one of the component's overlays, declared + inline in the component config or inherited from an overlay-dir + '.overlay.toml' file's [metadata] block. + - 'group': metadata attached to a component group the component is an + explicit member of (per the group's 'components' list). + +By default both sources are listed. Pass --overlays to show only overlay +metadata, or --groups to show only component-group metadata. + +This command is read-only and does not parse spec files or fetch upstream +sources, so it is fast and works even when locks are missing or stale. + +``` +azldev component metadata [flags] +``` + +### Examples + +``` + # List all metadata (overlays and groups) for all components + azldev component metadata -a + + # List metadata for one component + azldev component metadata -p curl + + # Only overlay metadata + azldev component metadata -p curl --overlays + + # Only component-group metadata + azldev component metadata -p curl --groups + + # List only entries carrying documentation metadata + azldev component metadata -a --only-annotated + + # Filter by category + azldev component metadata -a --category backport-dist-git + + # List only entries that can be upstreamed + azldev component metadata -a --upstreamability yes + + # JSON output for scripting + azldev component metadata -a -q -O json +``` + +### Options + +``` + -a, --all-components Include all components + --category string only include entries whose metadata declares this category + -p, --component stringArray Component name pattern + -g, --component-group stringArray Component group name + --groups list component-group metadata (default lists both overlays and groups) + -h, --help help for metadata + --only-annotated exclude entries that have no metadata + --overlays list overlay metadata (default lists both overlays and groups) + -s, --spec-path stringArray Spec path + --upstreamability string only include entries whose metadata declares this upstreamability ('yes', 'no', or 'unknown') +``` + +### Options inherited from parent commands + +``` + -y, --accept-all accept all prompts + --color mode output colorization mode {always, auto, never} (default auto) + --config-file stringArray additional TOML config file(s) to merge (may be repeated) + -n, --dry-run dry run only (do not take action) + --network-retries int maximum number of attempts for network operations (minimum 1) (default 3) + --no-default-config disable default configuration + -O, --output-format fmt output format {csv, json, markdown, table} (default table) + --permissive-config do not fail on unknown fields in TOML config files + -C, --project string path to Azure Linux project + -q, --quiet only enable minimal output + -v, --verbose enable verbose output +``` + +### SEE ALSO + +* [azldev component](azldev_component.md) - Manage components + diff --git a/docs/user/reference/cli/azldev_component_overlays.md b/docs/user/reference/cli/azldev_component_overlays.md deleted file mode 100644 index 3e5b0f9f..00000000 --- a/docs/user/reference/cli/azldev_component_overlays.md +++ /dev/null @@ -1,75 +0,0 @@ - - -## azldev component overlays - -List overlays and their metadata for components in this project - -### Synopsis - -List overlays defined on components in the project configuration, including any -documentation metadata (category, commits, upstream PR, bug links, etc.) attached -to each overlay. Overlays loaded from a component's overlay-dir (per-file -.overlay.toml format) inherit their metadata from the file's [metadata] block. - -This command is read-only and does not parse spec files or fetch upstream sources, -so it is fast and works even when locks are missing or stale. - -``` -azldev component overlays [flags] -``` - -### Examples - -``` - # List overlays for all components - azldev component overlays -a - - # List overlays for one component - azldev component overlays -p curl - - # List only overlays carrying documentation metadata - azldev component overlays -a --only-annotated - - # Filter by category - azldev component overlays -a --category backport-dist-git - - # List only overlays that can be upstreamed - azldev component overlays -a --upstreamability yes - - # JSON output for scripting - azldev component overlays -a -q -O json -``` - -### Options - -``` - -a, --all-components Include all components - --category string only include overlays whose metadata declares this category - -p, --component stringArray Component name pattern - -g, --component-group stringArray Component group name - -h, --help help for overlays - --only-annotated exclude overlays that have no metadata - -s, --spec-path stringArray Spec path - --upstreamability string only include overlays whose metadata declares this upstreamability ('yes', 'no', or 'unknown') -``` - -### Options inherited from parent commands - -``` - -y, --accept-all accept all prompts - --color mode output colorization mode {always, auto, never} (default auto) - --config-file stringArray additional TOML config file(s) to merge (may be repeated) - -n, --dry-run dry run only (do not take action) - --network-retries int maximum number of attempts for network operations (minimum 1) (default 3) - --no-default-config disable default configuration - -O, --output-format fmt output format {csv, json, markdown, table} (default table) - --permissive-config do not fail on unknown fields in TOML config files - -C, --project string path to Azure Linux project - -q, --quiet only enable minimal output - -v, --verbose enable verbose output -``` - -### SEE ALSO - -* [azldev component](azldev_component.md) - Manage components - diff --git a/docs/user/reference/config/overlays.md b/docs/user/reference/config/overlays.md index b38ffb9c..b39a2059 100644 --- a/docs/user/reference/config/overlays.md +++ b/docs/user/reference/config/overlays.md @@ -184,7 +184,7 @@ lines = ["- Fix CVE-2024-1234"] ### Inspecting overlay metadata -Use `azldev component overlays` to list overlays (and their metadata) for one or more components. The command is read-only and supports `--category` and `--only-annotated` filters; output is available as a table (default) or JSON (`-O json`). +Use `azldev component metadata` to list documentation metadata for one or more components. By default it lists metadata from both sources — component overlays and the component groups each component belongs to — tagging each row with its `source`. Pass `--overlays` or `--groups` to restrict to one source. The command is read-only and supports `--category`, `--only-annotated`, and `--upstreamability` filters; output is available as a table (default) or JSON (`-O json`). ## Examples diff --git a/internal/app/azldev/cmds/component/component.go b/internal/app/azldev/cmds/component/component.go index d04d8f7c..479d5f4f 100644 --- a/internal/app/azldev/cmds/component/component.go +++ b/internal/app/azldev/cmds/component/component.go @@ -32,5 +32,5 @@ components defined in the project configuration.`, queryOnAppInit(app, cmd) renderOnAppInit(app, cmd) updateOnAppInit(app, cmd) - overlaysOnAppInit(app, cmd) + metadataOnAppInit(app, cmd) } diff --git a/internal/app/azldev/cmds/component/metadata.go b/internal/app/azldev/cmds/component/metadata.go new file mode 100644 index 00000000..eb0ebe19 --- /dev/null +++ b/internal/app/azldev/cmds/component/metadata.go @@ -0,0 +1,371 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +package component + +import ( + "fmt" + "slices" + "sort" + "strings" + + "github.com/microsoft/azure-linux-dev-tools/internal/app/azldev" + "github.com/microsoft/azure-linux-dev-tools/internal/app/azldev/core/components" + "github.com/microsoft/azure-linux-dev-tools/internal/projectconfig" + "github.com/spf13/cobra" +) + +// MetadataSource identifies where a listed documentation-metadata entry originates. +type MetadataSource string + +const ( + // MetadataSourceOverlay marks an entry describing one of a component's overlays + // (declared inline in the component config or inherited from an overlay-dir file). + MetadataSourceOverlay MetadataSource = "overlay" + + // MetadataSourceGroup marks an entry describing a component group that the component is + // an explicit member of. + MetadataSourceGroup MetadataSource = "group" +) + +// MetadataOptions configures the 'component metadata' subcommand. +type MetadataOptions struct { + // ComponentFilter selects which components to inspect. + ComponentFilter components.ComponentFilter + + // Overlays selects overlay metadata for output. When both Overlays and Groups are + // false, both sources are listed. + Overlays bool + + // Groups selects component-group metadata for output. When both Overlays and Groups + // are false, both sources are listed. + Groups bool + + // Category, when non-empty, filters output to entries whose metadata declares this + // category. Entries with no metadata are excluded when this is set. + Category string + + // OnlyAnnotated, when true, excludes entries with no metadata. + OnlyAnnotated bool + + // Upstreamability, when non-empty, filters output to entries whose metadata declares + // this upstreamability ('yes', 'no', or 'unknown'). Entries with no metadata count as + // 'unknown'. + Upstreamability string +} + +// wantOverlays reports whether overlay entries should be listed. With no source flag set, +// both sources are listed. +func (o *MetadataOptions) wantOverlays() bool { + return o.Overlays || !o.Groups +} + +// wantGroups reports whether component-group entries should be listed. With no source flag +// set, both sources are listed. +func (o *MetadataOptions) wantGroups() bool { + return o.Groups || !o.Overlays +} + +func metadataOnAppInit(_ *azldev.App, parentCmd *cobra.Command) { + parentCmd.AddCommand(NewComponentMetadataCommand()) +} + +// NewComponentMetadataCommand constructs the 'component metadata' subcommand. +func NewComponentMetadataCommand() *cobra.Command { + options := &MetadataOptions{} + + cmd := &cobra.Command{ + Use: "metadata", + Short: "List documentation metadata for components' overlays and groups", + Long: `List documentation metadata (category, commits, upstream PR, bug links, +upstreamability, etc.) for the selected components. Metadata comes from two +sources, each tagged in the output: + + - 'overlay': metadata attached to one of the component's overlays, declared + inline in the component config or inherited from an overlay-dir + '.overlay.toml' file's [metadata] block. + - 'group': metadata attached to a component group the component is an + explicit member of (per the group's 'components' list). + +By default both sources are listed. Pass --overlays to show only overlay +metadata, or --groups to show only component-group metadata. + +This command is read-only and does not parse spec files or fetch upstream +sources, so it is fast and works even when locks are missing or stale.`, + Example: ` # List all metadata (overlays and groups) for all components + azldev component metadata -a + + # List metadata for one component + azldev component metadata -p curl + + # Only overlay metadata + azldev component metadata -p curl --overlays + + # Only component-group metadata + azldev component metadata -p curl --groups + + # List only entries carrying documentation metadata + azldev component metadata -a --only-annotated + + # Filter by category + azldev component metadata -a --category backport-dist-git + + # List only entries that can be upstreamed + azldev component metadata -a --upstreamability yes + + # JSON output for scripting + azldev component metadata -a -q -O json`, + RunE: azldev.RunFuncWithExtraArgs(func(env *azldev.Env, args []string) (interface{}, error) { + options.ComponentFilter.ComponentNamePatterns = append(args, options.ComponentFilter.ComponentNamePatterns...) + + return ListMetadata(env, options) + }), + ValidArgsFunction: components.GenerateComponentNameCompletions, + } + + azldev.ExportAsMCPTool(cmd) + + components.AddComponentFilterOptionsToCommand(cmd, &options.ComponentFilter) + + cmd.Flags().BoolVar(&options.Overlays, "overlays", false, + "list overlay metadata (default lists both overlays and groups)") + cmd.Flags().BoolVar(&options.Groups, "groups", false, + "list component-group metadata (default lists both overlays and groups)") + cmd.Flags().StringVar(&options.Category, "category", "", + "only include entries whose metadata declares this category") + cmd.Flags().BoolVar(&options.OnlyAnnotated, "only-annotated", false, + "exclude entries that have no metadata") + cmd.Flags().StringVar(&options.Upstreamability, "upstreamability", "", + "only include entries whose metadata declares this upstreamability ('yes', 'no', or 'unknown')") + + // This command is read-only; lock validation is irrelevant. + _ = cmd.Flags().MarkHidden("skip-lock-validation") + + return cmd +} + +// MetadataInfo is the per-entry output for the 'component metadata' subcommand. Each entry +// describes either one of a component's overlays or a component group it belongs to. +type MetadataInfo struct { + // Component is the name of the component the entry relates to; used as the table sort key. + Component string `json:"component" table:",sortkey"` + + // Source identifies whether the entry describes an overlay or a component group. + Source MetadataSource `json:"source"` + + // Index is the 1-based position of the overlay within the component's overlay list. It + // is 0 for group entries. + Index int `json:"index,omitempty" table:",omitempty"` + + // Group is the name of the component group for group entries. Empty for overlay entries. + Group string `json:"group,omitempty" table:",omitempty"` + + // Type is the overlay type (e.g. 'spec-set-tag', 'patch-add'). Empty for group entries. + Type projectconfig.ComponentOverlayType `json:"type,omitempty" table:",omitempty"` + + // Description is the overlay's or group's top-level human-readable description. + Description string `json:"description,omitempty" table:",omitempty"` + + // Category surfaces [projectconfig.OverlayMetadata.Category] for tabular output without + // forcing callers to drill into [MetadataInfo.Metadata]. Empty when the entry has no metadata. + Category projectconfig.OverlayCategory `json:"category,omitempty" table:",omitempty"` + + // Upstreamability surfaces [projectconfig.OverlayMetadata.Upstreamability] for tabular + // output. Always populated: entries without metadata (or with the field omitted) report + // 'unknown', so this is never empty and is always rendered. + Upstreamability projectconfig.OverlayUpstreamability `json:"upstreamability"` + + // Metadata is the full metadata for the entry. Nil when it has none. + Metadata *projectconfig.OverlayMetadata `json:"metadata,omitempty" table:"-"` +} + +// ListMetadata returns the documentation-metadata entries across the selected components. +// Overlay entries and/or component-group entries are included according to the source +// selectors in options (both by default). Lock validation is always skipped - this command +// is read-only. +func ListMetadata(env *azldev.Env, options *MetadataOptions) ([]MetadataInfo, error) { + if options.Category != "" && !projectconfig.OverlayCategory(options.Category).IsValid() { + return nil, fmt.Errorf("%w: unknown overlay category %#q", azldev.ErrInvalidUsage, options.Category) + } + + wantUpstreamability, err := normalizeUpstreamabilityFilter(options.Upstreamability) + if err != nil { + return nil, err + } + + options.ComponentFilter.SkipLockValidation = true + + resolver := components.NewResolver(env) + + comps, err := resolver.FindComponents(&options.ComponentFilter) + if err != nil { + return nil, fmt.Errorf("failed to resolve components:\n%w", err) + } + + results := make([]MetadataInfo, 0) + + for _, comp := range comps.Components() { + name := comp.GetName() + + for _, info := range collectMetadataInfos(env, name, comp.GetConfig(), options) { + if metadataInfoMatchesFilters(info, options, wantUpstreamability) { + results = append(results, info) + } + } + } + + sortMetadataInfos(results) + + return results, nil +} + +// collectMetadataInfos returns the metadata entries for a component, honoring the overlay +// and group source selectors in options. +func collectMetadataInfos( + env *azldev.Env, componentName string, config *projectconfig.ComponentConfig, options *MetadataOptions, +) []MetadataInfo { + infos := make([]MetadataInfo, 0, len(config.Overlays)) + + if options.wantOverlays() { + for idx := range config.Overlays { + overlay := &config.Overlays[idx] + infos = append(infos, buildOverlayMetadataInfo(componentName, idx+1, overlay)) + } + } + + if options.wantGroups() { + groupNames := slices.Clone(env.Config().GroupsByComponent[componentName]) + sort.Strings(groupNames) + + for _, groupName := range groupNames { + group := env.Config().ComponentGroups[groupName] + infos = append(infos, buildGroupMetadataInfo(componentName, groupName, group)) + } + } + + return infos +} + +// buildOverlayMetadataInfo constructs a [MetadataInfo] for one of a component's overlays. +func buildOverlayMetadataInfo( + componentName string, idx int, overlay *projectconfig.ComponentOverlay, +) MetadataInfo { + info := MetadataInfo{ + Component: componentName, + Source: MetadataSourceOverlay, + Index: idx, + Type: overlay.Type, + Description: overlay.Description, + Metadata: overlay.Metadata, + Upstreamability: projectconfig.OverlayUpstreamabilityUnknown, + } + + applyMetadataSummary(&info, overlay.Metadata) + + return info +} + +// buildGroupMetadataInfo constructs a [MetadataInfo] for a component group the component +// belongs to. +func buildGroupMetadataInfo( + componentName, groupName string, group projectconfig.ComponentGroupConfig, +) MetadataInfo { + info := MetadataInfo{ + Component: componentName, + Source: MetadataSourceGroup, + Group: groupName, + Description: group.Description, + Metadata: group.Metadata, + Upstreamability: projectconfig.OverlayUpstreamabilityUnknown, + } + + applyMetadataSummary(&info, group.Metadata) + + return info +} + +// applyMetadataSummary copies the category and upstreamability summary fields from the +// (possibly nil) metadata onto the info. Entries without metadata keep the 'unknown' +// upstreamability set by the caller. +func applyMetadataSummary(info *MetadataInfo, metadata *projectconfig.OverlayMetadata) { + if metadata == nil { + return + } + + info.Category = metadata.Category + if u := metadata.Upstreamability; u != "" { + info.Upstreamability = u + } +} + +// normalizeUpstreamabilityFilter validates a user-supplied '--upstreamability' filter +// value. An empty string means no filter was requested. Only 'yes', 'no', and 'unknown' +// are accepted; any other value yields an [azldev.ErrInvalidUsage] error. +func normalizeUpstreamabilityFilter(value string) (projectconfig.OverlayUpstreamability, error) { + switch value { + case "": + return "", nil + case string(projectconfig.OverlayUpstreamabilityUnknown): + return projectconfig.OverlayUpstreamabilityUnknown, nil + case string(projectconfig.OverlayUpstreamabilityYes): + return projectconfig.OverlayUpstreamabilityYes, nil + case string(projectconfig.OverlayUpstreamabilityNo): + return projectconfig.OverlayUpstreamabilityNo, nil + default: + return "", fmt.Errorf( + "%w: unknown upstreamability %#q; want 'yes', 'no', or 'unknown'", + azldev.ErrInvalidUsage, value, + ) + } +} + +// metadataInfoMatchesFilters reports whether a [MetadataInfo] passes the user-supplied +// category, only-annotated, and upstreamability filters. wantUpstreamability is the +// pre-normalized '--upstreamability' value computed once by [ListMetadata]. +func metadataInfoMatchesFilters( + info MetadataInfo, options *MetadataOptions, wantUpstreamability projectconfig.OverlayUpstreamability, +) bool { + if options.OnlyAnnotated && info.Metadata == nil { + return false + } + + if options.Category != "" && string(info.Category) != options.Category { + return false + } + + if options.Upstreamability != "" && info.Upstreamability != wantUpstreamability { + return false + } + + return true +} + +// sortMetadataInfos orders entries by component, then overlays before groups. Overlay +// entries keep their natural index order; group entries are ordered by group name. +func sortMetadataInfos(infos []MetadataInfo) { + slices.SortStableFunc(infos, func(left, right MetadataInfo) int { + if c := strings.Compare(left.Component, right.Component); c != 0 { + return c + } + + if c := metadataSourceRank(left.Source) - metadataSourceRank(right.Source); c != 0 { + return c + } + + if left.Source == MetadataSourceGroup { + return strings.Compare(left.Group, right.Group) + } + + return left.Index - right.Index + }) +} + +// metadataSourceRank assigns a stable ordering rank so overlay entries sort before group +// entries regardless of the source label's lexical order. +func metadataSourceRank(source MetadataSource) int { + if source == MetadataSourceOverlay { + return 0 + } + + return 1 +} diff --git a/internal/app/azldev/cmds/component/metadata_test.go b/internal/app/azldev/cmds/component/metadata_test.go new file mode 100644 index 00000000..39f341ac --- /dev/null +++ b/internal/app/azldev/cmds/component/metadata_test.go @@ -0,0 +1,304 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +package component_test + +import ( + "testing" + + "github.com/microsoft/azure-linux-dev-tools/internal/app/azldev/cmds/component" + "github.com/microsoft/azure-linux-dev-tools/internal/app/azldev/core/components" + "github.com/microsoft/azure-linux-dev-tools/internal/app/azldev/core/testutils" + "github.com/microsoft/azure-linux-dev-tools/internal/projectconfig" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestNewComponentMetadataCommand(t *testing.T) { + cmd := component.NewComponentMetadataCommand() + require.NotNil(t, cmd) + assert.Equal(t, "metadata", cmd.Use) +} + +// seedComponentsWithOverlays adds two components with a mix of annotated and unannotated +// overlays (no group membership). +func seedComponentsWithOverlays(t *testing.T, testEnv *testutils.TestEnv) { + t.Helper() + + testEnv.Config.Components["pkg-a"] = projectconfig.ComponentConfig{ + Name: "pkg-a", + Spec: projectconfig.SpecSource{Path: "/specs/pkg-a.spec"}, + Overlays: []projectconfig.ComponentOverlay{ + { + Type: projectconfig.ComponentOverlayAddSpecTag, + Tag: "Vendor", + Value: "Microsoft", + Metadata: &projectconfig.OverlayMetadata{ + Category: projectconfig.OverlayCategoryAZLBrandingPolicy, + }, + }, + { + Type: projectconfig.ComponentOverlayAddPatch, + Source: "patches/fix.patch", + Metadata: &projectconfig.OverlayMetadata{ + Category: projectconfig.OverlayCategoryBackportDistGit, + Commits: []string{"https://src.fedoraproject.org/rpms/pkg-a/c/abc"}, + }, + }, + { + // No metadata. + Type: projectconfig.ComponentOverlaySearchAndReplaceInSpec, + Regex: "foo", + Replacement: "bar", + }, + }, + } + + testEnv.Config.Components["pkg-b"] = projectconfig.ComponentConfig{ + Name: "pkg-b", + Spec: projectconfig.SpecSource{Path: "/specs/pkg-b.spec"}, + Overlays: []projectconfig.ComponentOverlay{ + { + Type: projectconfig.ComponentOverlayAddSpecTag, + Tag: "Packager", + Value: "azl", + Metadata: &projectconfig.OverlayMetadata{ + Category: projectconfig.OverlayCategoryAZLBuild, + PR: "https://github.com/example/repo/pull/1", + Upstreamability: projectconfig.OverlayUpstreamabilityYes, + }, + }, + }, + } +} + +func TestListMetadata_AllOverlays(t *testing.T) { + testEnv := testutils.NewTestEnv(t) + seedComponentsWithOverlays(t, testEnv) + + options := &component.MetadataOptions{ + ComponentFilter: components.ComponentFilter{IncludeAllComponents: true}, + } + + results, err := component.ListMetadata(testEnv.Env, options) + require.NoError(t, err) + require.Len(t, results, 4) + + // Sorted by (component, source, index). + assert.Equal(t, "pkg-a", results[0].Component) + assert.Equal(t, component.MetadataSourceOverlay, results[0].Source) + assert.Equal(t, 1, results[0].Index) + assert.Equal(t, projectconfig.OverlayCategoryAZLBrandingPolicy, results[0].Category) + + assert.Equal(t, "pkg-a", results[1].Component) + assert.Equal(t, 2, results[1].Index) + assert.Equal(t, projectconfig.OverlayCategoryBackportDistGit, results[1].Category) + require.NotNil(t, results[1].Metadata) + assert.Equal(t, []string{"https://src.fedoraproject.org/rpms/pkg-a/c/abc"}, results[1].Metadata.Commits) + + assert.Equal(t, "pkg-a", results[2].Component) + assert.Equal(t, 3, results[2].Index) + assert.Nil(t, results[2].Metadata, "overlay without metadata reports nil") + assert.Empty(t, results[2].Category) + + assert.Equal(t, "pkg-b", results[3].Component) + assert.Equal(t, projectconfig.OverlayCategoryAZLBuild, results[3].Category) +} + +func TestListMetadata_OnlyAnnotated(t *testing.T) { + testEnv := testutils.NewTestEnv(t) + seedComponentsWithOverlays(t, testEnv) + + options := &component.MetadataOptions{ + ComponentFilter: components.ComponentFilter{IncludeAllComponents: true}, + OnlyAnnotated: true, + } + + results, err := component.ListMetadata(testEnv.Env, options) + require.NoError(t, err) + require.Len(t, results, 3, "unannotated search-replace overlay must be excluded") + + for _, info := range results { + assert.NotNil(t, info.Metadata) + assert.NotEmpty(t, info.Category) + } +} + +func TestListMetadata_FilterByCategory(t *testing.T) { + testEnv := testutils.NewTestEnv(t) + seedComponentsWithOverlays(t, testEnv) + + options := &component.MetadataOptions{ + ComponentFilter: components.ComponentFilter{IncludeAllComponents: true}, + Category: string(projectconfig.OverlayCategoryBackportDistGit), + } + + results, err := component.ListMetadata(testEnv.Env, options) + require.NoError(t, err) + require.Len(t, results, 1) + assert.Equal(t, "pkg-a", results[0].Component) + assert.Equal(t, 2, results[0].Index) +} + +func TestListMetadata_UnknownCategoryRejected(t *testing.T) { + testEnv := testutils.NewTestEnv(t) + seedComponentsWithOverlays(t, testEnv) + + options := &component.MetadataOptions{ + ComponentFilter: components.ComponentFilter{IncludeAllComponents: true}, + Category: "bogus", + } + + _, err := component.ListMetadata(testEnv.Env, options) + require.Error(t, err) + assert.Contains(t, err.Error(), "unknown overlay category") +} + +func TestListMetadata_Upstreamable(t *testing.T) { + testEnv := testutils.NewTestEnv(t) + seedComponentsWithOverlays(t, testEnv) + + options := &component.MetadataOptions{ + ComponentFilter: components.ComponentFilter{IncludeAllComponents: true}, + Upstreamability: "yes", + } + + results, err := component.ListMetadata(testEnv.Env, options) + require.NoError(t, err) + require.Len(t, results, 1, "only the upstreamable overlay in pkg-b should be included") + assert.Equal(t, "pkg-b", results[0].Component) + assert.Equal(t, projectconfig.OverlayUpstreamabilityYes, results[0].Upstreamability) + assert.Equal(t, projectconfig.OverlayCategoryAZLBuild, results[0].Category) +} + +func TestListMetadata_UnknownUpstreamabilityRejected(t *testing.T) { + testEnv := testutils.NewTestEnv(t) + seedComponentsWithOverlays(t, testEnv) + + options := &component.MetadataOptions{ + ComponentFilter: components.ComponentFilter{IncludeAllComponents: true}, + Upstreamability: "maybe", + } + + _, err := component.ListMetadata(testEnv.Env, options) + require.Error(t, err) + assert.Contains(t, err.Error(), "unknown upstreamability") +} + +// seedComponentWithGroups adds a component with one annotated overlay that belongs to two +// groups, one annotated and one bare. +func seedComponentWithGroups(t *testing.T, testEnv *testutils.TestEnv) { + t.Helper() + + testEnv.Config.Components["pkg-a"] = projectconfig.ComponentConfig{ + Name: "pkg-a", + Spec: projectconfig.SpecSource{Path: "/specs/pkg-a.spec"}, + Overlays: []projectconfig.ComponentOverlay{ + { + Type: projectconfig.ComponentOverlayAddSpecTag, + Tag: "Vendor", + Value: "Microsoft", + Metadata: &projectconfig.OverlayMetadata{ + Category: projectconfig.OverlayCategoryAZLBrandingPolicy, + }, + }, + }, + } + + testEnv.Config.ComponentGroups["annotated-group"] = projectconfig.ComponentGroupConfig{ + Description: "Group with metadata", + Components: []string{"pkg-a"}, + Metadata: &projectconfig.OverlayMetadata{ + Category: projectconfig.OverlayCategoryAZLTestDisablement, + Upstreamability: projectconfig.OverlayUpstreamabilityNo, + }, + } + + testEnv.Config.ComponentGroups["bare-group"] = projectconfig.ComponentGroupConfig{ + Description: "Group without metadata", + Components: []string{"pkg-a"}, + } + + // Mirror the reverse index the loader builds for explicit group membership. + testEnv.Config.GroupsByComponent["pkg-a"] = []string{"annotated-group", "bare-group"} +} + +func TestListMetadata_DefaultListsBothSources(t *testing.T) { + testEnv := testutils.NewTestEnv(t) + seedComponentWithGroups(t, testEnv) + + options := &component.MetadataOptions{ + ComponentFilter: components.ComponentFilter{IncludeAllComponents: true}, + } + + results, err := component.ListMetadata(testEnv.Env, options) + require.NoError(t, err) + require.Len(t, results, 3, "one overlay plus two group memberships") + + // Overlay sorts before groups. + assert.Equal(t, component.MetadataSourceOverlay, results[0].Source) + assert.Equal(t, 1, results[0].Index) + assert.Empty(t, results[0].Group) + assert.Equal(t, projectconfig.OverlayCategoryAZLBrandingPolicy, results[0].Category) + + // Groups follow, ordered by group name. + assert.Equal(t, component.MetadataSourceGroup, results[1].Source) + assert.Equal(t, "annotated-group", results[1].Group) + assert.Equal(t, 0, results[1].Index) + require.NotNil(t, results[1].Metadata) + assert.Equal(t, projectconfig.OverlayCategoryAZLTestDisablement, results[1].Category) + assert.Equal(t, projectconfig.OverlayUpstreamabilityNo, results[1].Upstreamability) + + assert.Equal(t, component.MetadataSourceGroup, results[2].Source) + assert.Equal(t, "bare-group", results[2].Group) + assert.Nil(t, results[2].Metadata) + assert.Equal(t, projectconfig.OverlayUpstreamabilityUnknown, results[2].Upstreamability) +} + +func TestListMetadata_OverlaysOnly(t *testing.T) { + testEnv := testutils.NewTestEnv(t) + seedComponentWithGroups(t, testEnv) + + options := &component.MetadataOptions{ + ComponentFilter: components.ComponentFilter{IncludeAllComponents: true}, + Overlays: true, + } + + results, err := component.ListMetadata(testEnv.Env, options) + require.NoError(t, err) + require.Len(t, results, 1, "only the overlay entry is listed") + assert.Equal(t, component.MetadataSourceOverlay, results[0].Source) +} + +func TestListMetadata_GroupsOnly(t *testing.T) { + testEnv := testutils.NewTestEnv(t) + seedComponentWithGroups(t, testEnv) + + options := &component.MetadataOptions{ + ComponentFilter: components.ComponentFilter{IncludeAllComponents: true}, + Groups: true, + } + + results, err := component.ListMetadata(testEnv.Env, options) + require.NoError(t, err) + require.Len(t, results, 2, "only the two group entries are listed") + + for _, info := range results { + assert.Equal(t, component.MetadataSourceGroup, info.Source) + } +} + +func TestListMetadata_BothSelectorsListBoth(t *testing.T) { + testEnv := testutils.NewTestEnv(t) + seedComponentWithGroups(t, testEnv) + + options := &component.MetadataOptions{ + ComponentFilter: components.ComponentFilter{IncludeAllComponents: true}, + Overlays: true, + Groups: true, + } + + results, err := component.ListMetadata(testEnv.Env, options) + require.NoError(t, err) + require.Len(t, results, 3, "both selectors set lists overlays and groups") +} diff --git a/internal/app/azldev/cmds/component/overlays.go b/internal/app/azldev/cmds/component/overlays.go deleted file mode 100644 index 210c0d3c..00000000 --- a/internal/app/azldev/cmds/component/overlays.go +++ /dev/null @@ -1,234 +0,0 @@ -// Copyright (c) Microsoft Corporation. -// Licensed under the MIT License. - -package component - -import ( - "fmt" - "slices" - "strings" - - "github.com/microsoft/azure-linux-dev-tools/internal/app/azldev" - "github.com/microsoft/azure-linux-dev-tools/internal/app/azldev/core/components" - "github.com/microsoft/azure-linux-dev-tools/internal/projectconfig" - "github.com/spf13/cobra" -) - -// OverlaysOptions configures the 'component overlays' subcommand. -type OverlaysOptions struct { - // ComponentFilter selects which components to inspect. - ComponentFilter components.ComponentFilter - - // Category, when non-empty, filters output to overlays whose metadata declares this - // category. Overlays with no metadata are excluded when this is set. - Category string - - // OnlyAnnotated, when true, excludes overlays with no metadata. - OnlyAnnotated bool - - // Upstreamability, when non-empty, filters output to overlays whose metadata declares - // this upstreamability ('yes', 'no', or 'unknown'). Overlays with no metadata count as - // 'unknown'. - Upstreamability string -} - -func overlaysOnAppInit(_ *azldev.App, parentCmd *cobra.Command) { - parentCmd.AddCommand(NewComponentOverlaysCommand()) -} - -// NewComponentOverlaysCommand constructs the 'component overlays' subcommand. -func NewComponentOverlaysCommand() *cobra.Command { - options := &OverlaysOptions{} - - cmd := &cobra.Command{ - Use: "overlays", - Short: "List overlays and their metadata for components in this project", - Long: `List overlays defined on components in the project configuration, including any -documentation metadata (category, commits, upstream PR, bug links, etc.) attached -to each overlay. Overlays loaded from a component's overlay-dir (per-file -.overlay.toml format) inherit their metadata from the file's [metadata] block. - -This command is read-only and does not parse spec files or fetch upstream sources, -so it is fast and works even when locks are missing or stale.`, - Example: ` # List overlays for all components - azldev component overlays -a - - # List overlays for one component - azldev component overlays -p curl - - # List only overlays carrying documentation metadata - azldev component overlays -a --only-annotated - - # Filter by category - azldev component overlays -a --category backport-dist-git - - # List only overlays that can be upstreamed - azldev component overlays -a --upstreamability yes - - # JSON output for scripting - azldev component overlays -a -q -O json`, - RunE: azldev.RunFuncWithExtraArgs(func(env *azldev.Env, args []string) (interface{}, error) { - options.ComponentFilter.ComponentNamePatterns = append(args, options.ComponentFilter.ComponentNamePatterns...) - - return ListOverlays(env, options) - }), - ValidArgsFunction: components.GenerateComponentNameCompletions, - } - - azldev.ExportAsMCPTool(cmd) - - components.AddComponentFilterOptionsToCommand(cmd, &options.ComponentFilter) - - cmd.Flags().StringVar(&options.Category, "category", "", - "only include overlays whose metadata declares this category") - cmd.Flags().BoolVar(&options.OnlyAnnotated, "only-annotated", false, - "exclude overlays that have no metadata") - cmd.Flags().StringVar(&options.Upstreamability, "upstreamability", "", - "only include overlays whose metadata declares this upstreamability ('yes', 'no', or 'unknown')") - - // This command is read-only; lock validation is irrelevant. - _ = cmd.Flags().MarkHidden("skip-lock-validation") - - return cmd -} - -// OverlayInfo is the per-overlay output for the 'component overlays' subcommand. -type OverlayInfo struct { - // Component is the name of the component owning the overlay; used as the table sort key. - Component string `json:"component" table:",sortkey"` - - // Index is the 1-based position of the overlay within the component's overlay list. - Index int `json:"index"` - - // Type is the overlay type (e.g. 'spec-set-tag', 'patch-add'). - Type projectconfig.ComponentOverlayType `json:"type"` - - // Description is the overlay's top-level human-readable description. - Description string `json:"description,omitempty" table:",omitempty"` - - // Category surfaces [OverlayMetadata.Category] for tabular output without forcing - // callers to drill into [OverlayInfo.Metadata]. Empty when the overlay has no metadata. - Category projectconfig.OverlayCategory `json:"category,omitempty" table:",omitempty"` - - // Upstreamability surfaces [OverlayMetadata.Upstreamability] for tabular output. Always - // populated: overlays without metadata (or with the field omitted) report 'unknown', so - // this is never empty and is always rendered. - Upstreamability projectconfig.OverlayUpstreamability `json:"upstreamability"` - - // Metadata is the full metadata for the overlay. Nil when the overlay has none. - Metadata *projectconfig.OverlayMetadata `json:"metadata,omitempty" table:"-"` -} - -// ListOverlays returns one [OverlayInfo] per matching overlay across the selected -// components, applying the filters in options. Lock validation is always skipped — -// this command is read-only. -func ListOverlays(env *azldev.Env, options *OverlaysOptions) ([]OverlayInfo, error) { - if options.Category != "" && !projectconfig.OverlayCategory(options.Category).IsValid() { - return nil, fmt.Errorf("%w: unknown overlay category %#q", azldev.ErrInvalidUsage, options.Category) - } - - wantUpstreamability, err := normalizeUpstreamabilityFilter(options.Upstreamability) - if err != nil { - return nil, err - } - - options.ComponentFilter.SkipLockValidation = true - - resolver := components.NewResolver(env) - - comps, err := resolver.FindComponents(&options.ComponentFilter) - if err != nil { - return nil, fmt.Errorf("failed to resolve components:\n%w", err) - } - - results := make([]OverlayInfo, 0) - - for _, comp := range comps.Components() { - config := comp.GetConfig() - - for idx, overlay := range config.Overlays { - info := buildOverlayInfo(comp.GetName(), idx+1, &overlay) - - if !overlayInfoMatchesFilters(info, options, wantUpstreamability) { - continue - } - - results = append(results, info) - } - } - - slices.SortStableFunc(results, func(a, b OverlayInfo) int { - if c := strings.Compare(a.Component, b.Component); c != 0 { - return c - } - - return a.Index - b.Index - }) - - return results, nil -} - -// buildOverlayInfo constructs a single [OverlayInfo] for an overlay. -func buildOverlayInfo( - componentName string, idx int, overlay *projectconfig.ComponentOverlay, -) OverlayInfo { - info := OverlayInfo{ - Component: componentName, - Index: idx, - Type: overlay.Type, - Description: overlay.Description, - Metadata: overlay.Metadata, - Upstreamability: projectconfig.OverlayUpstreamabilityUnknown, - } - - if overlay.Metadata != nil { - info.Category = overlay.Metadata.Category - if u := overlay.Metadata.Upstreamability; u != "" { - info.Upstreamability = u - } - } - - return info -} - -// normalizeUpstreamabilityFilter validates a user-supplied '--upstreamability' filter -// value. An empty string means no filter was requested. Only 'yes', 'no', and 'unknown' -// are accepted; any other value yields an [azldev.ErrInvalidUsage] error. -func normalizeUpstreamabilityFilter(value string) (projectconfig.OverlayUpstreamability, error) { - switch value { - case "": - return "", nil - case string(projectconfig.OverlayUpstreamabilityUnknown): - return projectconfig.OverlayUpstreamabilityUnknown, nil - case string(projectconfig.OverlayUpstreamabilityYes): - return projectconfig.OverlayUpstreamabilityYes, nil - case string(projectconfig.OverlayUpstreamabilityNo): - return projectconfig.OverlayUpstreamabilityNo, nil - default: - return "", fmt.Errorf( - "%w: unknown upstreamability %#q; want 'yes', 'no', or 'unknown'", - azldev.ErrInvalidUsage, value, - ) - } -} - -// overlayInfoMatchesFilters reports whether an [OverlayInfo] passes the user-supplied -// category, only-annotated, and upstreamability filters. wantUpstreamability is the -// pre-normalized '--upstreamability' value computed once by [ListOverlays]. -func overlayInfoMatchesFilters( - info OverlayInfo, options *OverlaysOptions, wantUpstreamability projectconfig.OverlayUpstreamability, -) bool { - if options.OnlyAnnotated && info.Metadata == nil { - return false - } - - if options.Category != "" && string(info.Category) != options.Category { - return false - } - - if options.Upstreamability != "" && info.Upstreamability != wantUpstreamability { - return false - } - - return true -} diff --git a/internal/app/azldev/cmds/component/overlays_test.go b/internal/app/azldev/cmds/component/overlays_test.go deleted file mode 100644 index 1284216b..00000000 --- a/internal/app/azldev/cmds/component/overlays_test.go +++ /dev/null @@ -1,185 +0,0 @@ -// Copyright (c) Microsoft Corporation. -// Licensed under the MIT License. - -package component_test - -import ( - "testing" - - "github.com/microsoft/azure-linux-dev-tools/internal/app/azldev/cmds/component" - "github.com/microsoft/azure-linux-dev-tools/internal/app/azldev/core/components" - "github.com/microsoft/azure-linux-dev-tools/internal/app/azldev/core/testutils" - "github.com/microsoft/azure-linux-dev-tools/internal/projectconfig" - "github.com/stretchr/testify/assert" - "github.com/stretchr/testify/require" -) - -func TestNewComponentOverlaysCommand(t *testing.T) { - cmd := component.NewComponentOverlaysCommand() - require.NotNil(t, cmd) - assert.Equal(t, "overlays", cmd.Use) -} - -// seedComponentsWithOverlays adds two components with a mix of annotated and -// unannotated overlays. -func seedComponentsWithOverlays(t *testing.T, testEnv *testutils.TestEnv) { - t.Helper() - - testEnv.Config.Components["pkg-a"] = projectconfig.ComponentConfig{ - Name: "pkg-a", - Spec: projectconfig.SpecSource{Path: "/specs/pkg-a.spec"}, - Overlays: []projectconfig.ComponentOverlay{ - { - Type: projectconfig.ComponentOverlayAddSpecTag, - Tag: "Vendor", - Value: "Microsoft", - Metadata: &projectconfig.OverlayMetadata{ - Category: projectconfig.OverlayCategoryAZLBrandingPolicy, - }, - }, - { - Type: projectconfig.ComponentOverlayAddPatch, - Source: "patches/fix.patch", - Metadata: &projectconfig.OverlayMetadata{ - Category: projectconfig.OverlayCategoryBackportDistGit, - Commits: []string{"https://src.fedoraproject.org/rpms/pkg-a/c/abc"}, - }, - }, - { - // No metadata. - Type: projectconfig.ComponentOverlaySearchAndReplaceInSpec, - Regex: "foo", - Replacement: "bar", - }, - }, - } - - testEnv.Config.Components["pkg-b"] = projectconfig.ComponentConfig{ - Name: "pkg-b", - Spec: projectconfig.SpecSource{Path: "/specs/pkg-b.spec"}, - Overlays: []projectconfig.ComponentOverlay{ - { - Type: projectconfig.ComponentOverlayAddSpecTag, - Tag: "Packager", - Value: "azl", - Metadata: &projectconfig.OverlayMetadata{ - Category: projectconfig.OverlayCategoryAZLBuild, - PR: "https://github.com/example/repo/pull/1", - Upstreamability: projectconfig.OverlayUpstreamabilityYes, - }, - }, - }, - } -} - -func TestListOverlays_AllComponents(t *testing.T) { - testEnv := testutils.NewTestEnv(t) - seedComponentsWithOverlays(t, testEnv) - - options := &component.OverlaysOptions{ - ComponentFilter: components.ComponentFilter{IncludeAllComponents: true}, - } - - results, err := component.ListOverlays(testEnv.Env, options) - require.NoError(t, err) - require.Len(t, results, 4) - - // Sorted by (component, index). - assert.Equal(t, "pkg-a", results[0].Component) - assert.Equal(t, 1, results[0].Index) - assert.Equal(t, projectconfig.OverlayCategoryAZLBrandingPolicy, results[0].Category) - - assert.Equal(t, "pkg-a", results[1].Component) - assert.Equal(t, 2, results[1].Index) - assert.Equal(t, projectconfig.OverlayCategoryBackportDistGit, results[1].Category) - require.NotNil(t, results[1].Metadata) - assert.Equal(t, []string{"https://src.fedoraproject.org/rpms/pkg-a/c/abc"}, results[1].Metadata.Commits) - - assert.Equal(t, "pkg-a", results[2].Component) - assert.Equal(t, 3, results[2].Index) - assert.Nil(t, results[2].Metadata, "overlay without metadata reports nil") - assert.Empty(t, results[2].Category) - - assert.Equal(t, "pkg-b", results[3].Component) - assert.Equal(t, projectconfig.OverlayCategoryAZLBuild, results[3].Category) -} - -func TestListOverlays_OnlyAnnotated(t *testing.T) { - testEnv := testutils.NewTestEnv(t) - seedComponentsWithOverlays(t, testEnv) - - options := &component.OverlaysOptions{ - ComponentFilter: components.ComponentFilter{IncludeAllComponents: true}, - OnlyAnnotated: true, - } - - results, err := component.ListOverlays(testEnv.Env, options) - require.NoError(t, err) - require.Len(t, results, 3, "unannotated search-replace overlay must be excluded") - - for _, info := range results { - assert.NotNil(t, info.Metadata) - assert.NotEmpty(t, info.Category) - } -} - -func TestListOverlays_FilterByCategory(t *testing.T) { - testEnv := testutils.NewTestEnv(t) - seedComponentsWithOverlays(t, testEnv) - - options := &component.OverlaysOptions{ - ComponentFilter: components.ComponentFilter{IncludeAllComponents: true}, - Category: string(projectconfig.OverlayCategoryBackportDistGit), - } - - results, err := component.ListOverlays(testEnv.Env, options) - require.NoError(t, err) - require.Len(t, results, 1) - assert.Equal(t, "pkg-a", results[0].Component) - assert.Equal(t, 2, results[0].Index) -} - -func TestListOverlays_UnknownCategoryRejected(t *testing.T) { - testEnv := testutils.NewTestEnv(t) - seedComponentsWithOverlays(t, testEnv) - - options := &component.OverlaysOptions{ - ComponentFilter: components.ComponentFilter{IncludeAllComponents: true}, - Category: "bogus", - } - - _, err := component.ListOverlays(testEnv.Env, options) - require.Error(t, err) - assert.Contains(t, err.Error(), "unknown overlay category") -} - -func TestListOverlays_Upstreamable(t *testing.T) { - testEnv := testutils.NewTestEnv(t) - seedComponentsWithOverlays(t, testEnv) - - options := &component.OverlaysOptions{ - ComponentFilter: components.ComponentFilter{IncludeAllComponents: true}, - Upstreamability: "yes", - } - - results, err := component.ListOverlays(testEnv.Env, options) - require.NoError(t, err) - require.Len(t, results, 1, "only the upstreamable overlay in pkg-b should be included") - assert.Equal(t, "pkg-b", results[0].Component) - assert.Equal(t, projectconfig.OverlayUpstreamabilityYes, results[0].Upstreamability) - assert.Equal(t, projectconfig.OverlayCategoryAZLBuild, results[0].Category) -} - -func TestListOverlays_UnknownUpstreamabilityRejected(t *testing.T) { - testEnv := testutils.NewTestEnv(t) - seedComponentsWithOverlays(t, testEnv) - - options := &component.OverlaysOptions{ - ComponentFilter: components.ComponentFilter{IncludeAllComponents: true}, - Upstreamability: "maybe", - } - - _, err := component.ListOverlays(testEnv.Env, options) - require.Error(t, err) - assert.Contains(t, err.Error(), "unknown upstreamability") -} diff --git a/scenario/__snapshots__/TestMCPServerMode_1.snap.json b/scenario/__snapshots__/TestMCPServerMode_1.snap.json index 4e276a17..8d29f3ef 100755 --- a/scenario/__snapshots__/TestMCPServerMode_1.snap.json +++ b/scenario/__snapshots__/TestMCPServerMode_1.snap.json @@ -292,7 +292,7 @@ "openWorldHint": true, "readOnlyHint": false }, - "description": "List overlays and their metadata for components in this project", + "description": "List documentation metadata for components' overlays and groups", "inputSchema": { "properties": { "accept-all": { @@ -307,7 +307,7 @@ }, "category": { "default": "", - "description": "only include overlays whose metadata declares this category", + "description": "only include entries whose metadata declares this category", "type": "string" }, "color": { @@ -331,6 +331,11 @@ "description": "dry run only (do not take action)", "type": "boolean" }, + "groups": { + "default": false, + "description": "list component-group metadata (default lists both overlays and groups)", + "type": "boolean" + }, "network-retries": { "default": 3, "description": "maximum number of attempts for network operations (minimum 1)", @@ -343,13 +348,18 @@ }, "only-annotated": { "default": false, - "description": "exclude overlays that have no metadata", + "description": "exclude entries that have no metadata", "type": "boolean" }, "output-format": { "description": "output format {csv, json, markdown, table}", "type": "string" }, + "overlays": { + "default": false, + "description": "list overlay metadata (default lists both overlays and groups)", + "type": "boolean" + }, "permissive-config": { "default": false, "description": "do not fail on unknown fields in TOML config files", @@ -371,7 +381,7 @@ }, "upstreamability": { "default": "", - "description": "only include overlays whose metadata declares this upstreamability ('yes', 'no', or 'unknown')", + "description": "only include entries whose metadata declares this upstreamability ('yes', 'no', or 'unknown')", "type": "string" }, "verbose": { @@ -383,7 +393,7 @@ "required": [], "type": "object" }, - "name": "component-overlays" + "name": "component-metadata" }, { "annotations": { diff --git a/scenario/__snapshots__/TestSnapshotsContainer_config_generate-schema_stdout_1.snap b/scenario/__snapshots__/TestSnapshotsContainer_config_generate-schema_stdout_1.snap index cc48318f..923a40df 100755 --- a/scenario/__snapshots__/TestSnapshotsContainer_config_generate-schema_stdout_1.snap +++ b/scenario/__snapshots__/TestSnapshotsContainer_config_generate-schema_stdout_1.snap @@ -166,6 +166,11 @@ "title": "Description", "description": "Description of this component group" }, + "metadata": { + "$ref": "#/$defs/OverlayMetadata", + "title": "Metadata", + "description": "Optional documentation metadata for this component group" + }, "components": { "items": { "type": "string" diff --git a/scenario/__snapshots__/TestSnapshots_config_generate-schema_stdout_1.snap b/scenario/__snapshots__/TestSnapshots_config_generate-schema_stdout_1.snap index cc48318f..923a40df 100755 --- a/scenario/__snapshots__/TestSnapshots_config_generate-schema_stdout_1.snap +++ b/scenario/__snapshots__/TestSnapshots_config_generate-schema_stdout_1.snap @@ -166,6 +166,11 @@ "title": "Description", "description": "Description of this component group" }, + "metadata": { + "$ref": "#/$defs/OverlayMetadata", + "title": "Metadata", + "description": "Optional documentation metadata for this component group" + }, "components": { "items": { "type": "string" From 48a3372c6bf87d2fef0e93dacbcde009f166ebae Mon Sep 17 00:00:00 2001 From: Nan Liu Date: Mon, 22 Jun 2026 20:30:52 +0000 Subject: [PATCH 6/6] refactor(metadata): replace upstreamability enum with upstreamable bool Swap the tri-state OverlayUpstreamability enum (yes/no/unknown) for a *bool Upstreamable field on OverlayMetadata. A nil pointer (omitted in TOML) means upstreamability has not yet been assessed; true/false are the two assessed outcomes. The CLI flag is renamed --upstreamability -> --upstreamable and accepts 'true', 'false', or 'unknown'. 'unknown' matches both annotated entries that omit the field and entries with no metadata at all. Regenerates JSON schema, CLI reference docs, and scenario snapshots. --- .../cli/azldev_component_metadata.md | 4 +- .../user/reference/config/component-groups.md | 6 +- docs/user/reference/config/overlays.md | 4 +- .../app/azldev/cmds/component/metadata.go | 116 ++++++++++-------- .../azldev/cmds/component/metadata_test.go | 72 +++++++++-- internal/projectconfig/loader_test.go | 5 +- internal/projectconfig/overlay_metadata.go | 53 ++------ .../projectconfig/overlay_metadata_test.go | 27 ++-- .../TestMCPServerMode_1.snap.json | 4 +- ...ainer_config_generate-schema_stdout_1.snap | 14 +-- ...shots_config_generate-schema_stdout_1.snap | 14 +-- schemas/azldev.schema.json | 14 +-- 12 files changed, 173 insertions(+), 160 deletions(-) diff --git a/docs/user/reference/cli/azldev_component_metadata.md b/docs/user/reference/cli/azldev_component_metadata.md index 1cb2d2c0..fb6815d6 100644 --- a/docs/user/reference/cli/azldev_component_metadata.md +++ b/docs/user/reference/cli/azldev_component_metadata.md @@ -48,7 +48,7 @@ azldev component metadata [flags] azldev component metadata -a --category backport-dist-git # List only entries that can be upstreamed - azldev component metadata -a --upstreamability yes + azldev component metadata -a --upstreamable true # JSON output for scripting azldev component metadata -a -q -O json @@ -66,7 +66,7 @@ azldev component metadata [flags] --only-annotated exclude entries that have no metadata --overlays list overlay metadata (default lists both overlays and groups) -s, --spec-path stringArray Spec path - --upstreamability string only include entries whose metadata declares this upstreamability ('yes', 'no', or 'unknown') + --upstreamable string only include entries whose metadata declares this upstreamability ('true', 'false', or 'unknown') ``` ### Options inherited from parent commands diff --git a/docs/user/reference/config/component-groups.md b/docs/user/reference/config/component-groups.md index 619b23b0..d5c40cac 100644 --- a/docs/user/reference/config/component-groups.md +++ b/docs/user/reference/config/component-groups.md @@ -15,7 +15,7 @@ Component groups organize related components together and let you apply shared d ## Metadata -The optional `[component-groups..metadata]` table documents the group's intent and provenance. It shares exactly the same fields and validation rules as [overlay metadata](overlays.md): a required `category`, plus optional commit/PR/bug links and an upstreamability assessment. It is documentation only — it does not affect how members are resolved or built. +The optional `[component-groups..metadata]` table documents the group's intent and provenance. It shares exactly the same fields and validation rules as [overlay metadata](overlays.md): a required `category`, plus optional commit/PR/bug links and an upstreamable assessment. It is documentation only — it does not affect how members are resolved or built. | Field | TOML Key | Type | Required | Description | |-------|----------|------|----------|-------------| @@ -23,7 +23,7 @@ The optional `[component-groups..metadata]` table documents the group's in | Commits | `commits` | string array (URLs) | No | Upstream commit URLs this group references; required when `category = "backport-dist-git"` | | Upstream PR | `pr` | string (URL) | No | URL of the related upstream pull request | | Issue-tracker links | `bug` | string array (URLs) | No | URLs of related issue-tracker entries | -| Upstreamability | `upstreamability` | enum (`yes`/`no`/`unknown`) | No | Whether the group's change can be upstreamed (defaults to `unknown`) | +| Upstreamable | `upstreamable` | boolean | No | Whether the group's change can be upstreamed (`true`/`false`); omit when not yet assessed | ```toml [component-groups.check-skip-initial-failures] @@ -32,7 +32,7 @@ components = ["bats", "dnf", "git"] [component-groups.check-skip-initial-failures.metadata] category = "azl-test-disablement" -upstreamability = "unknown" +# upstreamable omitted: upstreamability for this group has not been assessed yet. ``` ## Component Membership diff --git a/docs/user/reference/config/overlays.md b/docs/user/reference/config/overlays.md index b39a2059..f4d12011 100644 --- a/docs/user/reference/config/overlays.md +++ b/docs/user/reference/config/overlays.md @@ -86,7 +86,7 @@ Overlays can carry an optional `metadata` table that documents *why* the overlay | Commits | `commits` | List of upstream commit URLs (Fedora dist-git or upstream project) that this overlay backports or implements. Each entry must be an absolute http(s) URL. | | PR | `pr` | URL of the upstream pull request that carries (or proposes) the fix. | | Bug | `bug` | List of bug-tracker URLs related to this overlay. | -| Upstreamability | `upstreamability` | Whether this change can be upstreamed: `"yes"`, `"no"`, or `"unknown"`. Defaults to `"unknown"` (not yet assessed) when omitted. | +| Upstreamable | `upstreamable` | Boolean indicating whether this change can be upstreamed: `true` or `false`. Omit the field when upstreamability has not yet been assessed. | ### Categories @@ -184,7 +184,7 @@ lines = ["- Fix CVE-2024-1234"] ### Inspecting overlay metadata -Use `azldev component metadata` to list documentation metadata for one or more components. By default it lists metadata from both sources — component overlays and the component groups each component belongs to — tagging each row with its `source`. Pass `--overlays` or `--groups` to restrict to one source. The command is read-only and supports `--category`, `--only-annotated`, and `--upstreamability` filters; output is available as a table (default) or JSON (`-O json`). +Use `azldev component metadata` to list documentation metadata for one or more components. By default it lists metadata from both sources — component overlays and the component groups each component belongs to — tagging each row with its `source`. Pass `--overlays` or `--groups` to restrict to one source. The command is read-only and supports `--category`, `--only-annotated`, and `--upstreamable` filters; output is available as a table (default) or JSON (`-O json`). ## Examples diff --git a/internal/app/azldev/cmds/component/metadata.go b/internal/app/azldev/cmds/component/metadata.go index eb0ebe19..fccf41a9 100644 --- a/internal/app/azldev/cmds/component/metadata.go +++ b/internal/app/azldev/cmds/component/metadata.go @@ -48,10 +48,10 @@ type MetadataOptions struct { // OnlyAnnotated, when true, excludes entries with no metadata. OnlyAnnotated bool - // Upstreamability, when non-empty, filters output to entries whose metadata declares - // this upstreamability ('yes', 'no', or 'unknown'). Entries with no metadata count as - // 'unknown'. - Upstreamability string + // Upstreamable, when non-empty, filters output to entries whose metadata declares this + // upstreamability ('true', 'false', or 'unknown'). Entries with no metadata, or whose + // metadata omits the field, count as 'unknown'. + Upstreamable string } // wantOverlays reports whether overlay entries should be listed. With no source flag set, @@ -111,7 +111,7 @@ sources, so it is fast and works even when locks are missing or stale.`, azldev component metadata -a --category backport-dist-git # List only entries that can be upstreamed - azldev component metadata -a --upstreamability yes + azldev component metadata -a --upstreamable true # JSON output for scripting azldev component metadata -a -q -O json`, @@ -135,8 +135,8 @@ sources, so it is fast and works even when locks are missing or stale.`, "only include entries whose metadata declares this category") cmd.Flags().BoolVar(&options.OnlyAnnotated, "only-annotated", false, "exclude entries that have no metadata") - cmd.Flags().StringVar(&options.Upstreamability, "upstreamability", "", - "only include entries whose metadata declares this upstreamability ('yes', 'no', or 'unknown')") + cmd.Flags().StringVar(&options.Upstreamable, "upstreamable", "", + "only include entries whose metadata declares this upstreamability ('true', 'false', or 'unknown')") // This command is read-only; lock validation is irrelevant. _ = cmd.Flags().MarkHidden("skip-lock-validation") @@ -170,10 +170,10 @@ type MetadataInfo struct { // forcing callers to drill into [MetadataInfo.Metadata]. Empty when the entry has no metadata. Category projectconfig.OverlayCategory `json:"category,omitempty" table:",omitempty"` - // Upstreamability surfaces [projectconfig.OverlayMetadata.Upstreamability] for tabular - // output. Always populated: entries without metadata (or with the field omitted) report - // 'unknown', so this is never empty and is always rendered. - Upstreamability projectconfig.OverlayUpstreamability `json:"upstreamability"` + // Upstreamable surfaces [projectconfig.OverlayMetadata.Upstreamable] for tabular + // output. Nil when the entry has no metadata or its metadata omits the field, which + // renders as a blank cell (upstreamability not assessed). + Upstreamable *bool `json:"upstreamable,omitempty" table:",omitempty"` // Metadata is the full metadata for the entry. Nil when it has none. Metadata *projectconfig.OverlayMetadata `json:"metadata,omitempty" table:"-"` @@ -188,7 +188,7 @@ func ListMetadata(env *azldev.Env, options *MetadataOptions) ([]MetadataInfo, er return nil, fmt.Errorf("%w: unknown overlay category %#q", azldev.ErrInvalidUsage, options.Category) } - wantUpstreamability, err := normalizeUpstreamabilityFilter(options.Upstreamability) + wantUpstreamable, err := normalizeUpstreamableFilter(options.Upstreamable) if err != nil { return nil, err } @@ -208,7 +208,7 @@ func ListMetadata(env *azldev.Env, options *MetadataOptions) ([]MetadataInfo, er name := comp.GetName() for _, info := range collectMetadataInfos(env, name, comp.GetConfig(), options) { - if metadataInfoMatchesFilters(info, options, wantUpstreamability) { + if metadataInfoMatchesFilters(info, options, wantUpstreamable) { results = append(results, info) } } @@ -251,13 +251,12 @@ func buildOverlayMetadataInfo( componentName string, idx int, overlay *projectconfig.ComponentOverlay, ) MetadataInfo { info := MetadataInfo{ - Component: componentName, - Source: MetadataSourceOverlay, - Index: idx, - Type: overlay.Type, - Description: overlay.Description, - Metadata: overlay.Metadata, - Upstreamability: projectconfig.OverlayUpstreamabilityUnknown, + Component: componentName, + Source: MetadataSourceOverlay, + Index: idx, + Type: overlay.Type, + Description: overlay.Description, + Metadata: overlay.Metadata, } applyMetadataSummary(&info, overlay.Metadata) @@ -271,12 +270,11 @@ func buildGroupMetadataInfo( componentName, groupName string, group projectconfig.ComponentGroupConfig, ) MetadataInfo { info := MetadataInfo{ - Component: componentName, - Source: MetadataSourceGroup, - Group: groupName, - Description: group.Description, - Metadata: group.Metadata, - Upstreamability: projectconfig.OverlayUpstreamabilityUnknown, + Component: componentName, + Source: MetadataSourceGroup, + Group: groupName, + Description: group.Description, + Metadata: group.Metadata, } applyMetadataSummary(&info, group.Metadata) @@ -284,46 +282,48 @@ func buildGroupMetadataInfo( return info } -// applyMetadataSummary copies the category and upstreamability summary fields from the -// (possibly nil) metadata onto the info. Entries without metadata keep the 'unknown' -// upstreamability set by the caller. +// applyMetadataSummary copies the category and upstreamable summary fields from the +// (possibly nil) metadata onto the info. Entries without metadata leave the fields unset +// (category empty, upstreamability not assessed). func applyMetadataSummary(info *MetadataInfo, metadata *projectconfig.OverlayMetadata) { if metadata == nil { return } info.Category = metadata.Category - if u := metadata.Upstreamability; u != "" { - info.Upstreamability = u - } + info.Upstreamable = metadata.Upstreamable } -// normalizeUpstreamabilityFilter validates a user-supplied '--upstreamability' filter -// value. An empty string means no filter was requested. Only 'yes', 'no', and 'unknown' -// are accepted; any other value yields an [azldev.ErrInvalidUsage] error. -func normalizeUpstreamabilityFilter(value string) (projectconfig.OverlayUpstreamability, error) { +// normalizeUpstreamableFilter validates a user-supplied '--upstreamable' filter value. An +// empty string means no filter was requested. Only 'true', 'false', and 'unknown' are +// accepted; any other value yields an [azldev.ErrInvalidUsage] error. The returned string +// Recognized values for the '--upstreamable' filter flag. Kept as named constants so +// [normalizeUpstreamableFilter] (which validates user input) and [upstreamableMatches] +// (which interprets it) stay in sync. +const ( + upstreamableFilterTrue = "true" + upstreamableFilterFalse = "false" + upstreamableFilterUnknown = "unknown" +) + +// is the normalized filter ("", "true", "false", or "unknown"). +func normalizeUpstreamableFilter(value string) (string, error) { switch value { - case "": - return "", nil - case string(projectconfig.OverlayUpstreamabilityUnknown): - return projectconfig.OverlayUpstreamabilityUnknown, nil - case string(projectconfig.OverlayUpstreamabilityYes): - return projectconfig.OverlayUpstreamabilityYes, nil - case string(projectconfig.OverlayUpstreamabilityNo): - return projectconfig.OverlayUpstreamabilityNo, nil + case "", upstreamableFilterTrue, upstreamableFilterFalse, upstreamableFilterUnknown: + return value, nil default: return "", fmt.Errorf( - "%w: unknown upstreamability %#q; want 'yes', 'no', or 'unknown'", + "%w: unknown upstreamable value %#q; want 'true', 'false', or 'unknown'", azldev.ErrInvalidUsage, value, ) } } // metadataInfoMatchesFilters reports whether a [MetadataInfo] passes the user-supplied -// category, only-annotated, and upstreamability filters. wantUpstreamability is the -// pre-normalized '--upstreamability' value computed once by [ListMetadata]. +// category, only-annotated, and upstreamable filters. wantUpstreamable is the +// pre-normalized '--upstreamable' value computed once by [ListMetadata]. func metadataInfoMatchesFilters( - info MetadataInfo, options *MetadataOptions, wantUpstreamability projectconfig.OverlayUpstreamability, + info MetadataInfo, options *MetadataOptions, wantUpstreamable string, ) bool { if options.OnlyAnnotated && info.Metadata == nil { return false @@ -333,13 +333,31 @@ func metadataInfoMatchesFilters( return false } - if options.Upstreamability != "" && info.Upstreamability != wantUpstreamability { + if wantUpstreamable != "" && !upstreamableMatches(info.Upstreamable, wantUpstreamable) { return false } return true } +// upstreamableMatches reports whether an entry's upstreamable value (nil meaning "not +// assessed") matches the normalized filter ('true', 'false', or 'unknown'). The 'unknown' +// arm matches both unannotated entries (no metadata at all) and annotated entries that +// omit the field, since both are unassessed; combine with '--only-annotated' to restrict +// to entries whose author left the field blank. +func upstreamableMatches(upstreamable *bool, want string) bool { + switch want { + case upstreamableFilterUnknown: + return upstreamable == nil + case upstreamableFilterTrue: + return upstreamable != nil && *upstreamable + case upstreamableFilterFalse: + return upstreamable != nil && !*upstreamable + default: + return false + } +} + // sortMetadataInfos orders entries by component, then overlays before groups. Overlay // entries keep their natural index order; group entries are ordered by group name. func sortMetadataInfos(infos []MetadataInfo) { diff --git a/internal/app/azldev/cmds/component/metadata_test.go b/internal/app/azldev/cmds/component/metadata_test.go index 39f341ac..aa80fc5b 100644 --- a/internal/app/azldev/cmds/component/metadata_test.go +++ b/internal/app/azldev/cmds/component/metadata_test.go @@ -10,6 +10,7 @@ import ( "github.com/microsoft/azure-linux-dev-tools/internal/app/azldev/core/components" "github.com/microsoft/azure-linux-dev-tools/internal/app/azldev/core/testutils" "github.com/microsoft/azure-linux-dev-tools/internal/projectconfig" + "github.com/samber/lo" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) @@ -63,9 +64,9 @@ func seedComponentsWithOverlays(t *testing.T, testEnv *testutils.TestEnv) { Tag: "Packager", Value: "azl", Metadata: &projectconfig.OverlayMetadata{ - Category: projectconfig.OverlayCategoryAZLBuild, - PR: "https://github.com/example/repo/pull/1", - Upstreamability: projectconfig.OverlayUpstreamabilityYes, + Category: projectconfig.OverlayCategoryAZLBuild, + PR: "https://github.com/example/repo/pull/1", + Upstreamable: lo.ToPtr(true), }, }, }, @@ -160,29 +161,75 @@ func TestListMetadata_Upstreamable(t *testing.T) { options := &component.MetadataOptions{ ComponentFilter: components.ComponentFilter{IncludeAllComponents: true}, - Upstreamability: "yes", + Upstreamable: "true", } results, err := component.ListMetadata(testEnv.Env, options) require.NoError(t, err) require.Len(t, results, 1, "only the upstreamable overlay in pkg-b should be included") assert.Equal(t, "pkg-b", results[0].Component) - assert.Equal(t, projectconfig.OverlayUpstreamabilityYes, results[0].Upstreamability) + require.NotNil(t, results[0].Upstreamable) + assert.True(t, *results[0].Upstreamable) assert.Equal(t, projectconfig.OverlayCategoryAZLBuild, results[0].Category) } -func TestListMetadata_UnknownUpstreamabilityRejected(t *testing.T) { +func TestListMetadata_UnknownUpstreamableRejected(t *testing.T) { testEnv := testutils.NewTestEnv(t) seedComponentsWithOverlays(t, testEnv) options := &component.MetadataOptions{ ComponentFilter: components.ComponentFilter{IncludeAllComponents: true}, - Upstreamability: "maybe", + Upstreamable: "maybe", } _, err := component.ListMetadata(testEnv.Env, options) require.Error(t, err) - assert.Contains(t, err.Error(), "unknown upstreamability") + assert.Contains(t, err.Error(), "unknown upstreamable value") +} + +func TestListMetadata_UpstreamableFalse(t *testing.T) { + testEnv := testutils.NewTestEnv(t) + seedComponentWithGroups(t, testEnv) + + options := &component.MetadataOptions{ + ComponentFilter: components.ComponentFilter{IncludeAllComponents: true}, + Upstreamable: "false", + } + + results, err := component.ListMetadata(testEnv.Env, options) + require.NoError(t, err) + require.Len(t, results, 1, "only the group annotated as not-upstreamable should be included") + assert.Equal(t, component.MetadataSourceGroup, results[0].Source) + assert.Equal(t, "annotated-group", results[0].Group) + require.NotNil(t, results[0].Upstreamable) + assert.False(t, *results[0].Upstreamable) +} + +func TestListMetadata_UpstreamableUnknown(t *testing.T) { + testEnv := testutils.NewTestEnv(t) + seedComponentWithGroups(t, testEnv) + + options := &component.MetadataOptions{ + ComponentFilter: components.ComponentFilter{IncludeAllComponents: true}, + Upstreamable: "unknown", + } + + results, err := component.ListMetadata(testEnv.Env, options) + require.NoError(t, err) + // 'unknown' matches both annotated entries that omit the field (the overlay) and + // unannotated entries (the bare group). The 'false'-annotated group is excluded. + require.Len(t, results, 2) + + for _, info := range results { + assert.Nil(t, info.Upstreamable) + } + + assert.Equal(t, component.MetadataSourceOverlay, results[0].Source) + require.NotNil(t, results[0].Metadata, "annotated overlay without 'upstreamable' must still match 'unknown'") + + assert.Equal(t, component.MetadataSourceGroup, results[1].Source) + assert.Equal(t, "bare-group", results[1].Group) + assert.Nil(t, results[1].Metadata, "unannotated entry must also match 'unknown'") } // seedComponentWithGroups adds a component with one annotated overlay that belongs to two @@ -209,8 +256,8 @@ func seedComponentWithGroups(t *testing.T, testEnv *testutils.TestEnv) { Description: "Group with metadata", Components: []string{"pkg-a"}, Metadata: &projectconfig.OverlayMetadata{ - Category: projectconfig.OverlayCategoryAZLTestDisablement, - Upstreamability: projectconfig.OverlayUpstreamabilityNo, + Category: projectconfig.OverlayCategoryAZLTestDisablement, + Upstreamable: lo.ToPtr(false), }, } @@ -247,12 +294,13 @@ func TestListMetadata_DefaultListsBothSources(t *testing.T) { assert.Equal(t, 0, results[1].Index) require.NotNil(t, results[1].Metadata) assert.Equal(t, projectconfig.OverlayCategoryAZLTestDisablement, results[1].Category) - assert.Equal(t, projectconfig.OverlayUpstreamabilityNo, results[1].Upstreamability) + require.NotNil(t, results[1].Upstreamable) + assert.False(t, *results[1].Upstreamable) assert.Equal(t, component.MetadataSourceGroup, results[2].Source) assert.Equal(t, "bare-group", results[2].Group) assert.Nil(t, results[2].Metadata) - assert.Equal(t, projectconfig.OverlayUpstreamabilityUnknown, results[2].Upstreamability) + assert.Nil(t, results[2].Upstreamable) } func TestListMetadata_OverlaysOnly(t *testing.T) { diff --git a/internal/projectconfig/loader_test.go b/internal/projectconfig/loader_test.go index 6d40fc88..7f347bf8 100644 --- a/internal/projectconfig/loader_test.go +++ b/internal/projectconfig/loader_test.go @@ -494,7 +494,7 @@ specs = ["SPECS/**/*.spec"] category = "backport-dist-git" commits = ["https://example.com/commit/abc"] pr = "https://example.com/pr/1" -upstreamability = "yes" +upstreamable = true ` ctx := testctx.NewCtx() @@ -509,7 +509,8 @@ upstreamability = "yes" assert.Equal(t, OverlayCategoryBackportDistGit, group.Metadata.Category) assert.Equal(t, []string{"https://example.com/commit/abc"}, group.Metadata.Commits) assert.Equal(t, "https://example.com/pr/1", group.Metadata.PR) - assert.Equal(t, OverlayUpstreamabilityYes, group.Metadata.Upstreamability) + require.NotNil(t, group.Metadata.Upstreamable) + assert.True(t, *group.Metadata.Upstreamable) } } diff --git a/internal/projectconfig/overlay_metadata.go b/internal/projectconfig/overlay_metadata.go index 0905d683..f80bb44f 100644 --- a/internal/projectconfig/overlay_metadata.go +++ b/internal/projectconfig/overlay_metadata.go @@ -63,32 +63,6 @@ func (c OverlayCategory) IsValid() bool { return slices.Contains(allOverlayCategories, c) } -// OverlayUpstreamability records whether an overlay's change can be sent upstream. It -// defaults to 'unknown' (not yet assessed) when the field is omitted. -type OverlayUpstreamability string - -const ( - // OverlayUpstreamabilityUnknown means upstreamability has not been assessed. It is the - // default when the field is omitted (an empty value is treated as 'unknown'). - OverlayUpstreamabilityUnknown OverlayUpstreamability = "unknown" - // OverlayUpstreamabilityYes means the change is a candidate for upstreaming. - OverlayUpstreamabilityYes OverlayUpstreamability = "yes" - // OverlayUpstreamabilityNo means the change is intentionally AZL-specific and will - // never be upstreamed. - OverlayUpstreamabilityNo OverlayUpstreamability = "no" -) - -// IsValid reports whether u is one of the recognized [OverlayUpstreamability] values. An -// empty value is accepted and means the field was omitted (defaults to 'unknown'). -func (u OverlayUpstreamability) IsValid() bool { - switch u { - case "", OverlayUpstreamabilityUnknown, OverlayUpstreamabilityYes, OverlayUpstreamabilityNo: - return true - default: - return false - } -} - // OverlayMetadata describes the intent and provenance of an overlay. It is documentation // only — it does not affect how the overlay is applied and is excluded from component // fingerprints. When present, it must declare a [OverlayMetadata.Category]; other fields @@ -107,9 +81,9 @@ type OverlayMetadata struct { // BugLinks holds URLs of issue-tracker entries related to this overlay. BugLinks []string `toml:"bug,omitempty" json:"bug,omitempty" validate:"omitempty,dive,http_url" jsonschema:"title=Issue-tracker links,description=URLs of related issue-tracker entries"` - // Upstreamability records whether this overlay's change can be upstreamed: 'yes', 'no', - // or 'unknown'. Omitting the field defaults to 'unknown' (not yet assessed). - Upstreamability OverlayUpstreamability `toml:"upstreamability,omitempty" json:"upstreamability,omitempty" jsonschema:"enum=yes,enum=no,enum=unknown,default=unknown,title=Upstreamability,description=Whether this overlay's change can be upstreamed: 'yes'; 'no'; or 'unknown' (the default)"` + // Upstreamable records whether this overlay's change can be upstreamed. It is omitted + // (nil) when upstreamability has not yet been assessed. + Upstreamable *bool `toml:"upstreamable,omitempty" json:"upstreamable,omitempty" jsonschema:"title=Upstreamable,description=Whether this overlay's change can be upstreamed; omit if not yet assessed"` } // clone returns a deep copy of the metadata. It is used to stamp a single file-level @@ -121,11 +95,16 @@ func (m *OverlayMetadata) clone() *OverlayMetadata { return nil } - cp := *m - cp.Commits = slices.Clone(m.Commits) - cp.BugLinks = slices.Clone(m.BugLinks) + cloned := *m + cloned.Commits = slices.Clone(m.Commits) + cloned.BugLinks = slices.Clone(m.BugLinks) - return &cp + if m.Upstreamable != nil { + upstreamable := *m.Upstreamable + cloned.Upstreamable = &upstreamable + } + + return &cloned } // metadataValidator is a shared, concurrency-safe validator instance reused across all @@ -154,14 +133,6 @@ func (m *OverlayMetadata) Validate() error { ) } - if !m.Upstreamability.IsValid() { - return fmt.Errorf( - "unknown upstreamability %#q; want %#q, %#q, or %#q", - string(m.Upstreamability), string(OverlayUpstreamabilityYes), - string(OverlayUpstreamabilityNo), string(OverlayUpstreamabilityUnknown), - ) - } - // Field-level constraints (for example 'pr' and 'bug' must be http(s) URLs) are // expressed as 'validate' struct tags and enforced here. if err := metadataValidator.Struct(m); err != nil { diff --git a/internal/projectconfig/overlay_metadata_test.go b/internal/projectconfig/overlay_metadata_test.go index 858a8ee0..1fc36fbf 100644 --- a/internal/projectconfig/overlay_metadata_test.go +++ b/internal/projectconfig/overlay_metadata_test.go @@ -7,6 +7,7 @@ import ( "testing" "github.com/microsoft/azure-linux-dev-tools/internal/projectconfig" + "github.com/samber/lo" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) @@ -48,27 +49,19 @@ func TestOverlayMetadata_Validate(t *testing.T) { { name: "azl-branding-policy may carry pr and commits", metadata: projectconfig.OverlayMetadata{ - Category: projectconfig.OverlayCategoryAZLBrandingPolicy, - PR: "https://github.com/example/repo/pull/2", - Commits: []string{"https://github.com/example/repo/commit/deadbeef"}, - Upstreamability: projectconfig.OverlayUpstreamabilityYes, + Category: projectconfig.OverlayCategoryAZLBrandingPolicy, + PR: "https://github.com/example/repo/pull/2", + Commits: []string{"https://github.com/example/repo/commit/deadbeef"}, + Upstreamable: lo.ToPtr(true), }, }, { - name: "upstreamability no is valid", + name: "upstreamable false is valid", metadata: projectconfig.OverlayMetadata{ - Category: projectconfig.OverlayCategoryAZLBuild, - Upstreamability: projectconfig.OverlayUpstreamabilityNo, + Category: projectconfig.OverlayCategoryAZLBuild, + Upstreamable: lo.ToPtr(false), }, }, - { - name: "unknown upstreamability rejected", - metadata: projectconfig.OverlayMetadata{ - Category: projectconfig.OverlayCategoryAZLBuild, - Upstreamability: projectconfig.OverlayUpstreamability("maybe"), - }, - errorContains: "unknown upstreamability", - }, { name: "missing category", metadata: projectconfig.OverlayMetadata{ @@ -138,9 +131,9 @@ func TestComponentOverlay_Validate_Metadata(t *testing.T) { require.NoError(t, overlay.Validate()) // Overlay with invalid metadata fails — wraps the metadata error. - overlay.Metadata.Upstreamability = projectconfig.OverlayUpstreamability("bogus") + overlay.Metadata.Category = projectconfig.OverlayCategory("bogus") err := overlay.Validate() require.Error(t, err) - assert.Contains(t, err.Error(), "unknown upstreamability") + assert.Contains(t, err.Error(), "unknown overlay category") } diff --git a/scenario/__snapshots__/TestMCPServerMode_1.snap.json b/scenario/__snapshots__/TestMCPServerMode_1.snap.json index 8d29f3ef..32874ac7 100755 --- a/scenario/__snapshots__/TestMCPServerMode_1.snap.json +++ b/scenario/__snapshots__/TestMCPServerMode_1.snap.json @@ -379,9 +379,9 @@ "description": "Spec path", "type": "string" }, - "upstreamability": { + "upstreamable": { "default": "", - "description": "only include entries whose metadata declares this upstreamability ('yes', 'no', or 'unknown')", + "description": "only include entries whose metadata declares this upstreamability ('true', 'false', or 'unknown')", "type": "string" }, "verbose": { diff --git a/scenario/__snapshots__/TestSnapshotsContainer_config_generate-schema_stdout_1.snap b/scenario/__snapshots__/TestSnapshotsContainer_config_generate-schema_stdout_1.snap index 923a40df..6cf89c26 100755 --- a/scenario/__snapshots__/TestSnapshotsContainer_config_generate-schema_stdout_1.snap +++ b/scenario/__snapshots__/TestSnapshotsContainer_config_generate-schema_stdout_1.snap @@ -766,16 +766,10 @@ "title": "Issue-tracker links", "description": "URLs of related issue-tracker entries" }, - "upstreamability": { - "type": "string", - "enum": [ - "yes", - "no", - "unknown" - ], - "title": "Upstreamability", - "description": "Whether this overlay's change can be upstreamed: 'yes'; 'no'; or 'unknown' (the default)", - "default": "unknown" + "upstreamable": { + "type": "boolean", + "title": "Upstreamable", + "description": "Whether this overlay's change can be upstreamed; omit if not yet assessed" } }, "additionalProperties": false, diff --git a/scenario/__snapshots__/TestSnapshots_config_generate-schema_stdout_1.snap b/scenario/__snapshots__/TestSnapshots_config_generate-schema_stdout_1.snap index 923a40df..6cf89c26 100755 --- a/scenario/__snapshots__/TestSnapshots_config_generate-schema_stdout_1.snap +++ b/scenario/__snapshots__/TestSnapshots_config_generate-schema_stdout_1.snap @@ -766,16 +766,10 @@ "title": "Issue-tracker links", "description": "URLs of related issue-tracker entries" }, - "upstreamability": { - "type": "string", - "enum": [ - "yes", - "no", - "unknown" - ], - "title": "Upstreamability", - "description": "Whether this overlay's change can be upstreamed: 'yes'; 'no'; or 'unknown' (the default)", - "default": "unknown" + "upstreamable": { + "type": "boolean", + "title": "Upstreamable", + "description": "Whether this overlay's change can be upstreamed; omit if not yet assessed" } }, "additionalProperties": false, diff --git a/schemas/azldev.schema.json b/schemas/azldev.schema.json index 923a40df..6cf89c26 100644 --- a/schemas/azldev.schema.json +++ b/schemas/azldev.schema.json @@ -766,16 +766,10 @@ "title": "Issue-tracker links", "description": "URLs of related issue-tracker entries" }, - "upstreamability": { - "type": "string", - "enum": [ - "yes", - "no", - "unknown" - ], - "title": "Upstreamability", - "description": "Whether this overlay's change can be upstreamed: 'yes'; 'no'; or 'unknown' (the default)", - "default": "unknown" + "upstreamable": { + "type": "boolean", + "title": "Upstreamable", + "description": "Whether this overlay's change can be upstreamed; omit if not yet assessed" } }, "additionalProperties": false,