diff --git a/cli/azd/docs/design/extension-flag-architecture.md b/cli/azd/docs/design/extension-flag-architecture.md new file mode 100644 index 00000000000..f356df63bbf --- /dev/null +++ b/cli/azd/docs/design/extension-flag-architecture.md @@ -0,0 +1,204 @@ +# Spec: Global Flags and Extension Flag Dispatch + +## Status + +**Documenting existing behavior** — this spec formalizes the flag contract that already exists in the azd + extension SDK implementation, and adds enforcement that was previously missing. + +## Goal + +Define how azd handles global flags when dispatching to extensions, including the pre-parsing pipeline, environment variable propagation, and the reserved flag registry that prevents namespace collisions. + +## Background + +azd extensions are standalone binaries that azd discovers, installs, and invokes as subcommands. When a user runs `azd model custom create --endpoint https://...`, azd: + +1. Pre-parses its own global flags from the full argument list +2. Launches the extension binary as a child process +3. Passes the raw arguments **and** global flag values (via environment variables) to the extension + +This creates a **shared flag namespace** — both azd and the extension parse the same `argv`. If an extension registers a flag that collides with an azd global flag (e.g., both use `-e`), azd's pre-parser consumes the value for its own purpose, and the extension either gets the wrong value or causes azd to error. + +Issue [#7271](https://github.com/Azure/azure-dev/issues/7271) demonstrated this: the `azd model` extension used `-e` for `--endpoint` (a URL), but azd's pre-parser treated the URL as an environment name and failed validation. + +## Architecture + +### Flag Flow Diagram + +``` +User runs: azd model custom create -e https://example.com/api + + ┌─────────────────────────────────────────────────────┐ + │ azd host process │ + │ │ + │ 1. ParseGlobalFlags(args) │ + │ - Reads: -e/--environment, --debug, --cwd, etc. │ + │ - UnknownFlags: true (ignores extension flags) │ + │ - Populates GlobalCommandOptions │ + │ │ + │ 2. extensions.go: DisableFlagParsing: true │ + │ - Cobra does NOT parse extension-specific flags │ + │ - Raw args passed through to extension │ + │ │ + │ 3. runner.go: Invoke() │ + │ - Converts GlobalCommandOptions → AZD_* env vars │ + │ - Launches extension binary with: │ + │ - Args: original argv (including -e value) │ + │ - Env: AZD_ENVIRONMENT, AZD_DEBUG, etc. │ + └──────────────────────┬──────────────────────────────┘ + │ + ▼ + ┌─────────────────────────────────────────────────────┐ + │ Extension binary (child process) │ + │ │ + │ 1. NewExtensionRootCommand() [SDK] │ + │ - Registers SAME global flags: │ + │ --environment/-e, --debug, --cwd/-C, etc. │ + │ - Falls back to AZD_* env vars if not on CLI │ + │ │ + │ 2. Extension-specific subcommands │ + │ - Register their OWN flags (--model, --version) │ + │ - Must NOT collide with reserved flags │ + └─────────────────────────────────────────────────────┘ +``` + +### Key Insight + +Both azd and the extension parse the **same arguments**. azd does not strip its global flags before passing args to extensions. This means: + +- If an extension reuses `-e` for `--endpoint`, azd's pre-parser sees `-e https://example.com/api` and tries to use the URL as an environment name +- The extension then also receives `-e https://example.com/api` in its args, but the SDK's root command binds `-e` to `--environment`, so the extension's own `-e` flag on a subcommand creates a conflict + +This is not a new restriction — it has been true since the extension system was designed. + +## Global Flags (Host Side) + +azd's global flags are registered through several mechanisms. All of them are reserved — extensions must not reuse these names regardless of how they are registered. + +### Pre-parsed Global Flags + +These flags are registered in `CreateGlobalFlagSet()` (`cmd/auto_install.go`) and pre-parsed by `ParseGlobalFlags()` **before** command dispatch. They are available to middleware and DI resolution even for extension commands: + +| Long Name | Short | Type | Default | Hidden | Description | +|-----------|-------|------|---------|--------|-------------| +| `cwd` | `C` | string | `""` | No | Sets the current working directory | +| `debug` | — | bool | `false` | No | Enables debugging and diagnostics logging | +| `no-prompt` | — | bool | `false` | No | Accepts default value instead of prompting | +| `trace-log-file` | — | string | `""` | Yes | Write a diagnostics trace to a file | +| `trace-log-url` | — | string | `""` | Yes | Send traces to an OpenTelemetry-compatible endpoint | + +### Root Command Persistent Flags + +These flags are registered on the root command's persistent flag set in `root.go` and are available to all subcommands: + +| Long Name | Short | Type | Default | Description | +|-----------|-------|------|---------|-------------| +| `environment` | `e` | string | `$AZURE_ENV_NAME` | The name of the environment to use | +| `output` | `o` | string | `"default"` | The output format (json, table, none) | + +### Cobra Built-in and Custom Command Flags + +| Long Name | Short | Description | +|-----------|-------|-------------| +| `help` | `h` | Help for the current command (cobra built-in) | +| `docs` | — | Opens documentation for the current command | + +### Pre-parsing Behavior + +`ParseGlobalFlags` uses `pflag.ParseErrorsAllowlist{UnknownFlags: true}` to silently ignore flags it doesn't recognize. This allows extension-specific flags (like `--model`, `--version`) to pass through without error. However, any flag that matches an azd global flag **will be consumed** by the pre-parser. + +## Global Flags (Extension SDK Side) + +The extension SDK's `NewExtensionRootCommand()` (`pkg/azdext/extension_command.go`) registers these persistent flags on every extension's root command: + +| Long Name | Short | Type | Default | Env Var Fallback | +|-----------|-------|------|---------|-----------------| +| `environment` | `e` | string | `""` | `AZD_ENVIRONMENT` | +| `cwd` | `C` | string | `""` | `AZD_CWD` | +| `debug` | — | bool | `false` | `AZD_DEBUG` | +| `no-prompt` | — | bool | `false` | `AZD_NO_PROMPT` | +| `output` | `o` | string | `"default"` | — | +| `trace-log-file` | — | string | `""` | — | +| `trace-log-url` | — | string | `""` | — | + +### Env Var Propagation + +azd passes global flag values to extensions via two mechanisms: + +1. **Environment variables** (`runner.go`): `AZD_DEBUG`, `AZD_NO_PROMPT`, `AZD_CWD`, `AZD_ENVIRONMENT` +2. **Raw args**: The original command-line arguments are passed through unchanged + +The SDK's `PersistentPreRunE` checks if each flag was explicitly set on the command line; if not, it falls back to the corresponding `AZD_*` environment variable. This dual-path design ensures global values are available whether the extension is invoked via azd or directly during development. + +## Reserved Flags + +### Definition + +A **reserved flag** is any flag that azd pre-parses from the command line before dispatching to extensions, or that the extension SDK registers on the root command. Extensions must not register flags with the same long name or short name on their subcommands. + +### The Reserved List + +The canonical list is maintained in two locations that are kept in sync by a test: + +- **Host side**: `internal/reserved_flags.go` — `reservedFlags` slice with `ReservedFlags()` getter and lookup helpers +- **SDK side**: `pkg/azdext/reserved_flags.go` — `reservedGlobalFlags` slice with validation + +| Long Name | Short | Reason Reserved | +|-----------|-------|----------------| +| `environment` | `e` | azd pre-parses for env selection; SDK registers on root | +| `cwd` | `C` | azd pre-parses for working directory; SDK registers on root | +| `debug` | — | azd pre-parses for debug mode; SDK registers on root | +| `no-prompt` | — | azd pre-parses for non-interactive mode; SDK registers on root | +| `output` | `o` | SDK registers on root for output format | +| `help` | `h` | cobra built-in; universal across all commands | +| `docs` | — | azd root command flag | +| `trace-log-file` | — | azd pre-parses for telemetry; SDK registers on root | +| `trace-log-url` | — | azd pre-parses for telemetry; SDK registers on root | + +## Enforcement + +### SDK-Level Validation + +`ValidateNoReservedFlagConflicts(root)` is called in `azdext.Run()` before command execution. It: + +1. Walks the entire command tree +2. For each command's flags, checks both long and short names against the reserved list +3. Skips flags on the root command's persistent flag set (those are the SDK-provided azd-compatible flags) +4. Returns a detailed error listing all conflicts with remediation guidance + +Any extension built with the azd SDK gets this check automatically — no opt-in required. + +### Sync Test + +A test (`TestReservedFlagsInSyncWithInternal`) ensures the SDK-side and host-side reserved flag lists stay in sync. If a developer adds a new global flag to one list but not the other, the test fails. + +## Adding a New Global Flag + +When azd needs a new global flag: + +1. Add the flag to `CreateGlobalFlagSet()` in `cmd/auto_install.go` +2. Add parsing logic to `ParseGlobalFlags()` in `cmd/auto_install.go` +3. Add to `reservedFlags` in `internal/reserved_flags.go` +4. Add to `reservedGlobalFlags` in `pkg/azdext/reserved_flags.go` +5. If it should be available to extensions: + - Register it in `NewExtensionRootCommand()` in `pkg/azdext/extension_command.go` + - Add env var propagation in `runner.go` + - Add env var fallback in `NewExtensionRootCommand`'s `PersistentPreRunE` +6. Run tests — the sync test will catch mismatches between steps 3 and 4 + +The reserved flag registry makes this process explicit and safe: any extension that happens to use the new flag name will get a clear error at startup instead of a mysterious runtime failure. + +## Implementation References + +| Component | File | +|-----------|------| +| Global flag registration | `cmd/auto_install.go` — `CreateGlobalFlagSet()` | +| Global flag pre-parsing | `cmd/auto_install.go` — `ParseGlobalFlags()` | +| Extension dispatch | `cmd/extensions.go` — `DisableFlagParsing: true` | +| InvokeOptions construction | `cmd/extensions.go` — global options propagation | +| Env var propagation | `pkg/extensions/runner.go` — `Invoke()` | +| SDK flag registration | `pkg/azdext/extension_command.go` — `NewExtensionRootCommand()` | +| SDK env var fallback | `pkg/azdext/extension_command.go` — `PersistentPreRunE` | +| Reserved flags (host) | `internal/reserved_flags.go` | +| Reserved flags (SDK) | `pkg/azdext/reserved_flags.go` | +| SDK enforcement | `pkg/azdext/reserved_flags.go` — `ValidateNoReservedFlagConflicts()` | +| Enforcement hook | `pkg/azdext/run.go` | diff --git a/cli/azd/docs/extensions/extensions-style-guide.md b/cli/azd/docs/extensions/extensions-style-guide.md index c478688fda8..58229e4479a 100644 --- a/cli/azd/docs/extensions/extensions-style-guide.md +++ b/cli/azd/docs/extensions/extensions-style-guide.md @@ -17,6 +17,63 @@ This guide provides design guidelines and best practices for developing extensio - Reuse established parameter patterns across new commands - Maintain consistent naming conventions (e.g., `--subscription`, `--name`, `--type`) - Provide sensible defaults to reduce cognitive load +- **Do not reuse reserved global flag names** — see section below + +### Reserved Global Flags + +azd pre-parses a set of global flags from the command line **before** dispatching to extensions. +The extension SDK (`NewExtensionRootCommand`) also registers these same flags on every +extension's root command. Because both azd and the extension parse the same `argv`, extensions +**must not** register flags that collide with these reserved names. + +This is not a new restriction — it has been true since the extension system was designed. +The SDK now enforces it at startup via `ValidateNoReservedFlagConflicts()`. + +#### Reserved flag names + +| Long Name | Short | Purpose | +|-----------|-------|---------| +| `environment` | `e` | Selects the azd environment | +| `cwd` | `C` | Sets the working directory | +| `debug` | — | Enables debug logging | +| `no-prompt` | — | Non-interactive mode | +| `output` | `o` | Output format (json, table, none) | +| `help` | `h` | Command help (cobra built-in) | +| `docs` | — | Opens command documentation | +| `trace-log-file` | — | Diagnostics trace file | +| `trace-log-url` | — | OpenTelemetry trace endpoint | + +#### What this means for extension authors + +**DO:** +- Use any flag name that is not in the table above +- Use any single-letter short flag except `e`, `C`, `o`, `h` +- Access the environment name via `extCtx.Environment` (the SDK provides it automatically) +- Ignore reserved flags you don't need — the SDK handles them + +**DON'T:** +- Register `--environment` or `-e` on any subcommand (use `--env-name` or `--target-env` if you need a second environment reference) +- Register `--debug`, `--cwd`, `--output`, `--help`, or their short forms for a different purpose +- Assume you can "override" a global flag on a subcommand — azd's pre-parser will consume it first + +#### What happens if you collide + +If your extension uses the azd SDK (`azdext.Run()`), the SDK validates all flags at startup. +A collision produces a clear error: + +``` +extension defines flags that conflict with reserved azd global flags: + - command "custom create": flag --endpoint/-e conflicts with reserved global flag --environment + (short flag -e is reserved by azd for --environment) +Remove or rename these flags to avoid conflicts with azd's global flags. +Reserved flags: environment, cwd, debug, no-prompt, output, help, docs, trace-log-file, trace-log-url +``` + +#### Background + +For the full technical specification of how flags flow between azd and extensions, including +the pre-parsing pipeline, environment variable propagation, and enforcement implementation, +see [Extension Flag Architecture Spec](../design/extension-flag-architecture.md). ### 3. **Help and Discoverability** diff --git a/cli/azd/internal/reserved_flags.go b/cli/azd/internal/reserved_flags.go new file mode 100644 index 00000000000..f7c3ca87d71 --- /dev/null +++ b/cli/azd/internal/reserved_flags.go @@ -0,0 +1,88 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. + +package internal + +// ReservedFlag describes a global flag that is owned by azd and must not be reused +// by extensions for a different purpose. Extensions that register a flag with the +// same short or long name will shadow the global flag, causing unpredictable behavior +// when azd tries to parse the command line before dispatching to the extension. +type ReservedFlag struct { + // Long is the full flag name (e.g. "environment"). Always present. + Long string + // Short is the single-character alias (e.g. "e"). Empty when there is no short form. + Short string + // Description explains the flag's purpose in azd. + Description string +} + +// reservedFlags is the canonical list of global flags that extensions must not reuse. +// It is derived from CreateGlobalFlagSet (auto_install.go), the root command's +// persistent flags, and the extension SDK's built-in flag set (extension_command.go). +// +// Keep this list in sync whenever a new global flag is added to azd. +var reservedFlags = []ReservedFlag{ + {Long: "environment", Short: "e", Description: "The name of the environment to use."}, + {Long: "cwd", Short: "C", Description: "Sets the current working directory."}, + {Long: "debug", Short: "", Description: "Enables debugging and diagnostics logging."}, + {Long: "no-prompt", Short: "", Description: "Accepts the default value instead of prompting."}, + {Long: "output", Short: "o", Description: "The output format (json, table, none)."}, + {Long: "help", Short: "h", Description: "Help for the current command."}, + {Long: "docs", Short: "", Description: "Opens the documentation for the current command."}, + {Long: "trace-log-file", Short: "", Description: "Write a diagnostics trace to a file."}, + {Long: "trace-log-url", Short: "", Description: "Send traces to an OpenTelemetry-compatible endpoint."}, +} + +// ReservedFlags returns a copy of the reserved flags list. +// The copy prevents callers from mutating the canonical list. +func ReservedFlags() []ReservedFlag { + out := make([]ReservedFlag, len(reservedFlags)) + copy(out, reservedFlags) + return out +} + +// reservedShortFlags is an index of short flag names built once at initialization time. +var reservedShortFlags = func() map[string]ReservedFlag { + m := make(map[string]ReservedFlag, len(reservedFlags)) + for _, f := range reservedFlags { + if f.Short != "" { + m[f.Short] = f + } + } + return m +}() + +// reservedLongFlags is an index of long flag names built once at initialization time. +var reservedLongFlags = func() map[string]ReservedFlag { + m := make(map[string]ReservedFlag, len(reservedFlags)) + for _, f := range reservedFlags { + m[f.Long] = f + } + return m +}() + +// IsReservedShortFlag returns true when the given single-character flag name +// (without the leading "-") is reserved by azd as a global flag. +func IsReservedShortFlag(short string) bool { + _, ok := reservedShortFlags[short] + return ok +} + +// IsReservedLongFlag returns true when the given long flag name +// (without the leading "--") is reserved by azd as a global flag. +func IsReservedLongFlag(long string) bool { + _, ok := reservedLongFlags[long] + return ok +} + +// GetReservedShortFlag returns the ReservedFlag for the given short name, if any. +func GetReservedShortFlag(short string) (ReservedFlag, bool) { + f, ok := reservedShortFlags[short] + return f, ok +} + +// GetReservedLongFlag returns the ReservedFlag for the given long name, if any. +func GetReservedLongFlag(long string) (ReservedFlag, bool) { + f, ok := reservedLongFlags[long] + return f, ok +} diff --git a/cli/azd/internal/reserved_flags_test.go b/cli/azd/internal/reserved_flags_test.go new file mode 100644 index 00000000000..1e4972a5de0 --- /dev/null +++ b/cli/azd/internal/reserved_flags_test.go @@ -0,0 +1,116 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. + +package internal + +import ( + "testing" + + "github.com/stretchr/testify/require" +) + +func TestReservedFlagsRegistryPopulated(t *testing.T) { + // Sanity check: the registry should contain the known global flags. + require.GreaterOrEqual(t, len(ReservedFlags()), 9, "expected at least 9 reserved flags") +} + +func TestIsReservedShortFlag(t *testing.T) { + tests := []struct { + short string + reserved bool + }{ + {"e", true}, + {"C", true}, + {"o", true}, + {"h", true}, + // Flags with no short form should NOT appear as short flags. + {"", false}, + // Arbitrary letters should not be reserved. + {"x", false}, + {"z", false}, + {"p", false}, + } + + for _, tt := range tests { + t.Run("short="+tt.short, func(t *testing.T) { + require.Equal(t, tt.reserved, IsReservedShortFlag(tt.short)) + }) + } +} + +func TestIsReservedLongFlag(t *testing.T) { + tests := []struct { + long string + reserved bool + }{ + {"environment", true}, + {"cwd", true}, + {"debug", true}, + {"no-prompt", true}, + {"output", true}, + {"help", true}, + {"docs", true}, + {"trace-log-file", true}, + {"trace-log-url", true}, + // Not reserved. + {"verbose", false}, + {"project-endpoint", false}, + {"", false}, + } + + for _, tt := range tests { + t.Run("long="+tt.long, func(t *testing.T) { + require.Equal(t, tt.reserved, IsReservedLongFlag(tt.long)) + }) + } +} + +func TestGetReservedShortFlag(t *testing.T) { + f, ok := GetReservedShortFlag("e") + require.True(t, ok) + require.Equal(t, "environment", f.Long) + require.Equal(t, "e", f.Short) + + f, ok = GetReservedShortFlag("C") + require.True(t, ok) + require.Equal(t, "cwd", f.Long) + + _, ok = GetReservedShortFlag("x") + require.False(t, ok) +} + +func TestGetReservedLongFlag(t *testing.T) { + f, ok := GetReservedLongFlag("debug") + require.True(t, ok) + require.Equal(t, "debug", f.Long) + require.Empty(t, f.Short, "debug has no short form") + + _, ok = GetReservedLongFlag("nonexistent") + require.False(t, ok) +} + +func TestReservedFlagsNoDuplicates(t *testing.T) { + seenLong := make(map[string]bool) + seenShort := make(map[string]bool) + + for _, f := range ReservedFlags() { + require.NotEmpty(t, f.Long, "every reserved flag must have a long name") + require.False(t, seenLong[f.Long], "duplicate long flag: %s", f.Long) + seenLong[f.Long] = true + + if f.Short != "" { + require.False(t, seenShort[f.Short], "duplicate short flag: %s", f.Short) + seenShort[f.Short] = true + } + } +} + +func TestReservedFlagsConsistentWithLookups(t *testing.T) { + // Every entry in ReservedFlags must be findable via the lookup helpers. + for _, f := range ReservedFlags() { + require.True(t, IsReservedLongFlag(f.Long), "long flag %q not found via IsReservedLongFlag", f.Long) + if f.Short != "" { + require.True(t, IsReservedShortFlag(f.Short), "short flag %q not found via IsReservedShortFlag", f.Short) + } + } +} diff --git a/cli/azd/pkg/azdext/reserved_flags.go b/cli/azd/pkg/azdext/reserved_flags.go new file mode 100644 index 00000000000..9faa535e277 --- /dev/null +++ b/cli/azd/pkg/azdext/reserved_flags.go @@ -0,0 +1,198 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. + +package azdext + +import ( + "fmt" + "strings" + + "github.com/spf13/cobra" + "github.com/spf13/pflag" +) + +// reservedFlag describes a global flag owned by azd that extensions must not reuse +// for a different purpose. +type reservedFlag struct { + Long string + Short string +} + +// reservedGlobalFlags is the canonical list of global flags that extensions must not reuse. +// Keep this in sync with internal.ReservedFlags and CreateGlobalFlagSet (auto_install.go). +var reservedGlobalFlags = []reservedFlag{ + {Long: "environment", Short: "e"}, + {Long: "cwd", Short: "C"}, + {Long: "debug", Short: ""}, + {Long: "no-prompt", Short: ""}, + {Long: "output", Short: "o"}, + {Long: "help", Short: "h"}, + {Long: "docs", Short: ""}, + {Long: "trace-log-file", Short: ""}, + {Long: "trace-log-url", Short: ""}, +} + +// reservedShorts and reservedLongs are lookup maps built at init time. +var ( + reservedShorts map[string]string // short -> long + reservedLongs map[string]bool +) + +func init() { + reservedShorts = make(map[string]string, len(reservedGlobalFlags)) + reservedLongs = make(map[string]bool, len(reservedGlobalFlags)) + for _, f := range reservedGlobalFlags { + reservedLongs[f.Long] = true + if f.Short != "" { + reservedShorts[f.Short] = f.Long + } + } +} + +// ReservedFlagNames returns the long names of all reserved global flags. +// This is intended for documentation and error messages. +func ReservedFlagNames() []string { + names := make([]string, len(reservedGlobalFlags)) + for i, f := range reservedGlobalFlags { + names[i] = f.Long + } + return names +} + +// FlagConflict describes a single flag that conflicts with a reserved azd global flag. +type FlagConflict struct { + // Command is the full command path (e.g. "model custom create"). + Command string + // FlagName is the long name of the conflicting flag. + FlagName string + // FlagShort is the short name of the conflicting flag (may be empty). + FlagShort string + // ReservedLong is the long name of the reserved flag it conflicts with. + ReservedLong string + // Reason describes why it conflicts (e.g. "short flag -e is reserved"). + Reason string +} + +func (c FlagConflict) String() string { + return fmt.Sprintf("command %q: flag %s conflicts with reserved global flag --%s (%s)", + c.Command, c.flagDisplay(), c.ReservedLong, c.Reason) +} + +func (c FlagConflict) flagDisplay() string { + if c.FlagShort != "" { + return fmt.Sprintf("--%s/-%s", c.FlagName, c.FlagShort) + } + return fmt.Sprintf("--%s", c.FlagName) +} + +// ValidateNoReservedFlagConflicts walks the command tree rooted at cmd and +// returns an error listing every extension-defined flag that collides with an +// azd reserved global flag. +// +// Flags registered on the root command's persistent flag set are allowed because +// the extension SDK intentionally mirrors azd's global flags there (see +// NewExtensionRootCommand). Only flags added by the extension on subcommands +// (local or inherited persistent flags not from root) are checked. +func ValidateNoReservedFlagConflicts(root *cobra.Command) error { + conflicts := collectConflicts(root, root) + if len(conflicts) == 0 { + return nil + } + + var b strings.Builder + b.WriteString("extension defines flags that conflict with reserved azd global flags:\n") + for _, c := range conflicts { + b.WriteString(" - ") + b.WriteString(c.String()) + b.WriteString("\n") + } + b.WriteString("Remove or rename these flags to avoid conflicts with azd's global flags.\n") + b.WriteString("Reserved flags: ") + b.WriteString(strings.Join(ReservedFlagNames(), ", ")) + return fmt.Errorf("%s", b.String()) +} + +// collectConflicts recursively collects flag conflicts from the command tree. +func collectConflicts(root, cmd *cobra.Command) []FlagConflict { + var conflicts []FlagConflict + + // Track which flags we've already checked to avoid duplicate reports + // when a flag appears in both Flags() and PersistentFlags(). + checked := make(map[string]struct{}) + + checkFlagOnce := func(f *pflag.Flag) { + if _, seen := checked[f.Name]; seen { + return + } + checked[f.Name] = struct{}{} + + // Skip flags that are the SDK-provided root persistent flags (same object). + // Use pointer equality so that a subcommand defining its own flag with the + // same name as a root persistent flag is still validated. + if rootFlag := root.PersistentFlags().Lookup(f.Name); rootFlag != nil && rootFlag == f { + return + } + + if c, ok := checkFlag(cmd, f); ok { + conflicts = append(conflicts, c) + } + } + + // Check flags defined directly on this command (not inherited from parents). + // We use cmd.Flags() instead of cmd.LocalFlags() because LocalFlags() + // triggers cobra's mergePersistentFlags, which panics on shorthand conflicts. + cmd.Flags().VisitAll(checkFlagOnce) + + // Also check persistent flags defined on this command. This catches cases where + // an extension defines a persistent flag (e.g. on a subcommand) that conflicts + // with a reserved flag but wouldn't appear in cmd.Flags(). + cmd.PersistentFlags().VisitAll(checkFlagOnce) + + // Recurse into subcommands. + for _, sub := range cmd.Commands() { + conflicts = append(conflicts, collectConflicts(root, sub)...) + } + + return conflicts +} + +// checkFlag checks a single flag against the reserved lists. +func checkFlag(cmd *cobra.Command, f *pflag.Flag) (FlagConflict, bool) { + cmdPath := commandPath(cmd) + + // Check short flag collision. + if f.Shorthand != "" { + if reservedLong, ok := reservedShorts[f.Shorthand]; ok { + return FlagConflict{ + Command: cmdPath, + FlagName: f.Name, + FlagShort: f.Shorthand, + ReservedLong: reservedLong, + Reason: fmt.Sprintf("short flag -%s is reserved by azd for --%s", f.Shorthand, reservedLong), + }, true + } + } + + // Check long flag collision (only if the long name is the same but used for + // a different purpose — identified by being on a subcommand, not root). + if reservedLongs[f.Name] { + return FlagConflict{ + Command: cmdPath, + FlagName: f.Name, + FlagShort: f.Shorthand, + ReservedLong: f.Name, + Reason: fmt.Sprintf("long flag --%s is reserved by azd", f.Name), + }, true + } + + return FlagConflict{}, false +} + +// commandPath returns the space-separated command path (excluding root). +func commandPath(cmd *cobra.Command) string { + var parts []string + for c := cmd; c != nil && c.HasParent(); c = c.Parent() { + parts = append([]string{c.Name()}, parts...) + } + return strings.Join(parts, " ") +} diff --git a/cli/azd/pkg/azdext/reserved_flags_test.go b/cli/azd/pkg/azdext/reserved_flags_test.go new file mode 100644 index 00000000000..049283c4007 --- /dev/null +++ b/cli/azd/pkg/azdext/reserved_flags_test.go @@ -0,0 +1,171 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. + +package azdext + +import ( + "testing" + + "github.com/azure/azure-dev/cli/azd/internal" + "github.com/spf13/cobra" + "github.com/stretchr/testify/require" +) + +// newPlainRoot creates a plain cobra root command (no SDK persistent flags). +// This simulates an extension that doesn't use NewExtensionRootCommand. +func newPlainRoot(name string) *cobra.Command { + return &cobra.Command{Use: name} +} + +func TestValidateNoReservedFlagConflicts_Clean(t *testing.T) { + root, _ := NewExtensionRootCommand(ExtensionCommandOptions{Name: "test"}) + sub := &cobra.Command{Use: "do"} + sub.Flags().StringP("endpoint", "p", "", "project endpoint") + sub.Flags().String("subscription", "", "subscription id") + root.AddCommand(sub) + + err := ValidateNoReservedFlagConflicts(root) + require.NoError(t, err, "no conflict expected for flags that don't collide") +} + +func TestValidateNoReservedFlagConflicts_ShortCollision(t *testing.T) { + // Use a plain root because cobra panics if a child redefines a shorthand + // already claimed by a parent's persistent flags (the SDK root has -e). + root := newPlainRoot("test") + sub := &cobra.Command{Use: "create"} + sub.Flags().StringP("project-endpoint", "e", "", "project endpoint") + root.AddCommand(sub) + + err := ValidateNoReservedFlagConflicts(root) + require.Error(t, err) + require.Contains(t, err.Error(), "short flag -e is reserved") + require.Contains(t, err.Error(), "project-endpoint") +} + +func TestValidateNoReservedFlagConflicts_LongCollision(t *testing.T) { + root := newPlainRoot("test") + sub := &cobra.Command{Use: "init"} + sub.Flags().StringP("environment", "n", "", "environment name or ID") + root.AddCommand(sub) + + err := ValidateNoReservedFlagConflicts(root) + require.Error(t, err) + require.Contains(t, err.Error(), "long flag --environment is reserved") +} + +func TestValidateNoReservedFlagConflicts_RootFlagsAllowed(t *testing.T) { + // The SDK's own root persistent flags (--environment, --debug, --cwd, etc.) + // mirror azd globals intentionally and must be allowed. + root, _ := NewExtensionRootCommand(ExtensionCommandOptions{Name: "test"}) + sub := &cobra.Command{Use: "list"} + sub.Flags().String("filter", "", "filter expression") + root.AddCommand(sub) + + err := ValidateNoReservedFlagConflicts(root) + require.NoError(t, err, "root persistent flags from SDK should be allowed") +} + +func TestValidateNoReservedFlagConflicts_MultipleCollisions(t *testing.T) { + root := newPlainRoot("test") + sub := &cobra.Command{Use: "create"} + sub.Flags().StringP("project-endpoint", "e", "", "endpoint") + sub.Flags().StringP("output-format", "o", "", "output format") + root.AddCommand(sub) + + err := ValidateNoReservedFlagConflicts(root) + require.Error(t, err) + require.Contains(t, err.Error(), "-e is reserved") + require.Contains(t, err.Error(), "-o is reserved") +} + +func TestValidateNoReservedFlagConflicts_NestedSubcommand(t *testing.T) { + root := newPlainRoot("test") + parent := &cobra.Command{Use: "model"} + child := &cobra.Command{Use: "create"} + child.Flags().StringP("project-endpoint", "e", "", "endpoint") + parent.AddCommand(child) + root.AddCommand(parent) + + err := ValidateNoReservedFlagConflicts(root) + require.Error(t, err) + require.Contains(t, err.Error(), "model create") + require.Contains(t, err.Error(), "-e is reserved") +} + +func TestValidateNoReservedFlagConflicts_SDKRootWithCollision(t *testing.T) { + // When using the SDK root, -C is already registered as a persistent flag. + // A subcommand that uses -C for a different flag (via StringVar + Shorthand) + // would be caught. We use a long-name collision instead to avoid cobra panic. + root, _ := NewExtensionRootCommand(ExtensionCommandOptions{Name: "test"}) + sub := &cobra.Command{Use: "init"} + // --docs collides with reserved long flag + sub.Flags().Bool("docs", false, "generate docs") + root.AddCommand(sub) + + err := ValidateNoReservedFlagConflicts(root) + require.Error(t, err) + require.Contains(t, err.Error(), "long flag --docs is reserved") +} + +func TestValidateNoReservedFlagConflicts_PersistentFlagCollision(t *testing.T) { + // A subcommand that defines a persistent flag colliding with a reserved name + // should be caught even though it uses PersistentFlags() not Flags(). + root := newPlainRoot("test") + sub := &cobra.Command{Use: "deploy"} + sub.PersistentFlags().Bool("debug", false, "extension debug mode") + root.AddCommand(sub) + + err := ValidateNoReservedFlagConflicts(root) + require.Error(t, err) + require.Contains(t, err.Error(), "long flag --debug is reserved") +} + +func TestReservedFlagNames(t *testing.T) { + names := ReservedFlagNames() + require.NotEmpty(t, names) + require.Contains(t, names, "environment") + require.Contains(t, names, "cwd") + require.Contains(t, names, "debug") + require.Contains(t, names, "output") + require.Contains(t, names, "help") +} + +func TestReservedFlagsInSyncWithInternal(t *testing.T) { + // Verify the SDK reserved flag list stays in sync with the internal registry. + // If this test fails, you added a flag to one list but not the other. + sdkFlags := reservedGlobalFlags + internalFlags := internal.ReservedFlags() + + // Build maps of long name -> short name for both sides. + sdkMap := make(map[string]string, len(sdkFlags)) + for _, f := range sdkFlags { + sdkMap[f.Long] = f.Short + } + + internalMap := make(map[string]string, len(internalFlags)) + for _, f := range internalFlags { + internalMap[f.Long] = f.Short + } + + // Check every SDK flag exists in internal with matching short name. + for long, short := range sdkMap { + internalShort, ok := internalMap[long] + require.True(t, ok, + "azdext reserved_flags.go has %q but internal.ReservedFlags() does not — add it to internal/reserved_flags.go", + long) + require.Equal(t, internalShort, short, + "short name mismatch for %q: internal has %q, SDK has %q", + long, internalShort, short) + } + + // Check every internal flag exists in SDK with matching short name. + for long, short := range internalMap { + sdkShort, ok := sdkMap[long] + require.True(t, ok, + "internal.ReservedFlags() has %q but azdext reserved_flags.go does not — add it to pkg/azdext/reserved_flags.go", + long) + require.Equal(t, sdkShort, short, + "short name mismatch for %q: SDK has %q, internal has %q", + long, sdkShort, short) + } +} diff --git a/cli/azd/pkg/azdext/run.go b/cli/azd/pkg/azdext/run.go index f4509e37f67..c420fd24caa 100644 --- a/cli/azd/pkg/azdext/run.go +++ b/cli/azd/pkg/azdext/run.go @@ -61,6 +61,13 @@ func Run(rootCmd *cobra.Command, opts ...RunOption) { o(&cfg) } + // Validate that extension-defined flags do not collide with azd reserved global flags. + // This check runs before execution so extension developers see the error immediately. + if err := ValidateNoReservedFlagConflicts(rootCmd); err != nil { + printError(err) + os.Exit(1) + } + if cfg.preExecute != nil { if err := cfg.preExecute(ctx, rootCmd); err != nil { if reportErr := ReportError(ctx, err); reportErr != nil {