Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions cmd/tscli/get/service/cli.go
Original file line number Diff line number Diff line change
Expand Up @@ -33,19 +33,19 @@ func Command() *cobra.Command {
}

path := fmt.Sprintf("/tailnet/{tailnet}/services/%s", url.PathEscape(serviceName))
var raw json.RawMessage
var resp map[string]any
if _, err := tscli.Do(
context.Background(),
client,
http.MethodGet,
path,
nil,
&raw,
&resp,
); err != nil {
return fmt.Errorf("failed to get service %s: %w", serviceName, err)
}

out, _ := json.MarshalIndent(raw, "", " ")
out, _ := json.MarshalIndent(resp, "", " ")
return output.Print(viper.GetString("output"), out)
},
}
Expand Down
20 changes: 19 additions & 1 deletion cmd/tscli/list/services/cli.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,10 @@ import (
"github.com/spf13/viper"
)

type listResponse struct {
VIPServices []map[string]any `json:"vipServices"`
}

func Command() *cobra.Command {
cmd := &cobra.Command{
Use: "services",
Expand All @@ -35,7 +39,21 @@ func Command() *cobra.Command {
return fmt.Errorf("failed to list services: %w", err)
}

out, _ := json.MarshalIndent(raw, "", " ")
out := raw
switch viper.GetString("output") {
case "pretty", "human":
var resp listResponse
if err := json.Unmarshal(raw, &resp); err != nil {
return fmt.Errorf("decode service list response: %w", err)
}

out, _ = json.MarshalIndent(resp.VIPServices, "", " ")
}

if viper.GetString("output") != "pretty" && viper.GetString("output") != "human" {
out, _ = json.MarshalIndent(raw, "", " ")
}

return output.Print(viper.GetString("output"), out)
},
}
Expand Down
28 changes: 24 additions & 4 deletions internal/testutil/apimock/fixtures.go
Original file line number Diff line number Diff line change
Expand Up @@ -207,13 +207,33 @@ func PostureIntegrationList() map[string]any {

func Service() map[string]any {
return map[string]any{
"name": "svc",
"ports": []int{443},
"name": "svc:demo-speedtest",
"addrs": []string{"100.82.154.103", "fd7a:115c:a1e0::b101:9bb1"},
"comment": "This Tailscale Service is managed by the Tailscale Kubernetes Operator",
"ports": []string{"tcp:443"},
"tags": []string{"tag:demo-speedtest"},
"annotations": map[string]any{
"tailscale.com/owner-references": `{"ownerRefs":[{"operatorID":"nbFLzCnzKQ11CNTRL"}]}`,
},
}
}

func ServiceList() []map[string]any {
return []map[string]any{Service()}
func ServiceList() map[string]any {
return map[string]any{
"vipServices": []map[string]any{
Service(),
{
"name": "svc:demo-streamer",
"addrs": []string{"100.106.40.81", "fd7a:115c:a1e0::da01:2897"},
"comment": "This Tailscale Service is managed by the Tailscale Kubernetes Operator",
"ports": []string{"tcp:443"},
"tags": []string{"tag:demo-streamer"},
"annotations": map[string]any{
"tailscale.com/owner-references": `{"ownerRefs":[{"operatorID":"nP1yvuuBKr11CNTRL"}]}`,
},
},
},
}
}

func ServiceDevices() []map[string]any {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
schema: spec-driven
created: 2026-04-12
Original file line number Diff line number Diff line change
@@ -0,0 +1,70 @@
## Context

`list services` and `get service` currently decode into `json.RawMessage`, re-marshal, and hand the payload to the generic output pipeline. That works for JSON and yaml, but the pretty/human renderer is optimized for top-level arrays or object records that already match the CLI's display expectations. Real service responses include nested service structures and collection wrappers that cause the generic formatter to flatten sibling service data into a single unreadable block.

The change touches multiple modules: service commands under `cmd/tscli`, shared output behavior under `pkg/output`, fixtures in `internal/testutil/apimock`, and example/integration tests under `test/cli`. The design needs to preserve existing flags and script-friendly structured output while fixing interactive readability.

## Goals / Non-Goals

**Goals:**
- Make `list services` pretty/human output render one service per record with stable field grouping.
- Make `get service` pretty/human output render a single service record consistently with other `get` commands.
- Preserve JSON and yaml response structure so existing scripts keep working.
- Add regression tests that exercise representative service payloads, including nested fields such as annotations.

**Non-Goals:**
- Redesign the entire pretty printer for every command shape in the CLI.
- Change service command flags, request paths, or output schema for JSON/yaml.
- Introduce config keys or output-mode-specific flags for service rendering.

## Decisions

### Decode service responses into service-shaped values before printing
The service commands will stop treating responses as opaque `json.RawMessage` for all output modes. Instead, they will decode into local service-oriented models or `map[string]any` structures that reflect the actual list and single-service payload shapes, then pass those values through the output layer.

Rationale:
- The bug starts at the boundary where the command loses shape information by treating the payload as undifferentiated raw JSON.
- Command-local shaping is lower risk than making the pretty printer infer special handling for one API family from arbitrary nested maps.
- JSON and yaml output can still be derived from the decoded structure without changing their semantics.

Alternatives considered:
- Teach the generic pretty printer to detect `vipServices`-style payloads. Rejected because it hardcodes service-specific API knowledge into a global renderer and risks unrelated regressions.
- Leave command decoding as-is and only patch wrapping/indent logic in `pkg/output`. Rejected because the malformed rendering is caused by the service payload shape entering the printer without command-aware normalization.

### Preserve the API response contract for structured modes
The implementation will keep JSON/yaml output schema-aligned with the API response while adapting pretty/human rendering to operate on the decoded service records.

Rationale:
- Scripts depend on structured output staying predictable.
- The user-visible bug is in pretty/human readability, not in the machine-readable modes.

Alternatives considered:
- Emit a simplified synthetic summary for all modes. Rejected because it would drop API-documented service fields and create compatibility risk for scripts.

### Add representative service output fixtures and mode-specific assertions
Tests will use richer service fixtures that include addresses, ports, tags, comments, and annotations. Example output coverage will assert not just JSON shape but also readable pretty/human rendering for `list services` and `get service`.

Rationale:
- The current tests pass because they only assert trivial JSON keys on simplistic fixtures.
- Regressions in interactive rendering need direct text assertions, not just structural JSON checks.

Alternatives considered:
- Rely only on unit tests for the pretty printer. Rejected because the defect is command-path-specific and must be covered end to end.

## Risks / Trade-offs

- [Service payload shape may differ between list and get responses] -> Use separate command-local response models or normalization paths and cover both with tests.
- [Command-local shaping could drift from the pinned schema over time] -> Keep JSON/yaml output schema-aligned and extend fixtures/tests to cover important service fields.
- [Changing shared output helpers could affect unrelated commands] -> Prefer minimal shared helper changes and keep service-specific adaptation close to the service commands.

## Migration Plan

No migration is required. The change is an in-place correction to existing command behavior:
1. Update service command decoding/output handling.
2. Expand fixtures and command-level tests for representative service payloads and pretty/human assertions.
3. Verify JSON/yaml output remains stable.
4. Rollback, if needed, by restoring the previous service command output path; no persisted data or config is affected.

## Open Questions

- Whether `list services devices` shares any of the same rendering issues once richer fixtures are added, and whether it should be covered in the same implementation pass if a related issue appears.
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
## Why

`tscli list services` and `tscli get service` currently produce incorrect pretty-mode output for real Tailscale service payloads, flattening nested service data into a hard-to-read stream instead of rendering one service record at a time. This needs to be fixed now because the commands are already exposed to users and the current output makes service inspection unreliable in the interactive modes people use most.

## What Changes

- Fix `list services` pretty and human output so service collections render as distinct service records instead of a collapsed nested blob.
- Fix `get service` pretty and human output so a single service renders with the same stable field formatting as other structured commands.
- Normalize service response decoding/output handling so collection and single-service commands preserve service fields such as addresses, ports, tags, comments, and annotations.
- Add output-focused regression coverage for service commands across supported output modes, with emphasis on pretty/human readability and stable JSON/yaml structure.
- Review adjacent service command output behavior for obvious formatting inconsistencies introduced by the same raw-response path and correct them where needed without changing command flags or API semantics.

## Capabilities

### New Capabilities

- `service-command-output`: Define stable user-facing output behavior for `list services` and `get service`, especially in pretty and human modes for nested service payloads.

### Modified Capabilities

- `cli-command-test-coverage`: Tighten command output coverage so service commands exercise supported output modes and catch regressions in rendered structure, not just JSON shape.

## Impact

- Affected command groups: `list services`, `get service`, and any shared service output helpers introduced for pretty/human rendering.
- Affected code: service CLI command implementations under `cmd/tscli/...`, shared output logic under `pkg/output`, and mock/example output tests under `test/cli`.
- Backward compatibility: command paths, flags, config keys, and API requests remain unchanged; the user-visible impact is corrected pretty/human rendering and stronger regression coverage for existing scripts and interactive use.
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
## MODIFIED Requirements

### Requirement: Output and exit semantics are stable
Command tests MUST validate output behavior and error semantics for supported output modes so scripts can depend on stable behavior.

#### Scenario: Successful command execution
- **WHEN** a command succeeds in mock-backed execution
- **THEN** tests MUST assert expected stdout structure/content and empty stderr

#### Scenario: Command execution error
- **WHEN** a command fails due to API or validation error
- **THEN** tests MUST assert non-zero error behavior and error details written to stderr without ambiguous output

#### Scenario: Pretty and human rendering for nested responses is covered
- **WHEN** a command returns nested collection or object payloads whose pretty or human rendering differs from raw JSON shape
- **THEN** automated tests MUST assert readable rendered structure in `pretty` and `human` modes
- **AND** the tests MUST catch record-collapsing, field duplication, and sibling-value leakage across rendered items

#### Scenario: Service command output regressions are detected
- **WHEN** `list services` or `get service` output handling changes
- **THEN** automated tests MUST verify supported output modes for representative service payloads including addresses, ports, tags, comments, and annotations
- **AND** tests MUST fail if service records are flattened into an unreadable pretty or human rendering
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
## ADDED Requirements

### Requirement: Service commands render stable structured output
`tscli list services` and `tscli get service` SHALL render service payloads as stable structured records across supported output modes without flattening or collapsing sibling service entries together.

#### Scenario: List services pretty output shows one service per record
- **WHEN** `tscli list services` runs in `pretty` mode against a response containing multiple services
- **THEN** the CLI SHALL render each service as a distinct record separated consistently from the next service
- **AND** fields from one service SHALL NOT be merged into or repeated under another service's rendered block

#### Scenario: List services preserves key service properties
- **WHEN** `tscli list services` receives service objects containing properties such as `name`, `addrs`, `ports`, `tags`, `comment`, and `annotations`
- **THEN** the CLI SHALL preserve those properties in structured output modes
- **AND** nested map fields such as `annotations` SHALL remain associated with the correct service record

#### Scenario: Get service pretty output shows a single service record
- **WHEN** `tscli get service --service <name>` runs in `pretty` or `human` mode
- **THEN** the CLI SHALL render the returned service as one structured service record
- **AND** the rendered output SHALL use stable field formatting consistent with other structured `get` commands

#### Scenario: Structured output remains script-friendly
- **WHEN** `tscli list services` or `tscli get service` runs in `json` or `yaml` mode
- **THEN** the CLI SHALL preserve the API response structure without synthetic summaries or lossy field dropping
- **AND** correcting pretty or human rendering SHALL NOT change command flags or request semantics
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
## 1. Service response shaping

- [x] 1.1 Inspect the `/services` and `/services/{serviceName}` response shapes and add service-oriented decoding/normalization for `list services` and `get service`.
- [x] 1.2 Update the service commands so pretty and human rendering operate on stable service records while JSON and yaml remain schema-aligned and script-friendly.
- [x] 1.3 Review adjacent service output paths for the same raw-response issue and fix any obvious formatting regressions discovered during implementation.

## 2. Output behavior verification

- [x] 2.1 Expand service fixtures to include representative fields such as addresses, ports, tags, comments, and annotations.
- [x] 2.2 Add command-level example/output tests for `list services` and `get service` that assert readable pretty and human rendering and stable JSON/yaml structure.
- [x] 2.3 Verify the updated service output behavior does not regress command exit semantics or compatibility for existing flags and scripts.

## 3. Documentation and validation

- [x] 3.1 Update any command docs or examples whose rendered output expectations change because of the service pretty/human fix.
- [x] 3.2 Run the relevant service command and docs test suites and capture any follow-up output issues uncovered by the richer fixtures.
10 changes: 10 additions & 0 deletions openspec/specs/cli-command-test-coverage/spec.md
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,16 @@ Command tests MUST validate output behavior and error semantics for supported ou
- **WHEN** a command fails due to API or validation error
- **THEN** tests MUST assert non-zero error behavior and error details written to stderr without ambiguous output

#### Scenario: Pretty and human rendering for nested responses is covered
- **WHEN** a command returns nested collection or object payloads whose pretty or human rendering differs from raw JSON shape
- **THEN** automated tests MUST assert readable rendered structure in `pretty` and `human` modes
- **AND** the tests MUST catch record-collapsing, field duplication, and sibling-value leakage across rendered items

#### Scenario: Service command output regressions are detected
- **WHEN** `list services` or `get service` output handling changes
- **THEN** automated tests MUST verify supported output modes for representative service payloads including addresses, ports, tags, comments, and annotations
- **AND** tests MUST fail if service records are flattened into an unreadable pretty or human rendering

### Requirement: Configuration precedence is verified
Automated tests MUST verify command configuration precedence of flags over environment variables over config files.

Expand Down
28 changes: 28 additions & 0 deletions openspec/specs/service-command-output/spec.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
## Purpose

Define stable output expectations for service-oriented CLI commands so human-readable rendering stays coherent while structured formats remain faithful to the API schema.

## Requirements

### Requirement: Service commands render stable structured output
`tscli list services` and `tscli get service` SHALL render service payloads as stable structured records across supported output modes without flattening or collapsing sibling service entries together.

#### Scenario: List services pretty output shows one service per record
- **WHEN** `tscli list services` runs in `pretty` mode against a response containing multiple services
- **THEN** the CLI SHALL render each service as a distinct record separated consistently from the next service
- **AND** fields from one service SHALL NOT be merged into or repeated under another service's rendered block

#### Scenario: List services preserves key service properties
- **WHEN** `tscli list services` receives service objects containing properties such as `name`, `addrs`, `ports`, `tags`, `comment`, and `annotations`
- **THEN** the CLI SHALL preserve those properties in structured output modes
- **AND** nested map fields such as `annotations` SHALL remain associated with the correct service record

#### Scenario: Get service pretty output shows a single service record
- **WHEN** `tscli get service --service <name>` runs in `pretty` or `human` mode
- **THEN** the CLI SHALL render the returned service as one structured service record
- **AND** the rendered output SHALL use stable field formatting consistent with other structured `get` commands

#### Scenario: Structured output remains script-friendly
- **WHEN** `tscli list services` or `tscli get service` runs in `json` or `yaml` mode
- **THEN** the CLI SHALL preserve the API response structure without synthetic summaries or lossy field dropping
- **AND** correcting pretty or human rendering SHALL NOT change command flags or request semantics
64 changes: 62 additions & 2 deletions pkg/output/human.go
Original file line number Diff line number Diff line change
Expand Up @@ -88,21 +88,81 @@ func fmtVal(v any) string {
// join scalars, otherwise indicate array length
var parts []string
for _, el := range x {
if s, ok := el.(string); ok && len(s) <= 40 {
if s, ok := inlineVal(el); ok {
parts = append(parts, s)
} else {
return fmt.Sprintf("[%d items]", len(x))
}
}
return strings.Join(parts, " ")
case map[string]any:
return fmt.Sprintf("{%d fields}", len(x))
if s, ok := inlineMap(x); ok {
return s
}
b, _ := json.Marshal(x)
return trunc(string(b))
default:
b, _ := json.Marshal(x)
return trunc(string(b))
}
}

func inlineVal(v any) (string, bool) {
switch x := v.(type) {
case nil:
return "null", true
case string:
return trunc(x), true
case json.Number:
return x.String(), true
case bool:
return fmt.Sprintf("%v", x), true
case []any:
var parts []string
for _, el := range x {
part, ok := inlineVal(el)
if !ok {
return "", false
}
parts = append(parts, part)
}
return "[" + strings.Join(parts, ", ") + "]", true
case map[string]any:
return inlineMap(x)
default:
return "", false
}
}

func inlineMap(m map[string]any) (string, bool) {
if len(m) == 0 {
return "{}", true
}

var keys []string
for k := range maps.Keys(m) {
keys = append(keys, k)
}
sort.Strings(keys)

var parts []string
totalLen := 2 // {}
for _, k := range keys {
part, ok := inlineVal(m[k])
if !ok {
return "", false
}
entry := k + ": " + part
totalLen += len(entry) + 2
if totalLen > 80 {
return "", false
}
parts = append(parts, entry)
}

return "{" + strings.Join(parts, ", ") + "}", true
}

func trunc(s string) string {
if len(s) > 80 {
return s[:77] + "…"
Expand Down
Loading
Loading