feat(container): add registry list command and public link on container create#614
feat(container): add registry list command and public link on container create#614fabienfleureau wants to merge 1 commit intomainfrom
Conversation
…er create - Add `qovery container registry` subcommand with `list` subcommand supporting `--organization` and `--json` flags - Add `--json` flag to `qovery container create`; JSON output now includes `public_link` when a port is configured and a link is available - Use `encoding/json` for JSON output in container create (replaces hand-rolled fmt.Sprintf to avoid escaping issues)
There was a problem hiding this comment.
Pull request overview
This PR adds two new features to the Qovery CLI: a qovery container registry list subcommand that lists all container registries for an organization, and a --json flag to qovery container create that outputs structured JSON including the service id, name, and optionally public_link after creation. It also replaces hand-rolled JSON strings with encoding/json marshalling in the create command.
Changes:
- Add
qovery container registry listsubcommand with--organizationand--jsonflags for tabular and JSON output - Add
--jsonflag toqovery container create, outputtingid,name, andpublic_linkusingencoding/jsoninstead of manual string formatting - Add
qovery container registryparent command as the new subcommand group host
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
cmd/container_registry.go |
New parent registry subcommand under container, following the same pattern as container_domain.go and container_env.go |
cmd/container_registry_list.go |
New list subcommand that fetches and displays container registries in tabular or JSON format |
cmd/container_create.go |
New create subcommand for container services with --json output including a public_link field |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if err != nil { | ||
| utils.PrintlnError(err) | ||
| os.Exit(1) | ||
| panic("unreachable") // staticcheck false possible: https://staticcheck.io/docs/checks#SA5011 |
There was a problem hiding this comment.
The comment at line 39 reads // staticcheck false possible: but should be // staticcheck false positive: to be consistent with all other identical comments throughout the codebase (e.g., lines 24, 32, 64, 92 in the same file, and all other cmd/*.go files).
| panic("unreachable") // staticcheck false possible: https://staticcheck.io/docs/checks#SA5011 | |
| panic("unreachable") // staticcheck false positive: https://staticcheck.io/docs/checks#SA5011 |
| if registry.Kind != nil { | ||
| kind = string(*registry.Kind) | ||
| } | ||
| data = append(data, []string{registry.Id, *registry.Name, kind, url}) |
There was a problem hiding this comment.
registry.Name is a *string field, as confirmed by the client-go type (set via SetName in tests, using &containerRegistryRequest.Name in the mock). At line 57, it is dereferenced with *registry.Name without a nil check, while registry.Url (line 50) and registry.Kind (line 54) are both guarded with nil checks before dereferencing. If Name is nil for any registry, this will panic at runtime. A nil guard (similar to how Url is handled) or use of a getter method should be applied here.
| data = append(data, []string{registry.Id, *registry.Name, kind, url}) | |
| name := "" | |
| if registry.Name != nil { | |
| name = *registry.Name | |
| } | |
| data = append(data, []string{registry.Id, name, kind, url}) |
| } | ||
| results = append(results, map[string]interface{}{ | ||
| "id": registry.Id, | ||
| "name": registry.Name, |
There was a problem hiding this comment.
In getContainerRegistryJsonOutput, registry.Name is stored directly into the map as a *string (line 82), unlike url and kind which are always resolved to plain string values (lines 72-79). When Name is nil, json.Marshal will output "name": null rather than "name": "", making the JSON output inconsistent with the other fields. The name field should be resolved to a plain string (possibly using a nil guard or a getter) just like url and kind are.
| Name string `json:"name"` | ||
| PublicLink string `json:"public_link,omitempty"` | ||
| }{Id: created.Id, Name: created.Name, PublicLink: publicLink} | ||
| j, _ := json.Marshal(out) |
There was a problem hiding this comment.
The json.Marshal error is silently discarded with _ on this line (j, _ := json.Marshal(out)). Throughout the codebase, every other use of json.Marshal in JSON output paths (e.g., container_list.go:85, cluster_list.go:79, container_registry_list.go:88) assigns the error and handles it with PrintlnError + os.Exit(1). This is inconsistent with the established codebase convention and could silently produce an empty output on a marshalling failure. The error should be checked and handled.
| j, _ := json.Marshal(out) | |
| j, err := json.Marshal(out) | |
| if err != nil { | |
| utils.PrintlnError(err) | |
| os.Exit(1) | |
| panic("unreachable") // staticcheck false positive: https://staticcheck.io/docs/checks#SA5011 | |
| } |
Summary
qovery container registry listsubcommand — lists all container registries for an organization with--organizationand--jsonflags--jsonflag toqovery container create— JSON output includesid,name, andpublic_link(when a port is configured and a link is available after creation)fmt.SprintfJSON string incontainer createwithencoding/json+ anonymous struct to avoid escaping issues on names containing special charactersTest plan
qovery container registry list --organization <name>returns tabular outputqovery container registry list --organization <name> --jsonreturns JSON array withid,name,kind,urlfieldsqovery container create --json ...with a port returns{"id":"...","name":"...","public_link":"..."}after creationqovery container create --json ...without a port returns{"id":"...","name":"..."}(nopublic_linkfield)