From 5cbf6961e348dbc2438cdd6a5a4f170814d86dee Mon Sep 17 00:00:00 2001 From: lei Date: Tue, 2 Jun 2026 18:26:51 +0300 Subject: [PATCH 1/3] feat(registry): beta enablement, active-profile resolution, interactive wizards Promote the registry command tree from env-gated pre-release to beta: - Remove the VERDA_REGISTRY_ENABLED gate; register unconditionally and mark "(beta)" in `verda --help` via the shared cmdutil.TagAnnotation. - Resolve the active profile (resolveProfile: explicit --profile > VERDA_PROFILE/auth.profile > "default") across all subcommands, so reads follow `verda auth use` like the rest of the CLI; --profile defaults to "". - configure: wizard profile picker + manual name/secret entry, wired --docker-config, overwrite-confirm guard, trimmed help. - ls: action-menu drill-down (Get pull URL / Delete) with a filterable tag picker; falls back to the Docker v2 tag list when Harbor's artifacts API is denied; graceful access-denied handling. - tags: interactive tag picker on a TTY (table when piped / -o json); bare `tags` points to `ls`. - copy: interactive wizard (source/access/scope/destination/confirm). - push: no-daemon fallback (source type -> path -> repo/tag) instead of erroring. - delete: exit cleanly on access_denied instead of looping. - Rename `login` -> `configure-docker` (alias `login`). Fixes: interactive pickers use cmd.Context() not the per-request timeout (no mid-prompt cancel); --src-auth basic validates --src-username before reading stdin (no lost one-shot secret). Co-Authored-By: Claude Opus 4.8 (1M context) --- internal/verda-cli/cmd/cmd.go | 10 +- internal/verda-cli/cmd/cmd_test.go | 16 - internal/verda-cli/cmd/registry/README.md | 83 ++-- internal/verda-cli/cmd/registry/configure.go | 222 +++++++--- .../verda-cli/cmd/registry/configure_test.go | 245 +++++++++++ internal/verda-cli/cmd/registry/copy.go | 115 ++++-- internal/verda-cli/cmd/registry/copy_test.go | 27 ++ .../verda-cli/cmd/registry/copy_wizard.go | 383 ++++++++++++++++++ .../cmd/registry/copy_wizard_test.go | 158 ++++++++ internal/verda-cli/cmd/registry/delete.go | 40 +- .../verda-cli/cmd/registry/delete_test.go | 34 ++ internal/verda-cli/cmd/registry/format.go | 23 ++ internal/verda-cli/cmd/registry/helper.go | 45 +- .../verda-cli/cmd/registry/helper_test.go | 44 ++ internal/verda-cli/cmd/registry/login.go | 21 +- internal/verda-cli/cmd/registry/ls.go | 293 +++++++++++--- internal/verda-cli/cmd/registry/ls_test.go | 160 +++++++- internal/verda-cli/cmd/registry/push.go | 171 ++++++-- internal/verda-cli/cmd/registry/push_test.go | 106 ++++- internal/verda-cli/cmd/registry/registry.go | 8 +- .../verda-cli/cmd/registry/registry_test.go | 7 +- internal/verda-cli/cmd/registry/show.go | 11 +- internal/verda-cli/cmd/registry/tags.go | 54 ++- internal/verda-cli/cmd/registry/tags_test.go | 53 +++ internal/verda-cli/cmd/registry/wizard.go | 209 +++++++++- .../verda-cli/cmd/registry/wizard_test.go | 104 ++++- 26 files changed, 2293 insertions(+), 349 deletions(-) create mode 100644 internal/verda-cli/cmd/registry/copy_wizard.go create mode 100644 internal/verda-cli/cmd/registry/copy_wizard_test.go create mode 100644 internal/verda-cli/cmd/registry/helper_test.go diff --git a/internal/verda-cli/cmd/cmd.go b/internal/verda-cli/cmd/cmd.go index a5e82a8..e2f8c1b 100644 --- a/internal/verda-cli/cmd/cmd.go +++ b/internal/verda-cli/cmd/cmd.go @@ -135,15 +135,13 @@ func NewRootCommand(ioStreams cmdutil.IOStreams) (*cobra.Command, *clioptions.Op images.NewCmdImages(f, ioStreams), instancetypes.NewCmdInstanceTypes(f, ioStreams), locations.NewCmdLocations(f, ioStreams), + registry.NewCmdRegistry(f, ioStreams), s3.NewCmdS3(f, ioStreams), sshkey.NewCmdSSHKey(f, ioStreams), startupscript.NewCmdStartupScript(f, ioStreams), template.NewCmdTemplate(f, ioStreams), volume.NewCmdVolume(f, ioStreams), } - if registryEnabled() { - resourceCmds = append(resourceCmds, registry.NewCmdRegistry(f, ioStreams)) - } groups := cmdutil.CommandGroups{ { @@ -205,12 +203,6 @@ func NewRootCommand(ioStreams cmdutil.IOStreams) (*cobra.Command, *clioptions.Op return cmd, opts } -// registryEnabled hides the registry subtree unless VERDA_REGISTRY_ENABLED is 1/true (pre-GA). -func registryEnabled() bool { - v := os.Getenv("VERDA_REGISTRY_ENABLED") - return v == "1" || v == "true" -} - // serverlessEnabled gates the container + batchjob subtrees behind // VERDA_SERVERLESS_ENABLED=1/true (pre-GA). Without it the Serverless group is // not registered; `verda container`/`verda batchjob` return "unknown command". diff --git a/internal/verda-cli/cmd/cmd_test.go b/internal/verda-cli/cmd/cmd_test.go index 3bb6108..a8d3f04 100644 --- a/internal/verda-cli/cmd/cmd_test.go +++ b/internal/verda-cli/cmd/cmd_test.go @@ -20,22 +20,6 @@ import ( "github.com/spf13/cobra" ) -func TestRegistryEnabled_EnvVar(t *testing.T) { - for _, tc := range []struct { - val string - want bool - }{ - {"1", true}, {"true", true}, {"0", false}, {"", false}, {"yes", false}, - } { - t.Run(tc.val, func(t *testing.T) { - t.Setenv("VERDA_REGISTRY_ENABLED", tc.val) - if got := registryEnabled(); got != tc.want { - t.Fatalf("registryEnabled()=%v, want %v", got, tc.want) - } - }) - } -} - func TestSkipCredentialResolution_RegistryChildren(t *testing.T) { parent := &cobra.Command{Use: "registry"} child := &cobra.Command{Use: "configure"} diff --git a/internal/verda-cli/cmd/registry/README.md b/internal/verda-cli/cmd/registry/README.md index 1fff48b..74752be 100644 --- a/internal/verda-cli/cmd/registry/README.md +++ b/internal/verda-cli/cmd/registry/README.md @@ -2,7 +2,7 @@ Manage Verda Container Registry (VCR, `vccr.io`) credentials, browse repositories, push local Docker images, and copy images between registries. Credentials are stored separately from the main API credentials under `verda_registry_` prefixed keys in the shared profile system. -> **Pre-release.** The `registry` command tree is gated behind `VERDA_REGISTRY_ENABLED=1` and hidden from `verda --help`. Without the env var, `verda registry ...` returns "unknown command". When the feature ships GA, delete `registryEnabled()` in `internal/verda-cli/cmd/cmd.go`, drop the gate in `NewRootCommand`, and remove `Hidden: true` from `internal/verda-cli/cmd/registry/registry.go`. +> **Beta.** The `registry` command tree is enabled by default and listed in `verda --help` as `registry … (beta)`. No env var is required. One flag is still inert pending follow-up: `push --no-mount` (accepted, prints a notice, no effect). ## Commands @@ -10,9 +10,9 @@ Manage Verda Container Registry (VCR, `vccr.io`) credentials, browse repositorie |---------|---------| | `verda registry configure` | Save VCR credentials (paste `docker login` from the web UI, flags, or wizard) | | `verda registry show` | Print credential status + expiry (no secrets) | -| `verda registry login` | Write `~/.docker/config.json` for `docker pull` / compose / helm / nerdctl | +| `verda registry configure-docker` (alias `login`) | Write `~/.docker/config.json` for `docker pull` / compose / helm / nerdctl | | `verda registry ls` | List repositories in the active Verda project | -| `verda registry tags ` | List tags in a repository plus per-tag digest + size | +| `verda registry tags ` | List tags in a repository (interactive picker → pull URL on a TTY; table with digest + size when piped / `-o json`) | | `verda registry push [image...]` | Push local images (daemon / OCI layout / tarball); zero-arg launches interactive picker | | `verda registry copy []` (alias `cp`) | Copy an image between registries | | `verda registry delete []` (aliases `del`, `rm`) | Delete a repository or a single image (tag / digest); zero-arg launches interactive flow | @@ -23,13 +23,15 @@ The parent command also accepts the alias `vcr`, so `verda vcr ls` works identic `--profile`, `--credentials-file`, `--debug`, `--agent`, `-o json`/`yaml`, `--timeout`. +**Profile selection.** With no `--profile`, registry commands use the **active profile** — the one set by `verda auth use ` (`auth.profile` in `~/.verda/config.yaml`) or `VERDA_PROFILE` — falling back to `default`. So after `verda auth use production`, `verda registry ls`/`configure`/… all operate on the `production` profile without repeating `--profile`. An explicit `--profile X` always overrides. + ## Quick start ### 1. Create credentials in the web UI, then configure the CLI ```bash # Paste the full docker login command the UI prints -VERDA_REGISTRY_ENABLED=1 verda registry configure \ +verda registry configure \ --paste "docker login -u vcr-+ -p vccr.io" ``` @@ -38,7 +40,7 @@ Or run without flags on a TTY to drive the interactive wizard, which asks for th ### 2. Verify ```bash -VERDA_REGISTRY_ENABLED=1 verda registry show +verda registry show # registry_configured: true # expires_at: 2026-05-20T00:00:00Z # days_remaining: 30 @@ -48,10 +50,13 @@ VERDA_REGISTRY_ENABLED=1 verda registry show ```bash # From the Docker daemon -VERDA_REGISTRY_ENABLED=1 verda registry push my-app:v1.0.0 +verda registry push my-app:v1.0.0 -# Or launch the interactive picker (no positional args) on a TTY -VERDA_REGISTRY_ENABLED=1 verda registry push +# Or launch the interactive picker (no positional args) on a TTY. +# Daemon running → pick from your local images. +# Daemon NOT running → fall back to a guided prompt: choose an OCI layout or +# tarball, give a path + destination repo/tag. +verda registry push ``` ### 4. List repositories and tags @@ -59,48 +64,53 @@ VERDA_REGISTRY_ENABLED=1 verda registry push ```bash # Every repository in the active project, then pick one to see its image list # (digest / tags / size / push / pull — the same view Harbor's web UI shows) -VERDA_REGISTRY_ENABLED=1 verda registry ls +verda registry ls # Scriptable (piping suppresses the picker; JSON/YAML do too) -VERDA_REGISTRY_ENABLED=1 verda registry ls | less -VERDA_REGISTRY_ENABLED=1 verda registry ls -o json +verda registry ls | less +verda registry ls -o json # Tags inside one repository (digest + size per tag) -VERDA_REGISTRY_ENABLED=1 verda registry tags my-app +verda registry tags my-app ``` ### 5. Copy from another registry ```bash +# Interactive wizard: prompts for source → access → scope → destination → confirm +verda registry copy + # Copy a public image from Docker Hub to VCR, preserving repo/tag -VERDA_REGISTRY_ENABLED=1 verda registry copy docker.io/library/nginx:1.25 +verda registry copy docker.io/library/nginx:1.25 # Copy every tag -VERDA_REGISTRY_ENABLED=1 verda registry copy docker.io/library/nginx --all-tags +verda registry copy docker.io/library/nginx --all-tags # Copy to a custom destination -VERDA_REGISTRY_ENABLED=1 verda registry copy gcr.io/my-project/app:v1 my-app:prod +verda registry copy gcr.io/my-project/app:v1 my-app:prod ``` +On a terminal, running `copy` with **no arguments** launches a wizard that mirrors `s3 cp`'s flow: enter the **source** image (validated), pick **source access** (public via `docker login` / anonymous / username+password), choose **scope** (just this tag / all tags), confirm the **destination** (pre-filled with the synthesized `vccr.io//:`), then review a `Will run: verda registry copy …` preview before it executes. Esc steps back a question; Ctrl+C exits. Scripts / `--agent` / `-o json` must pass `` explicitly. + ### 6. Delete a repository or image ```bash # Delete a single image (by tag or digest) -VERDA_REGISTRY_ENABLED=1 verda registry delete my-app:v1.2.3 -VERDA_REGISTRY_ENABLED=1 verda registry delete my-app@sha256:abcdef... +verda registry delete my-app:v1.2.3 +verda registry delete my-app@sha256:abcdef... # Delete an entire repository (all artifacts + tags) -VERDA_REGISTRY_ENABLED=1 verda registry delete my-app --yes +verda registry delete my-app --yes # Zero-arg: interactive picker + multi-select (Space/Ctrl+A/Enter) -VERDA_REGISTRY_ENABLED=1 verda registry delete +verda registry delete ``` ## Configuration ### Where the values come from -The web UI's "Registry credentials created" dialog shows three fields. Map them to `configure` flags: +In the Verda dashboard: **select your project** (skipped automatically if you only have one) → **Credentials** → **Create credentials** → Provider **Verda**, enter a name and an expiry (Label, in days) → **Create credentials**. The dialog that appears shows three fields. Map them to `configure` flags: | Web UI field | CLI flag | |---|---| @@ -135,14 +145,18 @@ When `--endpoint` is omitted, the CLI prints a one-line `Using registry endpoint `--expires-in ` overrides the 30-day default expiry. `--profile ` writes to a named profile section. +Re-running `configure` against a profile that already holds registry credentials **replaces** them. On a terminal you're asked to confirm first (`Profile "x" already has registry credentials … Replace them?`); declining writes nothing. In agent mode and non-interactive (piped) runs the replace proceeds without a prompt — rotation is the explicit intent — and a one-line `Replacing existing registry credentials in profile "x".` note is printed to stderr (non-agent) so it isn't silent. Other keys in the profile (S3, API) and other profiles are never touched. + > **Credentials are write-once from the user's side.** Verda's API never returns the secret again — only the credential name. If the secret is lost, delete + recreate the credential in the web UI and re-run `configure`. The CLI cannot fetch the secret on your behalf. -## Login (docker config merge) +## Configure Docker (docker config merge) + +> Named `configure-docker` (alias `login`, mirroring `docker login` / `gcloud auth configure-docker`). It does **not** contact the registry — it's a local merge into `~/.docker/config.json`. ```bash -verda registry login # merge default profile into ~/.docker/config.json -verda registry login --profile staging # merge a non-default profile -verda registry login --config /tmp/dc.json # write to a non-default docker config path +verda registry configure-docker # merge default profile into ~/.docker/config.json +verda registry configure-docker --profile staging # merge a non-default profile +verda registry configure-docker --config /tmp/dc.json # write to a non-default docker config path ``` `login` is a **local file merge** — it never talks to the registry. Existing entries for other registries and unknown top-level keys (`credsStore`, `credHelpers`, `HttpHeaders`, `psFormat`, ...) are preserved verbatim. @@ -164,7 +178,15 @@ verda registry tags my-app --all verda registry tags vccr.io/my-project/my-app # fully qualified form works too ``` -On a terminal, `ls` prints one row per repository and lets you pick one; selecting a row fetches that repo's per-artifact detail and renders an image-list card (**DIGEST**, **TAGS**, **SIZE**, **PUSHED**, **PULLED**) — the same view Harbor's UI shows when you expand a repository row. Pick "Exit" (or press Ctrl-C at the picker) to quit. +On a terminal, `ls` prints one row per repository and lets you pick one. Selecting a repository opens an **action menu**: + +- **Get pull URL** — a filterable, newest-first **tag picker**; pick a tag (type to filter) and `ls` prints that tag's full, copy-pasteable pull reference (`vccr.io//:`, or `@` for untagged artifacts) and exits — getting the URL is the goal. +- **Delete image(s)…** — the same multi-select + confirmation flow as `verda registry delete` (space to mark, Ctrl+A to select all, a red "cannot be undone" confirm). Interactive only. +- **← Back** to the repository list. Esc backs up one level; Ctrl+C quits. + +To go straight to a known repo's tags, use `verda registry tags ` — it opens the **same tag picker** on a terminal, or prints a detailed per-tag table (digest / size) when piped or with `-o json`. + +If your credential can list repositories but not their *artifacts* (a Harbor 403 — some credentials lack the project image-permission), `ls` falls back to the Docker v2 tag list (the same one `verda registry tags` reads) so the **Get pull URL** tag picker still works — just without sizes/dates, and without the **Delete** action (delete goes through the same denied Harbor API). If even the v2 tag list is denied, it shows just the repository pull reference (`vccr.io//`, which you can still `docker pull`). When `ls` is piped or redirected, or when `-o json` / `-o yaml` is set, the picker is suppressed and a single deterministic document is emitted instead. This is what scripts and CI should rely on. @@ -264,7 +286,7 @@ No extra flags — this is the default path: ```bash echo "$GITHUB_PAT" | docker login ghcr.io -u USERNAME --password-stdin -VERDA_REGISTRY_ENABLED=1 verda registry copy \ +verda registry copy \ ghcr.io/acme/private-app:v1 \ acme/private-app:v1 ``` @@ -274,7 +296,7 @@ VERDA_REGISTRY_ENABLED=1 verda registry copy \ The secret **must** come via stdin; passing it as a flag is not supported: ```bash -echo "$SRC_PASSWORD" | VERDA_REGISTRY_ENABLED=1 verda registry copy \ +echo "$SRC_PASSWORD" | verda registry copy \ private.example.com/team/app:v1 \ --src-auth basic \ --src-username jdoe \ @@ -431,7 +453,7 @@ The artifact-count / digest / removed-tags fields come from a best-effort pre-de `--profile`, `--credentials-file`. ### `tags` -`--profile`, `--credentials-file`, `--limit`, `--all`. +`--profile`, `--credentials-file`, `--limit`, `--all`. On a terminal `tags ` is an interactive filterable picker (type to filter; select a tag → prints its pull reference and exits — the same picker `ls`'s drill-down uses); piped / `-o json` / `--agent` print the per-tag table instead. **Bare `tags`** (no repo) returns a usage error pointing at `tags ` / `ls` — `tags` is tag-centric; browsing repositories interactively is `ls`'s job. ### `push` `--profile`, `--credentials-file`, `--repo`, `--tag`, `--source` (`auto|daemon|oci|tar`), `--jobs`, `--image-jobs`, `--retries`, `--progress` (`auto|plain|json|none`), `--no-mount` (currently a no-op; prints a notice). @@ -471,9 +493,8 @@ Bubbletea output always goes to **stderr** so stdout stays clean for scripted co ## Environment -- `VERDA_REGISTRY_ENABLED=1` -- **required** to register any `registry` subcommand (pre-release gate) - `VERDA_REGISTRY_CREDENTIALS_FILE` -- override the default credentials file path (`~/.verda/credentials`); useful in tests and CI -- `DOCKER_CONFIG` -- honoured by `verda registry login` when `--config` is not passed +- `DOCKER_CONFIG` -- honoured by `verda registry configure-docker` when `--config` is not passed - `DOCKER_HOST` -- honoured by the daemon source in `push --source daemon` and `--source auto` ## Multiple profiles @@ -488,7 +509,7 @@ Switch per-command with `--profile staging` or persist it with `verda auth use s ## Interactive vs Non-Interactive -- `configure` has an interactive bubbletea wizard that drives the `--paste` flow plus expiry + docker-config options. Supply `--paste` or `--username/--password-stdin/--endpoint` to skip the wizard entirely. +- `configure` has an interactive bubbletea wizard: pick or create a profile (each annotated with whether it already holds registry credentials), then choose how you have the credential — **paste** the full `docker login` command, or **enter the credential name + secret** separately (matching the web UI's two copy options). The manual path doesn't ask for a host: the project id is parsed from the credential name and the base is `vccr.io` (or the profile's saved host). Then expiry + docker-config. Supply `--paste` or `--username/--password-stdin` to skip the wizard entirely. - `push` with zero positional args launches an interactive daemon-image picker when stderr is a TTY and `--agent` is off. Under `--agent` or a non-TTY, zero-arg push returns a structured "interactive push requires a TTY" error. - `copy` prompts for overwrite confirmation when the destination tag already exists. Pass `--overwrite` / `--yes` to skip the prompt, or run under `--agent` to force the caller to make the decision (agent mode returns `CONFIRMATION_REQUIRED` rather than auto-confirming). - `delete` with zero positional args on a TTY launches the interactive repo picker + sub-menu + multi-select image flow (Space to toggle, Ctrl+A to select all, Enter to confirm). With a positional target, it shows a one-shot confirmation dialog; `--yes` skips it. Under `--agent`, `--yes` is mandatory — missing it returns `CONFIRMATION_REQUIRED`. diff --git a/internal/verda-cli/cmd/registry/configure.go b/internal/verda-cli/cmd/registry/configure.go index 575e2a0..e7477bc 100644 --- a/internal/verda-cli/cmd/registry/configure.go +++ b/internal/verda-cli/cmd/registry/configure.go @@ -15,6 +15,7 @@ package registry import ( + "context" "errors" "fmt" "io" @@ -51,6 +52,13 @@ type configureOptions struct { ExpiresInDays int Paste string DockerConfig bool + + // Secret is populated only by the wizard's manual-entry path (the + // "Enter credential name + secret" mode). The flag path reads the secret + // from stdin instead, so it stays empty there. resolveRegistryInputs uses + // a non-empty Secret to distinguish the wizard-manual path from the + // --password-stdin path. + Secret string } // NewCmdConfigure creates the `verda registry configure` command. @@ -61,7 +69,6 @@ type configureOptions struct { // input flags are supplied on a TTY. func NewCmdConfigure(f cmdutil.Factory, ioStreams cmdutil.IOStreams) *cobra.Command { opts := &configureOptions{ - Profile: defaultProfileName, ExpiresInDays: defaultExpiresInDays, } @@ -69,65 +76,31 @@ func NewCmdConfigure(f cmdutil.Factory, ioStreams cmdutil.IOStreams) *cobra.Comm Use: "configure", Short: registryConfigureShort, Long: cmdutil.LongDesc(` - Save Verda Container Registry credentials to the shared credentials - file. Credentials are stored alongside API and S3 keys using - verda_registry_ prefixed keys; existing keys in the same profile - section are preserved. - - Default file: ~/.verda/credentials - Override with --credentials-file or VERDA_REGISTRY_CREDENTIALS_FILE. - - Where to find the values - The Verda web UI's "Registry credentials created" dialog shows: - - Full credentials name → --username - Secret → stdin (with --password-stdin) - Registry authentication command → paste verbatim into --paste - - The third field is the only place the registry URL appears after - you close the dialog, so the easiest path is to copy that whole - string and use --paste. The URL looks like: - - docker login -u vcr-+ -p - - where is ` + "`vccr.io`" + ` on production and a longer - staging hostname on non-production deployments. - - Two non-interactive input modes are supported: - - --paste (recommended) Paste the full ` + "`docker login ...`" + ` - command the web UI prints. The host is extracted - automatically. - --username + --password-stdin [+ --endpoint] - Classic Docker-style: username as a flag, secret on - stdin. --endpoint defaults to the previously-saved - endpoint for this profile, or ` + "`" + defaultRegistryEndpoint + "`" + ` on - production. Staging/custom deployments must pass - --endpoint explicitly the first time. + Save Verda Container Registry credentials to ~/.verda/credentials + (override with --credentials-file or VERDA_REGISTRY_CREDENTIALS_FILE). + Stored with verda_registry_ prefixed keys alongside API and S3 keys; + other keys in the profile are preserved. + + Three ways to provide credentials: + • interactive — run with no flags on a terminal; the wizard guides you, + including where to find the credential in the web UI. + • --paste — paste the web UI's "Registry authentication command" (the + docker login … line); the host is auto-detected. + • --username + --password-stdin [--endpoint] — Docker-style; --endpoint + defaults to the profile's saved host, else vccr.io. + + Find the credential in the web UI under: select project → Credentials → + Create credentials. `), Example: cmdutil.Examples(` - # Recommended: paste the full command from the web UI's - # "Registry authentication command" field + # Interactive wizard (recommended for first-time setup) + verda registry configure + + # Paste the web UI's "Registry authentication command" verda registry configure --paste "docker login -u vcr-abc+cli -p s3cret vccr.io" - # Production: --endpoint defaults to vccr.io, so only - # --username and the stdin secret are required - echo -n "$SECRET" | verda registry configure \ - --username vcr-abc+cli \ - --password-stdin - - # Staging / custom: pass --endpoint explicitly the first time; - # subsequent rotations on the same profile reuse the saved host - echo -n "$SECRET" | verda registry configure \ - --username vcr-abc+cli \ - --password-stdin \ - --endpoint registry.staging.internal.datacrunch.io - - # Different profile, custom expiry - verda registry configure \ - --profile staging \ - --paste "docker login -u vcr-u+n -p pw vccr.io" \ - --expires-in 7 + # Username + secret on stdin (--endpoint defaults to vccr.io) + echo -n "$SECRET" | verda registry configure --username vcr-abc+cli --password-stdin `), Args: cobra.NoArgs, RunE: func(cmd *cobra.Command, args []string) error { @@ -136,7 +109,7 @@ func NewCmdConfigure(f cmdutil.Factory, ioStreams cmdutil.IOStreams) *cobra.Comm } flags := cmd.Flags() - flags.StringVar(&opts.Profile, "profile", opts.Profile, "Credentials profile name") + flags.StringVar(&opts.Profile, "profile", opts.Profile, "Credentials profile name (default: active profile)") flags.StringVar(&opts.CredentialsFile, "credentials-file", "", "Path to the shared credentials file") flags.StringVar(&opts.Username, "username", "", "Registry username (vcr-+)") flags.BoolVar(&opts.PasswordStdin, "password-stdin", false, "Read the registry secret from stdin") @@ -145,7 +118,7 @@ func NewCmdConfigure(f cmdutil.Factory, ioStreams cmdutil.IOStreams) *cobra.Comm "or \""+defaultRegistryEndpoint+"\" on production.") flags.IntVar(&opts.ExpiresInDays, "expires-in", opts.ExpiresInDays, "Days from now until the credentials expire") flags.StringVar(&opts.Paste, "paste", "", "Full `docker login ...` command to parse (alternative to --username/--password-stdin)") - flags.BoolVar(&opts.DockerConfig, "docker-config", false, "Also write ~/.docker/config.json (not yet implemented in this subcommand)") + flags.BoolVar(&opts.DockerConfig, "docker-config", false, "Also write ~/.docker/config.json (same merge as `verda registry configure-docker`)") return cmd } @@ -157,6 +130,9 @@ func runConfigure(cmd *cobra.Command, f cmdutil.Factory, ioStreams cmdutil.IOStr // derived Username/Endpoint) and opts.ExpiresInDays / opts.DockerConfig // before falling through to the flag-driven resolution path below. if shouldRunConfigureWizard(f, opts) { + // The docker-login string lives several clicks deep in the web UI; + // tell the user exactly where to find it before the paste prompt. + printConfigureIntro(ioStreams) flow := buildConfigureFlow(opts) engine := wizard.NewEngine(f.Prompter(), f.Status(), wizard.WithOutput(ioStreams.ErrOut), wizard.WithExitConfirmation()) @@ -165,19 +141,31 @@ func runConfigure(cmd *cobra.Command, f cmdutil.Factory, ioStreams cmdutil.IOStr } } + // Resolve the target profile now that the wizard (if any) has run: an + // explicit --profile or the picker's choice is already on opts.Profile; + // otherwise fall back to the active profile so configure writes where the + // read commands will look. + opts.Profile = resolveProfile(opts.Profile) + creds, err := resolveRegistryInputs(cmd, f, ioStreams, opts) if err != nil { return err } - if opts.DockerConfig { - _, _ = fmt.Fprintln(ioStreams.ErrOut, - "TODO: --docker-config is accepted but not yet wired in `registry configure`; "+ - "use `verda registry login` (Task 13) or the interactive wizard (Task 7) "+ - "to also update ~/.docker/config.json.") + path := credentialsFilePath(opts.CredentialsFile) + + // The registry secret is write-once (the API never returns it), so + // replacing a profile's existing credentials is irreversible. Confirm on a + // TTY; proceed (with a note) for scripted rotation. + proceed, err := confirmOverwriteIfConfigured(cmd.Context(), f, ioStreams, opts.Profile, path) + if err != nil { + return err + } + if !proceed { + _, _ = fmt.Fprintln(ioStreams.ErrOut, "Canceled.") + return nil } - path := credentialsFilePath(opts.CredentialsFile) if err := options.WriteRegistryCredentialsToProfile(path, opts.Profile, creds); err != nil { return err } @@ -187,9 +175,93 @@ func runConfigure(cmd *cobra.Command, f cmdutil.Factory, ioStreams cmdutil.IOStr _, _ = fmt.Fprintf(ioStreams.ErrOut, "Credentials expire at %s (in %d days)\n", creds.ExpiresAt.Format(time.RFC3339), opts.ExpiresInDays) } + + if opts.DockerConfig { + writeDockerConfigForConfigure(ioStreams, creds) + } return nil } +// printConfigureIntro tells the user exactly where the docker-login string +// lives in the Verda web UI before the wizard prompts for it. The path is +// several clicks deep, so new users otherwise can't find the "Registry +// authentication command" the paste step asks for. Mirrors s3's +// printConfigureIntro. Goes to ErrOut so stdout stays clean. +func printConfigureIntro(ioStreams cmdutil.IOStreams) { + w := ioStreams.ErrOut + _, _ = fmt.Fprintln(w, "\n Create a Container Registry credential in the Verda dashboard first:") + _, _ = fmt.Fprintln(w, " 1. Open the dashboard and select your project (skipped if you have only one).") + _, _ = fmt.Fprintln(w, " 2. Go to Credentials → Create credentials.") + _, _ = fmt.Fprintln(w, " 3. Provider: Verda. Enter a name and an expiry (Label, in days),") + _, _ = fmt.Fprintln(w, " then click \"Create credentials\".") + _, _ = fmt.Fprintln(w, " 4. In the dialog, copy either the full \"Registry authentication command\"") + _, _ = fmt.Fprintln(w, " (the `docker login …` line) or the \"Full credentials name\" + \"Secret\".") + _, _ = fmt.Fprintln(w, " The secret is shown only once.") + _, _ = fmt.Fprintln(w, "\n The wizard will ask which of those you have.") + _, _ = fmt.Fprintln(w) +} + +// confirmOverwriteIfConfigured guards the irreversible replace of a profile's +// existing registry credentials. Returns (proceed, err): +// +// - profile has no registry creds yet → (true, nil); no prompt. +// - interactive TTY → prompt to replace; decline → (false, nil). +// - agent / non-TTY → (true, nil); rotation is the explicit +// intent and scripts must not block. A non-agent run still emits a one-line +// "Replacing…" note so the overwrite isn't silent. +// +// A failed load is treated as "nothing to overwrite" (true, nil): the worst +// case is we skip a warning, never that we block a legitimate first-time write. +func confirmOverwriteIfConfigured(ctx context.Context, f cmdutil.Factory, ioStreams cmdutil.IOStreams, profile, path string) (bool, error) { + existing, err := options.LoadRegistryCredentialsForProfile(path, profile) + if err != nil || existing == nil || !existing.HasCredentials() { + return true, nil //nolint:nilerr // intentional: load failure = nothing to overwrite, proceed + } + + detail := existing.Username + if !existing.ExpiresAt.IsZero() { + detail = fmt.Sprintf("%s, expires %s", existing.Username, existing.ExpiresAt.Format("2006-01-02")) + } + + if f.AgentMode() || !isTerminalFn(ioStreams.Out) { + if !f.AgentMode() { + _, _ = fmt.Fprintf(ioStreams.ErrOut, + "Replacing existing registry credentials in profile %q (%s).\n", profile, detail) + } + return true, nil + } + + confirmed, err := f.Prompter().Confirm(ctx, + fmt.Sprintf("Profile %q already has registry credentials (%s). Replace them?", profile, detail)) + if err != nil { + if cmdutil.IsPromptCancel(err) { + return false, nil + } + return false, err + } + return confirmed, nil +} + +// writeDockerConfigForConfigure honors the --docker-config flag (and the +// wizard's "Also write ~/.docker/config.json?" step) by merging the just-saved +// credentials into the Docker config via the same writer `registry configure-docker` +// uses. A failure here is non-fatal: the Verda credentials are already saved, +// so we warn and point at `registry configure-docker` for a retry rather than failing the +// whole command. +func writeDockerConfigForConfigure(ioStreams cmdutil.IOStreams, creds *options.RegistryCredentials) { + dockerPath, err := resolveDockerConfigPath("") + if err == nil { + err = writeDockerLogin(dockerPath, creds) + } + if err != nil { + _, _ = fmt.Fprintf(ioStreams.ErrOut, + "warning: could not write Docker config: %v\nRun `verda registry configure-docker` to retry.\n", err) + return + } + _, _ = fmt.Fprintf(ioStreams.ErrOut, + "Also wrote %s for use with docker pull / docker-compose / helm.\n", dockerPath) +} + // resolveRegistryInputs decides which non-interactive path to use (paste or // flags+stdin) and returns populated credentials, or a usage error if neither // input mode is satisfied. @@ -210,6 +282,28 @@ func resolveRegistryInputs(cmd *cobra.Command, f cmdutil.Factory, ioStreams cmdu ExpiresAt: expiresAt, }, nil + case strings.TrimSpace(opts.Username) != "" && opts.Secret != "": + // Wizard manual-entry path: the user typed the full credential name and + // secret (the secret never touches stdin). The web UI's name+secret + // fields don't carry a host, so derive it the same way the flag path + // does — the profile's saved endpoint, else the production base + // (vccr.io) — and parse the project id out of the credential name. + projectID, err := projectIDFromUsername(opts.Username) + if err != nil { + return nil, cmdutil.UsageErrorf(cmd, "%s", err.Error()) + } + endpoint, source := resolveEndpointForFlags(opts) + if source != endpointSourceFlag && !f.AgentMode() { + _, _ = fmt.Fprintf(ioStreams.ErrOut, "Using registry endpoint %q (%s).\n", endpoint, source) + } + return &options.RegistryCredentials{ + Username: opts.Username, + Secret: opts.Secret, + Endpoint: endpoint, + ProjectID: projectID, + ExpiresAt: expiresAt, + }, nil + case strings.TrimSpace(opts.Username) != "" && opts.PasswordStdin: endpoint, source := resolveEndpointForFlags(opts) if endpoint == "" { diff --git a/internal/verda-cli/cmd/registry/configure_test.go b/internal/verda-cli/cmd/registry/configure_test.go index 6c69a61..a5aea74 100644 --- a/internal/verda-cli/cmd/registry/configure_test.go +++ b/internal/verda-cli/cmd/registry/configure_test.go @@ -16,6 +16,7 @@ package registry import ( "bytes" + "encoding/base64" "os" "path/filepath" "strings" @@ -25,6 +26,7 @@ import ( "gopkg.in/ini.v1" cmdutil "github.com/verda-cloud/verda-cli/internal/verda-cli/cmd/util" + tuitest "github.com/verda-cloud/verdagostack/pkg/tui/testing" ) // newTestStreams returns IOStreams backed by buffers, with `stdin` providing @@ -362,6 +364,106 @@ verda_registry_project_id = abc if !strings.Contains(errOut.String(), "saved in profile") { t.Errorf("stderr should explain reuse of saved endpoint, got: %q", errOut.String()) } + // Non-TTY overwrite of an already-configured profile proceeds (rotation + // intent) but isn't silent — it notes the replace on stderr. + if !strings.Contains(errOut.String(), "Replacing existing registry credentials") { + t.Errorf("stderr should note the credential replace, got: %q", errOut.String()) + } +} + +// TestConfigure_OverwriteExistingProfile_TTYDeclineAborts: on a terminal, +// configuring a profile that already has registry credentials prompts before +// the irreversible replace. Declining writes nothing. +func TestConfigure_OverwriteExistingProfile_TTYDeclineAborts(t *testing.T) { + dir := t.TempDir() + path := filepath.Join(dir, "credentials") + t.Setenv("VERDA_REGISTRY_CREDENTIALS_FILE", path) + t.Setenv("VERDA_HOME", dir) + + existing := `[default] +verda_registry_username = vcr-abc+oldcli +verda_registry_secret = oldsecret +verda_registry_endpoint = vccr.io +verda_registry_project_id = abc +` + if err := os.WriteFile(path, []byte(existing), 0o600); err != nil { + t.Fatal(err) + } + + withForcedTTY(t, true) + mock := tuitest.New().AddConfirm(false) // decline the replace + f := cmdutil.NewTestFactory(mock) + streams, out, errOut := newTestStreams("") + + err := runConfigureForTest(t, f, streams, + "--paste", "docker login -u vcr-abc+newcli -p newsecret vccr.io", + ) + if err != nil { + t.Fatalf("run: %v", err) + } + + cfg, err := ini.Load(path) + if err != nil { + t.Fatalf("load: %v", err) + } + // Old credentials must be intact — nothing was written. + if got := cfg.Section("default").Key("verda_registry_secret").String(); got != "oldsecret" { + t.Errorf("secret = %q, want oldsecret (declined overwrite)", got) + } + if got := cfg.Section("default").Key("verda_registry_username").String(); got != "vcr-abc+oldcli" { + t.Errorf("username = %q, want vcr-abc+oldcli (declined overwrite)", got) + } + if !strings.Contains(errOut.String(), "Canceled") { + t.Errorf("expected Canceled on stderr, got: %q", errOut.String()) + } + if strings.Contains(out.String(), "saved to profile") { + t.Errorf("should not have reported a save; stdout: %q", out.String()) + } +} + +// TestConfigure_OverwriteExistingProfile_TTYConfirmReplaces: accepting the +// prompt rotates the credentials in place. +func TestConfigure_OverwriteExistingProfile_TTYConfirmReplaces(t *testing.T) { + dir := t.TempDir() + path := filepath.Join(dir, "credentials") + t.Setenv("VERDA_REGISTRY_CREDENTIALS_FILE", path) + t.Setenv("VERDA_HOME", dir) + + existing := `[default] +verda_registry_username = vcr-abc+oldcli +verda_registry_secret = oldsecret +verda_registry_endpoint = vccr.io +verda_registry_project_id = abc +` + if err := os.WriteFile(path, []byte(existing), 0o600); err != nil { + t.Fatal(err) + } + + withForcedTTY(t, true) + mock := tuitest.New().AddConfirm(true) // accept the replace + f := cmdutil.NewTestFactory(mock) + streams, out, _ := newTestStreams("") + + err := runConfigureForTest(t, f, streams, + "--paste", "docker login -u vcr-abc+newcli -p newsecret vccr.io", + ) + if err != nil { + t.Fatalf("run: %v", err) + } + + cfg, err := ini.Load(path) + if err != nil { + t.Fatalf("load: %v", err) + } + if got := cfg.Section("default").Key("verda_registry_secret").String(); got != "newsecret" { + t.Errorf("secret = %q, want newsecret (confirmed overwrite)", got) + } + if got := cfg.Section("default").Key("verda_registry_username").String(); got != "vcr-abc+newcli" { + t.Errorf("username = %q, want vcr-abc+newcli (confirmed overwrite)", got) + } + if !strings.Contains(out.String(), "saved to profile") { + t.Errorf("expected save confirmation; stdout: %q", out.String()) + } } // TestConfigure_UsernamePasswordStdin_FlagWinsOverSaved: an explicit @@ -475,6 +577,149 @@ func TestConfigure_UsernamePasswordStdin_SuggestsPasteOnUsageError(t *testing.T) } } +// TestConfigure_DockerConfigFlagWritesDockerConfig verifies that --docker-config +// merges the just-saved credentials into ~/.docker/config.json (here redirected +// via DOCKER_CONFIG) using the same writer `registry login` uses. +func TestConfigure_DockerConfigFlagWritesDockerConfig(t *testing.T) { + dir := t.TempDir() + path := filepath.Join(dir, "credentials") + t.Setenv("VERDA_REGISTRY_CREDENTIALS_FILE", path) + t.Setenv("VERDA_HOME", dir) + dockerDir := filepath.Join(dir, "docker") + t.Setenv("DOCKER_CONFIG", dockerDir) + + f := cmdutil.NewTestFactory(nil) + streams, _, errOut := newTestStreams("") + + err := runConfigureForTest(t, f, streams, + "--paste", "docker login -u vcr-abc+cli -p s3cret vccr.io", + "--docker-config", + ) + if err != nil { + t.Fatalf("run: %v", err) + } + + data, err := os.ReadFile(filepath.Join(dockerDir, "config.json")) + if err != nil { + t.Fatalf("docker config not written: %v", err) + } + wantAuth := base64.StdEncoding.EncodeToString([]byte("vcr-abc+cli:s3cret")) + if !strings.Contains(string(data), wantAuth) { + t.Errorf("docker config missing base64 auth entry; got: %s", data) + } + if !strings.Contains(string(data), "vccr.io") { + t.Errorf("docker config missing registry host; got: %s", data) + } + if !strings.Contains(errOut.String(), "Also wrote") { + t.Errorf("expected docker-config confirmation on stderr, got: %q", errOut.String()) + } +} + +// TestResolveRegistryInputs_WizardManualPath: the wizard manual path provides +// Username + Secret (no stdin, no endpoint). The endpoint is derived (vccr.io +// base / saved host) and the project id is parsed from the credential name. +func TestResolveRegistryInputs_WizardManualPath(t *testing.T) { + dir := t.TempDir() + t.Setenv("VERDA_REGISTRY_CREDENTIALS_FILE", filepath.Join(dir, "credentials")) + + f := cmdutil.NewTestFactory(nil) + streams, _, _ := newTestStreams("") + cmd := NewCmdConfigure(f, streams) + + opts := &configureOptions{ + Profile: defaultProfileName, + Username: "vcr-abc+cli", + Secret: "s3cret", + ExpiresInDays: defaultExpiresInDays, + } + creds, err := resolveRegistryInputs(cmd, f, streams, opts) + if err != nil { + t.Fatalf("resolveRegistryInputs: %v", err) + } + if creds.Username != "vcr-abc+cli" { + t.Errorf("Username = %q, want vcr-abc+cli", creds.Username) + } + if creds.Secret != "s3cret" { + t.Errorf("Secret = %q, want s3cret", creds.Secret) + } + if creds.ProjectID != "abc" { + t.Errorf("ProjectID = %q, want abc (parsed from credential name)", creds.ProjectID) + } + if creds.Endpoint != defaultRegistryEndpoint { + t.Errorf("Endpoint = %q, want %q (production base)", creds.Endpoint, defaultRegistryEndpoint) + } +} + +// TestResolveRegistryInputs_WizardManualPath_ReusesSavedEndpoint: when the +// profile already has a saved host (e.g. staging), the manual path reuses it +// rather than defaulting to vccr.io. +func TestResolveRegistryInputs_WizardManualPath_ReusesSavedEndpoint(t *testing.T) { + dir := t.TempDir() + path := filepath.Join(dir, "credentials") + t.Setenv("VERDA_REGISTRY_CREDENTIALS_FILE", path) + + existing := `[default] +verda_registry_endpoint = registry.staging.internal.datacrunch.io +` + if err := os.WriteFile(path, []byte(existing), 0o600); err != nil { + t.Fatal(err) + } + + f := cmdutil.NewTestFactory(nil) + streams, _, _ := newTestStreams("") + cmd := NewCmdConfigure(f, streams) + + opts := &configureOptions{ + Profile: defaultProfileName, + Username: "vcr-abc+cli", + Secret: "s3cret", + ExpiresInDays: defaultExpiresInDays, + } + creds, err := resolveRegistryInputs(cmd, f, streams, opts) + if err != nil { + t.Fatalf("resolveRegistryInputs: %v", err) + } + if creds.Endpoint != "registry.staging.internal.datacrunch.io" { + t.Errorf("Endpoint = %q, want the saved staging host", creds.Endpoint) + } +} + +// TestConfigure_WritesToActiveProfile: with no --profile, configure writes to +// the active profile (VERDA_PROFILE here), not the literal "default". This is +// the write side of the active-profile fix; the read commands resolve the same +// way via resolveProfile/loadCredsFromFactory. +func TestConfigure_WritesToActiveProfile(t *testing.T) { + dir := t.TempDir() + path := filepath.Join(dir, "credentials") + t.Setenv("VERDA_REGISTRY_CREDENTIALS_FILE", path) + t.Setenv("VERDA_HOME", dir) + t.Setenv("VERDA_PROFILE", "production") + + f := cmdutil.NewTestFactory(nil) + streams, out, _ := newTestStreams("") + + err := runConfigureForTest(t, f, streams, + "--paste", "docker login -u vcr-abc+cli -p s3cret vccr.io", + ) + if err != nil { + t.Fatalf("run: %v", err) + } + + cfg, err := ini.Load(path) + if err != nil { + t.Fatalf("load: %v", err) + } + if !cfg.Section("production").HasKey("verda_registry_username") { + t.Error("credentials should have been written to the active profile [production]") + } + if cfg.Section("default").HasKey("verda_registry_username") { + t.Error("credentials must NOT leak into [default] when the active profile is production") + } + if !strings.Contains(out.String(), `profile "production"`) { + t.Errorf("should report writing to the active profile, got: %q", out.String()) + } +} + func TestConfigure_ExpiresInRespected(t *testing.T) { dir := t.TempDir() path := filepath.Join(dir, "credentials") diff --git a/internal/verda-cli/cmd/registry/copy.go b/internal/verda-cli/cmd/registry/copy.go index a35c98f..39e333a 100644 --- a/internal/verda-cli/cmd/registry/copy.go +++ b/internal/verda-cli/cmd/registry/copy.go @@ -114,7 +114,6 @@ type copyPayload struct { // credentials, with the secret read from stdin. func NewCmdCopy(f cmdutil.Factory, ioStreams cmdutil.IOStreams) *cobra.Command { opts := ©Options{ - Profile: defaultProfileName, Retries: 5, Progress: progressAuto, SrcAuth: srcAuthDockerConfig, @@ -139,7 +138,7 @@ func NewCmdCopy(f cmdutil.Factory, ioStreams cmdutil.IOStreams) *cobra.Command { and registered credential helpers by default. Pass --src-auth anonymous to skip the keychain entirely or --src-auth basic plus --src-username / --src-password-stdin - to supply explicit credentials. + to supply explicit credentials. Run with no arguments on a terminal to launch an interactive wizard that prompts for the source, access mode, scope (this tag / all tags), and destination, then runs the copy. `), Example: cmdutil.Examples(` # Copy a public image from Docker Hub to VCR under the same repo/tag @@ -156,14 +155,22 @@ func NewCmdCopy(f cmdutil.Factory, ioStreams cmdutil.IOStreams) *cobra.Command { # JSON output for scripts verda registry copy docker.io/library/nginx:1.25 -o json `), - Args: cobra.RangeArgs(1, 2), + // 0 args on an interactive terminal launches the wizard; scripts / + // agent / json callers must pass explicitly. + Args: cobra.MaximumNArgs(2), RunE: func(cmd *cobra.Command, args []string) error { + if len(args) == 0 { + if copyWizardEligible(f, ioStreams) { + return runCopyWizard(cmd, f, ioStreams, opts) + } + return cmdutil.UsageErrorf(cmd, "copy requires a source image: verda registry copy []") + } return runCopy(cmd, f, ioStreams, opts, args) }, } flags := cmd.Flags() - flags.StringVar(&opts.Profile, "profile", opts.Profile, "Credentials profile name") + flags.StringVar(&opts.Profile, "profile", opts.Profile, "Credentials profile name (default: active profile)") flags.StringVar(&opts.CredentialsFile, "credentials-file", "", "Path to credentials file") flags.IntVar(&opts.Jobs, "jobs", 0, "Layer-level parallelism (0 = ggcr default)") flags.IntVar(&opts.ImageJobs, "image-jobs", 0, "Image-level parallelism for --all-tags (0 = auto)") @@ -207,11 +214,27 @@ func runCopy(cmd *cobra.Command, f cmdutil.Factory, ioStreams cmdutil.IOStreams, } } - srcAuth, err := buildSourceAuth(opts, ioStreams.In) + // Flag path: the --src-auth basic secret comes from stdin. The wizard + // supplies it from a prompt and calls runCopyResolved directly. + basicPassword, err := readBasicSourcePassword(opts, ioStreams.In) + if err != nil { + return err + } + srcAuth, err := buildSourceAuth(opts, basicPassword) if err != nil { return err } + return runCopyResolved(cmd, f, ioStreams, opts, args, creds, srcRef, srcAuth) +} + +// runCopyResolved runs a copy once the source reference and source-side +// authenticator are resolved. Shared by the flag path (runCopy) and the +// interactive wizard (runCopyWizard), so both get identical dry-run, overwrite, +// --all-tags, progress, and structured-output behavior. +// +//nolint:gocritic // hugeParam: Ref is an immutable value type; contract uses value receivers uniformly (see refname.go). +func runCopyResolved(cmd *cobra.Command, f cmdutil.Factory, ioStreams cmdutil.IOStreams, opts *copyOptions, args []string, creds *options.RegistryCredentials, srcRef Ref, srcAuth authn.Authenticator) error { // --yes implies --overwrite — matches s3 rm/rb's pattern where --yes // means "I've already decided." Keeping the two as separate flags lets // a user say "overwrite yes, but also surface progress" without @@ -490,11 +513,55 @@ func resolveCopyDestination(args []string, src Ref, creds *options.RegistryCrede }, nil } -// buildSourceAuth constructs the authn.Authenticator for the source side -// based on --src-auth + associated flags. docker-config routes through -// the swappable sourceKeychainBuilder so tests can assert which keychain -// was selected without driving a real credential store. -func buildSourceAuth(opts *copyOptions, stdin io.Reader) (authn.Authenticator, error) { +// readBasicSourcePassword reads the --src-auth basic secret from stdin on the +// flag path, enforcing the --src-password-stdin contract. Returns "" for +// non-basic auth (the password is unused there); the wizard skips this and +// supplies the password from a prompt instead. +func readBasicSourcePassword(opts *copyOptions, stdin io.Reader) (string, error) { + if opts.SrcAuth != srcAuthBasic { + return "", nil + } + // Validate --src-username BEFORE touching stdin: otherwise a missing + // username drains the piped secret first and the user must re-pipe it + // (painful when it's a one-shot token from a subprocess). + if opts.SrcUsername == "" { + return "", &cmdutil.AgentError{ + Code: kindRegistryInvalidReference, + Message: "--src-auth basic requires --src-username", + ExitCode: cmdutil.ExitBadArgs, + } + } + if !opts.SrcPasswordStdin { + return "", &cmdutil.AgentError{ + Code: kindRegistryInvalidReference, + Message: "--src-auth basic requires --src-password-stdin (read from stdin)", + ExitCode: cmdutil.ExitBadArgs, + } + } + if stdin == nil { + return "", &cmdutil.AgentError{ + Code: kindRegistryInvalidReference, + Message: "no stdin available to read --src-password-stdin", + ExitCode: cmdutil.ExitBadArgs, + } + } + pw, err := readSecretFromStdin(stdin) + if err != nil { + return "", &cmdutil.AgentError{ + Code: kindRegistryInvalidReference, + Message: fmt.Sprintf("reading source password from stdin: %v", err), + ExitCode: cmdutil.ExitBadArgs, + } + } + return pw, nil +} + +// buildSourceAuth constructs the authn.Authenticator for the source side based +// on --src-auth. The basic secret is supplied by the caller (stdin on the flag +// path, a prompt in the wizard). docker-config routes through the swappable +// sourceKeychainBuilder so tests can assert which keychain was selected without +// driving a real credential store. +func buildSourceAuth(opts *copyOptions, basicPassword string) (authn.Authenticator, error) { switch opts.SrcAuth { case srcAuthAnonymous: return authn.Anonymous, nil @@ -507,38 +574,16 @@ func buildSourceAuth(opts *copyOptions, stdin io.Reader) (authn.Authenticator, e ExitCode: cmdutil.ExitBadArgs, } } - if !opts.SrcPasswordStdin { - return nil, &cmdutil.AgentError{ - Code: kindRegistryInvalidReference, - Message: "--src-auth basic requires --src-password-stdin (read from stdin)", - ExitCode: cmdutil.ExitBadArgs, - } - } - if stdin == nil { - return nil, &cmdutil.AgentError{ - Code: kindRegistryInvalidReference, - Message: "no stdin available to read --src-password-stdin", - ExitCode: cmdutil.ExitBadArgs, - } - } - pw, err := readSecretFromStdin(stdin) - if err != nil { - return nil, &cmdutil.AgentError{ - Code: kindRegistryInvalidReference, - Message: fmt.Sprintf("reading source password from stdin: %v", err), - ExitCode: cmdutil.ExitBadArgs, - } - } - if pw == "" { + if basicPassword == "" { return nil, &cmdutil.AgentError{ Code: kindRegistryInvalidReference, - Message: "empty password on stdin for --src-password-stdin", + Message: "empty source password for --src-auth basic", ExitCode: cmdutil.ExitBadArgs, } } return authn.FromConfig(authn.AuthConfig{ Username: opts.SrcUsername, - Password: pw, + Password: basicPassword, }), nil case srcAuthDockerConfig, "": diff --git a/internal/verda-cli/cmd/registry/copy_test.go b/internal/verda-cli/cmd/registry/copy_test.go index d169307..16e9ca0 100644 --- a/internal/verda-cli/cmd/registry/copy_test.go +++ b/internal/verda-cli/cmd/registry/copy_test.go @@ -221,6 +221,33 @@ func TestCopy_SrcAuthAnonymous(t *testing.T) { } } +// TestCopy_SrcAuthBasic_MissingUsernameDoesNotDrainStdin: --src-auth basic +// without --src-username errors on the username BEFORE reading stdin, so a +// piped one-shot secret isn't consumed (the user can re-run without re-piping). +func TestCopy_SrcAuthBasic_MissingUsernameDoesNotDrainStdin(t *testing.T) { + writeCopyCredsFile(t, "vccr.io", "proj") + + in := bytes.NewBufferString("s3cret-token\n") + streams := cmdutil.IOStreams{In: in, Out: &bytes.Buffer{}, ErrOut: &bytes.Buffer{}} + + f := cmdutil.NewTestFactory(nil) + err := runCopyForTest(t, f, streams, + "docker.io/library/nginx:1.25", + "--src-auth", "basic", + "--src-password-stdin", // username omitted on purpose + ) + if err == nil { + t.Fatal("expected an error when --src-username is missing") + } + if !strings.Contains(err.Error(), "requires --src-username") { + t.Errorf("error = %v, want it to mention 'requires --src-username'", err) + } + // The secret on stdin must be intact — the username check fires first. + if in.Len() == 0 { + t.Error("stdin was drained before the --src-username check; the piped secret is lost") + } +} + // TestCopy_SrcAuthBasic: --src-auth basic + --src-username + secret on // stdin builds an authn.AuthConfig with those values and uses it on the // source side. We verify this by pointing the source at a tiny server diff --git a/internal/verda-cli/cmd/registry/copy_wizard.go b/internal/verda-cli/cmd/registry/copy_wizard.go new file mode 100644 index 0000000..3b5e4f0 --- /dev/null +++ b/internal/verda-cli/cmd/registry/copy_wizard.go @@ -0,0 +1,383 @@ +// Copyright 2026 Verda Cloud Oy +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package registry + +import ( + "context" + "fmt" + "strings" + + "github.com/spf13/cobra" + "github.com/verda-cloud/verdagostack/pkg/tui" + + cmdutil "github.com/verda-cloud/verda-cli/internal/verda-cli/cmd/util" + "github.com/verda-cloud/verda-cli/internal/verda-cli/options" +) + +// copy wizard steps. Mirrors s3 cp's runUploadWizard step-machine: each step +// prompts, Esc steps back one, Ctrl+C exits, and a final confirm previews the +// equivalent command before running the real copy pipeline. +const ( + copyStepSource = iota + copyStepAccess + copyStepScope + copyStepDest + copyStepConfirm +) + +// copyWizardEligible reports whether to launch the interactive copy wizard: +// an interactive terminal, not agent mode, and the default table output +// (json/yaml callers must pass explicitly so scripts stay deterministic). +func copyWizardEligible(f cmdutil.Factory, ioStreams cmdutil.IOStreams) bool { + return !f.AgentMode() && !isStructuredFormat(f.OutputFormat()) && isTerminalFn(ioStreams.Out) +} + +// runCopyWizard guides an interactive copy: source image (validated) -> source +// access (public/anonymous/basic) -> scope (this tag / all tags) -> destination +// (pre-filled from the active VCR project) -> confirm. On confirm it runs the +// same pipeline as the flag path via runCopyResolved, so dry-run-free copies get +// the progress view, overwrite guard, and --all-tags fan-out unchanged. +// +// Navigation honors the hint bar: Esc steps BACK one question, Ctrl+C exits. +// Pickers return the raw prompter error so classifyWizardNav can tell the two +// apart. +// +//nolint:gocyclo // Interactive step machine with per-step back/exit navigation — inherently complex. +func runCopyWizard(cmd *cobra.Command, f cmdutil.Factory, ioStreams cmdutil.IOStreams, opts *copyOptions) error { + // Unbounded: interactive think-time (and the copy itself, inside + // runCopyResolved) must not hit the short per-request --timeout. Ctrl+C + // cancels. + ctx := cmd.Context() + + creds, err := loadCredsFromFactory(f, opts.Profile, opts.CredentialsFile) + if err != nil { + return checkExpiry(nil) + } + if err := checkExpiry(creds); err != nil { + return err + } + if creds.Endpoint == "" || creds.ProjectID == "" { + return &cmdutil.AgentError{ + Code: kindRegistryNotConfigured, + Message: "Registry is not configured. Run `verda registry configure` first.", + ExitCode: cmdutil.ExitAuth, + } + } + + prompter := f.Prompter() + var ( + srcRaw string + srcRef Ref + srcPassword string + dstRaw string + ) + step := copyStepSource + for { + switch step { + case copyStepSource: + raw, ref, perr := promptCopySource(ctx, prompter, ioStreams) + if perr != nil { + if cmdutil.IsPromptCancel(perr) { + return nil // first step: any cancel exits + } + return perr + } + if raw == "" { + return nil + } + srcRaw, srcRef = raw, ref + step = copyStepAccess + + case copyStepAccess: + pw, aerr := promptCopyAccess(ctx, prompter, opts) + if back, exit, fatal := classifyWizardNav(aerr, false); fatal != nil { + return fatal + } else if exit { + return nil + } else if back { + step = copyStepSource + continue + } + srcPassword = pw + step = copyStepScope + + case copyStepScope: + serr := promptCopyScope(ctx, prompter, opts) + if back, exit, fatal := classifyWizardNav(serr, false); fatal != nil { + return fatal + } else if exit { + return nil + } else if back { + step = copyStepAccess + continue + } + step = copyStepDest + + case copyStepDest: + dst, derr := promptCopyDest(ctx, prompter, cmd, opts, srcRaw, srcRef, creds) + if back, exit, fatal := classifyWizardNav(derr, false); fatal != nil { + return fatal + } else if exit { + return nil + } else if back { + step = copyStepScope + continue + } + dstRaw = dst + step = copyStepConfirm + + case copyStepConfirm: + back, cerr := confirmAndRunCopy(cmd, f, ioStreams, opts, creds, srcRaw, srcRef, srcPassword, dstRaw) + if cerr != nil { + return cerr + } + if back { + step = copyStepDest + continue + } + return nil + } + } +} + +// classifyWizardNav maps a picker's returned error into back/exit/real outcomes. +// Esc (IsPromptBack) steps back; Ctrl+C (IsPromptInterrupt) exits; a non-prompt +// error propagates; nil advances. Mirrors s3 cp's classifyNav. +func classifyWizardNav(err error, firstStep bool) (back, exit bool, fatal error) { + switch { + case err == nil: + return false, false, nil + case cmdutil.IsPromptInterrupt(err): + return false, true, nil + case cmdutil.IsPromptBack(err): + if firstStep { + return false, true, nil + } + return true, false, nil + default: + return false, false, err + } +} + +// promptCopySource asks for the source image and validates it parses. A bad +// reference re-prompts (not a navigation error); Esc/Ctrl+C returns the raw +// prompter error; empty input returns ("", Ref{}, nil) so the caller exits. +func promptCopySource(ctx context.Context, prompter tui.Prompter, ioStreams cmdutil.IOStreams) (string, Ref, error) { + for { + raw, err := prompter.TextInput(ctx, "Source image (e.g. docker.io/library/nginx:1.25)") + if err != nil { + return "", Ref{}, err + } + raw = strings.TrimSpace(raw) + if raw == "" { + return "", Ref{}, nil + } + ref, perr := Parse(raw) + if perr != nil { + _, _ = fmt.Fprintf(ioStreams.ErrOut, " %v — try again.\n", perr) + continue + } + return raw, ref, nil + } +} + +// promptCopyAccess selects the source-side authentication mode and, for the +// basic option, prompts for username + (masked) password. Sets opts.SrcAuth / +// opts.SrcUsername and returns the password (empty for the non-basic modes). +// Returns the raw prompter error from the Select so the caller can classify +// Esc (back) vs Ctrl+C (exit); a canceled basic sub-prompt loops back to the +// mode list rather than exiting. +func promptCopyAccess(ctx context.Context, prompter tui.Prompter, opts *copyOptions) (string, error) { + const ( + accessPublic = iota + accessAnonymous + accessBasic + ) + choices := []string{ + "Public / via `docker login` (default)", + "Anonymous", + "Username + password", + } + for { + idx, serr := prompter.Select(ctx, "Source access", choices, tui.WithShowHints(true)) + if serr != nil { + return "", serr // raw: caller classifies Esc vs Ctrl+C + } + switch idx { + case accessPublic: + opts.SrcAuth = srcAuthDockerConfig + return "", nil + case accessAnonymous: + opts.SrcAuth = srcAuthAnonymous + return "", nil + case accessBasic: + uname, pw, berr := promptBasicCreds(ctx, prompter) + if berr != nil { + if cmdutil.IsPromptInterrupt(berr) { + return "", berr + } + continue // Esc / empty on a sub-prompt → back to the mode list + } + if uname == "" || pw == "" { + continue // incomplete → back to the mode list + } + opts.SrcAuth = srcAuthBasic + opts.SrcUsername = uname + return pw, nil + } + } +} + +// promptBasicCreds collects a username and masked password for --src-auth basic. +// Empty input (after a clean prompt) returns "" so the caller loops; a canceled +// prompt returns the raw error. +func promptBasicCreds(ctx context.Context, prompter tui.Prompter) (username, password string, err error) { + u, err := prompter.TextInput(ctx, "Source username") + if err != nil { + return "", "", err + } + u = strings.TrimSpace(u) + if u == "" { + return "", "", nil + } + p, err := prompter.Password(ctx, "Source password") + if err != nil { + return "", "", err + } + return u, p, nil +} + +// promptCopyScope asks whether to copy a single tag or every tag in the source +// repository, setting opts.AllTags. Returns the raw prompter error. +func promptCopyScope(ctx context.Context, prompter tui.Prompter, opts *copyOptions) error { + idx, err := prompter.Select(ctx, "What to copy", + []string{"Just this image (tag)", "All tags in the repository"}, + tui.WithShowHints(true)) + if err != nil { + return err + } + opts.AllTags = idx == 1 + return nil +} + +// promptCopyDest asks for the destination, pre-filled with the synthesized VCR +// reference (single tag) or repository base (--all-tags). The user accepts it +// with Enter or edits it. Returns the raw prompter error. +// +//nolint:gocritic // hugeParam: Ref is an immutable value type; contract uses value receivers uniformly (see refname.go). +func promptCopyDest(ctx context.Context, prompter tui.Prompter, cmd *cobra.Command, opts *copyOptions, srcRaw string, srcRef Ref, creds *options.RegistryCredentials) (string, error) { + suggested := copyDestSuggestion(cmd, opts, srcRaw, srcRef, creds) + label := "Destination" + if opts.AllTags { + label = "Destination repository (all tags)" + } + dst, err := prompter.TextInput(ctx, label, tui.WithDefault(suggested)) + if err != nil { + return "", err + } + dst = strings.TrimSpace(dst) + if dst == "" { + dst = suggested + } + return dst, nil +} + +// copyDestSuggestion computes the default destination shown in the dest step, +// reusing the same resolution the flag path uses. Falls back to "" if creds +// can't synthesize one (already guarded upstream, so this is defensive). +// +//nolint:gocritic // hugeParam: Ref uses value receivers uniformly (see refname.go). +func copyDestSuggestion(cmd *cobra.Command, opts *copyOptions, srcRaw string, srcRef Ref, creds *options.RegistryCredentials) string { + if opts.AllTags { + base, err := resolveCopyAllTagsDestination(cmd, []string{copyTaglessSource(srcRef)}, srcRef, creds) + if err != nil { + return "" + } + return base.String() + } + dst, err := resolveCopyDestination([]string{srcRaw}, srcRef, creds) + if err != nil { + return "" + } + return dst.String() +} + +// copyTaglessSource renders the source as a tag-less repository path +// (//) so --all-tags isn't rejected for carrying an +// explicit :tag the user may have typed. +// +//nolint:gocritic // hugeParam: Ref uses value receivers uniformly (see refname.go). +func copyTaglessSource(srcRef Ref) string { + return srcRef.Host + "/" + srcRef.FullRepository() +} + +// confirmAndRunCopy previews the equivalent command, confirms (default Yes), and +// runs the real copy via runCopyResolved. back=true means Esc -> return to the +// destination step; Ctrl+C or "no" is a clean exit (back=false, err=nil). +// +//nolint:gocritic // hugeParam: Ref uses value receivers uniformly (see refname.go). +func confirmAndRunCopy(cmd *cobra.Command, f cmdutil.Factory, ioStreams cmdutil.IOStreams, opts *copyOptions, creds *options.RegistryCredentials, srcRaw string, srcRef Ref, srcPassword, dstRaw string) (back bool, err error) { + // Build the positional args the resolved copy expects. --all-tags needs a + // tag-less source (and destination); single-tag passes the source verbatim. + srcArg := srcRaw + if opts.AllTags { + srcArg = copyTaglessSource(srcRef) + } + args := []string{srcArg, dstRaw} + + _, _ = fmt.Fprintf(ioStreams.ErrOut, "\n Will run: %s\n\n", copyCommandPreview(args, opts)) + + confirmed, cerr := f.Prompter().Confirm(cmd.Context(), "Proceed with copy? (esc to go back)", tui.WithConfirmDefault(true)) + if cerr != nil { + if cmdutil.IsPromptBack(cerr) { + return true, nil + } + if cmdutil.IsPromptInterrupt(cerr) { + return false, nil + } + return false, cerr + } + if !confirmed { + _, _ = fmt.Fprintln(ioStreams.ErrOut, "Canceled.") + return false, nil + } + + srcAuth, aerr := buildSourceAuth(opts, srcPassword) + if aerr != nil { + return false, aerr + } + return false, runCopyResolved(cmd, f, ioStreams, opts, args, creds, srcRef, srcAuth) +} + +// copyCommandPreview renders the equivalent `verda registry copy …` command so +// the user sees exactly what the wizard will run (and can reproduce it later). +func copyCommandPreview(args []string, opts *copyOptions) string { + var b strings.Builder + b.WriteString("verda registry copy ") + b.WriteString(strings.Join(args, " ")) + if opts.AllTags { + b.WriteString(" --all-tags") + } + if opts.SrcAuth != "" && opts.SrcAuth != srcAuthDockerConfig { + b.WriteString(" --src-auth ") + b.WriteString(opts.SrcAuth) + } + if opts.SrcAuth == srcAuthBasic { + b.WriteString(" --src-username ") + b.WriteString(opts.SrcUsername) + b.WriteString(" --src-password-stdin") + } + return b.String() +} diff --git a/internal/verda-cli/cmd/registry/copy_wizard_test.go b/internal/verda-cli/cmd/registry/copy_wizard_test.go new file mode 100644 index 0000000..f2665ce --- /dev/null +++ b/internal/verda-cli/cmd/registry/copy_wizard_test.go @@ -0,0 +1,158 @@ +// Copyright 2026 Verda Cloud Oy +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package registry + +import ( + "context" + "errors" + "strings" + "testing" + + "github.com/google/go-containerregistry/pkg/authn" + v1 "github.com/google/go-containerregistry/pkg/v1" + + cmdutil "github.com/verda-cloud/verda-cli/internal/verda-cli/cmd/util" + "github.com/verda-cloud/verda-cli/internal/verda-cli/options" + tuitest "github.com/verda-cloud/verdagostack/pkg/tui/testing" +) + +// readCaptureRegistry records the ref passed to Read, then errors so the copy +// short-circuits before any network I/O. Used to assert the wizard built the +// expected source reference and reached the real copy path. +type readCaptureRegistry struct { + Registry + gotRef string +} + +func (r *readCaptureRegistry) Read(_ context.Context, ref string) (v1.Image, error) { + r.gotRef = ref + return nil, errors.New("intercept: no network") +} + +// headNotFoundRegistry answers every Head with a not-found AgentError so the +// overwrite guard treats the destination as absent and proceeds. +type headNotFoundRegistry struct { + Registry + gotRef string +} + +func (h *headNotFoundRegistry) Head(_ context.Context, ref string) (*v1.Descriptor, error) { + h.gotRef = ref + return nil, &cmdutil.AgentError{Code: kindRegistryTagNotFound, Message: "not found"} +} + +func TestCopyCommandPreview(t *testing.T) { + t.Parallel() + cases := []struct { + name string + args []string + opts *copyOptions + want string + }{ + { + "single public", + []string{"docker.io/library/nginx:1.25", "vccr.io/p/library/nginx:1.25"}, + ©Options{SrcAuth: srcAuthDockerConfig}, + "verda registry copy docker.io/library/nginx:1.25 vccr.io/p/library/nginx:1.25", + }, + { + "all tags", + []string{"docker.io/library/nginx", "vccr.io/p/library/nginx"}, + ©Options{SrcAuth: srcAuthDockerConfig, AllTags: true}, + "verda registry copy docker.io/library/nginx vccr.io/p/library/nginx --all-tags", + }, + { + "basic auth", + []string{"private.example.com/app:v1", "vccr.io/p/app:v1"}, + ©Options{SrcAuth: srcAuthBasic, SrcUsername: "jdoe"}, + "verda registry copy private.example.com/app:v1 vccr.io/p/app:v1 --src-auth basic --src-username jdoe --src-password-stdin", + }, + { + "anonymous", + []string{"src:v1", "dst:v1"}, + ©Options{SrcAuth: srcAuthAnonymous}, + "verda registry copy src:v1 dst:v1 --src-auth anonymous", + }, + } + for _, c := range cases { + t.Run(c.name, func(t *testing.T) { + if got := copyCommandPreview(c.args, c.opts); got != c.want { + t.Errorf("preview = %q, want %q", got, c.want) + } + }) + } +} + +// TestCopyWizard_RunsCopyWithCollectedInputs drives the full wizard (source -> +// access -> scope -> destination -> confirm) and asserts it reaches the real +// copy with the source it collected. Registries are stubbed so no network I/O +// occurs. +func TestCopyWizard_RunsCopyWithCollectedInputs(t *testing.T) { + writeCopyCredsFile(t, "vccr.io", "proj") + + src := &readCaptureRegistry{} + prevSrc := sourceRegistryBuilder + sourceRegistryBuilder = func(_ authn.Authenticator, _ RetryConfig) Registry { return src } + t.Cleanup(func() { sourceRegistryBuilder = prevSrc }) + + dst := &headNotFoundRegistry{} + prevClient := clientBuilder + clientBuilder = func(_ *options.RegistryCredentials, _ RetryConfig) Registry { return dst } + t.Cleanup(func() { clientBuilder = prevClient }) + + mock := tuitest.New(). + AddTextInput("docker.io/library/nginx:1.25"). // source image + AddSelect(0). // access: public / docker login + AddSelect(0). // scope: just this tag + AddTextInput(""). // destination: accept default + AddConfirm(true) // confirm + f := cmdutil.NewTestFactory(mock) + streams, _, _ := copyStreams("") + + opts := ©Options{Retries: 5, Progress: progressAuto, SrcAuth: srcAuthDockerConfig} + cmd := NewCmdCopy(f, streams) + cmd.SetContext(context.Background()) + + // The copy fails at the stubbed source Read (no network) — that's expected; + // we only assert the wizard reached the copy with the inputs it gathered. + _ = runCopyWizard(cmd, f, streams, opts) + + if !strings.Contains(src.gotRef, "library/nginx:1.25") { + t.Errorf("source Read ref = %q, want it to contain library/nginx:1.25", src.gotRef) + } + // The default destination was synthesized under the active VCR project. + if !strings.Contains(dst.gotRef, "vccr.io/proj/library/nginx:1.25") { + t.Errorf("destination Head ref = %q, want vccr.io/proj/library/nginx:1.25", dst.gotRef) + } +} + +// TestCopy_NoArgsNonTTYUsageError: with no positional args and no terminal, the +// wizard must not launch — the command returns a usage error so scripts fail +// loudly instead of hanging on a prompt. +func TestCopy_NoArgsNonTTYUsageError(t *testing.T) { + withForcedTTY(t, false) + writeCopyCredsFile(t, "vccr.io", "proj") + + f := cmdutil.NewTestFactory(nil) + streams, _, _ := copyStreams("") + + err := runCopyForTest(t, f, streams /* no args */) + if err == nil { + t.Fatal("expected a usage error with no args on a non-terminal") + } + if !strings.Contains(err.Error(), "requires a source") { + t.Errorf("unexpected error: %v", err) + } +} diff --git a/internal/verda-cli/cmd/registry/delete.go b/internal/verda-cli/cmd/registry/delete.go index 572ed1d..80a36f6 100644 --- a/internal/verda-cli/cmd/registry/delete.go +++ b/internal/verda-cli/cmd/registry/delete.go @@ -81,7 +81,7 @@ type deleteResult struct { // positional target is required AND --yes is mandatory; missing --yes // surfaces a CONFIRMATION_REQUIRED AgentError so automations can retry. func NewCmdDelete(f cmdutil.Factory, ioStreams cmdutil.IOStreams) *cobra.Command { - opts := &deleteOptions{Profile: defaultProfileName} + opts := &deleteOptions{} cmd := &cobra.Command{ Use: "delete [REPOSITORY[:TAG|@DIGEST]]", @@ -133,7 +133,7 @@ func NewCmdDelete(f cmdutil.Factory, ioStreams cmdutil.IOStreams) *cobra.Command } flags := cmd.Flags() - flags.StringVar(&opts.Profile, "profile", opts.Profile, "Credentials profile to read") + flags.StringVar(&opts.Profile, "profile", opts.Profile, "Credentials profile to read (default: active profile)") flags.StringVar(&opts.CredentialsFile, "credentials-file", "", "Path to the shared Verda credentials file") flags.BoolVarP(&opts.Yes, "yes", "y", false, "Skip confirmation (required in agent mode)") @@ -187,7 +187,12 @@ func runDelete(cmd *cobra.Command, f cmdutil.Factory, ioStreams cmdutil.IOStream ExitCode: cmdutil.ExitBadArgs, } } - return runDeleteInteractive(ctx, f, ioStreams, lister, creds) + // cmd.Context() (not the per-request timeout ctx): the interactive + // flow includes user think-time across the repo picker, sub-menu, + // multi-select and confirm, so a 30s --timeout must not cancel a + // prompt mid-flow. Ctrl+C is the stop signal. The non-interactive + // target path below keeps the bounded ctx for its single API call. + return runDeleteInteractive(cmd.Context(), f, ioStreams, lister, creds) } return runDeleteTarget(ctx, f, ioStreams, lister, creds, target, opts.Yes) @@ -433,7 +438,7 @@ func runDeleteInteractive(ctx context.Context, f cmdutil.Factory, ioStreams cmdu } labels = append(labels, "Exit") - idx, err := prompter.Select(ctx, "Select repository to manage (type to filter)", labels, tui.WithShowHints(true)) + idx, err := prompter.Select(ctx, registryBreadcrumb(creds.Endpoint, ""), labels, tui.WithShowHints(true)) if err != nil { return nil //nolint:nilerr // intentional: prompter cancel is a clean exit } @@ -444,8 +449,15 @@ func runDeleteInteractive(ctx context.Context, f cmdutil.Factory, ioStreams cmdu selected := repos[idx] exit, err := runDeleteRepoMenu(ctx, f, ioStreams, lister, creds, selected) if err != nil { - // Surface the error but keep the outer loop alive so the - // user can recover (same contract as ls_test's + // access_denied is a permission wall, not a transient blip: + // every repo will fail the same way, so surface the actionable + // error once and exit rather than looping the user back into a + // picker that can only re-fail (mirrors ls's handling). + if isAccessDenied(err) { + return err + } + // Transient (e.g. 5xx): keep the outer loop alive so the user + // can recover (same contract as ls_test's // "ArtifactsErrorStaysInLoop"). _, _ = fmt.Fprintf(ioStreams.ErrOut, "Error: %v\n", err) continue @@ -482,7 +494,7 @@ func runDeleteRepoMenu(ctx context.Context, f cmdutil.Factory, ioStreams cmdutil for { idx, err := prompter.Select(ctx, - fmt.Sprintf("What would you like to delete in %s?", repo.Name), choices, tui.WithShowHints(true)) + registryBreadcrumb(creds.Endpoint, repo.Name), choices, tui.WithShowHints(true)) if err != nil { return true, nil //nolint:nilerr // intentional: prompter cancel is a clean exit } @@ -510,9 +522,9 @@ func runDeleteRepoMenu(ctx context.Context, f cmdutil.Factory, ioStreams cmdutil } // runDeleteImagesInteractive lists artifacts in repo and runs a -// MultiSelect picker over them. Ctrl-A (the built-in "select all" in -// our bubbletea MultiSelect) is surfaced in the prompt label so users -// discover it even when they skip the hint bar. +// MultiSelect picker over them. The hint bar (WithMultiSelectShowHints) +// advertises the toggle / ctrl+a-select-all / enter keys, matching the +// s3 browser's multi-select affordances. // // The confirm step matches the "Delete image" dialog from the web UI: // one row per selected artifact, red warning, yes/no. @@ -534,8 +546,8 @@ func runDeleteImagesInteractive(ctx context.Context, f cmdutil.Factory, ioStream prompter := f.Prompter() indices, err := prompter.MultiSelect(ctx, - "Select image(s) to delete (space toggles, ctrl+a selects all, enter confirms)", - labels) + "Select image(s) to delete", + labels, tui.WithMultiSelectShowHints(true)) if err != nil { // User canceled the picker — back to the menu. return nil //nolint:nilerr // intentional: prompter cancel is a clean exit @@ -721,8 +733,8 @@ func formatArtifactRow(a *ArtifactInfo) string { if a.Size > 0 { size = formatBytes(a.Size) } - return fmt.Sprintf("%-22s %-30s %10s", - shortDigest(a.Digest), truncateField(tag, 30), size) + return fmt.Sprintf("%s %-22s %-30s %10s", + artifactGlyph, shortDigest(a.Digest), truncateField(tag, 30), size) } // truncateField narrows a field to width, adding "..." when truncated. diff --git a/internal/verda-cli/cmd/registry/delete_test.go b/internal/verda-cli/cmd/registry/delete_test.go index fb29dbf..d02e423 100644 --- a/internal/verda-cli/cmd/registry/delete_test.go +++ b/internal/verda-cli/cmd/registry/delete_test.go @@ -458,6 +458,40 @@ func TestDelete_Interactive_ImageBatch(t *testing.T) { } } +// TestDelete_Interactive_AccessDeniedExits: when the image-delete flow hits a +// permission wall (Harbor 403), the interactive loop surfaces the actionable +// error once and exits, instead of bouncing the user back to the repo picker +// to retry-and-re-fail. +func TestDelete_Interactive_AccessDeniedExits(t *testing.T) { + writeLsCredsFile(t, validLsCredsBody("vccr.io", "abc")) + lister := &fakeLister{ + repos: sampleRepos(), + artifactsErr: &cmdutil.AgentError{ + Code: kindRegistryAccessDenied, + Message: "denied", + }, + } + withFakeHarborLister(t, lister) + withForcedTTY(t, true) + + // pick repo 0 -> menu "Delete image(s)" (0) -> ListArtifacts 403 -> exit. + mock := tuitest.New().AddSelect(0).AddSelect(0) + f := cmdutil.NewTestFactory(mock) + streams, _, _ := newLsStreams() + + err := runDeleteForTest(t, f, streams) + if err == nil { + t.Fatal("expected access_denied to propagate and exit, got nil") + } + var ae *cmdutil.AgentError + if !errors.As(err, &ae) || ae.Code != kindRegistryAccessDenied { + t.Errorf("error = %v, want registry_access_denied", err) + } + if len(lister.deletedArtifacts) != 0 { + t.Errorf("nothing should have been deleted, got %+v", lister.deletedArtifacts) + } +} + // TestDelete_Interactive_ImageBatch_SelectAll mirrors the user pressing // Ctrl+A inside the MultiSelect: every artifact is queued for delete in // a single batch. tuitest's AddMultiSelect([]int{0,1,2,...}) emulates diff --git a/internal/verda-cli/cmd/registry/format.go b/internal/verda-cli/cmd/registry/format.go index cd666c1..aea224d 100644 --- a/internal/verda-cli/cmd/registry/format.go +++ b/internal/verda-cli/cmd/registry/format.go @@ -87,6 +87,29 @@ func isStructuredFormat(format string) bool { // Harbor's web UI phrasing so the CLI and UI read the same. const untaggedLabel = "" +// Interactive-picker glyphs mirror the s3 browser's iconography so the two +// command trees read the same in interactive mode: 📦 a repository, 📄 an +// image (artifact). See cmd/s3/browse.go for the originating convention. +const ( + repoGlyph = "📦" + artifactGlyph = "📄" +) + +// registryBreadcrumb renders an interactive picker title as a path rooted at +// the registry host, mirroring s3's `s3://bucket/key` breadcrumb idiom. repo +// is "" at the repository-list level. Falls back to the production host when +// the saved endpoint is blank (legacy creds configured without an explicit +// endpoint). +func registryBreadcrumb(host, repo string) string { + if host == "" { + host = defaultRegistryEndpoint + } + if repo == "" { + return host + "/" + } + return host + "/" + repo +} + // formatMMSS renders a duration as MM:SS for durations under an hour and // HH:MM:SS once it reaches one hour. Negative durations clamp to "00:00". // Pure; no clock access. diff --git a/internal/verda-cli/cmd/registry/helper.go b/internal/verda-cli/cmd/registry/helper.go index 4c27429..d19dce1 100644 --- a/internal/verda-cli/cmd/registry/helper.go +++ b/internal/verda-cli/cmd/registry/helper.go @@ -66,26 +66,31 @@ func buildHarborLister(creds *options.RegistryCredentials, cfg RetryConfig) Repo return harborListerBuilder(creds, cfg) } -// loadCredsFromFactory loads registry credentials for the current -// profile, applying the s3-style fallback to the default profile name -// when Profile is empty. -// -// Registry commands are exempt from Options.Complete() (see -// cmd/cmd.go skipCredentialResolution), so AuthOptions.Profile is never -// auto-resolved. Without the fallback, an unset profile would make -// ini.v1 load the synthetic DEFAULT section instead of the user's -// [default] section, surfacing a spurious "not configured" error right -// after a successful `verda registry configure`. -func loadCredsFromFactory(f cmdutil.Factory, profileOverride, fileOverride string) (*options.RegistryCredentials, error) { - profile := profileOverride - if profile == "" { - if opts := f.Options(); opts != nil && opts.AuthOptions != nil { - profile = opts.AuthOptions.Profile - } - } - if profile == "" { - profile = defaultProfileName - } +// loadCredsFromFactory loads registry credentials for the resolved profile +// (see resolveProfile). +func loadCredsFromFactory(_ cmdutil.Factory, profileOverride, fileOverride string) (*options.RegistryCredentials, error) { + profile := resolveProfile(profileOverride) path := credentialsFilePath(fileOverride) return options.LoadRegistryCredentialsForProfile(path, profile) } + +// resolveProfile picks the credentials profile a registry command acts on: +// an explicit --profile flag wins, else the CLI's active profile (VERDA_PROFILE +// or auth.profile in the config), else "default". +// +// Registry commands are in skipCredentialResolution, so — unlike `verda vm` — +// they don't inherit the active profile from options.Complete(). Resolving it +// here keeps `registry ls/tags/push/...` consistent with `registry configure` +// and the rest of the CLI: a user who ran `verda auth use ` (or set +// VERDA_PROFILE) gets that profile for registry too. Idempotent — resolving an +// already-explicit profile returns it unchanged. +// +// The "default" fallback also avoids ini.v1 loading its synthetic DEFAULT +// section for a blank profile, which would surface a spurious "not configured" +// error right after a successful `verda registry configure`. +func resolveProfile(flagProfile string) string { + if p := options.ActiveProfile(flagProfile); p != "" { + return p + } + return defaultProfileName +} diff --git a/internal/verda-cli/cmd/registry/helper_test.go b/internal/verda-cli/cmd/registry/helper_test.go new file mode 100644 index 0000000..76cedba --- /dev/null +++ b/internal/verda-cli/cmd/registry/helper_test.go @@ -0,0 +1,44 @@ +// Copyright 2026 Verda Cloud Oy +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package registry + +import "testing" + +// TestResolveProfile pins the precedence registry commands use to pick a +// credentials profile: explicit flag > VERDA_PROFILE > "default". (auth.profile +// from the config is covered by ActiveProfile's own tests; viper is empty in +// this unit context, so it resolves to "default".) +func TestResolveProfile(t *testing.T) { + t.Run("no flag, no env -> default", func(t *testing.T) { + t.Setenv("VERDA_PROFILE", "") + if got := resolveProfile(""); got != defaultProfileName { + t.Errorf("resolveProfile(\"\") = %q, want %q", got, defaultProfileName) + } + }) + + t.Run("VERDA_PROFILE wins over default", func(t *testing.T) { + t.Setenv("VERDA_PROFILE", "production") + if got := resolveProfile(""); got != "production" { + t.Errorf("resolveProfile(\"\") = %q, want %q", got, "production") + } + }) + + t.Run("explicit flag wins over env", func(t *testing.T) { + t.Setenv("VERDA_PROFILE", "production") + if got := resolveProfile("staging"); got != "staging" { + t.Errorf("resolveProfile(\"staging\") = %q, want %q (flag wins)", got, "staging") + } + }) +} diff --git a/internal/verda-cli/cmd/registry/login.go b/internal/verda-cli/cmd/registry/login.go index c2c0543..a3e531d 100644 --- a/internal/verda-cli/cmd/registry/login.go +++ b/internal/verda-cli/cmd/registry/login.go @@ -28,14 +28,14 @@ import ( "github.com/verda-cloud/verda-cli/internal/verda-cli/options" ) -// loginOptions bundles the flag state for `verda registry login`. +// loginOptions bundles the flag state for `verda registry configure-docker`. type loginOptions struct { Profile string CredentialsFile string DockerConfig string } -// NewCmdLogin creates the `verda registry login` command. +// NewCmdLogin creates the `verda registry configure-docker` command. // // login writes the active registry credentials into ~/.docker/config.json // so third-party tools (docker pull, docker-compose, helm pull oci://, @@ -43,13 +43,12 @@ type loginOptions struct { // registry — it is a pure file-merge. `configure` is for storing Verda's // own credentials; `login` is for interop. func NewCmdLogin(f cmdutil.Factory, ioStreams cmdutil.IOStreams) *cobra.Command { - opts := &loginOptions{ - Profile: defaultProfileName, - } + opts := &loginOptions{} cmd := &cobra.Command{ - Use: "login", - Short: "Write VCR credentials into ~/.docker/config.json", + Use: "configure-docker", + Aliases: []string{"login"}, + Short: "Set up Docker/compose/helm to pull from VCR (writes ~/.docker/config.json)", Long: cmdutil.LongDesc(` Write the active Verda Container Registry credentials into the Docker config file so third-party tools (docker pull, docker-compose, @@ -65,13 +64,13 @@ func NewCmdLogin(f cmdutil.Factory, ioStreams cmdutil.IOStreams) *cobra.Command `), Example: cmdutil.Examples(` # Write the default profile into ~/.docker/config.json - verda registry login + verda registry configure-docker # Use a specific profile - verda registry login --profile staging + verda registry configure-docker --profile staging # Write to a non-default Docker config file - verda registry login --config /tmp/docker/config.json + verda registry configure-docker --config /tmp/docker/config.json `), Args: cobra.NoArgs, RunE: func(cmd *cobra.Command, args []string) error { @@ -80,7 +79,7 @@ func NewCmdLogin(f cmdutil.Factory, ioStreams cmdutil.IOStreams) *cobra.Command } flags := cmd.Flags() - flags.StringVar(&opts.Profile, "profile", opts.Profile, "Credentials profile to read") + flags.StringVar(&opts.Profile, "profile", opts.Profile, "Credentials profile to read (default: active profile)") flags.StringVar(&opts.CredentialsFile, "credentials-file", "", "Path to the shared Verda credentials file") flags.StringVar(&opts.DockerConfig, "config", "", "Path to the Docker config file (default ~/.docker/config.json)") diff --git a/internal/verda-cli/cmd/registry/ls.go b/internal/verda-cli/cmd/registry/ls.go index d4ebc8c..86ba256 100644 --- a/internal/verda-cli/cmd/registry/ls.go +++ b/internal/verda-cli/cmd/registry/ls.go @@ -16,15 +16,16 @@ package registry import ( "context" + "errors" "fmt" "sort" "strconv" - "strings" "github.com/spf13/cobra" "github.com/verda-cloud/verdagostack/pkg/tui" cmdutil "github.com/verda-cloud/verda-cli/internal/verda-cli/cmd/util" + "github.com/verda-cloud/verda-cli/internal/verda-cli/options" ) // lsOptions bundles flag state for `verda registry ls`. @@ -50,7 +51,7 @@ type lsPayload struct { // UI's "Image list" view. When piped or redirected (or with -o json / // yaml), ls prints a single non-interactive table/document. func NewCmdLs(f cmdutil.Factory, ioStreams cmdutil.IOStreams) *cobra.Command { - opts := &lsOptions{Profile: defaultProfileName} + opts := &lsOptions{} cmd := &cobra.Command{ Use: "ls", @@ -87,7 +88,7 @@ func NewCmdLs(f cmdutil.Factory, ioStreams cmdutil.IOStreams) *cobra.Command { } flags := cmd.Flags() - flags.StringVar(&opts.Profile, "profile", opts.Profile, "Credentials profile to read") + flags.StringVar(&opts.Profile, "profile", opts.Profile, "Credentials profile to read (default: active profile)") flags.StringVar(&opts.CredentialsFile, "credentials-file", "", "Path to the shared Verda credentials file") return cmd @@ -162,14 +163,20 @@ func runLs(cmd *cobra.Command, f cmdutil.Factory, ioStreams cmdutil.IOStreams, o // TTY: interactive drill-down. Mirrors cmd/vm/list.go — use the // factory's prompter so agent mode / automation still gets a // structured AgentError instead of a blocked terminal. - return runLsInteractive(ctx, f, ioStreams, lister, payload) + // + // Pass cmd.Context() (NOT the per-request timeout ctx used for the + // listing above): the interactive loop includes user think-time, so a + // 30s --timeout must not cancel a prompt mid-browse. Ctrl+C is the stop + // signal; per-call API timeouts inside the loop are a future refinement. + return runLsInteractive(cmd.Context(), f, ioStreams, lister, payload, creds) } // runLsInteractive loops: render a single-line summary per repo, let the // user pick one, then render that repo's artifact card. Selecting Exit // (or Ctrl-C on the picker) returns nil — same contract as `vm ls`. func runLsInteractive(ctx context.Context, f cmdutil.Factory, ioStreams cmdutil.IOStreams, - lister RepositoryLister, payload lsPayload) error { + lister RepositoryLister, payload lsPayload, creds *options.RegistryCredentials) error { + host := creds.Endpoint _, _ = fmt.Fprintf(ioStreams.ErrOut, " %d repository(ies) in project %s\n\n", len(payload.Repositories), payload.Project) @@ -181,7 +188,7 @@ func runLsInteractive(ctx context.Context, f cmdutil.Factory, ioStreams cmdutil. labels = append(labels, "Exit") for { - idx, err := prompter.Select(ctx, "Select repository (type to filter)", labels, tui.WithShowHints(true)) + idx, err := prompter.Select(ctx, registryBreadcrumb(host, ""), labels, tui.WithShowHints(true)) if err != nil { // Prompter-layer cancellation (Ctrl-C, ESC) returns a // sentinel error; vm list treats it as a clean exit. @@ -193,14 +200,49 @@ func runLsInteractive(ctx context.Context, f cmdutil.Factory, ioStreams cmdutil. repo := payload.Repositories[idx] arts, artErr := lister.ListArtifacts(ctx, payload.Project, repo.Name) - if artErr != nil { - // Keep the loop alive so one flaky repo doesn't kick the - // user out of the picker (matches vm list's fetch-details - // error handling). + switch { + case artErr == nil: + // Repo action menu: copy a pull URL or delete image(s). + exit, perr := runRepoActions(ctx, f, ioStreams, lister, creds, repo, arts) + if perr != nil { + return perr + } + if exit { + return nil + } + case isAccessDenied(artErr): + // Harbor's artifact API is denied for this credential, but the + // Docker v2 tag list (what `registry tags` uses) usually isn't — + // fall back to it so the tag picker / pull URLs still work. Delete + // stays unavailable here (it goes through the same denied Harbor + // API). If even the v2 list is denied, show just the repo path. + base := repoPullBase(host, payload.Project, &repo) + if tags := tagsViaRegistry(ctx, creds, repo); len(tags) > 0 { + _, _ = fmt.Fprintln(ioStreams.ErrOut, + " Listing tags via the registry API (this credential can't read Harbor image details; sizes/dates omitted).") + exit, perr := runTagPicker(ctx, f, ioStreams, repo.Name, base, tagEntriesFromNames(tags)) + if perr != nil { + return perr + } + if exit { + return nil + } + continue + } + renderRepoPullPath(ioStreams, host, payload.Project, repo) + exit, perr := cmdutil.PromptBackOrExit(ctx, prompter) + if perr != nil { + return perr + } + if exit { + return nil + } + default: + // Transient (e.g. 5xx): surface it and keep the picker open so one + // flaky repo doesn't kick the user out (matches vm list's + // fetch-details error handling). _, _ = fmt.Fprintf(ioStreams.ErrOut, "Error: %v\n", artErr) - continue } - renderRepoArtifacts(ioStreams, repo, arts) } } @@ -258,58 +300,203 @@ func formatRepoRow(r *RepositoryInfo) string { if len(name) > 48 { name = name[:45] + "..." } - return fmt.Sprintf("%-48s %4d artifact(s) %6d pull(s) %s", - name, r.ArtifactCount, r.PullCount, updated) + return fmt.Sprintf("%s %-48s %4d artifact(s) %6d pull(s) %s", + repoGlyph, name, r.ArtifactCount, r.PullCount, updated) } -// renderRepoArtifacts prints the per-repo detail card: header + image -// list. Columns mirror Harbor's UI ("Image list" section): DIGEST / TAGS -// / SIZE / PUSHED / PULLED. PullTime=zero means "never pulled" and is -// rendered as "--" (Harbor shows an em-dash for the same state). -func renderRepoArtifacts(ioStreams cmdutil.IOStreams, repo RepositoryInfo, arts []ArtifactInfo) { - _, _ = fmt.Fprintf(ioStreams.Out, "\n %s\n", repo.Name) - if repo.FullName != "" && repo.FullName != repo.Name { - _, _ = fmt.Fprintf(ioStreams.Out, " (full path: %s)\n", repo.FullName) +// runRepoActions is the per-repo action menu shown after a repo is selected in +// the interactive browser: copy a pull URL (tag picker) or delete image(s). +// The delete path reuses runDeleteImagesInteractive so there is exactly one +// destructive flow (same multi-select + red-warning confirm as +// `verda registry delete`). Returns exit=true only on Ctrl+C; Esc / "Back" +// returns to the repository list. +func runRepoActions(ctx context.Context, f cmdutil.Factory, ioStreams cmdutil.IOStreams, + lister RepositoryLister, creds *options.RegistryCredentials, repo RepositoryInfo, arts []ArtifactInfo) (bool, error) { + const ( + actPull = iota + actDelete + actBack + ) + choices := []string{ + "Get pull URL (pick a tag)", + "Delete image(s)…", + "← Back to repositories", } - _, _ = fmt.Fprintf(ioStreams.Out, " %d artifact(s), %d pull(s)\n\n", - repo.ArtifactCount, repo.PullCount) - if len(arts) == 0 { - _, _ = fmt.Fprintln(ioStreams.Out, " No artifacts.") - return + prompter := f.Prompter() + title := repo.Name + " — choose an action" + for { + idx, err := prompter.Select(ctx, title, choices, tui.WithShowHints(true)) + if err != nil { + if cmdutil.IsPromptInterrupt(err) { + return true, nil // Ctrl+C quits the command + } + return false, nil // Esc → back to the repository list + } + switch idx { + case actPull: + base := repoPullBase(creds.Endpoint, creds.ProjectID, &repo) + exit, perr := runTagPicker(ctx, f, ioStreams, repo.Name, base, buildTagEntries(arts)) + if perr != nil { + return false, perr + } + if exit { + return true, nil + } + case actDelete: + // Same flow `verda registry delete` uses: multi-select + confirm. + if derr := runDeleteImagesInteractive(ctx, f, ioStreams, lister, creds, repo); derr != nil { + return false, derr + } + // Refresh artifacts so a subsequent "Get pull URL" reflects deletes. + if refreshed, rerr := lister.ListArtifacts(ctx, creds.ProjectID, repo.Name); rerr == nil { + arts = refreshed + } + case actBack: + return false, nil + } } +} - _, _ = fmt.Fprintf(ioStreams.Out, " %-22s %-30s %10s %-19s %-19s\n", - "DIGEST", "TAGS", "SIZE", "PUSHED", "PULLED") - _, _ = fmt.Fprintf(ioStreams.Out, " %-22s %-30s %10s %-19s %-19s\n", - "------", "----", "----", "------", "------") +// tagEntry is one row in the drill-down tag picker. suffix is appended to the +// repo's pull base to form the full reference: ":" for tagged artifacts, +// "@" for untagged ones (SBOMs, referrers). +type tagEntry struct { + label string + suffix string +} - for i := range arts { - a := &arts[i] - tags := strings.Join(a.Tags, ", ") - if tags == "" { - tags = untaggedLabel - } - if len(tags) > 30 { - tags = tags[:27] + "..." +// buildTagEntries flattens a repo's artifacts into one picker row per tag, +// newest first (by artifact push time). Untagged artifacts become a single +// " " row that resolves to an @digest reference. +func buildTagEntries(arts []ArtifactInfo) []tagEntry { + sorted := make([]ArtifactInfo, len(arts)) + copy(sorted, arts) + sort.SliceStable(sorted, func(i, j int) bool { + return sorted[i].PushTime.After(sorted[j].PushTime) + }) + + entries := make([]tagEntry, 0, len(sorted)) + for i := range sorted { + a := &sorted[i] + size := formatBytes(a.Size) + age := formatAgo(a.PushTime, nil) + if len(a.Tags) == 0 { + entries = append(entries, tagEntry{ + label: fmt.Sprintf("%-30s %10s %s", untaggedLabel+" "+shortDigest(a.Digest), size, age), + suffix: "@" + a.Digest, + }) + continue } - pushed := "--" - if !a.PushTime.IsZero() { - pushed = a.PushTime.UTC().Format("2006-01-02 15:04:05") + for _, t := range a.Tags { + entries = append(entries, tagEntry{ + label: fmt.Sprintf("%-30s %10s %s", t, size, age), + suffix: ":" + t, + }) } - pulled := "--" - if !a.PullTime.IsZero() { - pulled = a.PullTime.UTC().Format("2006-01-02 15:04:05") + } + return entries +} + +// tagEntriesFromNames builds picker rows from bare tag names — used by the +// access-denied fallback, where only the Docker v2 tag list is available (no +// per-artifact size/push-time, so labels are just the tag). +func tagEntriesFromNames(tags []string) []tagEntry { + entries := make([]tagEntry, 0, len(tags)) + for _, t := range tags { + entries = append(entries, tagEntry{label: t, suffix: ":" + t}) + } + return entries +} + +// runTagPicker drives the repo drill-down over a prebuilt entry list: filter +// (type-to-filter on the Select) and pick a tag, then print its full, +// copy-pasteable pull reference (base + suffix) and quit — getting the URL is +// the goal, so once it's on stdout the user can copy it and move on rather than +// being parked back in the picker. Returns quit=true when a tag was picked or +// Ctrl+C; Esc or the trailing "Back" row returns to the previous menu. +func runTagPicker(ctx context.Context, f cmdutil.Factory, ioStreams cmdutil.IOStreams, + repoName, base string, entries []tagEntry) (bool, error) { + if len(entries) == 0 { + _, _ = fmt.Fprintf(ioStreams.Out, "\n %s\n No tags.\n Pull:\n %s\n\n", repoName, base) + return false, nil + } + + const backLabel = "← Back" + labels := make([]string, 0, len(entries)+1) + for i := range entries { + labels = append(labels, entries[i].label) + } + labels = append(labels, backLabel) + + prompter := f.Prompter() + title := repoName + " — select a tag (type to filter)" + idx, err := prompter.Select(ctx, title, labels, tui.WithShowHints(true)) + if err != nil { + if cmdutil.IsPromptInterrupt(err) { + return true, nil // Ctrl+C quits the whole command } - _, _ = fmt.Fprintf(ioStreams.Out, " %-22s %-30s %10s %-19s %-19s\n", - shortDigest(a.Digest), - tags, - formatBytes(a.Size), - pushed, - pulled, - ) + return false, nil // Esc → back to the previous menu + } + if idx == len(entries) { // "← Back" + return false, nil + } + // The chosen tag's full pull reference, on stdout so it's clean to copy. + // Then quit: the user got what they came for. + _, _ = fmt.Fprintf(ioStreams.Out, " %s%s\n", base, entries[idx].suffix) + return true, nil +} + +// renderRepoPullPath prints just the repository's pull reference, used when the +// credential can list repositories but not their artifacts (Harbor 403 on the +// artifacts endpoint). It surfaces the one actionable thing — the path the user +// can `docker pull` — instead of an error. The explanatory note goes to ErrOut +// so the reference on stdout stays clean to copy. +func renderRepoPullPath(ioStreams cmdutil.IOStreams, host, project string, repo RepositoryInfo) { + _, _ = fmt.Fprintf(ioStreams.Out, "\n %s\n", repo.Name) + _, _ = fmt.Fprintln(ioStreams.ErrOut, + " Tag details need a credential with pull permission. You can still pull the repository:") + _, _ = fmt.Fprintf(ioStreams.Out, " Pull:\n %s\n\n", repoPullBase(host, project, &repo)) +} + +// repoPullBase builds the registry pull reference for a repository: +// "//". It prefers the Harbor-supplied FullName +// (already "/") and falls back to project + Name. Host defaults +// to the production endpoint for legacy creds that never stored one. Append +// ":" or "@" for a specific artifact. +func repoPullBase(host, project string, repo *RepositoryInfo) string { + full := repo.FullName + if full == "" { + full = project + "/" + repo.Name + } + if host == "" { + host = defaultRegistryEndpoint + } + return host + "/" + full +} + +// tagsViaRegistry falls back to the Docker Registry v2 tag list (the API +// `registry tags` uses) when Harbor's artifact API is denied. Many credentials +// can pull/list over v2 even without the Harbor project permission the +// artifacts endpoint requires. Returns nil on any error so the caller degrades +// to showing just the repository path. +func tagsViaRegistry(ctx context.Context, creds *options.RegistryCredentials, repo RepositoryInfo) []string { + full := repo.FullName + if full == "" { + full = creds.ProjectID + "/" + repo.Name } - _, _ = fmt.Fprintln(ioStreams.Out) + tags, err := buildClient(creds, RetryConfig{}).Tags(ctx, full) + if err != nil { + return nil + } + return tags +} + +// isAccessDenied reports whether err is a translated registry_access_denied +// AgentError (Harbor 403) — a permission state, not a transient failure. +func isAccessDenied(err error) bool { + var ae *cmdutil.AgentError + return errors.As(err, &ae) && ae.Code == kindRegistryAccessDenied } // shortDigest returns the first 19 chars of a digest ("sha256:abcdef012") diff --git a/internal/verda-cli/cmd/registry/ls_test.go b/internal/verda-cli/cmd/registry/ls_test.go index a42df4c..026011e 100644 --- a/internal/verda-cli/cmd/registry/ls_test.go +++ b/internal/verda-cli/cmd/registry/ls_test.go @@ -26,9 +26,21 @@ import ( "go.yaml.in/yaml/v3" cmdutil "github.com/verda-cloud/verda-cli/internal/verda-cli/cmd/util" + "github.com/verda-cloud/verda-cli/internal/verda-cli/options" tuitest "github.com/verda-cloud/verdagostack/pkg/tui/testing" ) +// withFakeRegistryTags swaps clientBuilder so the ggcr Registry's Tags returns +// the given function — used to drive the access-denied v2-tag fallback in ls. +func withFakeRegistryTags(t *testing.T, tagsFn func(ctx context.Context, repo string) ([]string, error)) { + t.Helper() + orig := clientBuilder + clientBuilder = func(_ *options.RegistryCredentials, _ RetryConfig) Registry { + return &tagsOnlyRegistry{tagsFn: tagsFn} + } + t.Cleanup(func() { clientBuilder = orig }) +} + // runLsForTest exercises the real flag-parsing path so tests match // production argv behavior. func runLsForTest(t *testing.T, f cmdutil.Factory, streams cmdutil.IOStreams, args ...string) error { @@ -297,9 +309,9 @@ func TestLs_Interactive_DrillsIntoArtifacts(t *testing.T) { withFakeHarborLister(t, lister) withForcedTTY(t, true) - // Two selects: pick the first repo (library/hello-world after sort), - // then pick Exit (index 2 == len(repos)). - mock := tuitest.New().AddSelect(0).AddSelect(2) + // repo 0 -> action "Get pull URL" (0) -> tag "latest" (0): prints the URL + // and exits (picking a tag is the terminal action). + mock := tuitest.New().AddSelect(0).AddSelect(0).AddSelect(0) f := cmdutil.NewTestFactory(mock) streams, out, errOut := newLsStreams() @@ -311,18 +323,9 @@ func TestLs_Interactive_DrillsIntoArtifacts(t *testing.T) { lister.gotArtifactRepo, "library/hello-world") } - got := out.String() - for _, want := range []string{ - "library/hello-world", - "DIGEST", "TAGS", "SIZE", "PUSHED", "PULLED", - "sha256:d1a8d0a4eeb6", - "latest", - "3.92 KiB", - "2026-04-22 13:40:05", - } { - if !strings.Contains(got, want) { - t.Errorf("drill-down output missing %q:\n%s", want, got) - } + // Selecting the "latest" tag prints its full, copy-pasteable pull ref. + if got := out.String(); !strings.Contains(got, "vccr.io/abc/library/hello-world:latest") { + t.Errorf("expected the tag's pull reference on stdout, got:\n%s", got) } // The header summary ("N repository(ies) in project X") goes to @@ -333,9 +336,8 @@ func TestLs_Interactive_DrillsIntoArtifacts(t *testing.T) { } } -// TestLs_Interactive_UntaggedArtifact verifies the renderer surfaces -// for an artifact with an empty tag list (common for -// dangling referrer manifests and SBOM artifacts). +// TestLs_Interactive_UntaggedArtifact verifies an artifact with no tags is +// pickable as an "" row that resolves to an @digest pull reference. func TestLs_Interactive_UntaggedArtifact(t *testing.T) { writeLsCredsFile(t, validLsCredsBody("vccr.io", "abc")) lister := &fakeLister{ @@ -350,15 +352,18 @@ func TestLs_Interactive_UntaggedArtifact(t *testing.T) { withFakeHarborLister(t, lister) withForcedTTY(t, true) - mock := tuitest.New().AddSelect(0).AddSelect(1) // pick repo then Exit. + // repo 0 -> action "Get pull URL" (0) -> untagged row (0): prints the + // @digest reference and exits. + mock := tuitest.New().AddSelect(0).AddSelect(0).AddSelect(0) f := cmdutil.NewTestFactory(mock) streams, out, _ := newLsStreams() if err := runLsForTest(t, f, streams); err != nil { t.Fatalf("ls: %v", err) } - if got := out.String(); !strings.Contains(got, "") { - t.Errorf("expected marker, got:\n%s", got) + // Untagged artifacts resolve to an @digest reference. + if got := out.String(); !strings.Contains(got, "vccr.io/abc/library/hello-world@sha256:deadbeef") { + t.Errorf("expected @digest pull reference, got:\n%s", got) } } @@ -391,6 +396,119 @@ func TestLs_Interactive_ArtifactsErrorStaysInLoop(t *testing.T) { } } +// TestLs_Interactive_ArtifactsAccessDeniedShowsPullPath: when the credential +// can list repositories but not view artifacts (Harbor 403), selecting a repo +// shows its copy-pasteable pull reference (not the tag list, which is denied, +// and not a confusing full-table dump) and keeps the picker open. +func TestLs_Interactive_ArtifactsAccessDeniedShowsPullPath(t *testing.T) { + writeLsCredsFile(t, validLsCredsBody("vccr.io", "abc")) + lister := &fakeLister{ + repos: sampleRepos(), + artifactsErr: &cmdutil.AgentError{ + Code: kindRegistryAccessDenied, + Message: "denied", + }, + } + withFakeHarborLister(t, lister) + // v2 tag list is ALSO denied here → degrade to just the repo pull path. + withFakeRegistryTags(t, func(_ context.Context, _ string) ([]string, error) { + return nil, errors.New("denied") + }) + withForcedTTY(t, true) + + // Pick repo 0 -> denied -> pull path + Back/Exit gate -> choose Exit (idx 1). + mock := tuitest.New().AddSelect(0).AddSelect(1) + f := cmdutil.NewTestFactory(mock) + streams, out, errOut := newLsStreams() + + if err := runLsForTest(t, f, streams); err != nil { + t.Fatalf("ls: %v", err) + } + if !strings.Contains(errOut.String(), "pull permission") { + t.Errorf("expected the tags-unavailable note on ErrOut, got:\n%s", errOut.String()) + } + // The copy-pasteable repository pull reference is on stdout. + if !strings.Contains(out.String(), "vccr.io/abc/library/hello-world") { + t.Errorf("expected the repository pull path on stdout, got:\n%s", out.String()) + } + // No artifact table (that's what was denied) and no full repo-list dump. + if strings.Contains(out.String(), "DIGEST") || strings.Contains(out.String(), "REPOSITORY") { + t.Errorf("should not have printed an artifact card or the repo table, got:\n%s", out.String()) + } +} + +// TestLs_Interactive_ArtifactsAccessDenied_FallsBackToV2Tags: when Harbor's +// artifact API is denied but the Docker v2 tag list works, the drill-down still +// offers the tag picker → pull URL via the v2 fallback. +func TestLs_Interactive_ArtifactsAccessDenied_FallsBackToV2Tags(t *testing.T) { + writeLsCredsFile(t, validLsCredsBody("vccr.io", "abc")) + lister := &fakeLister{ + repos: sampleRepos(), + artifactsErr: &cmdutil.AgentError{ + Code: kindRegistryAccessDenied, + Message: "denied", + }, + } + withFakeHarborLister(t, lister) + withFakeRegistryTags(t, func(_ context.Context, _ string) ([]string, error) { + return []string{"v1", "v2"}, nil + }) + withForcedTTY(t, true) + + // repo 0 -> v2 fallback tag picker -> pick "v1" (0): prints URL and exits. + mock := tuitest.New().AddSelect(0).AddSelect(0) + f := cmdutil.NewTestFactory(mock) + streams, out, errOut := newLsStreams() + + if err := runLsForTest(t, f, streams); err != nil { + t.Fatalf("ls: %v", err) + } + if !strings.Contains(errOut.String(), "registry API") { + t.Errorf("expected the v2-fallback note on ErrOut, got:\n%s", errOut.String()) + } + if !strings.Contains(out.String(), "vccr.io/abc/library/hello-world:v1") { + t.Errorf("expected the v1 pull reference on stdout, got:\n%s", out.String()) + } +} + +// TestLs_Interactive_DeleteImagesFromActionMenu verifies the repo action menu +// can delete images: picking "Delete image(s)" runs the same multi-select + +// confirm flow as `verda registry delete`, recording the deletion. +func TestLs_Interactive_DeleteImagesFromActionMenu(t *testing.T) { + writeLsCredsFile(t, validLsCredsBody("vccr.io", "abc")) + lister := &fakeLister{ + repos: sampleRepos(), + artifactsByRepo: map[string][]ArtifactInfo{ + "library/hello-world": sampleArtifacts(), + }, + } + withFakeHarborLister(t, lister) + withForcedTTY(t, true) + + // repo 0 -> action "Delete image(s)" (1) -> multi-select artifact 0 -> + // confirm -> action menu "Back" (2) -> repo "Exit" (2). + mock := tuitest.New(). + AddSelect(0). + AddSelect(1). + AddMultiSelect([]int{0}). + AddConfirm(true). + AddSelect(2). + AddSelect(2) + f := cmdutil.NewTestFactory(mock) + streams, _, _ := newLsStreams() + + if err := runLsForTest(t, f, streams); err != nil { + t.Fatalf("ls: %v", err) + } + if len(lister.deletedArtifacts) != 1 { + t.Fatalf("expected 1 deleted artifact, got %d (%+v)", + len(lister.deletedArtifacts), lister.deletedArtifacts) + } + if lister.deletedArtifacts[0].Repo != "library/hello-world" { + t.Errorf("deleted from repo %q, want library/hello-world", lister.deletedArtifacts[0].Repo) + } +} + // TestLs_Interactive_EmptyRepos short-circuits: an empty project prints // the friendly message and never enters the picker loop. func TestLs_Interactive_EmptyRepos(t *testing.T) { diff --git a/internal/verda-cli/cmd/registry/push.go b/internal/verda-cli/cmd/registry/push.go index 0de0f98..4825271 100644 --- a/internal/verda-cli/cmd/registry/push.go +++ b/internal/verda-cli/cmd/registry/push.go @@ -19,11 +19,14 @@ import ( "errors" "fmt" "io" + "os" + "path/filepath" "strings" "time" tea "charm.land/bubbletea/v2" "github.com/spf13/cobra" + "github.com/verda-cloud/verdagostack/pkg/tui" v1 "github.com/google/go-containerregistry/pkg/v1" @@ -85,7 +88,6 @@ type pushPayload struct { // worker pool but has no effect yet. func NewCmdPush(f cmdutil.Factory, ioStreams cmdutil.IOStreams) *cobra.Command { opts := &pushOptions{ - Profile: defaultProfileName, Source: string(SourceAuto), ImageJobs: 1, Retries: 5, @@ -140,7 +142,7 @@ func NewCmdPush(f cmdutil.Factory, ioStreams cmdutil.IOStreams) *cobra.Command { } flags := cmd.Flags() - flags.StringVar(&opts.Profile, "profile", opts.Profile, "Credentials profile name") + flags.StringVar(&opts.Profile, "profile", opts.Profile, "Credentials profile name (default: active profile)") flags.StringVar(&opts.CredentialsFile, "credentials-file", "", "Path to credentials file") flags.StringVar(&opts.Repo, "repo", "", "Override destination repository name (single-image only)") flags.StringVar(&opts.Tag, "tag", "", "Override destination tag (single-image only)") @@ -167,7 +169,7 @@ func runPush(cmd *cobra.Command, f cmdutil.Factory, ioStreams cmdutil.IOStreams, } if len(args) == 0 { - picked, proceed, err := resolveZeroArgs(cmd.Context(), f, ioStreams, creds) + picked, proceed, err := resolveZeroArgs(cmd.Context(), f, ioStreams, creds, opts) if err != nil { return err } @@ -591,6 +593,7 @@ func resolveZeroArgs( f cmdutil.Factory, ioStreams cmdutil.IOStreams, creds *options.RegistryCredentials, + opts *pushOptions, ) (refs []string, proceed bool, err error) { if !isInteractivePush(f, ioStreams.ErrOut) { return nil, false, &cmdutil.AgentError{ @@ -602,7 +605,7 @@ func resolveZeroArgs( ExitCode: cmdutil.ExitBadArgs, } } - return listAndPickImages(ctx, f, ioStreams, creds) + return listAndPickImages(ctx, f, ioStreams, creds, opts) } // listAndPickImages is the zero-arg interactive flow. It pings the @@ -619,16 +622,18 @@ func listAndPickImages( f cmdutil.Factory, ioStreams cmdutil.IOStreams, creds *options.RegistryCredentials, + opts *pushOptions, ) (refs []string, proceed bool, err error) { lister, lerr := daemonListerBuilder() if lerr != nil { - return nil, false, interactiveNoImageSourceError(lerr) + // No daemon to enumerate — offer a file source instead of dead-ending. + return pushFileSourceFallback(ctx, f, ioStreams, opts) } pingCtx, pingCancel := context.WithTimeout(ctx, daemonPingTimeout) defer pingCancel() if perr := lister.Ping(pingCtx); perr != nil { - return nil, false, interactiveNoImageSourceError(perr) + return pushFileSourceFallback(ctx, f, ioStreams, opts) } listCtx, listCancel := context.WithTimeout(ctx, f.Options().Timeout) @@ -695,22 +700,142 @@ func runPickerTUI( return refs, true } -// interactiveNoImageSourceError is the zero-arg picker counterpart to -// daemonUnreachableError. The message lives here rather than in -// source.go so the friendly "pass image refs" hint lines up with the -// zero-arg flow (where --source isn't the only fix available). -func interactiveNoImageSourceError(underlying error) error { - return &cmdutil.AgentError{ - Code: kindRegistryNoImageSource, - Message: fmt.Sprintf( - "Docker daemon not reachable (%s); "+ - "pass image references as arguments (e.g. `verda registry push my-app:v1`) "+ - "or use --source oci|tar with a path", - underlying.Error(), - ), - Details: map[string]any{ - "daemon_error": underlying.Error(), - }, - ExitCode: cmdutil.ExitAPI, +// pushFileSourceFallback runs when a zero-arg interactive push can't reach the +// Docker daemon. Instead of dead-ending, it guides the user to push from a +// local OCI layout or image tarball: pick the source type, the path, and a +// destination repo/tag (a path can't carry one, so the repo is pre-filled from +// the path's basename and editable). It sets opts.Source/Repo/Tag and returns +// the path as the single "ref" the normal push pipeline consumes. A cancel at +// any prompt returns (nil, false, nil) → the caller exits 0 quietly. +// +//nolint:gocyclo // Interactive step machine with per-step back/exit navigation — inherently complex. +func pushFileSourceFallback(ctx context.Context, f cmdutil.Factory, ioStreams cmdutil.IOStreams, opts *pushOptions) (refs []string, proceed bool, err error) { + prompter := f.Prompter() + + const ( + stepSource = iota + stepPath + stepRepo + stepTag + ) + var ( + srcType ImageSource + label string + path string + repo string + ) + step := stepSource + for { + switch step { + case stepSource: + idx, serr := prompter.Select(ctx, + "Docker daemon not reachable — push from a file instead", + []string{"OCI layout (directory)", "Image tarball (.tar file)", "Cancel"}, + tui.WithShowHints(true)) + if serr != nil { + // First step: Esc and Ctrl+C both exit cleanly (nothing to go + // back to). A real I/O error propagates. + if cmdutil.IsPromptCancel(serr) { + return nil, false, nil + } + return nil, false, serr + } + switch idx { + case 0: + srcType, label = SourceOCI, "OCI layout directory" + case 1: + srcType, label = SourceTar, "image tarball (.tar)" + default: + return nil, false, nil // Cancel + } + step = stepPath + + case stepPath: + p, perr := prompter.TextInput(ctx, "Path to the "+label) + if back, exit, fatal := classifyWizardNav(perr, false); fatal != nil { + return nil, false, fatal + } else if exit { + return nil, false, nil + } else if back { + step = stepSource + continue + } + p = strings.TrimSpace(p) + if p == "" { + step = stepSource // empty → back to the source choice + continue + } + if _, statErr := os.Stat(p); statErr != nil { + _, _ = fmt.Fprintf(ioStreams.ErrOut, " %v — try again.\n", statErr) + continue // re-prompt the path + } + path = p + step = stepRepo + + case stepRepo: + r, rerr := prompter.TextInput(ctx, "Destination repository (under your VCR project)", + tui.WithDefault(defaultRepoFromPath(path))) + if back, exit, fatal := classifyWizardNav(rerr, false); fatal != nil { + return nil, false, fatal + } else if exit { + return nil, false, nil + } else if back { + step = stepPath + continue + } + r = strings.TrimSpace(r) + if r == "" { + r = defaultRepoFromPath(path) + } + if r == "" { + _, _ = fmt.Fprintln(ioStreams.ErrOut, " repository cannot be empty — try again.") + continue + } + if r == "." || r == ".." { + // These would reach ggcr's repository parser and fail with a + // cryptic "can only contain …" error; reject up front. + _, _ = fmt.Fprintln(ioStreams.ErrOut, " repository name cannot be \".\" or \"..\" — try again.") + continue + } + repo = r + step = stepTag + + case stepTag: + tg, terr := prompter.TextInput(ctx, "Tag", tui.WithDefault(defaultTag)) + if back, exit, fatal := classifyWizardNav(terr, false); fatal != nil { + return nil, false, fatal + } else if exit { + return nil, false, nil + } else if back { + step = stepRepo + continue + } + tg = strings.TrimSpace(tg) + if tg == "" { + tg = defaultTag + } + opts.Source = string(srcType) + opts.Repo = repo + opts.Tag = tg + return []string{path}, true, nil + } + } +} + +// defaultRepoFromPath derives a sensible destination repository from a source +// path: the basename with a known image-archive extension stripped +// (./build/my-app.tar → "my-app", ./layout/ → "layout"). +func defaultRepoFromPath(p string) string { + base := filepath.Base(strings.TrimRight(p, "/")) + // "." (from "." or "/") and ".." (from "../") aren't usable repo names — + // return "" so the prompt re-asks rather than pre-filling garbage. + if base == "." || base == ".." { + return "" + } + for _, ext := range []string{".tar.gz", ".tgz", ".tar", ".oci"} { + if strings.HasSuffix(base, ext) { + return strings.TrimSuffix(base, ext) + } } + return base } diff --git a/internal/verda-cli/cmd/registry/push_test.go b/internal/verda-cli/cmd/registry/push_test.go index 6679496..371947e 100644 --- a/internal/verda-cli/cmd/registry/push_test.go +++ b/internal/verda-cli/cmd/registry/push_test.go @@ -21,6 +21,8 @@ import ( "fmt" "io" "net/http" + "os" + "path/filepath" "strings" "testing" @@ -30,6 +32,7 @@ import ( cmdutil "github.com/verda-cloud/verda-cli/internal/verda-cli/cmd/util" "github.com/verda-cloud/verda-cli/internal/verda-cli/options" + tuitest "github.com/verda-cloud/verdagostack/pkg/tui/testing" ) // ---------- push test helpers ---------- @@ -632,8 +635,9 @@ func TestPush_InteractiveMode_NonTTYErrors(t *testing.T) { } } -// TestPush_InteractiveMode_NoDaemon: zero args + TTY + daemon ping -// fails → registry_no_image_source error. Picker never invoked. +// TestPush_InteractiveMode_NoDaemon: zero args + TTY + daemon ping fails → +// instead of erroring, push offers the file-source fallback. Canceling it exits +// cleanly (no error, nothing pushed) and the daemon image picker never runs. func TestPush_InteractiveMode_NoDaemon(t *testing.T) { writeLsCredsFile(t, healthyPushCredsBody("vccr.io", "proj")) @@ -649,22 +653,12 @@ func TestPush_InteractiveMode_NoDaemon(t *testing.T) { return nil, false }) - f := cmdutil.NewTestFactory(nil) + // Fallback source picker → "Cancel" (index 2): clean exit, nothing pushed. + f := cmdutil.NewTestFactory(tuitest.New().AddSelect(2)) streams, _, _ := newLsStreams() - err := runPushForTest(t, f, streams) - if err == nil { - t.Fatal("expected registry_no_image_source error when daemon unreachable") - } - var ae *cmdutil.AgentError - if !errors.As(err, &ae) { - t.Fatalf("expected *AgentError, got %T (%v)", err, err) - } - if ae.Code != kindRegistryNoImageSource { - t.Errorf("Code = %q, want %q", ae.Code, kindRegistryNoImageSource) - } - if !strings.Contains(ae.Message, "Docker daemon not reachable") { - t.Errorf("expected friendly daemon message, got: %v", ae.Message) + if err := runPushForTest(t, f, streams); err != nil { + t.Fatalf("canceling the file-source fallback should exit cleanly, got: %v", err) } if daemon.pingHits == 0 { t.Error("expected Ping to be attempted") @@ -673,7 +667,85 @@ func TestPush_InteractiveMode_NoDaemon(t *testing.T) { t.Errorf("List should NOT be called when Ping fails, got %d call(s)", daemon.listHits) } if pickerHits != 0 { - t.Errorf("picker should NOT be invoked when daemon unreachable, got %d call(s)", pickerHits) + t.Errorf("daemon image picker should NOT run when daemon unreachable, got %d call(s)", pickerHits) + } + if len(fake.loadCalls) != 0 { + t.Errorf("nothing should be loaded/pushed on cancel, got %+v", fake.loadCalls) + } +} + +// TestPushFileSourceFallback_Tarball drives the no-daemon fallback to +// completion: pick "tarball", give an existing path, accept the derived repo, +// type a tag. It sets opts.Source/Repo/Tag and returns the path as the ref. +func TestPushFileSourceFallback_Tarball(t *testing.T) { + dir := t.TempDir() + tarPath := filepath.Join(dir, "my-app.tar") + if err := os.WriteFile(tarPath, []byte("x"), 0o600); err != nil { + t.Fatal(err) + } + + mock := tuitest.New(). + AddSelect(1). // Image tarball + AddTextInput(tarPath). // path + AddTextInput(""). // repo: accept the derived default + AddTextInput("v9") // tag + f := cmdutil.NewTestFactory(mock) + streams, _, _ := newLsStreams() + opts := &pushOptions{Source: string(SourceAuto)} + + refs, proceed, err := pushFileSourceFallback(context.Background(), f, streams, opts) + if err != nil || !proceed { + t.Fatalf("fallback = (%v, %v, %v), want (refs, true, nil)", refs, proceed, err) + } + if len(refs) != 1 || refs[0] != tarPath { + t.Errorf("refs = %v, want [%s]", refs, tarPath) + } + if opts.Source != string(SourceTar) { + t.Errorf("opts.Source = %q, want %q", opts.Source, SourceTar) + } + if opts.Repo != "my-app" { + t.Errorf("opts.Repo = %q, want my-app (derived from basename)", opts.Repo) + } + if opts.Tag != "v9" { + t.Errorf("opts.Tag = %q, want v9", opts.Tag) + } +} + +// TestDefaultRepoFromPath: basename with archive extensions stripped, and the +// "." / ".." sentinels (from ".", "/", "../") map to "" so the repo prompt +// re-asks instead of pre-filling an unusable name. +func TestDefaultRepoFromPath(t *testing.T) { + t.Parallel() + cases := map[string]string{ + "./build/my-app.tar": "my-app", + "my-app.tar.gz": "my-app", + "layout/": "layout", + "./my-app": "my-app", + ".": "", + "/": "", + "../": "", + "..": "", + } + for in, want := range cases { + if got := defaultRepoFromPath(in); got != want { + t.Errorf("defaultRepoFromPath(%q) = %q, want %q", in, got, want) + } + } +} + +// TestPushFileSourceFallback_Cancel: choosing "Cancel" returns (nil, false, +// nil) and leaves opts untouched. +func TestPushFileSourceFallback_Cancel(t *testing.T) { + f := cmdutil.NewTestFactory(tuitest.New().AddSelect(2)) // Cancel + streams, _, _ := newLsStreams() + opts := &pushOptions{Source: string(SourceAuto)} + + refs, proceed, err := pushFileSourceFallback(context.Background(), f, streams, opts) + if refs != nil || proceed || err != nil { + t.Fatalf("fallback = (%v, %v, %v), want (nil, false, nil)", refs, proceed, err) + } + if opts.Source != string(SourceAuto) { + t.Errorf("opts.Source = %q, want unchanged (auto)", opts.Source) } } diff --git a/internal/verda-cli/cmd/registry/registry.go b/internal/verda-cli/cmd/registry/registry.go index 5ff4085..1592d5e 100644 --- a/internal/verda-cli/cmd/registry/registry.go +++ b/internal/verda-cli/cmd/registry/registry.go @@ -23,10 +23,10 @@ import ( // NewCmdRegistry creates the parent `verda registry` command. func NewCmdRegistry(f cmdutil.Factory, ioStreams cmdutil.IOStreams) *cobra.Command { cmd := &cobra.Command{ - Use: "registry", - Aliases: []string{"vccr", "vcr"}, - Short: "Manage the Verda Cloud Container Registry (vccr.io)", - Hidden: true, // pre-GA; also gated in cmd/cmd.go via VERDA_REGISTRY_ENABLED + Use: "registry", + Aliases: []string{"vccr", "vcr"}, + Short: "Manage the Verda Cloud Container Registry (vccr.io)", + Annotations: map[string]string{cmdutil.TagAnnotation: "beta"}, Long: cmdutil.LongDesc(` Manage Verda Container Registry (vccr.io) credentials, browse repositories, push local Docker images, and copy images between diff --git a/internal/verda-cli/cmd/registry/registry_test.go b/internal/verda-cli/cmd/registry/registry_test.go index 2adadb3..d3c89cf 100644 --- a/internal/verda-cli/cmd/registry/registry_test.go +++ b/internal/verda-cli/cmd/registry/registry_test.go @@ -30,8 +30,11 @@ func TestNewCmdRegistry_BasicWiring(t *testing.T) { if cmd.Use != "registry" { t.Fatalf("Use = %q, want %q", cmd.Use, "registry") } - if !cmd.Hidden { - t.Errorf("command should be Hidden pre-GA") + if cmd.Hidden { + t.Errorf("command should be visible in help (beta, no longer pre-GA gated)") + } + if got := cmd.Annotations[cmdutil.TagAnnotation]; got != "beta" { + t.Errorf("TagAnnotation = %q, want %q (renders as \"registry (beta)\" in help)", got, "beta") } if !sliceContains(cmd.Aliases, "vccr") { t.Errorf("alias \"vccr\" missing; got %v", cmd.Aliases) diff --git a/internal/verda-cli/cmd/registry/show.go b/internal/verda-cli/cmd/registry/show.go index 4d28c41..ce4c721 100644 --- a/internal/verda-cli/cmd/registry/show.go +++ b/internal/verda-cli/cmd/registry/show.go @@ -67,7 +67,7 @@ func NewCmdShow(f cmdutil.Factory, ioStreams cmdutil.IOStreams) *cobra.Command { } flags := cmd.Flags() - flags.StringVar(&profile, "profile", defaultProfileName, "Credentials profile to show") + flags.StringVar(&profile, "profile", "", "Credentials profile to show (default: active profile)") flags.StringVar(&credentialsFile, "credentials-file", "", "Path to the shared credentials file") return cmd @@ -76,11 +76,10 @@ func NewCmdShow(f cmdutil.Factory, ioStreams cmdutil.IOStreams) *cobra.Command { // runShow is the RunE body, split out for testability. func runShow(f cmdutil.Factory, ioStreams cmdutil.IOStreams, profile, credentialsFile string) error { // Registry commands are in skipCredentialResolution, so AuthOptions.Profile - // is never resolved. Mirror the s3 convention and fall back to the default - // profile when --profile was explicitly set to an empty string. - if profile == "" { - profile = defaultProfileName - } + // is never resolved. Resolve the active profile here (explicit --profile > + // VERDA_PROFILE / auth.profile > "default") so `show` reports the same + // profile the other registry commands act on. + profile = resolveProfile(profile) path := credentialsFilePath(credentialsFile) outputFormat := f.OutputFormat() diff --git a/internal/verda-cli/cmd/registry/tags.go b/internal/verda-cli/cmd/registry/tags.go index d6edbbf..2732b18 100644 --- a/internal/verda-cli/cmd/registry/tags.go +++ b/internal/verda-cli/cmd/registry/tags.go @@ -71,8 +71,7 @@ type tagsPayload struct { // --limit 0; the two flags together are redundant but not an error. func NewCmdTags(f cmdutil.Factory, ioStreams cmdutil.IOStreams) *cobra.Command { opts := &tagsOptions{ - Profile: defaultProfileName, - Limit: defaultTagsLimit, + Limit: defaultTagsLimit, } cmd := &cobra.Command{ @@ -80,7 +79,11 @@ func NewCmdTags(f cmdutil.Factory, ioStreams cmdutil.IOStreams) *cobra.Command { Short: "List tags in a container repository", Long: cmdutil.LongDesc(` List every tag in the given repository plus per-tag metadata - (digest, size). + (digest, size). On a terminal, tags are shown as an interactive, + filterable picker — select one to print its full pull reference and + exit (the same picker "verda registry ls" uses when you drill into a + repository). Piped output, -o json/yaml, or --agent print the table + instead. The repository may be passed as a full reference ("vccr.io//") or a short reference (""). Short @@ -116,14 +119,24 @@ func NewCmdTags(f cmdutil.Factory, ioStreams cmdutil.IOStreams) *cobra.Command { # JSON output for scripts verda registry tags my-app -o json `), - Args: cobra.ExactArgs(1), + // MaximumNArgs (not ExactArgs) so a bare `tags` returns a helpful + // pointer to `ls` / `tags ` instead of cobra's terse + // "accepts 1 arg(s), received 0". `tags` is tag-centric (one repo); + // browsing repositories interactively is `ls`'s job. + Args: cobra.MaximumNArgs(1), RunE: func(cmd *cobra.Command, args []string) error { + if len(args) == 0 { + return cmdutil.UsageErrorf(cmd, + "tags needs a repository.\n"+ + " verda registry tags list that repository's tags\n"+ + " verda registry ls browse repositories interactively, then tags") + } return runTags(cmd, f, ioStreams, opts, args[0]) }, } flags := cmd.Flags() - flags.StringVar(&opts.Profile, "profile", opts.Profile, "Credentials profile to read") + flags.StringVar(&opts.Profile, "profile", opts.Profile, "Credentials profile to read (default: active profile)") flags.StringVar(&opts.CredentialsFile, "credentials-file", "", "Path to the shared Verda credentials file") flags.IntVar(&opts.Limit, "limit", opts.Limit, "Cap on per-tag metadata lookups (0 = unlimited)") flags.BoolVar(&opts.All, "all", false, "Run metadata lookups for every tag (overrides --limit)") @@ -214,10 +227,41 @@ func runTags(cmd *cobra.Command, f cmdutil.Factory, ioStreams cmdutil.IOStreams, return werr } } + + // Interactive terminal: present the same filterable tag picker `ls`'s + // drill-down uses; selecting a tag prints its full pull reference. Piped / + // redirected / -o json / --agent fall through to the plain table so scripts + // stay deterministic. The picker uses cmd.Context() (not the per-request + // timeout ctx) so user think-time can't cancel it mid-prompt. + if !f.AgentMode() && isTerminalFn(ioStreams.Out) { + base := repoPullBase(creds.Endpoint, creds.ProjectID, &RepositoryInfo{FullName: fullRepo}) + _, perr := runTagPicker(cmd.Context(), f, ioStreams, ref.Repository, base, tagRowsToEntries(rows, metadataCap)) + return perr + } + renderTagsHuman(ioStreams, payload, metadataCap) return nil } +// tagRowsToEntries turns fetched tag rows into picker entries: a "tag size" +// label (size renders "--" for rows past the metadata cap) and a ":" +// suffix appended to the repo's pull base. Used by the interactive `tags` +// picker, reusing ls.go's runTagPicker. +func tagRowsToEntries(rows []tagRow, metadataCap int) []tagEntry { + entries := make([]tagEntry, 0, len(rows)) + for i := range rows { + size := "--" + if i < metadataCap { + size = formatBytes(rows[i].Size) + } + entries = append(entries, tagEntry{ + label: fmt.Sprintf("%-30s %10s", rows[i].Tag, size), + suffix: ":" + rows[i].Tag, + }) + } + return entries +} + // startTagsSpinner starts the listing spinner (if the factory exposes a // status impl). Returns nil when Status is unavailable — callers must handle // a nil spinner. diff --git a/internal/verda-cli/cmd/registry/tags_test.go b/internal/verda-cli/cmd/registry/tags_test.go index f2fbbeb..b76da7d 100644 --- a/internal/verda-cli/cmd/registry/tags_test.go +++ b/internal/verda-cli/cmd/registry/tags_test.go @@ -26,6 +26,7 @@ import ( cmdutil "github.com/verda-cloud/verda-cli/internal/verda-cli/cmd/util" "github.com/verda-cloud/verda-cli/internal/verda-cli/options" + tuitest "github.com/verda-cloud/verdagostack/pkg/tui/testing" ) // runTagsForTest exercises the real flag-parsing path so tests match @@ -411,6 +412,58 @@ func (r *tagsOnlyRegistry) Head(ctx context.Context, ref string) (*v1.Descriptor return nil, errors.New("head not implemented") } +// TestTags_Interactive_PicksTagPrintsPullRef: on a terminal, `tags ` +// presents the filterable tag picker (reusing ls's runTagPicker); selecting a +// tag prints its full pull reference and exits. Non-TTY callers (the other +// tags tests) still get the table. +func TestTags_Interactive_PicksTagPrintsPullRef(t *testing.T) { + fake := &tagsOnlyRegistry{ + tagsFn: func(_ context.Context, _ string) ([]string, error) { + return []string{"v1", "v2"}, nil + }, + headFn: func(_ context.Context, _ string) (*v1.Descriptor, error) { + return &v1.Descriptor{Size: 1024}, nil + }, + } + orig := clientBuilder + clientBuilder = func(_ *options.RegistryCredentials, _ RetryConfig) Registry { return fake } + t.Cleanup(func() { clientBuilder = orig }) + + writeLsCredsFile(t, healthyVCRCredsBody("vccr.io", "proj")) + withForcedTTY(t, true) + + // Pick the first tag (v1) → prints its pull reference and exits. + f := cmdutil.NewTestFactory(tuitest.New().AddSelect(0)) + streams, out, _ := newLsStreams() + + if err := runTagsForTest(t, f, streams, "my-app"); err != nil { + t.Fatalf("tags (interactive): %v", err) + } + if got := out.String(); !strings.Contains(got, "vccr.io/proj/my-app:v1") { + t.Errorf("expected pull reference on stdout, got:\n%s", got) + } + // The picker is used, not the table. + if strings.Contains(out.String(), "DIGEST") { + t.Errorf("expected the picker (no table), got:\n%s", out.String()) + } +} + +// TestTags_NoArg_FriendlyError: bare `tags` (no repo) returns a helpful +// message pointing at `tags ` / `ls`, not cobra's terse arg error. +func TestTags_NoArg_FriendlyError(t *testing.T) { + writeLsCredsFile(t, healthyVCRCredsBody("vccr.io", "proj")) + f := cmdutil.NewTestFactory(nil) + streams, _, _ := newLsStreams() + + err := runTagsForTest(t, f, streams /* no repo */) + if err == nil { + t.Fatal("expected an error for bare `tags`") + } + if !strings.Contains(err.Error(), "needs a repository") { + t.Errorf("expected a friendly 'needs a repository' message, got: %v", err) + } +} + // TestTags_NetworkError: Tags returns a connection-refused error; the // command surfaces it as registry_unreachable. func TestTags_NetworkError(t *testing.T) { diff --git a/internal/verda-cli/cmd/registry/wizard.go b/internal/verda-cli/cmd/registry/wizard.go index ad09f53..3ea1d29 100644 --- a/internal/verda-cli/cmd/registry/wizard.go +++ b/internal/verda-cli/cmd/registry/wizard.go @@ -15,27 +15,49 @@ package registry import ( + "context" "errors" "fmt" "strconv" "strings" + "github.com/verda-cloud/verdagostack/pkg/tui" "github.com/verda-cloud/verdagostack/pkg/tui/bubbletea" "github.com/verda-cloud/verdagostack/pkg/tui/wizard" + + "github.com/verda-cloud/verda-cli/internal/verda-cli/options" +) + +// newProfileSentinel is the Choice value for "create a new profile". A NUL +// byte can't occur in an INI section name, so it never collides with a real +// profile. Mirrors the s3 wizard's sentinel. +const newProfileSentinel = "\x00new-profile" + +// Credential input modes offered by the wizard, matching the two ways the web +// UI lets you copy a credential: the whole `docker login …` command, or the +// "Full credential name" + "Secret" fields separately. +const ( + inputModePaste = "paste" + inputModeManual = "manual" ) // buildConfigureFlow builds the interactive wizard flow for registry // credential configuration. // -// paste → expires-in → docker-config +// profile (pick or create) → [new name] → input mode +// ├─ paste: paste docker-login command +// └─ manual: username → secret +// → expires-in → docker-config // // The paste step asks the user to paste the full `docker login ...` command // the Verda web UI prints when a credential is provisioned. Its validator // routes the string through parseDockerLogin so the user sees the parser's -// diagnostic in-place and can try again without restarting the flow. On -// success the parsed username/secret/host/project-id are written directly -// onto opts so the existing flag-driven persistence code path in configure.go -// handles the rest. +// diagnostic in-place and can try again without restarting the flow. The +// manual path mirrors the web UI's separate "Full credential name" + "Secret" +// fields; the host isn't asked for because it's derived in configure.go (the +// project id is parsed out of the credential name, the base is vccr.io / the +// profile's saved host). Both paths populate opts so the flag-driven +// resolution path in configure.go handles persistence. func buildConfigureFlow(opts *configureOptions) *wizard.Flow { return &wizard.Flow{ Name: "registry-configure", @@ -44,13 +66,180 @@ func buildConfigureFlow(opts *configureOptions) *wizard.Flow { {ID: "hints", View: wizard.NewHintBarView(wizard.WithHintStyle(bubbletea.HintStyle()))}, }, Steps: []wizard.Step{ + configureStepProfile(opts), + configureStepNewProfileName(opts), + configureStepInputMode(), configureStepPaste(opts), + configureStepUsername(opts), + configureStepSecret(opts), configureStepExpiresIn(opts), configureStepDockerConfig(opts), }, } } +// configureStepInputMode asks whether the user has the full `docker login` +// command (paste) or the credential name + secret separately (manual). The +// chosen value is read by the paste / username / secret / endpoint steps' +// ShouldSkip predicates via collected["input-mode"]. +func configureStepInputMode() wizard.Step { + return wizard.Step{ + Name: "input-mode", + Description: "How do you have the credential?", + Prompt: wizard.SelectPrompt, + Required: true, + Loader: func(_ context.Context, _ tui.Prompter, _ tui.Status, _ *wizard.Store) ([]wizard.Choice, error) { + return []wizard.Choice{ + {Label: "Paste the docker login command (recommended)", Value: inputModePaste}, + {Label: "Enter credential name + secret", Value: inputModeManual}, + }, nil + }, + Default: func(_ map[string]any) any { return inputModePaste }, + Setter: func(any) {}, + Resetter: func() {}, + IsSet: func() bool { return false }, + Value: func() any { return inputModePaste }, + } +} + +// configureStepUsername (manual path) prompts for the "Full credential name" +// the web UI shows, validating the vcr-+ shape so the user +// sees a clear error before the flow moves on. Resetter is a no-op so that, in +// paste mode where this step is skipped, it doesn't clobber the username the +// paste step parsed onto opts. +func configureStepUsername(opts *configureOptions) wizard.Step { + return wizard.Step{ + Name: "username", + Description: "Full credential name (vcr-+)", + Prompt: wizard.TextInputPrompt, + Required: true, + DependsOn: []string{"input-mode"}, + ShouldSkip: func(collected map[string]any) bool { + v, _ := collected["input-mode"].(string) + return v != inputModeManual + }, + Validate: func(v any) error { + s := strings.TrimSpace(v.(string)) + if s == "" { + return errors.New("credential name cannot be empty") + } + if _, err := projectIDFromUsername(s); err != nil { + return err + } + return nil + }, + Setter: func(v any) { opts.Username = strings.TrimSpace(v.(string)) }, + Resetter: func() {}, + IsSet: func() bool { return false }, + Value: func() any { return opts.Username }, + } +} + +// configureStepSecret (manual path) prompts for the "Secret" field. Masked +// input; the value is stored verbatim (not trimmed) since a secret may contain +// surrounding whitespace. No-op Resetter for the same reason as username. +func configureStepSecret(opts *configureOptions) wizard.Step { + return wizard.Step{ + Name: "secret", + Description: "Secret", + Prompt: wizard.PasswordPrompt, + Required: true, + DependsOn: []string{"input-mode"}, + ShouldSkip: func(collected map[string]any) bool { + v, _ := collected["input-mode"].(string) + return v != inputModeManual + }, + Validate: func(v any) error { + if v.(string) == "" { + return errors.New("secret cannot be empty") + } + return nil + }, + Setter: func(v any) { opts.Secret = v.(string) }, + Resetter: func() {}, + IsSet: func() bool { return false }, + Value: func() any { return opts.Secret }, + } +} + +// registryProfileChoices lists existing credential profiles (each tagged with +// whether it already holds registry credentials) plus a trailing "create new" +// option. Reading the file fails soft: on any error the user still gets the +// create-new choice. Mirrors the s3 wizard's profileChoices. +func registryProfileChoices(path string) []wizard.Choice { + profiles, _ := options.ListProfiles(path) + choices := make([]wizard.Choice, 0, len(profiles)+1) + for _, p := range profiles { + desc := "no registry credentials yet" + if creds, err := options.LoadRegistryCredentialsForProfile(path, p); err == nil && creds.HasCredentials() { + desc = "registry configured" + } + choices = append(choices, wizard.Choice{Label: p, Value: p, Description: desc}) + } + return append(choices, wizard.Choice{Label: "+ Create new profile…", Value: newProfileSentinel}) +} + +// configureStepProfile lets the user pick an existing profile or create a new +// one, defaulting the selection to the active profile. A --profile flag +// (opts.Profile != default) pre-sets it and skips this step. +func configureStepProfile(opts *configureOptions) wizard.Step { + return wizard.Step{ + Name: "profile", + Description: "Profile", + Prompt: wizard.SelectPrompt, + Required: true, + Loader: func(_ context.Context, _ tui.Prompter, _ tui.Status, _ *wizard.Store) ([]wizard.Choice, error) { + // List the same file the save writes to (must honor --credentials-file). + return registryProfileChoices(credentialsFilePath(opts.CredentialsFile)), nil + }, + Default: func(_ map[string]any) any { + if p := options.ActiveProfile(""); p != "" { + return p + } + return defaultProfileName + }, + // Sentinel ("create new") leaves opts.Profile alone; the new-name step + // sets it. Picking an existing profile sets it directly. + Setter: func(v any) { + if s, _ := v.(string); s != "" && s != newProfileSentinel { + opts.Profile = s + } + }, + Resetter: func() { opts.Profile = defaultProfileName }, + IsSet: func() bool { return opts.Profile != "" && opts.Profile != defaultProfileName }, + Value: func() any { return opts.Profile }, + } +} + +// configureStepNewProfileName prompts for the name only when the user chose +// "create new" in the profile step. +func configureStepNewProfileName(opts *configureOptions) wizard.Step { + return wizard.Step{ + Name: "new-profile-name", + Description: "New profile name", + Prompt: wizard.TextInputPrompt, + Required: true, + DependsOn: []string{"profile"}, + ShouldSkip: func(collected map[string]any) bool { + v, _ := collected["profile"].(string) + return v != newProfileSentinel + }, + Validate: func(v any) error { + if strings.TrimSpace(v.(string)) == "" { + return errors.New("profile name cannot be empty") + } + return nil + }, + Setter: func(v any) { opts.Profile = strings.TrimSpace(v.(string)) }, + // No-op: opts.Profile is owned by the profile step. Resetting it here + // (called when this step is skipped for an existing profile) would + // clobber the selected profile back to the default. + Resetter: func() {}, + IsSet: func() bool { return false }, + Value: func() any { return opts.Profile }, + } +} + // configureStepPaste asks the user to paste the full `docker login ...` // command from the Verda web UI and routes it through parseDockerLogin. On // success the parsed fields populate opts so the existing persistence path @@ -65,6 +254,11 @@ func configureStepPaste(opts *configureOptions) wizard.Step { Description: "Paste the 'Registry authentication command' from the Verda UI (docker login -u … -p … )", Prompt: wizard.TextInputPrompt, Required: true, + DependsOn: []string{"input-mode"}, + ShouldSkip: func(collected map[string]any) bool { + v, _ := collected["input-mode"].(string) + return v == inputModeManual + }, Validate: func(v any) error { s, _ := v.(string) if strings.TrimSpace(s) == "" { @@ -147,9 +341,8 @@ func configureStepExpiresIn(opts *configureOptions) wizard.Step { } // configureStepDockerConfig asks whether to also write ~/.docker/config.json. -// The actual write happens in `verda registry login` (Task 13); for now we -// only store the boolean and the RunE will print a notice so the user knows -// they still need to run login to materialize the docker config. +// When true, runConfigure performs the same merge `verda registry configure-docker` does +// (via writeDockerLogin) right after saving the Verda credentials. func configureStepDockerConfig(opts *configureOptions) wizard.Step { return wizard.Step{ Name: "docker-config", diff --git a/internal/verda-cli/cmd/registry/wizard_test.go b/internal/verda-cli/cmd/registry/wizard_test.go index c158786..ef4eff0 100644 --- a/internal/verda-cli/cmd/registry/wizard_test.go +++ b/internal/verda-cli/cmd/registry/wizard_test.go @@ -17,6 +17,7 @@ package registry import ( "context" "io" + "path/filepath" "strings" "testing" @@ -34,10 +35,14 @@ func TestBuildConfigureFlow_Structure(t *testing.T) { if flow == nil { t.Fatal("buildConfigureFlow returned nil") } - if len(flow.Steps) != 3 { - t.Fatalf("flow has %d steps, want 3", len(flow.Steps)) + if len(flow.Steps) != 8 { + t.Fatalf("flow has %d steps, want 8", len(flow.Steps)) + } + wantNames := []string{ + "profile", "new-profile-name", "input-mode", + "paste", "username", "secret", + "expires-in", "docker-config", } - wantNames := []string{"paste", "expires-in", "docker-config"} for i, want := range wantNames { if flow.Steps[i].Name != want { t.Errorf("step %d: got %q, want %q", i, flow.Steps[i].Name, want) @@ -50,7 +55,7 @@ func TestBuildConfigureFlow_PasteValidator(t *testing.T) { opts := &configureOptions{ExpiresInDays: defaultExpiresInDays} flow := buildConfigureFlow(opts) - paste := flow.Steps[0] + paste := flow.Steps[3] if paste.Validate == nil { t.Fatal("paste step has no Validate func") @@ -78,7 +83,7 @@ func TestBuildConfigureFlow_ExpiresInValidator(t *testing.T) { opts := &configureOptions{ExpiresInDays: defaultExpiresInDays} flow := buildConfigureFlow(opts) - step := flow.Steps[1] + step := flow.Steps[6] if step.Validate == nil { t.Fatal("expires-in step has no Validate func") @@ -116,7 +121,7 @@ func TestBuildConfigureFlow_DockerConfigDefaultsYes(t *testing.T) { opts := &configureOptions{ExpiresInDays: defaultExpiresInDays} flow := buildConfigureFlow(opts) - step := flow.Steps[2] + step := flow.Steps[7] if step.Default == nil { t.Fatal("docker-config step has no Default func") @@ -130,11 +135,13 @@ func TestBuildConfigureFlow_DockerConfigDefaultsYes(t *testing.T) { } } -// TestBuildConfigureFlow_HappyPath drives the full 3-step flow through the -// wizard engine in test mode and asserts opts is populated as the flag-path -// expects. Mirrors s3/wizard_test.go TestBuildConfigureFlowHappyPath. +// TestBuildConfigureFlow_HappyPath drives the full flow through the wizard +// engine in test mode and asserts opts is populated as the flag-path expects. +// Mirrors s3/wizard_test.go TestBuildConfigureFlowHappyPath, including the +// profile picker → new-name steps. func TestBuildConfigureFlow_HappyPath(t *testing.T) { - t.Parallel() + // No t.Parallel: t.Setenv isolates the credentials file the profile Loader reads. + t.Setenv("VERDA_REGISTRY_CREDENTIALS_FILE", filepath.Join(t.TempDir(), "credentials")) opts := &configureOptions{ Profile: defaultProfileName, @@ -144,6 +151,9 @@ func TestBuildConfigureFlow_HappyPath(t *testing.T) { engine := wizard.NewEngine(nil, nil, wizard.WithOutput(io.Discard), wizard.WithTestResults( + wizard.SelectResult(0), // profile: empty file → only "+ Create new profile…" + wizard.TextResult("staging"), // new profile name + wizard.SelectResult(0), // input-mode: paste (index 0) wizard.TextResult("docker login -u vcr-abc+cli -p s3cret vccr.io"), // paste wizard.TextResult("7"), // expires-in wizard.ConfirmResult(true), // docker-config @@ -154,6 +164,9 @@ func TestBuildConfigureFlow_HappyPath(t *testing.T) { t.Fatalf("wizard Run failed: %v", err) } + if opts.Profile != "staging" { + t.Errorf("Profile = %q, want %q", opts.Profile, "staging") + } if opts.Username != "vcr-abc+cli" { t.Errorf("Username = %q, want %q", opts.Username, "vcr-abc+cli") } @@ -170,3 +183,74 @@ func TestBuildConfigureFlow_HappyPath(t *testing.T) { t.Error("DockerConfig = false, want true") } } + +// TestBuildConfigureFlow_PresetProfileSkipsPicker verifies a --profile flag +// (opts.Profile != default) pre-sets the profile and skips both the picker and +// the new-name step, so only paste / expires-in / docker-config prompt. +func TestBuildConfigureFlow_PresetProfileSkipsPicker(t *testing.T) { + // No t.Parallel: t.Setenv isolates the credentials file the profile Loader reads. + t.Setenv("VERDA_REGISTRY_CREDENTIALS_FILE", filepath.Join(t.TempDir(), "credentials")) + + opts := &configureOptions{Profile: "prod", ExpiresInDays: defaultExpiresInDays} + flow := buildConfigureFlow(opts) + engine := wizard.NewEngine(nil, nil, + wizard.WithOutput(io.Discard), + wizard.WithTestResults( + wizard.SelectResult(0), // input-mode: paste + wizard.TextResult("docker login -u vcr-abc+cli -p s3cret vccr.io"), // paste + wizard.TextResult("0"), // expires-in (no expiry) + wizard.ConfirmResult(false), // docker-config + ), + ) + + if err := engine.Run(context.Background(), flow); err != nil { + t.Fatalf("wizard Run failed: %v", err) + } + if opts.Profile != "prod" { + t.Errorf("Profile = %q, want %q (preset, picker skipped)", opts.Profile, "prod") + } + if opts.Username != "vcr-abc+cli" { + t.Errorf("Username = %q, want %q", opts.Username, "vcr-abc+cli") + } +} + +// TestBuildConfigureFlow_ManualEntry drives the "Enter credential name + +// secret" path: input-mode=manual skips the paste step and prompts for the +// credential name and secret instead. The endpoint is NOT prompted — it's +// derived later in resolveRegistryInputs — so opts.Paste stays empty and the +// name/secret land on opts. +func TestBuildConfigureFlow_ManualEntry(t *testing.T) { + // No t.Parallel: t.Setenv isolates the credentials file the profile Loader reads. + t.Setenv("VERDA_REGISTRY_CREDENTIALS_FILE", filepath.Join(t.TempDir(), "credentials")) + + opts := &configureOptions{Profile: defaultProfileName, ExpiresInDays: defaultExpiresInDays} + flow := buildConfigureFlow(opts) + engine := wizard.NewEngine(nil, nil, + wizard.WithOutput(io.Discard), + wizard.WithTestResults( + wizard.SelectResult(0), // profile: create new + wizard.TextResult("prod"), // new profile name + wizard.SelectResult(1), // input-mode: manual (index 1) + wizard.TextResult("vcr-abc+cli"), // username + wizard.TextResult("s3cret"), // secret + wizard.TextResult("30"), // expires-in + wizard.ConfirmResult(false), // docker-config + ), + ) + + if err := engine.Run(context.Background(), flow); err != nil { + t.Fatalf("wizard Run failed: %v", err) + } + if opts.Profile != "prod" { + t.Errorf("Profile = %q, want %q", opts.Profile, "prod") + } + if opts.Username != "vcr-abc+cli" { + t.Errorf("Username = %q, want %q", opts.Username, "vcr-abc+cli") + } + if opts.Secret != "s3cret" { + t.Errorf("Secret = %q, want %q", opts.Secret, "s3cret") + } + if opts.Paste != "" { + t.Errorf("Paste = %q, want empty (manual mode skips the paste step)", opts.Paste) + } +} From dd3e05939508d437950758fc46967bb0a83d15f6 Mon Sep 17 00:00:00 2001 From: lei Date: Tue, 2 Jun 2026 18:27:58 +0300 Subject: [PATCH 2/3] update claude.md --- CLAUDE.md | 2 +- internal/verda-cli/cmd/registry/CLAUDE.md | 41 ++++++++++++++--------- 2 files changed, 27 insertions(+), 16 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index eced4bb..bfc2ce2 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -39,7 +39,7 @@ Each command directory has its own `CLAUDE.md` (domain knowledge) and `README.md | `cmd/volume/` | CLAUDE.md, README.md | Volume lifecycle, trash, actions | | `cmd/sshkey/` | CLAUDE.md, README.md | SSH key management | | `cmd/startupscript/` | CLAUDE.md, README.md | Startup script management | -| `cmd/registry/` | CLAUDE.md, README.md | Container registry (vccr.io): configure, show, login, ls, tags, push, copy — pre-release behind `VERDA_REGISTRY_ENABLED=1` | +| `cmd/registry/` | CLAUDE.md, README.md | Container registry (vccr.io): configure, configure-docker (alias login), show, ls, tags, push, copy, delete — beta (enabled by default, marked `(beta)` in `verda --help`) | | `cmd/update/` | CLAUDE.md, README.md | CLI self-update | | `cmd/settings/` | CLAUDE.md, README.md | CLI settings management | | `cmd/availability/` | — | Instance availability by location | diff --git a/internal/verda-cli/cmd/registry/CLAUDE.md b/internal/verda-cli/cmd/registry/CLAUDE.md index 238d9d4..0994e0b 100644 --- a/internal/verda-cli/cmd/registry/CLAUDE.md +++ b/internal/verda-cli/cmd/registry/CLAUDE.md @@ -5,13 +5,13 @@ ## Quick Reference - Parent: `verda registry` (aliases: `vccr` canonical, `vcr` legacy) -- Subcommands: `configure`, `show`, `login`, `ls`, `tags`, `push`, `copy` (alias `cp`), `delete` (aliases `del`, `rm`) +- Subcommands: `configure`, `configure-docker` (alias `login`), `show`, `ls`, `tags`, `push`, `copy` (alias `cp`), `delete` (aliases `del`, `rm`) - Files: - - `registry.go` -- Parent command registration. `Hidden: true`; register gated on `VERDA_REGISTRY_ENABLED=1` in `cmd/cmd.go`. + - `registry.go` -- Parent command registration. Visible in `verda --help` with a `(beta)` marker on `Short`; registered unconditionally in `cmd/cmd.go` (no env gate). - `configure.go` -- Credential setup (three input modes: `--paste`, `--username/--password-stdin/--endpoint`, interactive wizard). - `wizard.go` -- Bubbletea wizard steps for `configure`. - `show.go` -- Credential status readout (no secrets); near-expiry warning when < 7 days. - - `login.go` -- Writes `~/.docker/config.json` so docker/compose/helm/nerdctl can auth against VCR. Pure file merge; no registry call. + - `login.go` -- The `configure-docker` command (alias `login`; `NewCmdLogin`/`runLogin` keep the internal "login" names since the merge writes a docker *login* entry). Writes `~/.docker/config.json` so docker/compose/helm/nerdctl can auth against VCR. Pure file merge; no registry call. - `client.go` -- `Registry` interface + `ggcrRegistry` production implementation. All `remote.*` and `authn`/`name` imports isolated here + a few siblings. - `helper.go` -- `clientBuilder` / `daemonListerBuilder` / `sourceLoaderBuilder` swap points. `loadCredsFromFactory` + default-profile fallback. - `path.go` -- Credentials file path resolution (`--credentials-file` > `VERDA_REGISTRY_CREDENTIALS_FILE` > `options.DefaultCredentialsFilePath()`). @@ -27,11 +27,12 @@ - `harbor.go` -- `RepositoryLister` interface (`ListRepositories` + `ListArtifacts` + `DeleteRepository` + `DeleteArtifact`) + `harborClient` implementation. Talks to Harbor's REST API v2.0 (`/api/v2.0/projects`, `/api/v2.0/repositories`, `/api/v2.0/projects/{p}/repositories/{r}[/artifacts[/{ref}]]`) using Basic auth. Intentionally separate from the `Registry` interface (which is Docker Registry v2 / ggcr shaped); see "List Repositories (ls)" and "Delete (`delete`)" below. - `ls.go` -- List repositories in the active Verda project via `harborClient`. Non-TTY + structured output render a flat table/document; TTY output routes through `f.Prompter().Select` so the user can pick a repo to drill into, at which point `ListArtifacts` fetches per-artifact detail (digest / tags / size / push / pull) for a Harbor-UI-style image card. - `delete.go` -- Delete a repository (all artifacts / tags) or a single image (artifact) in the active project via `harborClient`. Positional target shapes: `REPOSITORY`, `REPOSITORY:TAG`, `REPOSITORY@DIGEST`. TTY + no arg enters an interactive flow (pick repo → sub-menu → `MultiSelect` over artifacts for batch delete, or whole-repo delete). Agent mode requires `--yes`; missing `--yes` returns `CONFIRMATION_REQUIRED`. Reuses `Normalize` from `refname.go` plus a local `classifyTarget` to distinguish "bare repo" vs "artifact by tag" (because `Normalize` defaults `Tag` to `"latest"` for push/copy semantics — that default would smuggle a tag-delete otherwise). - - `tags.go` -- List tags + per-tag digest/size via `Head`. - - `push.go` -- Push local images (daemon/oci/tar). Handles zero-arg interactive picker. `runPickerFn` swap point. + - `tags.go` -- List tags + per-tag digest/size via `Head`. `Args: MaximumNArgs(1)`. On a TTY (non-agent, table format) `tags ` reuses `ls.go`'s `runTagPicker` (`tagRowsToEntries` → filterable picker → prints a pull ref, using `cmd.Context()` so think-time can't cancel it); piped / structured / agent render the table. **Bare `tags`** (no repo) returns a friendly usage error pointing at `tags ` / `ls` — `tags` is tag-centric (one repo); browsing repositories interactively is `ls`'s job (intentionally NOT duplicated here). + - `push.go` -- Push local images (daemon/oci/tar). Zero-arg on a TTY: if the daemon is reachable → daemon-image picker (`runPickerFn` swap point); if **not** reachable → `pushFileSourceFallback` (prompt source type → path → destination repo/tag, sets `opts.Source/Repo/Tag`) instead of dead-ending. `resolveZeroArgs`/`listAndPickImages` take `opts` so the fallback can set the source. - `push_view.go` -- Bubbletea model + renderer for the push/copy progress view. `isTerminalFn` swap point. - `push_picker.go` -- Bubbletea model for the interactive daemon-image picker (selectable list + `formatAgo` helper). - - `copy.go` -- Copy single ref / `--all-tags` between registries. `sourceKeychainBuilder` / `sourceRegistryBuilder` swap points; worker pool + overwrite guard. + - `copy.go` -- Copy single ref / `--all-tags` between registries. `sourceKeychainBuilder` / `sourceRegistryBuilder` swap points; worker pool + overwrite guard. `runCopy` (flag path) reads the `--src-auth basic` secret from stdin then calls `runCopyResolved`, the shared post-auth executor; `buildSourceAuth(opts, basicPassword)` takes the password as a string (stdin on the flag path, a prompt in the wizard). + - `copy_wizard.go` -- Interactive `copy` wizard (zero args on a TTY, gated by `copyWizardEligible`). s3-cp-style step-machine: source → access → scope → destination → confirm, Esc=back/Ctrl+C=exit. Collects into `opts` + a prompted password, then calls `runCopyResolved` so the wizard and flag paths share dry-run/overwrite/all-tags/progress behavior. `confirmAndRunCopy` previews the equivalent `verda registry copy …` command before executing. - `*_test.go` alongside each file. ## Domain-Specific Logic @@ -52,9 +53,11 @@ Registry credentials are **write-once from the user's side**. The Verda API's `G - `translateErrorWithExpiry(err, creds)` (`errors.go`) maps a server-side 401 to `registry_credential_expired` when `creds.ExpiresAt` is in the past, otherwise to `registry_auth_failed`. This is the preferred error-translation entry point for any command that has creds in hand. - Legacy rows with `ExpiresAt.IsZero()` are treated as "not expired" — the server is authoritative. This keeps pre-Task-9 credentials working without a migration. -### Profile Fallback (registry-specific) +### Profile resolution (registry-specific) -Registry commands are in `skipCredentialResolution` (see `cmd/cmd.go`), so `Options.Complete()` never runs and `AuthOptions.Profile` stays empty. `loadCredsFromFactory` in `helper.go` therefore falls back to `defaultProfileName` (`"default"`) when the profile is blank. Without this fallback, `LoadRegistryCredentialsForProfile(path, "")` would resolve ini.v1's synthetic `DEFAULT` section instead of the user's `[default]` section, and every registry command would falsely report "not configured" right after a successful `verda registry configure`. This mirrors the s3 package's pattern. +Registry commands are in `skipCredentialResolution` (see `cmd/cmd.go`), so `Options.Complete()` never runs and the active profile is **not** auto-resolved the way it is for `verda vm`. `resolveProfile(flagProfile)` in `helper.go` replicates the precedence locally: **explicit `--profile` > `VERDA_PROFILE` / `auth.profile` (via `options.ActiveProfile`) > `"default"`**. Every subcommand's `--profile` flag now defaults to `""` (not `defaultProfileName`) so "unspecified" reaches `resolveProfile` and picks up the active profile; `loadCredsFromFactory` and `show`/`configure` all route through it. + +This matters because `configure`'s wizard picker already defaults to the active profile (`ActiveProfile`). Before the fix, the read commands hardcoded `"default"`, so a user whose active profile was e.g. `production` configured `production` but `ls`/`tags`/… silently read the stale `default` section — surfacing as a spurious `registry_credential_expired`. The `"default"` final fallback also avoids `LoadRegistryCredentialsForProfile(path, "")` resolving ini.v1's synthetic `DEFAULT` section instead of the user's `[default]`. An explicit `--profile X` always wins (resolution is idempotent). s3 has the same hardcoded-default shape and is a candidate for the same fix. ### ggcr Isolation Discipline @@ -135,11 +138,10 @@ Before each `Write`, we `Head` the destination ref: `--dry-run` skips the guard entirely by design (it's asking "what would happen", not doing). -### Feature Gate +### Release Status (beta) -- Parent command hidden via `Hidden: true` in `registry.go` so `verda --help` doesn't advertise it pre-GA. -- Registration in `cmd/cmd.go` gated by `registryEnabled()` which reads `VERDA_REGISTRY_ENABLED=1` / `=true`. Without the env var, the command tree isn't registered at all and `verda registry ...` returns "unknown command". -- **GA flip**: delete `registryEnabled()`, drop the gate in `NewRootCommand`, and remove `Hidden: true` from `registry.go`. +- Registered unconditionally in `cmd/cmd.go` (the old `registryEnabled()` / `VERDA_REGISTRY_ENABLED` gate is gone) and listed in `verda --help` as `registry … (beta)` — the `(beta)` marker lives in the parent `Short` in `registry.go`, mirroring `cmd/mcp`'s convention. +- **Beta caveat still open**: `push --no-mount` is accepted but a no-op (prints a notice). Resolve or hide it before dropping the `(beta)` marker for full GA. (`configure --docker-config` is now fully wired — see below.) ## Gotchas & Edge Cases @@ -150,8 +152,12 @@ Before each `Write`, we `Head` the destination ref: - `TestPush_InteractiveMode_HappyPath` (and similar) uses `--progress plain` to bypass the TUI branch; the real bubbletea path is hard to drive in unit tests, so it's covered by model-level tests in `push_view_test.go` / `push_picker_test.go` instead. - `pushViewModel` (`push_view.go`) is **reused by `copy`** for both single-ref and `--all-tags`. The heading still says "Pushing N images" even in copy mode — a minor label mismatch, acceptable for v1; easy to swap in a dedicated copy header later. - The pre-flight `Head` on a copy destination relies on `translateTransportError` mapping a bare 404 (no structured `Diagnostic` body) to `registry_tag_not_found`. Without that fallthrough, non-existent destinations would surface as `registry_internal_error` and the overwrite guard would bail out before the first write. -- `configure --docker-config` is accepted but not yet wired — it prints a TODO notice pointing at `verda registry login`. +- `configure --docker-config` (and the wizard's "Also write ~/.docker/config.json?" step) writes the Docker config via the same `writeDockerLogin` merge `verda registry login` uses. A write failure is non-fatal — the Verda credentials are already saved, so configure warns and points at `registry login` for a retry rather than failing. +- Wizard input modes (`configureStepInputMode` in `wizard.go`): the `input-mode` Select gates the paste step vs. the manual `username` + `secret` steps via each step's `ShouldSkip` reading `collected["input-mode"]`. The manual path has **no endpoint step** — the host is derived in `resolveRegistryInputs` (the `Username != "" && Secret != ""` branch) via `resolveEndpointForFlags` (saved host → `vccr.io`), with the project id parsed from the credential name by `projectIDFromUsername`. The manual branch is distinguished from the `--password-stdin` flag branch by a non-empty `opts.Secret` (the flag path leaves `Secret` empty and reads stdin). Skipped manual steps use no-op `Resetter`s so paste-mode doesn't clobber the paste-parsed `opts.Username`. +- Overwrite guard (`confirmOverwriteIfConfigured` in `configure.go`): re-configuring a profile that already has registry creds is an **irreversible** replace (the secret is write-once). On a TTY it prompts to confirm (decline → clean abort, nothing written); in agent/non-TTY it proceeds (rotation intent) with a non-agent stderr note. Detection is `options.LoadRegistryCredentialsForProfile(...).HasCredentials()`; a load error is treated as "nothing to overwrite" so a first-time write is never blocked. The wizard's profile picker independently annotates each profile `registry configured` / `no registry credentials yet`. - Bubbletea output always goes to `ioStreams.ErrOut`. Stdout stays clean for structured / scripted consumption. +- **Interactive prompts must use `cmd.Context()`, not the per-request timeout ctx** (`context.WithTimeout(cmd.Context(), f.Options().Timeout)`). Think-time across a picker/wizard easily exceeds the 30s default `--timeout`; a bounded ctx cancels the prompt mid-flow and the cancel is swallowed as a clean exit (the TUI just vanishes). `ls`/`delete`/`tags`/`copy`/`push` interactive flows pass `cmd.Context()`; the bounded ctx is used only for the up-front listing API call. (Per-call timeouts inside the interactive loop are a future refinement.) +- `--src-auth basic` validation order matters: `readBasicSourcePassword` checks `--src-username` is set BEFORE reading stdin, so a missing username doesn't drain (and lose) a piped one-shot secret. `buildSourceAuth` keeps its own username + empty-password checks as the wizard-path guard. - `splitLocalRef` in `push.go` intentionally does **not** use `Normalize()` — Normalize prefixes with `creds.ProjectID`, which is correct for VCR destinations but would corrupt a local `my-app:v1` source ref. The host heuristic mirrors `isShortRef` (first segment is a host iff it contains `.` / `:` or is `localhost`). ## Relationships @@ -204,11 +210,15 @@ Harbor's top-level `/repositories` endpoint has *two* filter syntaxes that look 1. `-o json|yaml` → `WriteStructured` → return. The picker never runs; scripts always get a deterministic document. 2. `!isTerminalFn(ioStreams.Out)` → `renderLsHuman` → return. Same table the pre-drill-down `ls` used to produce. Piping to `less` / `jq` / CI logs hits this path. -3. TTY → `runLsInteractive` loops the user through `prompter.Select` → `ListArtifacts(project, repo.Name)` → `renderRepoArtifacts`. Selecting "Exit" (the appended trailing label) or a prompter cancel error returns nil cleanly, same contract as `cmd/vm/list.go`. A transient `ListArtifacts` error is printed to ErrOut and the picker stays open — matching `vm list`'s "one flaky fetch doesn't kick the user out" behavior. +3. TTY → `runLsInteractive` loops the repo picker (`prompter.Select`). Selecting a repo calls `ListArtifacts` then `runRepoActions` — a per-repo **action menu** (`prompter.Select`): **Get pull URL** → `runTagPicker`; **Delete image(s)…** → `runDeleteImagesInteractive` (the *same* multi-select + red-warning confirm flow `verda registry delete` uses — one destructive code path, not a copy); **← Back**. Selecting "Exit" on the repo picker or a prompter cancel returns nil cleanly, same contract as `cmd/vm/list.go`. `runLsInteractive` takes `*options.RegistryCredentials` (not just the host string) so it can hand creds to the delete flow. After a delete, `runRepoActions` re-fetches artifacts so a subsequent pull-URL pick reflects the change (the repo-picker counts are built once and may be stale — acceptable for v1). + + `runTagPicker` lists the repo's tags newest-first (`buildTagEntries` sorts artifacts by `PushTime` desc, one row per tag, `` rows for tagless artifacts), `prompter.Select`'s type-to-filter handles filtering, and selecting a row prints that tag's full pull reference (`repoPullBase + ":" + tag`, or `@digest` for untagged) to **stdout** so it's clean to copy, then **quits** — getting the URL is the goal, so a positive selection is the terminal action (`runTagPicker` returns quit=true). Esc / "Back" returns to the action menu; Ctrl+C quits. + + `ListArtifacts` errors are classified by `isAccessDenied` (a translated `registry_access_denied` AgentError = Harbor 403). Harbor's artifact API needs a project permission the VCR robots issued today often lack, but the **Docker v2 tag list** (`Registry.Tags`, what `verda registry tags` uses) usually works for the same credential. So on access-denied, `tagsViaRegistry` falls back to `buildClient(creds, …).Tags(repo.FullName)` and, if it returns tags, drives the same `runTagPicker` (names only — no size/push-time from v2) so pull URLs still work. **Delete is intentionally not offered on the fallback path** — it goes through the same denied Harbor API. Only if the v2 list is *also* denied does `renderRepoPullPath` show just the repository pull reference. **Transient** (5xx) errors print to ErrOut and re-prompt — matching `vm list`'s "one flaky fetch doesn't kick the user out" behavior. `ListArtifacts` percent-encodes the repo name (`url.PathEscape`) because Harbor routes the path; the regression test `TestHarbor_ListArtifacts_URLEscapesRepoSlash` asserts the encoded form reaches the server. Unlike the repository-listing endpoint, the project-scoped `/repositories/{r}/artifacts` endpoint *does* work for the robot accounts VCR mints today (confirmed against staging, Apr 2026). If that changes, `translateHarborError` already maps 401/403 to the appropriate `registry_*` AgentError. -The picker's per-row label is built by `formatRepoRow` (repo name padded to 48 cols + right-aligned artifact/pull counts + short-form "UPDATED" date). The image-list card is rendered by `renderRepoArtifacts`, columns mirror Harbor's web UI (`DIGEST` / `TAGS` / `SIZE` / `PUSHED` / `PULLED`). Zero-valued `PullTime` (Harbor's "never pulled" sentinel is the 0001-01-01 epoch) renders as `--`. Untagged artifacts (SBOMs, referrer manifests) show `` in the TAGS column. +The repo-picker per-row label is built by `formatRepoRow` (repo name padded to 48 cols + right-aligned artifact/pull counts + short-form "UPDATED" date). The interactive drill-down does **not** render the old `DIGEST`/`TAGS`/`SIZE`/`PUSHED`/`PULLED` table — that detailed, non-interactive view now lives only in `verda registry tags `; the `ls` drill-down is the action menu + tag picker described above. `repoPullBase` (used for both the tag picker and the access-denied path) prefers Harbor's `FullName` (already `/`) and falls back to `project + "/" + Name`, defaulting the host to `defaultRegistryEndpoint`. ## Delete (`delete` / aliases `del`, `rm`) @@ -250,6 +260,7 @@ Harbor returns HTTP 412 when a project policy — **Tag Immutability** or **Tag 2. Inner menu: "Delete image(s) from X", "Delete repository X (all images)", "Back to repository list", "Exit". 3. The image-delete sub-flow uses `prompter.MultiSelect` with a label that surfaces the **Ctrl+A** "select all" keystroke — the bubbletea `MultiSelect` already supports it natively (see `verdagostack/pkg/tui/bubbletea/multiselect.go`), so we just advertise it in the prompt label and tests emulate it via `AddMultiSelect([]int{0, 1, ..., n-1})`. 4. The image batch runs sequentially (Harbor has no bulk-delete endpoint). Failures on individual artifacts are collected and reported at the end; survivors still get deleted — users generally want partial progress, not all-or-nothing. +5. Error handling in the outer loop classifies via `isAccessDenied` (shared with `ls`): a `registry_access_denied` (Harbor 403) is a permission wall — surface the actionable error once and **return** (exit), rather than looping the user back into a picker that can only re-fail. This is the common case for credentials minted before the Harbor image permission was granted (the error text tells them to re-`configure` with a fresh credential). Transient (5xx) errors still print + `continue` so one flaky fetch doesn't eject the user. ### Structured output (agent mode) From 7b26b7e7798a289615ae0174988030c4c9a0dc65 Mon Sep 17 00:00:00 2001 From: lei Date: Tue, 2 Jun 2026 18:31:43 +0300 Subject: [PATCH 3/3] test(registry): annotate G304 gosec false positive in configure_test The docker-config read is from the test's own temp dir; same //nolint:gosec pattern already used in s3/browse_test.go. Fixes the gosec CI job (golangci-lint --no-config --default=none -E gosec ./...). Co-Authored-By: Claude Opus 4.8 (1M context) --- internal/verda-cli/cmd/registry/configure_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/verda-cli/cmd/registry/configure_test.go b/internal/verda-cli/cmd/registry/configure_test.go index a5aea74..35e3d3d 100644 --- a/internal/verda-cli/cmd/registry/configure_test.go +++ b/internal/verda-cli/cmd/registry/configure_test.go @@ -599,7 +599,7 @@ func TestConfigure_DockerConfigFlagWritesDockerConfig(t *testing.T) { t.Fatalf("run: %v", err) } - data, err := os.ReadFile(filepath.Join(dockerDir, "config.json")) + data, err := os.ReadFile(filepath.Join(dockerDir, "config.json")) //nolint:gosec // G304: reads from the test's own temp dir if err != nil { t.Fatalf("docker config not written: %v", err) }