fix(server/show): include security_groups in JSON/YAML output (closes #191)

cmd/server/list.go

@@ -62,6 +62,30 @@ var listCmd = &cobra.Command{
 	},
 }
 
+// serverShowDetail is the JSON/YAML/CSV shape of `server show`. It embeds
+// the raw model.Server (so existing fields keep their tag names) and adds
+// security_groups + ports, which are derived from a separate /v2.0/ports
+// query (#191). Only non-nil fields appear in JSON output thanks to
+// omitempty, so name-only consumers stay backwards-compatible.
+type serverShowDetail struct {
+	*model.Server  `yaml:",inline"`
+	SecurityGroups []string                 `json:"security_groups,omitempty" yaml:"security_groups,omitempty"`
+	Ports          []serverShowDetailPort   `json:"ports,omitempty" yaml:"ports,omitempty"`
+	Volumes        []serverShowDetailVolume `json:"volumes,omitempty" yaml:"volumes,omitempty"`
+}
+
+type serverShowDetailPort struct {
+	ID         string   `json:"id" yaml:"id"`
+	MACAddress string   `json:"mac_address" yaml:"mac_address"`
+	IPs        []string `json:"ips" yaml:"ips"`
+}
+
+type serverShowDetailVolume struct {
+	ID     string `json:"id" yaml:"id"`
+	Device string `json:"device" yaml:"device"`
+	SizeGB int    `json:"size_gb,omitempty" yaml:"size_gb,omitempty"`
+}
+
 var showCmd = &cobra.Command{
 	Use:   "show <id|name>",
 	Short: "Show server details",
@@ -77,11 +101,6 @@ var showCmd = &cobra.Command{
 			return err
 		}
 
-		format := cmdutil.GetFormat(cmd)
-		if format != "" && format != "table" {
-			return output.New(format).Format(os.Stdout, server)
-		}
-
 		// Resolve flavor name
 		flavorDisplay := server.Flavor.ID
 		if f, err := compute.GetFlavor(server.Flavor.ID); err == nil {
@@ -119,61 +138,92 @@ var showCmd = &cobra.Command{
 			}
 		}
 
-		// Human-readable key-value output
-		printServerDetail(server, flavorDisplay, imageDisplay)
-
-		// Volume attachments (non-fatal)
-		if len(attachments) > 0 {
-			fmt.Println("Volumes:")
-			for _, a := range attachments {
-				size := ""
-				if vol, err := volumeAPI.GetVolume(a.VolumeID); err == nil {
-					size = fmt.Sprintf(" %dGB", vol.Size)
-				}
-				fmt.Printf("  %s %s%s\n", a.VolumeID, a.Device, size)
-			}
-		}
-
-		// Ports and Security Groups (non-fatal)
+		// Resolve ports + security groups via the network API. Used by both
+		// the table renderer (existing behaviour) and structured-format
+		// output (#191 — JSON/YAML/CSV previously dropped this entirely).
 		networkAPI := api.NewNetworkAPI(client)
-		if ports, err := networkAPI.ListPortsByDevice(server.ID); err == nil && len(ports) > 0 {
-			// Build SG ID-to-name map
+		ports, _ := networkAPI.ListPortsByDevice(server.ID)
+		var sgNames []string
+		var detailPorts []serverShowDetailPort
+		if len(ports) > 0 {
 			sgMap := make(map[string]string)
 			if sgs, err := networkAPI.ListSecurityGroups(); err == nil {
 				for _, sg := range sgs {
 					sgMap[sg.ID] = sg.Name
 				}
 			}
-
-			fmt.Println("Ports:")
+			detailPorts = make([]serverShowDetailPort, 0, len(ports))
 			for _, p := range ports {
-				var ips []string
+				ips := make([]string, 0, len(p.FixedIPs))
 				for _, ip := range p.FixedIPs {
 					ips = append(ips, ip.IPAddress)
 				}
-				fmt.Printf("  %s mac=%s ips=[%s]\n", p.ID, p.MACAddress, strings.Join(ips, ","))
+				detailPorts = append(detailPorts, serverShowDetailPort{
+					ID:         p.ID,
+					MACAddress: p.MACAddress,
+					IPs:        ips,
+				})
 			}
-
-			// Collect unique SGs across all ports
 			sgSeen := make(map[string]bool)
-			var sgNames []string
 			for _, p := range ports {
 				for _, sgID := range p.SecurityGroups {
-					if !sgSeen[sgID] {
-						sgSeen[sgID] = true
-						name := sgMap[sgID]
-						if name == "" {
-							name = sgID
-						}
-						sgNames = append(sgNames, name)
+					if sgSeen[sgID] {
+						continue
 					}
+					sgSeen[sgID] = true
+					name := sgMap[sgID]
+					if name == "" {
+						name = sgID
+					}
+					sgNames = append(sgNames, name)
 				}
 			}
-			if len(sgNames) > 0 {
-				fmt.Println("Security Groups:")
-				for _, name := range sgNames {
-					fmt.Printf("  %s\n", name)
+		}
+
+		format := cmdutil.GetFormat(cmd)
+		if format != "" && format != "table" {
+			detailVolumes := make([]serverShowDetailVolume, 0, len(attachments))
+			for _, a := range attachments {
+				v := serverShowDetailVolume{ID: a.VolumeID, Device: a.Device}
+				if vol, err := volumeAPI.GetVolume(a.VolumeID); err == nil {
+					v.SizeGB = vol.Size
 				}
+				detailVolumes = append(detailVolumes, v)
+			}
+			detail := serverShowDetail{
+				Server:         server,
+				SecurityGroups: sgNames,
+				Ports:          detailPorts,
+				Volumes:        detailVolumes,
+			}
+			return output.New(format).Format(os.Stdout, detail)
+		}
+
+		// Human-readable key-value output
+		printServerDetail(server, flavorDisplay, imageDisplay)
+
+		// Volume attachments (non-fatal)
+		if len(attachments) > 0 {
+			fmt.Println("Volumes:")
+			for _, a := range attachments {
+				size := ""
+				if vol, err := volumeAPI.GetVolume(a.VolumeID); err == nil {
+					size = fmt.Sprintf(" %dGB", vol.Size)
+				}
+				fmt.Printf("  %s %s%s\n", a.VolumeID, a.Device, size)
+			}
+		}
+
+		if len(detailPorts) > 0 {
+			fmt.Println("Ports:")
+			for _, p := range detailPorts {
+				fmt.Printf("  %s mac=%s ips=[%s]\n", p.ID, p.MACAddress, strings.Join(p.IPs, ","))
+			}
+		}
+		if len(sgNames) > 0 {
+			fmt.Println("Security Groups:")
+			for _, name := range sgNames {
+				fmt.Printf("  %s\n", name)
 			}
 		}
 

cmd/server/list_test.go

@@ -0,0 +1,88 @@
+package server
+
+import (
+	"encoding/json"
+	"strings"
+	"testing"
+
+	"gopkg.in/yaml.v3"
+
+	"github.com/crowdy/conoha-cli/internal/model"
+)
+
+// #191: server show --format json must include security_groups, not null.
+// The detail struct embeds *model.Server so existing fields stay at the
+// top level of the JSON object (and YAML mapping).
+func TestServerShowDetail_JSONShape(t *testing.T) {
+	d := serverShowDetail{
+		Server:         &model.Server{ID: "srv-abc", Name: "web1", Status: "ACTIVE"},
+		SecurityGroups: []string{"IPv4v6-SSH", "3000-9999"},
+		Ports: []serverShowDetailPort{
+			{ID: "port-1", MACAddress: "fa:16:3e:01:02:03", IPs: []string{"10.0.0.5"}},
+		},
+		Volumes: []serverShowDetailVolume{
+			{ID: "vol-1", Device: "/dev/vda", SizeGB: 100},
+		},
+	}
+
+	b, err := json.Marshal(d)
+	if err != nil {
+		t.Fatalf("json.Marshal: %v", err)
+	}
+	out := string(b)
+
+	for _, want := range []string{
+		`"id":"srv-abc"`,
+		`"name":"web1"`,
+		`"security_groups":["IPv4v6-SSH","3000-9999"]`,
+		`"ports":[`,
+		`"volumes":[`,
+	} {
+		if !strings.Contains(out, want) {
+			t.Errorf("expected JSON to contain %q, got: %s", want, out)
+		}
+	}
+
+	// Round-trip: structured consumers must be able to read security_groups.
+	var parsed map[string]any
+	if err := json.Unmarshal(b, &parsed); err != nil {
+		t.Fatalf("json.Unmarshal: %v", err)
+	}
+	sgs, ok := parsed["security_groups"].([]any)
+	if !ok || len(sgs) != 2 {
+		t.Errorf("security_groups not a 2-element array: %v", parsed["security_groups"])
+	}
+}
+
+func TestServerShowDetail_YAMLShape(t *testing.T) {
+	d := serverShowDetail{
+		Server:         &model.Server{ID: "srv-abc", Name: "web1"},
+		SecurityGroups: []string{"web"},
+	}
+	b, err := yaml.Marshal(d)
+	if err != nil {
+		t.Fatalf("yaml.Marshal: %v", err)
+	}
+	out := string(b)
+	// embedded *model.Server should be inlined (no `server:` nesting)
+	if strings.Contains(out, "Server:") || strings.Contains(out, "server:") {
+		t.Errorf("expected embedded struct to be inlined, got: %s", out)
+	}
+	if !strings.Contains(out, "id: srv-abc") || !strings.Contains(out, "security_groups:") {
+		t.Errorf("expected flattened YAML, got: %s", out)
+	}
+}
+
+// When SecurityGroups is empty, omitempty must drop the field rather
+// than emitting `"security_groups":null` (which would re-introduce the
+// original bug shape).
+func TestServerShowDetail_EmptySGsOmitted(t *testing.T) {
+	d := serverShowDetail{Server: &model.Server{ID: "srv-x"}}
+	b, err := json.Marshal(d)
+	if err != nil {
+		t.Fatalf("json.Marshal: %v", err)
+	}
+	if strings.Contains(string(b), "security_groups") {
+		t.Errorf("expected security_groups to be omitted when nil, got: %s", string(b))
+	}
+}