diff --git a/apps/cinc/cmd/acl.go b/apps/cinc/cmd/acl.go index a93cf54..6e00177 100644 --- a/apps/cinc/cmd/acl.go +++ b/apps/cinc/cmd/acl.go @@ -282,8 +282,10 @@ func explainACLChangeError(err error, grant bool, target, members string, groups // error itself when it isn't a server response. func serverMessage(err error) string { var resp *cinc.ErrorResponse - if errors.As(err, &resp) && len(resp.Messages) > 0 { - return strings.Join(resp.Messages, "; ") + if errors.As(err, &resp) { + if msg := resp.ServerMessage(); msg != "" { + return msg + } } return err.Error() } diff --git a/apps/cinc/cmd/node.go b/apps/cinc/cmd/node.go index 9aa1528..6d1f3e4 100644 --- a/apps/cinc/cmd/node.go +++ b/apps/cinc/cmd/node.go @@ -3,6 +3,8 @@ package cmd import ( "bufio" "fmt" + "maps" + "slices" "strings" "text/tabwriter" "time" @@ -581,10 +583,33 @@ func nodeSSHHost(node *cinc.Node, attr string) (string, error) { if attr == "name" { return node.Name, nil } - if _, ok := node.Attribute(attr); !ok { + value, ok := node.Attribute(attr) + if !ok { return "", fmt.Errorf("node %q has no %q attribute to connect to. Pick another with --attribute, or pass --attribute name to use the node name", node.Name, attr) } - return node.AttributeString(attr), nil + host, ok := node.AttributeScalar(attr) + if !ok { + return "", fmt.Errorf("node %q's %q attribute isn't a single value we can connect to%s", node.Name, attr, nestedAttributeHint(attr, value)) + } + return host, nil +} + +// nestedAttributeHint suggests dotted paths into an attribute that holds an +// object, e.g. "; try --attribute cloud.public_hostname". +func nestedAttributeHint(attr string, value any) string { + m, ok := value.(map[string]any) + if !ok || len(m) == 0 { + return "" + } + keys := slices.Sorted(maps.Keys(m)) + if len(keys) > 3 { + keys = keys[:3] + } + paths := make([]string, len(keys)) + for i, k := range keys { + paths[i] = attr + "." + k + } + return "; it holds an object, so name a key inside it, such as --attribute " + strings.Join(paths, " or ") } func remoteOptions(flags nodeSSHFlags) remote.SSHOptions { diff --git a/apps/cinc/cmd/node_test.go b/apps/cinc/cmd/node_test.go index b922c00..eea87a3 100644 --- a/apps/cinc/cmd/node_test.go +++ b/apps/cinc/cmd/node_test.go @@ -922,3 +922,21 @@ func TestNodeSSHTargetsReportsUndecodableRow(t *testing.T) { t.Errorf("node ssh error = %v, want the decode failure", err) } } + +// An attribute that holds an object can't be a host to connect to. Before, +// its Go map text ("map[...]") went to SSH as the host name. +func TestNodeSSHHostRejectsNonScalarAttribute(t *testing.T) { + node := &cinc.Node{Name: "web01", Automatic: cinc.Attributes{"cloud": map[string]any{"public_hostname": "web01.cloud.test"}}} + _, err := nodeSSHHost(node, "cloud") + if err == nil { + t.Fatal("want an error for an attribute that holds an object") + } + for _, want := range []string{"web01", `"cloud"`, "cloud.public_hostname"} { + if !strings.Contains(err.Error(), want) { + t.Errorf("error %q should mention %s", err, want) + } + } + if strings.Contains(err.Error(), "map[") { + t.Errorf("error %q leaks Go map formatting", err) + } +}