From ba21e076bb6713aade1dd1f62e3324fc08a61e48 Mon Sep 17 00:00:00 2001 From: Irena Rindos Date: Tue, 13 Feb 2024 15:56:13 -0500 Subject: [PATCH] cli(aliases): Support aliases for connect and targets in the CLI (#4356) * cli(aliases): Support aliases for boundary connect and boundary targets CLI --- internal/cmd/base/base.go | 9 ++ internal/cmd/base/base_test.go | 75 +++++++++++++ .../cmd/commands/aliasescmd/target_funcs.go | 7 ++ internal/cmd/commands/connect/connect.go | 9 ++ .../commands/targetscmd/ssh_targets.gen.go | 11 ++ .../cmd/commands/targetscmd/targets.gen.go | 11 ++ .../commands/targetscmd/tcp_targets.gen.go | 11 ++ internal/cmd/gencli/input.go | 92 ++++++++++------ internal/cmd/gencli/templates.go | 13 +++ internal/tests/cli/boundary/_aliases.bash | 70 ++++++++++++ internal/tests/cli/boundary/_connect.bash | 6 + .../tests/cli/boundary/_targets_alias.bash | 58 ++++++++++ internal/tests/cli/boundary/alias.bats | 102 +++++++++++++++++ .../tests/cli/boundary/target_aliases.bats | 103 ++++++++++++++++++ 14 files changed, 541 insertions(+), 36 deletions(-) create mode 100644 internal/cmd/base/base_test.go create mode 100644 internal/tests/cli/boundary/_aliases.bash create mode 100644 internal/tests/cli/boundary/_targets_alias.bash create mode 100644 internal/tests/cli/boundary/alias.bats create mode 100644 internal/tests/cli/boundary/target_aliases.bats diff --git a/internal/cmd/base/base.go b/internal/cmd/base/base.go index c59836aac..e6b324c27 100644 --- a/internal/cmd/base/base.go +++ b/internal/cmd/base/base.go @@ -343,6 +343,15 @@ func (c *Command) Client(opt ...Option) (*api.Client, error) { return c.client, nil } +// If the first arg isn't a flag, extract it as the alias and return the remaining args +func ExtractAliasFromArgs(inArgs []string) (string, []string) { + if len(inArgs) > 0 && inArgs[0][0] != '-' { + return inArgs[0], inArgs[1:] + } + + return "", inArgs +} + type FlagSetBit uint const ( diff --git a/internal/cmd/base/base_test.go b/internal/cmd/base/base_test.go new file mode 100644 index 000000000..7faecb87a --- /dev/null +++ b/internal/cmd/base/base_test.go @@ -0,0 +1,75 @@ +// Copyright (c) HashiCorp, Inc. +// SPDX-License-Identifier: BUSL-1.1 + +package base + +import ( + "testing" + + "github.com/stretchr/testify/assert" +) + +func Test_extractAliasFromArgs(t *testing.T) { + tests := []struct { + name string + args []string + wantArgs []string + wantAlias string + }{ + { + name: "no-args", + }, + { + name: "no-alias", + args: []string{"-flag", "value"}, + wantArgs: []string{"-flag", "value"}, + }, + { + name: "no-alias-equal-flag", + args: []string{"-flag=value"}, + wantArgs: []string{"-flag=value"}, + }, + { + name: "alias-and-flag", + args: []string{"alias.value", "-flag", "value"}, + wantArgs: []string{"-flag", "value"}, + wantAlias: "alias.value", + }, + { + name: "alias-and-flags", + args: []string{"alias.value", "-flag", "value", "-flag2", "value2"}, + wantArgs: []string{"-flag", "value", "-flag2", "value2"}, + wantAlias: "alias.value", + }, + { + name: "alias-and-equal-flag", + args: []string{"alias.value", "-flag=value"}, + wantArgs: []string{"-flag=value"}, + wantAlias: "alias.value", + }, + { + name: "alias-and-equal-flags", + args: []string{"alias.value", "-flag=value", "-flag2=value2"}, + wantArgs: []string{"-flag=value", "-flag2=value2"}, + wantAlias: "alias.value", + }, + { + name: "alias-mixed-flags", + args: []string{"alias.value", "-flag", "value", "-flag=value"}, + wantArgs: []string{"-flag", "value", "-flag=value"}, + wantAlias: "alias.value", + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + assert := assert.New(t) + + alias, args := ExtractAliasFromArgs(tt.args) + assert.Equal(args, tt.wantArgs) + + if tt.wantAlias != "" { + assert.Equal(alias, tt.wantAlias) + } + }) + } +} diff --git a/internal/cmd/commands/aliasescmd/target_funcs.go b/internal/cmd/commands/aliasescmd/target_funcs.go index 961c3cce1..d90a20748 100644 --- a/internal/cmd/commands/aliasescmd/target_funcs.go +++ b/internal/cmd/commands/aliasescmd/target_funcs.go @@ -108,6 +108,13 @@ func extraTargetFlagHandlingFuncImpl(c *TargetCommand, _ *base.FlagSets, opts *[ attributes[name] = value } + switch c.flagValue { + case "": + case "null": + default: + *opts = append(*opts, aliases.WithValue(c.flagValue)) + } + switch c.flagDestinationId { case "": case "null": diff --git a/internal/cmd/commands/connect/connect.go b/internal/cmd/commands/connect/connect.go index e840506ba..a54d218cc 100644 --- a/internal/cmd/commands/connect/connect.go +++ b/internal/cmd/commands/connect/connect.go @@ -257,6 +257,9 @@ func (c *Command) Run(args []string) (retCode int) { f := c.Flags() + var alias string + alias, args = base.ExtractAliasFromArgs(args) + if err := f.Parse(args); err != nil { c.PrintCliError(err) return base.CommandUserError @@ -271,6 +274,12 @@ func (c *Command) Run(args []string) (retCode int) { defer c.proxyCancel() switch { + case alias != "": + if c.flagTargetId != "" && (c.flagTargetName != "" || c.FlagScopeId != "" || c.FlagScopeName != "") { + c.PrintCliError(errors.New("Cannot specify a Target alias and also other lookup parameters")) + return base.CommandUserError + } + c.flagTargetId = alias case c.flagAuthzToken != "": switch { case c.flagTargetId != "": diff --git a/internal/cmd/commands/targetscmd/ssh_targets.gen.go b/internal/cmd/commands/targetscmd/ssh_targets.gen.go index b2ca443fa..c57f0ff95 100644 --- a/internal/cmd/commands/targetscmd/ssh_targets.gen.go +++ b/internal/cmd/commands/targetscmd/ssh_targets.gen.go @@ -120,6 +120,17 @@ func (c *SshCommand) Run(args []string) int { f := c.Flags() + var alias string + alias, args = base.ExtractAliasFromArgs(args) + + if alias != "" { + if c.FlagId != "" { + c.PrintCliError(errors.New("Cannot specify both an alias and id; choose one or the other")) + return base.CommandUserError + } + c.FlagId = alias + } + if err := f.Parse(args); err != nil { c.PrintCliError(err) return base.CommandUserError diff --git a/internal/cmd/commands/targetscmd/targets.gen.go b/internal/cmd/commands/targetscmd/targets.gen.go index 4aa222e48..e5730bac9 100644 --- a/internal/cmd/commands/targetscmd/targets.gen.go +++ b/internal/cmd/commands/targetscmd/targets.gen.go @@ -142,6 +142,17 @@ func (c *Command) Run(args []string) int { f := c.Flags() + var alias string + alias, args = base.ExtractAliasFromArgs(args) + + if alias != "" { + if c.FlagId != "" { + c.PrintCliError(errors.New("Cannot specify both an alias and id; choose one or the other")) + return base.CommandUserError + } + c.FlagId = alias + } + if err := f.Parse(args); err != nil { c.PrintCliError(err) return base.CommandUserError diff --git a/internal/cmd/commands/targetscmd/tcp_targets.gen.go b/internal/cmd/commands/targetscmd/tcp_targets.gen.go index ff068c591..69a410dff 100644 --- a/internal/cmd/commands/targetscmd/tcp_targets.gen.go +++ b/internal/cmd/commands/targetscmd/tcp_targets.gen.go @@ -120,6 +120,17 @@ func (c *TcpCommand) Run(args []string) int { f := c.Flags() + var alias string + alias, args = base.ExtractAliasFromArgs(args) + + if alias != "" { + if c.FlagId != "" { + c.PrintCliError(errors.New("Cannot specify both an alias and id; choose one or the other")) + return base.CommandUserError + } + c.FlagId = alias + } + if err := f.Parse(args); err != nil { c.PrintCliError(err) return base.CommandUserError diff --git a/internal/cmd/gencli/input.go b/internal/cmd/gencli/input.go index 376906af6..4c1fd3d96 100644 --- a/internal/cmd/gencli/input.go +++ b/internal/cmd/gencli/input.go @@ -91,6 +91,17 @@ type cmdInfo struct { SkipClientCallActions []string SkipFiltering bool + + // UsesAlias controls whether to attempt to extract aliases from the CLI args + UsesAlias bool + + // AliasFieldFlag controls which command flag to substitute a found alias for + AliasFieldFlag string + + // FlagNameOverwrittenByAlias is used in the CLI template to populate an error message. + // It controls what to print as the field replaced by an alias + // if both an alias and the FlagNameOverwrittenByAlias are provided. + FlagNameOverwrittenByAlias string } var inputStructs = map[string][]*cmdInfo{ @@ -665,46 +676,55 @@ var inputStructs = map[string][]*cmdInfo{ }, "targets": { { - ResourceType: resource.Target.String(), - Pkg: "targets", - StdActions: []string{"read", "delete", "list"}, - HasExtraCommandVars: true, - HasExtraHelpFunc: true, - HasExampleCliOutput: true, - HasName: true, - HasDescription: true, - Container: "Scope", - VersionedActions: []string{"add-host-sources", "remove-host-sources", "set-host-sources", "add-credential-sources", "remove-credential-sources", "set-credential-sources"}, + ResourceType: resource.Target.String(), + Pkg: "targets", + StdActions: []string{"read", "delete", "list"}, + HasExtraCommandVars: true, + HasExtraHelpFunc: true, + HasExampleCliOutput: true, + HasName: true, + HasDescription: true, + Container: "Scope", + VersionedActions: []string{"add-host-sources", "remove-host-sources", "set-host-sources", "add-credential-sources", "remove-credential-sources", "set-credential-sources"}, + UsesAlias: true, + AliasFieldFlag: "FlagId", + FlagNameOverwrittenByAlias: "id", }, { - ResourceType: resource.Target.String(), - Pkg: "targets", - StdActions: []string{"create", "update"}, - SubActionPrefix: "tcp", - HasExtraCommandVars: true, - SkipNormalHelp: true, - HasExtraHelpFunc: true, - HasId: true, - HasName: true, - Container: "Scope", - HasDescription: true, - VersionedActions: []string{"update"}, - NeedsSubtypeInCreate: true, + ResourceType: resource.Target.String(), + Pkg: "targets", + StdActions: []string{"create", "update"}, + SubActionPrefix: "tcp", + HasExtraCommandVars: true, + SkipNormalHelp: true, + HasExtraHelpFunc: true, + HasId: true, + HasName: true, + Container: "Scope", + HasDescription: true, + VersionedActions: []string{"update"}, + NeedsSubtypeInCreate: true, + UsesAlias: true, + AliasFieldFlag: "FlagId", + FlagNameOverwrittenByAlias: "id", }, { - ResourceType: resource.Target.String(), - Pkg: "targets", - StdActions: []string{"create", "update"}, - SubActionPrefix: "ssh", - HasExtraCommandVars: true, - SkipNormalHelp: true, - HasExtraHelpFunc: true, - HasId: true, - HasName: true, - Container: "Scope", - HasDescription: true, - VersionedActions: []string{"update"}, - NeedsSubtypeInCreate: true, + ResourceType: resource.Target.String(), + Pkg: "targets", + StdActions: []string{"create", "update"}, + SubActionPrefix: "ssh", + HasExtraCommandVars: true, + SkipNormalHelp: true, + HasExtraHelpFunc: true, + HasId: true, + HasName: true, + Container: "Scope", + HasDescription: true, + VersionedActions: []string{"update"}, + NeedsSubtypeInCreate: true, + UsesAlias: true, + AliasFieldFlag: "FlagId", + FlagNameOverwrittenByAlias: "id", }, }, "users": { diff --git a/internal/cmd/gencli/templates.go b/internal/cmd/gencli/templates.go index 2880dc1c2..440740520 100644 --- a/internal/cmd/gencli/templates.go +++ b/internal/cmd/gencli/templates.go @@ -287,6 +287,19 @@ func (c *{{ camelCase .SubActionPrefix }}Command) Run(args []string) int { f := c.Flags() + {{ if .UsesAlias }} + var alias string + alias, args = base.ExtractAliasFromArgs(args) + + if alias != ""{ + if c.{{ .AliasFieldFlag }} != "" { + c.PrintCliError(errors.New("Cannot specify both an alias and {{ .FlagNameOverwrittenByAlias }}; choose one or the other")) + return base.CommandUserError + } + c.{{ .AliasFieldFlag }} = alias + } + {{ end }} + if err := f.Parse(args); err != nil { c.PrintCliError(err) return base.CommandUserError diff --git a/internal/tests/cli/boundary/_aliases.bash b/internal/tests/cli/boundary/_aliases.bash new file mode 100644 index 000000000..2c6bf37c6 --- /dev/null +++ b/internal/tests/cli/boundary/_aliases.bash @@ -0,0 +1,70 @@ +# Copyright (c) HashiCorp, Inc. +# SPDX-License-Identifier: BUSL-1.1 + +load _authorized_actions + +function create_target_alias() { + local value=$1 + local destid=$2 + boundary aliases create target \ + -value $value \ + -destination-id $destid \ + -format json +} + +function read_alias(){ + boundary aliases read -id $1 -format json +} + +function delete_alias(){ + boundary aliases delete -id $1 -format json +} + +function list_alias(){ + boundary aliases list -format json +} + +function update_target_alias_value(){ + local aid=$1 + local value=$2 + boundary aliases update target \ + -id $aid \ + -value $value \ + -format json +} + +function update_target_alias_host_id(){ + local aid=$1 + local hostid=$2 + boundary aliases update target \ + -id $aid \ + -authorize-session-host-id $hostid \ + -format json +} + +function update_target_alias_remove_destination_id(){ + local aid=$1 + boundary aliases update target \ + -id $aid \ + -destination-id null \ + -format json +} + +function update_target_alias_destination_id(){ + local aid=$1 + local destid=$2 + boundary aliases update target \ + -id $aid \ + -destination-id $destid \ + -format json +} + +function alias_id_from_target_alias(){ + local alias=$1 + strip $(list_alias | jq -c ".items[] | select(.value | contains(\"$alias\")) | .[\"id\"]") +} + +function host_id_from_alias_id(){ + local aid=$1 + strip $(read_alias $aid | jq -c ".item | .attributes | .authorize_session_arguments | .host_id") +} \ No newline at end of file diff --git a/internal/tests/cli/boundary/_connect.bash b/internal/tests/cli/boundary/_connect.bash index 95a8b6eba..9242ce7a0 100644 --- a/internal/tests/cli/boundary/_connect.bash +++ b/internal/tests/cli/boundary/_connect.bash @@ -6,3 +6,9 @@ function connect_nc() { # Note: When this command returns, the session immediately goes into a "canceling" state echo "foo" | boundary connect -exec nc -target-id $id -- {{boundary.ip}} {{boundary.port}} } + +function connect_alias() { + local alias=$1 + # Note: When this command returns, the session immediately goes into a "canceling" state + echo "foo" | boundary connect $alias -exec nc -- {{boundary.ip}} {{boundary.port}} +} \ No newline at end of file diff --git a/internal/tests/cli/boundary/_targets_alias.bash b/internal/tests/cli/boundary/_targets_alias.bash new file mode 100644 index 000000000..7d757d3e5 --- /dev/null +++ b/internal/tests/cli/boundary/_targets_alias.bash @@ -0,0 +1,58 @@ +# Copyright (c) HashiCorp, Inc. +# SPDX-License-Identifier: BUSL-1.1 + +load _authorized_actions + +function read_target_by_alias() { + boundary targets read $1 -format json +} + +function update_address_by_alias() { + local aid=$1 + local addr=$2 + boundary targets update tcp $aid -address $2 +} + +function delete_target_by_alias() { + boundary targets delete $1 -format json +} + +function add_target_host_sources_by_alias() { + local tid=$1 + local hostSources=${@:2} + for i in "${hostSources}"; do + hostSource+="-host-source $i " + done + + boundary targets add-host-sources $tid $hostSource +} + +function target_has_host_source_id_by_alias() { + local tid=$1 + local format=$2 + local hostSources=${@:3} + + if [[ "$format" == "json" ]]; then + ids=$(boundary targets read $tid -format json | jq '.item.host_sources[].id') + for i in "${hostSources}"; do + local hsid=$i + for id in $ids; do + if [ $(strip "$id") == "$hsid" ]; then + return 0 + fi + done + done + return 1 + elif [[ "$format" == "table" ]]; then + for i in "${hostSources}"; do + pattern="Host Sources:.*ID:.*$i*" + OUTPUT=$(boundary targets read $tid -format table) + if ! [[ $OUTPUT =~ $pattern ]]; then + echo "Host source id '$i' not found on target" + return 1 + fi + done + return 0 + fi + return 1 +} \ No newline at end of file diff --git a/internal/tests/cli/boundary/alias.bats b/internal/tests/cli/boundary/alias.bats new file mode 100644 index 000000000..ac2ce7ab8 --- /dev/null +++ b/internal/tests/cli/boundary/alias.bats @@ -0,0 +1,102 @@ +#!/usr/bin/env bats + +load _auth +load _connect +load _aliases +load _helpers +load _targets + +export ALIAS_VALUE='test' +export UPDATE_ALIAS_VALUE='test.change' + +@test "boundary/login: can login as admin user" { + run login $DEFAULT_LOGIN + [ "$status" -eq 0 ] +} + +@test "boundary/alias: admin user can create alias" { + run create_target_alias $ALIAS_VALUE $DEFAULT_TARGET + echo $output + [ "$status" -eq 0 ] +} + +@test "boundary/alias: admin user can not create already created alias" { + run create_target_alias $ALIAS_VALUE $DEFAULT_TARGET + [ "$status" -eq 1 ] +} + +@test "boundary/alias: admin user can list aliases" { + run list_alias + [ "$status" -eq 0 ] +} + +@test "boundary/alias: admin user can read created alias" { + local id=$(alias_id_from_target_alias $ALIAS_VALUE) + run read_alias $id + [ "$status" -eq 0 ] +} + +@test "boundary/alias: admin user can update created alias " { + local id=$(alias_id_from_target_alias $ALIAS_VALUE) + run update_target_alias_host_id $id $DEFAULT_HOST + [ "$status" -eq 0 ] + local hostid=$(host_id_from_alias_id $id) + [ "$hostid" == "$DEFAULT_HOST" ] +} + +@test "boundary/alias: admin user can remove destination id and host id will also be cleared out" { + local id=$(alias_id_from_target_alias $ALIAS_VALUE) + run update_target_alias_remove_destination_id $id + local hostid=$(host_id_from_alias_id $id) + [ ! -z "$hostid" ] +} + +@test "boundary/alias: admin user can update alias value" { + local id=$(alias_id_from_target_alias $ALIAS_VALUE) + run update_target_alias_value $id $UPDATE_ALIAS_VALUE + [ "$status" -eq 0 ] +} + +@test "boundary/alias: admin user can delete alias" { + local id=$(alias_id_from_target_alias $UPDATE_ALIAS_VALUE) + run delete_alias $id + [ "$status" -eq 0 ] +} + +@test "boundary/alias: admin user can re-create alias" { + run create_target_alias $ALIAS_VALUE $DEFAULT_TARGET + echo $output + [ "$status" -eq 0 ] +} + +@test "boundary/alias: admin user can connect to target using an alias" { + run connect_alias $ALIAS_VALUE + [ "$status" -eq 0 ] +} + +@test "boundary/alias: admin user can read target using an alias" { + run read_target $ALIAS_VALUE + [ "$status" -eq 0 ] +} + +@test "boundary/alias: can login as unpriv user" { + run login $DEFAULT_UNPRIVILEGED_LOGIN + [ "$status" -eq 0 ] +} + +@test "boundary/alias: unpriv user cannot list aliases" { + run list_alias + [ "$status" -eq 1 ] +} + +@test "boundary/alias: unpriv user cannot read aliases" { + local id=$(alias_id_from_target_alias $ALIAS_VALUE) + run read_alias $id + [ "$status" -eq 1 ] +} + +@test "boundary/alias: unpriv user cannot create aliases" { + run create_target_alias bogus $DEFAULT_TARGET + echo $output + [ "$status" -eq 1 ] +} \ No newline at end of file diff --git a/internal/tests/cli/boundary/target_aliases.bats b/internal/tests/cli/boundary/target_aliases.bats new file mode 100644 index 000000000..2c3faa59a --- /dev/null +++ b/internal/tests/cli/boundary/target_aliases.bats @@ -0,0 +1,103 @@ +#!/usr/bin/env bats + +load _auth +load _connect +load _aliases +load _helpers +load _targets +load _targets_alias + +export ALIAS_VALUE='target.alias' + +@test "boundary/login: can login as admin user" { + run login $DEFAULT_LOGIN + [ "$status" -eq 0 ] +} + +@test "boundary/alias: admin user can create target alias" { + run create_target_alias $ALIAS_VALUE $DEFAULT_TARGET + echo $output + [ "$status" -eq 0 ] +} + +@test "boundary/alias/target: admin user can connect to default target using alias" { + run connect_alias $ALIAS_VALUE + [ "$status" -eq 0 ] +} + +@test "boundary/alias/target: admin user can read default target using alias" { + run read_target_by_alias $ALIAS_VALUE + [ "$status" -eq 0 ] +} + +@test "boundary/login: can login as unpriv user" { + run login $DEFAULT_UNPRIVILEGED_LOGIN + [ "$status" -eq 0 ] +} + +@test "boundary/alias/target: unpriv user can connect to default target" { + run connect_alias $ALIAS_VALUE + [ "$status" -eq 0 ] +} + +@test "boundary/alias/target: unpriv user can read default target" { + run read_target_by_alias $ALIAS_VALUE + [ "$status" -eq 0 ] +} + +@test "boundary/alias/target: login back in as admin user" { + run login $DEFAULT_LOGIN + [ "$status" -eq 0 ] +} + +@test "boundary/target: admin user can create target" { + run create_tcp_target $DEFAULT_P_ID 22 $TGT_NAME + echo $output + [ "$status" -eq 0 ] +} + +@test "boundary/target: admin user can update alias to use created target" { + local aid=$(alias_id_from_target_alias $ALIAS_VALUE) + local tid=$(target_id_from_name $DEFAULT_P_ID $TGT_NAME) + run update_target_alias_destination_id $aid $tid + echo $output + [ "$status" -eq 0 ] +} + +@test "boundary/target: admin user can read created target using alias" { + run read_target_by_alias $ALIAS_VALUE + [ "$status" -eq 0 ] +} + +@test "boundary/alias/target: admin user can add default host set to created target using an alias" { + run add_target_host_sources_by_alias $ALIAS_VALUE $DEFAULT_HOST_SET + echo "$output" + [ "$status" -eq 0 ] +} + +@test "boundary/alias/target: created target has default host set" { + local format="json" + run target_has_host_source_id_by_alias $ALIAS_VALUE $format $DEFAULT_HOST_SET + echo "$output" + [ "$status" -eq 0 ] +} + +@test "boundary/alias/target: default user can connect to created target using alias" { + run connect_alias $ALIAS_VALUE + echo "connecting to $ALIAS_VALUE: $output" + [ "$status" -eq 0 ] +} + +@test "boundary/alias/target: admin user set can set a client port using an alias" { + run update_tcp_target $ALIAS_VALUE -default-client-port 1234 + echo "$output" + [ "$status" -eq 0 ] + + run read_target_by_alias $ALIAS_VALUE + [ "$status" -eq 0 ] + got=$(echo "$output") + + echo "$got" + run field_eq "$got" ".item.attributes.default_client_port" "1234" + [ "$status" -eq 0 ] +} \ No newline at end of file