Skip to content
Draft
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
16 changes: 13 additions & 3 deletions pkg/client/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -51,12 +51,22 @@ func APIErrorMessage(status int, body []byte) string {
return "Harness returned an HTML page (possible redirect, proxy, or WAF). Check your API URL and credentials."
}
}
// Try to extract a message from JSON
// Prefer RFC 7807 / FME v4 problem+JSON `detail` over legacy `message`.
var parsed struct {
Message string `json:"message"`
Detail string `json:"detail"`
Title string `json:"title"`
}
if json.Unmarshal(body, &parsed) == nil && parsed.Message != "" {
return parsed.Message
if json.Unmarshal(body, &parsed) == nil {
if parsed.Detail != "" {
return parsed.Detail
}
if parsed.Message != "" {
return parsed.Message
}
if parsed.Title != "" {
return parsed.Title
}
}
if len(body) > 200 {
return string(body[:200]) + "..."
Expand Down
40 changes: 40 additions & 0 deletions pkg/client/error_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
// Copyright © 2026 Harness Inc.
// SPDX-License-Identifier: Apache-2.0

package client

import "testing"

func TestAPIErrorMessage_ProblemJSONUsesDetail(t *testing.T) {
body := []byte(`{"type":"https://developer.harness.io/docs/api-reference/errors#has-dependents","status":400,"title":"Resource Has Dependents","errorType":"hasDependents","detail":"Environment d264a970-9756-11f1-87f2-ca24eb84ddac cannot be archived, it has apitokens associated [tok-1]"}`)
got := APIErrorMessage(400, body)
if want := "Environment d264a970-9756-11f1-87f2-ca24eb84ddac cannot be archived, it has apitokens associated [tok-1]"; got != want {
t.Fatalf("APIErrorMessage = %q, want detail %q", got, want)
}
}

func TestAPIErrorMessage_PrefersDetailOverMessage(t *testing.T) {
body := []byte(`{"message":"generic","detail":"specific dependent list"}`)
got := APIErrorMessage(400, body)
if got != "specific dependent list" {
t.Fatalf("APIErrorMessage = %q, want detail", got)
}
}

func TestAPIErrorMessage_FallsBackToMessage(t *testing.T) {
body := []byte(`{"message":"legacy harness error"}`)
got := APIErrorMessage(400, body)
if got != "legacy harness error" {
t.Fatalf("APIErrorMessage = %q, want message", got)
}
}

func TestAPIErrorMessage_DoesNotTruncateDetail(t *testing.T) {
detail := "Environment abc cannot be archived, it has apitokens associated [" +
"tok-aaaaaaaa-bbbb-cccc-dddd-eeeeeeeeeeee, tok-ffffffff-1111-2222-3333-444444444444]"
body := []byte(`{"errorType":"hasDependents","detail":"` + detail + `"}`)
got := APIErrorMessage(400, body)
if got != detail {
t.Fatalf("truncated or rewritten detail: %q", got)
}
}
12 changes: 11 additions & 1 deletion pkg/registry/buildctx.go
Original file line number Diff line number Diff line change
Expand Up @@ -277,7 +277,17 @@ func buildCtx(cmd *cobra.Command, cs *spec.CommandSpec, args []string, r *Regist
return nil, err
}
for _, f := range cs.Flags {
if f.Required && cmdctx.GetString(ctx.FlagValues, f.Name) == "" {
if !f.Required {
continue
}
var missing bool
switch {
case f.IsArray || f.IsMulti:
missing = len(cmdctx.GetStringSlice(ctx.FlagValues, f.Name)) == 0
default:
missing = cmdctx.GetString(ctx.FlagValues, f.Name) == ""
}
if missing {
if len(f.CompletionValues) > 0 {
return nil, fmt.Errorf("flag --%s is required (%s)", f.Name, strings.Join(f.CompletionValues, ", "))
}
Expand Down
31 changes: 31 additions & 0 deletions pkg/registry/buildctx_workflow_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -159,6 +159,37 @@ func TestBuildCtx_WorkflowRequiredFlag(t *testing.T) {
}
}

func TestBuildCtx_WorkflowRequiredArrayFlag(t *testing.T) {
r := New()
registerWorkflowExecute(t, r, "reqarrayflag", &spec.CommandSpec{
Flags: []spec.Flag{
{Name: "keys", Required: true, IsArray: true, Description: "keys to remove"},
},
})
cs := r.GetSpec(VerbExecute, "reqarrayflag")

t.Run("missing", func(t *testing.T) {
cmd := buildWorkflowTestCmd(t, r, cs)
_, err := buildCtx(cmd, cs, []string{"my-id"}, r)
if err == nil {
t.Fatal("buildCtx() = nil, want error")
}
if !strings.Contains(err.Error(), "flag --keys is required") {
t.Fatalf("buildCtx() error %q missing expected substring", err)
}
})

t.Run("provided", func(t *testing.T) {
cmd := buildWorkflowTestCmd(t, r, cs)
if err := cmd.ParseFlags([]string{"--keys", "key-one,key-two"}); err != nil {
t.Fatalf("ParseFlags: %v", err)
}
if _, err := buildCtx(cmd, cs, []string{"my-id"}, r); err != nil {
t.Fatalf("buildCtx() = %v, want no error", err)
}
})
}

func TestBuildCtx_WorkflowIdPartsTooMany(t *testing.T) {
r := New()
registerWorkflowExecute(t, r, "cluster", &spec.CommandSpec{
Expand Down
10 changes: 5 additions & 5 deletions pkg/registry/endpoint.go
Original file line number Diff line number Diff line change
Expand Up @@ -290,7 +290,7 @@ func RunEndpoint(ctx *cmdctx.Ctx, ep *spec.EndpointSpec) (any, error) {
}
defer closeW()
if ctx.VerbHandler == VerbUpdate || ep.CreateStrategy == spec.CreateStrategySetFields {
return nil, PrintMutableFieldTable(w, MutableFields(resolveNounDef(ctx)))
return nil, PrintMutableFieldTable(w, MutableFields(resolveNounDef(ctx), ep.FieldsExtra))
}
return nil, PrintFieldTable(w, fields)
}
Expand Down Expand Up @@ -769,9 +769,9 @@ func runGetThenUpdate(ctx *cmdctx.Ctx, ep *spec.EndpointSpec, c *client.Client,
return nil, fmt.Errorf("get-then-%s: unmarshaling picked item: %w", strings.ToLower(method), err)
}

// Build a fieldID→FieldDef map from the noun's mutable fields for --set/--del resolution.
// Build a fieldID→FieldDef map from noun + fields_extra mutable paths for --set/--del.
fieldPaths := map[string]spec.FieldDef{}
for _, f := range MutableFields(resolveNounDef(ctx)) {
for _, f := range MutableFields(resolveNounDef(ctx), ep.FieldsExtra) {
fieldPaths[f.ID] = f
}

Expand Down Expand Up @@ -882,9 +882,9 @@ func runSetFields(ctx *cmdctx.Ctx, ep *spec.EndpointSpec, c *client.Client, path
}
}

// Build fieldID→FieldDef map from the noun's mutable fields.
// Build fieldID→FieldDef map from noun + fields_extra mutable paths.
fieldPaths := map[string]spec.FieldDef{}
for _, f := range MutableFields(resolveNounDef(ctx)) {
for _, f := range MutableFields(resolveNounDef(ctx), ep.FieldsExtra) {
fieldPaths[f.ID] = f
}

Expand Down
15 changes: 11 additions & 4 deletions pkg/registry/fields.go
Original file line number Diff line number Diff line change
Expand Up @@ -155,13 +155,20 @@ func (r *Registry) ResolveCommandFields(cs *spec.CommandSpec) []spec.FieldDef {
return append(base, ep.FieldsExtra...)
}

// MutableFields returns only the writable fields for a noun (those with mutable_path set).
func MutableFields(noun *spec.NounDef) []spec.FieldDef {
if noun == nil {
// MutableFields returns writable fields (mutable_path set) from the noun plus
// any command-level fields_extra. Variant commands (e.g. feature_flag:definition)
// declare extra writable paths on the endpoint, not on the noun.
func MutableFields(noun *spec.NounDef, extra []spec.FieldDef) []spec.FieldDef {
var src []spec.FieldDef
if noun != nil {
src = append(src, noun.Fields...)
}
src = append(src, extra...)
if len(src) == 0 {
return nil
}
var out []spec.FieldDef
for _, f := range noun.Fields {
for _, f := range src {
if f.MutablePath != "" {
out = append(out, f)
}
Expand Down
26 changes: 24 additions & 2 deletions pkg/registry/fields_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -328,7 +328,7 @@ func TestResolveCommandFields_FieldsNounOverride(t *testing.T) {
// ---------------------------------------------------------------------------

func TestMutableFields_NilNoun(t *testing.T) {
if got := MutableFields(nil); got != nil {
if got := MutableFields(nil, nil); got != nil {
t.Fatalf("expected nil for nil noun, got %v", got)
}
}
Expand All @@ -338,12 +338,34 @@ func TestMutableFields_FiltersNonMutable(t *testing.T) {
{ID: "name", Expr: "it.name", MutablePath: "name"},
{ID: "status", Expr: "it.status"}, // not mutable
}}
got := MutableFields(nd)
got := MutableFields(nd, nil)
if len(got) != 1 || got[0].ID != "name" {
t.Fatalf("MutableFields = %v, want only 'name'", got)
}
}

func TestMutableFields_IncludesFieldsExtra(t *testing.T) {
nd := &spec.NounDef{Fields: []spec.FieldDef{
{ID: "name", Expr: "it.name", MutablePath: "name"},
{ID: "status", Expr: "it.status"},
}}
extra := []spec.FieldDef{
{ID: "traffic_allocation", Expr: "string(it.trafficAllocation)", MutablePath: "trafficAllocation"},
{ID: "environment", Expr: "it.environment.name"}, // display-only extra
}
got := MutableFields(nd, extra)
if len(got) != 2 {
t.Fatalf("MutableFields = %v, want name + traffic_allocation", got)
}
ids := map[string]string{}
for _, f := range got {
ids[f.ID] = f.MutablePath
}
if ids["name"] != "name" || ids["traffic_allocation"] != "trafficAllocation" {
t.Fatalf("MutableFields paths = %v", ids)
}
}

// ---------------------------------------------------------------------------
// PrintMutableFieldTable
// ---------------------------------------------------------------------------
Expand Down
Loading