diff --git a/CHANGELOG.md b/CHANGELOG.md index fcbbbda238..e612959424 100644 --- a/CHANGELOG.md +++ b/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) diff --git a/internal/cmd/base/format.go b/internal/cmd/base/format.go index a72b3ec038..5db3e1797e 100644 --- a/internal/cmd/base/format.go +++ b/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: diff --git a/internal/tests/cli/boundary/_accounts.bash b/internal/tests/cli/boundary/_accounts.bash index 9444bc31ad..a7fd24abf5 100644 --- a/internal/tests/cli/boundary/_accounts.bash +++ b/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() { diff --git a/internal/tests/cli/boundary/_credential_libraries.bash b/internal/tests/cli/boundary/_credential_libraries.bash index 606399e6b6..6d3c43646e 100644 --- a/internal/tests/cli/boundary/_credential_libraries.bash +++ b/internal/tests/cli/boundary/_credential_libraries.bash @@ -20,7 +20,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() { diff --git a/internal/tests/cli/boundary/_credential_stores.bash b/internal/tests/cli/boundary/_credential_stores.bash index 26a8fb9077..8255564221 100644 --- a/internal/tests/cli/boundary/_credential_stores.bash +++ b/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() { diff --git a/internal/tests/cli/boundary/_credentials.bash b/internal/tests/cli/boundary/_credentials.bash index 4a37b0aada..0d17bfd19a 100644 --- a/internal/tests/cli/boundary/_credentials.bash +++ b/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\"]") } diff --git a/internal/tests/cli/boundary/_groups.bash b/internal/tests/cli/boundary/_groups.bash index ec7b721792..61202e0feb 100644 --- a/internal/tests/cli/boundary/_groups.bash +++ b/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 } diff --git a/internal/tests/cli/boundary/_helpers.bash b/internal/tests/cli/boundary/_helpers.bash index 9a9c331250..efa9d49d3d 100644 --- a/internal/tests/cli/boundary/_helpers.bash +++ b/internal/tests/cli/boundary/_helpers.bash @@ -24,9 +24,8 @@ 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() { diff --git a/internal/tests/cli/boundary/_host_catalogs.bash b/internal/tests/cli/boundary/_host_catalogs.bash index fdfe55a4cf..6b61f3383a 100644 --- a/internal/tests/cli/boundary/_host_catalogs.bash +++ b/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 } diff --git a/internal/tests/cli/boundary/_hosts.bash b/internal/tests/cli/boundary/_hosts.bash index 7be00c4ef9..3a14b619c3 100644 --- a/internal/tests/cli/boundary/_hosts.bash +++ b/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 } diff --git a/internal/tests/cli/boundary/_roles.bash b/internal/tests/cli/boundary/_roles.bash index 55b026f112..f689f9c7ae 100644 --- a/internal/tests/cli/boundary/_roles.bash +++ b/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 } diff --git a/internal/tests/cli/boundary/_scopes.bash b/internal/tests/cli/boundary/_scopes.bash index 84032c3524..8624adca05 100644 --- a/internal/tests/cli/boundary/_scopes.bash +++ b/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 } diff --git a/internal/tests/cli/boundary/_targets.bash b/internal/tests/cli/boundary/_targets.bash index 5e7644d152..ae786bf727 100644 --- a/internal/tests/cli/boundary/_targets.bash +++ b/internal/tests/cli/boundary/_targets.bash @@ -32,7 +32,7 @@ function read_target() { } function delete_target() { - boundary targets delete -id $1 + boundary targets delete -id $1 -format json } function list_targets() { @@ -65,20 +65,20 @@ function target_id_from_name() { function target_host_source_ids() { local tid=$1 - boundary targets read -id $tid -format json | jq '.item.host_sources[].id' + boundary targets read -id $tid -format json | jq '.item.host_sources[].id' } function target_has_host_source_id() { local tid=$1 local hsid=$2 - ids=$(target_host_source_ids $tid) + ids=$(target_host_source_ids $tid) for id in $ids; do if [ $(strip "$id") == "$hsid" ]; then - return 0 + return 0 fi done - return 1 + return 1 } function has_default_target_actions() { @@ -88,7 +88,7 @@ function has_default_target_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 } diff --git a/internal/tests/cli/boundary/_users.bash b/internal/tests/cli/boundary/_users.bash index 315bef4b9b..92b4b5e284 100644 --- a/internal/tests/cli/boundary/_users.bash +++ b/internal/tests/cli/boundary/_users.bash @@ -9,7 +9,7 @@ function read_user() { } function delete_user() { - boundary users delete -id $1 + boundary users delete -id $1 -format json } function list_users() { @@ -28,8 +28,8 @@ function has_default_user_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 } diff --git a/internal/tests/cli/boundary/_workers.bash b/internal/tests/cli/boundary/_workers.bash index a429e589f1..adb0c8c615 100644 --- a/internal/tests/cli/boundary/_workers.bash +++ b/internal/tests/cli/boundary/_workers.bash @@ -15,7 +15,7 @@ function update_worker() { } function delete_worker() { - boundary workers delete -id $1 + boundary workers delete -id $1 -format json } function list_workers() { @@ -34,7 +34,7 @@ function has_default_worker_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 } diff --git a/internal/tests/cli/boundary/credential_libraries.bats b/internal/tests/cli/boundary/credential_libraries.bats index 10077ff4a3..c26405bf9d 100755 --- a/internal/tests/cli/boundary/credential_libraries.bats +++ b/internal/tests/cli/boundary/credential_libraries.bats @@ -73,6 +73,13 @@ export NEW_VAULT_LIB="test_vault" [ "$status" -eq 0 ] run has_status_code "$output" "204" [ "$status" -eq 0 ] + + run delete_credential_library $clid + echo "$output" + [ "$status" -eq 1 ] + run has_status_code "$output" "404" + echo "$output" + [ "$status" -eq 0 ] } @test "boundary/credential-libraries: can create $NEW_VAULT_LIB vault library in credential store $NEW_STORE deprecated subcommand" { diff --git a/version/feature_manager.go b/version/feature_manager.go index 8995c73dcc..0628d9e766 100644 --- a/version/feature_manager.go +++ b/version/feature_manager.go @@ -23,6 +23,7 @@ type Feature int const ( UnknownFeature Feature = iota MultiHopSessionFeature + IncludeStatusInCli ) var featureMap map[Feature]MetadataConstraint @@ -52,6 +53,10 @@ func init() { Constraints: mustNewConstraints(">= 0.1.0"), // This feature exists at 0.1.0 and above } */ + featureMap[IncludeStatusInCli] = MetadataConstraint{ + MetaInfo: []Metadata{OSS, HCP}, + Constraints: mustNewConstraints("< 0.14.0"), + } } func metadataStringToMetadata(m string) Metadata {