Merge pull request #2887 from hashicorp/tmessi-fix-cli-ui-tests

fix(cli): Status code on api error

CHANGELOG.md

@@ -34,6 +34,12 @@ Canonical reference for changes, improvements, and bugfixes for Boundary.
   subcommand, will have the subtype set to `vault-generic`. The deprecated
   subtype and subcommands will be removed in boundary 0.14.0, at which point
   `vault-generic` must be used.
+* In Boundary 0.1.8 using the `-format=json` option with the cli would provide
+  a `status_code` for successful API requests from the cli. However, in the
+  case where an error was returned, the JSON would use `status` instead. This
+  inconsistency has been fixed, with `status_code` being used in both cases.
+  For error cases `status` will still be populated, but is deprecated and will
+  be removed in 0.14.0.
 
 ### New and Improved
 
@@ -68,6 +74,11 @@ Canonical reference for changes, improvements, and bugfixes for Boundary.
 * data warehouse: Fix bug that caused credential dimensions to not get
     associated with session facts ([PR](https://github.com/hashicorp/boundary/pull/2787)).
 * sessions: Fix two authorizeSession race conditions in handleProxy. ([PR](https://github.com/hashicorp/boundary/pull/2795))
+* cli: When using `-format=json` the JSON was inconsistent in how it reported
+  status codes. In successful cases it would use `status_code`, but in error
+  cases it would use `status`. Now `status_code` is used in both cases. In
+  error cases `status` is still populated, see the deprecations above for
+  more details. ([PR](https://github.com/hashicorp/boundary/pull/2887))
 
 ## 0.11.2 (2022/12/09)
 

internal/cmd/base/format.go

@@ -12,6 +12,7 @@ import (
 	"github.com/hashicorp/boundary/api"
 	"github.com/hashicorp/boundary/api/plugins"
 	"github.com/hashicorp/boundary/api/scopes"
+	"github.com/hashicorp/boundary/version"
 	"github.com/mitchellh/cli"
 	"github.com/mitchellh/go-wordwrap"
 	"github.com/pkg/errors"
@@ -181,16 +182,32 @@ func (c *Command) PrintApiError(in *api.Error, contextStr string, opt ...Option)
 	opts := getOpts(opt...)
 	switch Format(c.UI) {
 	case "json":
-		output := struct {
-			Context  string          `json:"context,omitempty"`
-			Status   int             `json:"status"`
-			ApiError json.RawMessage `json:"api_error"`
-		}{
-			Context:  contextStr,
-			Status:   in.Response().StatusCode(),
-			ApiError: in.Response().Body.Bytes(),
+		var b []byte
+		if version.SupportsFeature(version.Binary, version.IncludeStatusInCli) {
+			output := struct {
+				Context    string          `json:"context,omitempty"`
+				StatusCode int             `json:"status_code"`
+				Status     int             `json:"status"`
+				ApiError   json.RawMessage `json:"api_error"`
+			}{
+				Context:    contextStr,
+				StatusCode: in.Response().StatusCode(),
+				Status:     in.Response().StatusCode(),
+				ApiError:   in.Response().Body.Bytes(),
+			}
+			b, _ = JsonFormatter{}.Format(output)
+		} else {
+			output := struct {
+				Context    string          `json:"context,omitempty"`
+				StatusCode int             `json:"status_code"`
+				ApiError   json.RawMessage `json:"api_error"`
+			}{
+				Context:    contextStr,
+				StatusCode: in.Response().StatusCode(),
+				ApiError:   in.Response().Body.Bytes(),
+			}
+			b, _ = JsonFormatter{}.Format(output)
 		}
-		b, _ := JsonFormatter{}.Format(output)
 		c.UI.Error(string(b))
 
 	default:

internal/tests/cli/boundary/_accounts.bash

@@ -8,7 +8,7 @@ function read_account() {
 }
 
 function delete_account() {
-  boundary accounts delete -id $1
+  boundary accounts delete -id $1 -format json
 }
 
 function list_accounts() {

internal/tests/cli/boundary/_credential_libraries.bash

@@ -5,6 +5,11 @@ function create_vault_ssh_certificate_library() {
     create vault-ssh-certificate $@
 }
 
+function update_vault_ssh_certificate_library() {
+  boundary credential-libraries \
+    update vault-ssh-certificate $@
+}
+
 function create_vault_generic_library() {
   boundary credential-libraries \
     create vault-generic $@
@@ -20,7 +25,7 @@ function read_credential_library() {
 }
 
 function delete_credential_library() {
-  boundary credential-libraries delete -id $1
+  boundary credential-libraries delete -id $1 -format json
 }
 
 function list_credential_libraries() {

internal/tests/cli/boundary/_credential_stores.bash

@@ -19,7 +19,7 @@ function read_credential_store() {
 }
 
 function delete_credential_store() {
-  boundary credential-stores delete -id $1
+  boundary credential-stores delete -id $1 -format json
 }
 
 function list_credential_stores() {

internal/tests/cli/boundary/_credentials.bash

@@ -5,7 +5,7 @@ function create_username_password_credential() {
   local sid=$2
   local user=$3
   local pass=$4
-  
+
   export BP="${pass}"
   boundary credentials create username-password \
     -name $name \
@@ -32,7 +32,7 @@ function read_credential() {
 }
 
 function delete_credential() {
-  boundary credentials delete -id $1
+  boundary credentials delete -id $1 -format json
 }
 
 function list_credentials() {
@@ -42,6 +42,6 @@ function list_credentials() {
 function credential_id() {
   local name=$1
   local sid=$2
-  
+
   strip $(list_credentials $sid | jq -c ".items[] | select(.name | contains(\"$name\")) | .[\"id\"]")
 }

internal/tests/cli/boundary/_groups.bash

@@ -9,7 +9,7 @@ function read_group() {
 }
 
 function delete_group() {
-  boundary groups delete -id $1
+  boundary groups delete -id $1 -format json
 }
 
 function list_groups() {
@@ -27,7 +27,7 @@ function group_id() {
 
 function group_member_ids() {
   local gid=$1
-  boundary groups read -id $gid -format json | jq '.item["members"][]["id"]'  
+  boundary groups read -id $gid -format json | jq '.item["members"][]["id"]'
 }
 
 function group_has_member_id() {
@@ -36,10 +36,10 @@ function group_has_member_id() {
   ids=$(group_member_ids $gid)
   for id in $ids; do
     if [ $(strip "$id") == "$mid" ]; then
-      return 0 
+      return 0
     fi
   done
-  return 1 
+  return 1
 }
 
 function has_default_group_actions() {
@@ -51,6 +51,6 @@ function has_default_group_actions() {
     $(has_authorized_action "$out" "$action") || {
       echo "failed to find $action action in output: $out"
       return 1
-    } 
+    }
   done
 }

internal/tests/cli/boundary/_helpers.bash

@@ -24,11 +24,18 @@ function strip_all() {
 function has_status_code() {
   local json=$1
   local code=$2
-  if [ echo "$json"|jq -c ".status_code == $code" ]; then
-    return 1
-  fi
+  echo "checking .status_code == $code in $json"
+  echo "$json" | jq -e ".status_code == $code"
 }
 
 diag() {
     echo "$@" | sed -e 's/^/# /' >&3 ;
 }
+
+function field_eq() {
+    local json=$1
+    local field=$2
+    local expected=$3
+    echo "checking $field == $expected in $json"
+    echo "$json" | jq -e "$field == $expected"
+}

internal/tests/cli/boundary/_host_catalogs.bash

@@ -15,7 +15,7 @@ function read_host_catalog() {
 }
 
 function delete_host_catalog() {
-  boundary host-catalogs delete -id $1
+  boundary host-catalogs delete -id $1 -format json
 }
 
 function list_host_catalogs() {
@@ -35,7 +35,7 @@ function has_default_host_catalog_actions() {
   for action in ${actions[@]}; do
     $(has_authorized_action "$out" "$action") || {
       echo "failed to find $action action in output: $out"
-      return 1 
-    } 
+      return 1
+    }
   done
 }

internal/tests/cli/boundary/_hosts.bash

@@ -3,7 +3,7 @@ load _authorized_actions
 function create_host() {
   local name=$1
   local hcid=$2
-  
+
   boundary hosts create static \
     -name $name \
     -description 'test host' \
@@ -16,7 +16,7 @@ function read_host() {
 }
 
 function delete_host() {
-  boundary hosts delete -id $1
+  boundary hosts delete -id $1 -format json
 }
 
 function list_hosts() {
@@ -26,7 +26,7 @@ function list_hosts() {
 function host_id() {
   local name=$1
   local hcid=$2
-  
+
   strip $(list_hosts $hcid | jq -c ".items[] | select(.name | contains(\"$name\")) | .[\"id\"]")
 }
 
@@ -37,7 +37,7 @@ function has_default_host_actions() {
   for action in ${actions[@]}; do
     $(has_authorized_action "$out" "$action") || {
       echo "failed to find $action action in output: $out"
-      return 1 
-    } 
+      return 1
+    }
   done
 }

internal/tests/cli/boundary/_roles.bash

@@ -4,7 +4,7 @@ function create_role() {
   local sid=$1
   local name=$2
   local gsid=$3
-  
+
   boundary roles create \
     -scope-id $sid \
     -name $name \
@@ -17,7 +17,7 @@ function read_role() {
 }
 
 function delete_role() {
-  boundary roles delete -id $1
+  boundary roles delete -id $1 -format json
 }
 
 function list_roles() {
@@ -27,28 +27,28 @@ function list_roles() {
 function assoc_role_grant() {
   local grant=$1
   local id=$2
-  
+
   boundary roles add-grants -grant $grant -id $id
 }
 
 function assoc_role_principal() {
   local principal=$1
   local id=$2
-  
-  boundary roles add-principals -principal $principal -id $id 
+
+  boundary roles add-principals -principal $principal -id $id
 }
 
 function remove_role_grant() {
   local grant=$1
   local id=$2
-  
+
   boundary roles remove-grants -grant $grant -id $id
 }
 
 function remove_role_principal() {
   local principal=$1
   local id=$2
-  
+
   boundary roles remove-principals -principal $principal -id $id
 }
 
@@ -60,7 +60,7 @@ function role_id() {
 
 function role_principal_ids() {
   local rid=$1
-  strip $(read_role $rid | jq '.item["principals"][]["id"]') 
+  strip $(read_role $rid | jq '.item["principals"][]["id"]')
 }
 
 function role_has_principal_id() {
@@ -69,10 +69,10 @@ function role_has_principal_id() {
   local ids=$(role_principal_ids $rid)
   for id in $ids; do
     if [ $(strip "$id") == "$pid" ]; then
-      return 0 
+      return 0
     fi
   done
-  return 1 
+  return 1
 }
 
 function role_grants() {
@@ -86,10 +86,10 @@ function role_has_grant() {
   local hasgrants=$(role_grants $rid)
   for grant in $hasgrants; do
     if [ $(strip_all "$grant") == "$g" ]; then
-      return 0 
+      return 0
     fi
   done
-  return 1 
+  return 1
 }
 
 function has_default_role_actions() {
@@ -99,7 +99,7 @@ function has_default_role_actions() {
   for action in ${actions[@]}; do
     $(has_authorized_action "$out" "$action") || {
       echo "failed to find $action action in output: $out"
-      return 1 
-    } 
+      return 1
+    }
   done
 }

internal/tests/cli/boundary/_scopes.bash

@@ -16,7 +16,7 @@ function read_scope() {
 function delete_scope() {
   local sid=$1
 
-  boundary scopes delete -id $sid
+  boundary scopes delete -id $sid -format json
 }
 
 function list_scopes() {
@@ -26,7 +26,7 @@ function list_scopes() {
 function scope_id() {
   local name=$1
   local sid=$2
-  
+
   strip $(list_scopes $sid | jq -c ".items[] | select(.name | contains(\"$name\")) | .[\"id\"]")
 }
 
@@ -37,7 +37,7 @@ function has_default_scope_actions() {
   for action in ${actions[@]}; do
     $(has_authorized_action "$out" "$action") || {
       echo "failed to find $action action in output: $out"
-      return 1 
-    } 
+      return 1
+    }
   done
 }