From 5dbcda512ae91463317d17f776ac7ae6b92ab3bf Mon Sep 17 00:00:00 2001
From: Johan Brandhorst-Satzkorn <johan.brandhorst@gmail.com>
Date: Tue, 20 Sep 2022 11:39:28 -0700
Subject: [PATCH] Fix enos bats test errors (#2468)

* test(cli): Remove skips in CI for cli tests

* fix(enos): Fix enos bats tests

The tests still sometimes suffer from timing issues,
but I was able to run a successful run (setup, run, destroy)
of the tests from my machine at least once.

There were 2 issues:

- Some tests in groups.bats were hardcoding the default user id
- The default grants for a user in a project were missing
   the permission to connect to a target. We now add this
   explicitly.

Co-authored-by: Timothy Messier <tim.messier@gmail.com>
---
 enos/modules/test_cli_ui/tests.tf         | 30 +++++++++++++----------
 internal/tests/cli/boundary/_helpers.bash |  3 +--
 internal/tests/cli/boundary/groups.bats   | 12 ++-------
 internal/tests/cli/boundary/roles.bats    | 23 ++---------------
 internal/tests/cli/boundary/sessions.bats | 14 +++--------
 internal/tests/cli/boundary/target.bats   |  5 ----
 6 files changed, 25 insertions(+), 62 deletions(-)

diff --git a/enos/modules/test_cli_ui/tests.tf b/enos/modules/test_cli_ui/tests.tf
index ecfc4e4da3..13d1a75ab0 100644
--- a/enos/modules/test_cli_ui/tests.tf
+++ b/enos/modules/test_cli_ui/tests.tf
@@ -67,19 +67,6 @@ resource "enos_local_exec" "create_account" {
   inline = ["${var.local_boundary_dir}/boundary accounts create password -auth-method-id ${var.auth_method_id} -login-name ${local.test_user} -name ${local.test_user} -password env://BP -description 'test user' -format json"]
 }
 
-resource "enos_local_exec" "create_role" {
-  environment = local.base_environment
-  inline      = ["${var.local_boundary_dir}/boundary roles create -name='testrolerole' -scope-id='global' -format json"]
-}
-locals {
-  role_id = jsondecode(enos_local_exec.create_role.stdout).item.id
-}
-
-resource "enos_local_exec" "add_grants" {
-  environment = local.base_environment
-  inline      = ["${var.local_boundary_dir}/boundary roles add-grants -id=${local.role_id} -grant='id=hcst_9kF4FooBar;type=*;actions=create,delete,list,update' -format json"]
-}
-
 locals {
   account_id = jsondecode(enos_local_exec.create_account.stdout).item.id
 }
@@ -98,6 +85,23 @@ resource "enos_local_exec" "set_accounts" {
   inline      = ["${var.local_boundary_dir}/boundary users set-accounts -id ${local.user_id} -account ${local.account_id}"]
 }
 
+resource "enos_local_exec" "get_role_id" {
+  environment = local.base_environment
+  inline      = ["${var.local_boundary_dir}/boundary roles list -scope-id=${var.project_scope_id} -format json"]
+}
+
+locals {
+  # Get the role used for u_auth within the project
+  role_id = jsondecode(enos_local_exec.get_role_id.stdout).items[1].id
+}
+
+resource "enos_local_exec" "add_grants" {
+  depends_on  = [enos_local_exec.get_role_id]
+  environment = local.base_environment
+  inline      = ["${var.local_boundary_dir}/boundary roles add-grants -id=${local.role_id} -grant='id=*;type=target;actions=authorize-session' -format json"]
+}
+
+
 resource "enos_local_exec" "run_bats" {
   depends_on = [enos_local_exec.create_user]
   environment = {
diff --git a/internal/tests/cli/boundary/_helpers.bash b/internal/tests/cli/boundary/_helpers.bash
index b41f2926d8..9a9c331250 100644
--- a/internal/tests/cli/boundary/_helpers.bash
+++ b/internal/tests/cli/boundary/_helpers.bash
@@ -12,7 +12,6 @@ export DEFAULT_HOST_CATALOG="${DEFAULT_HOST_CATALOG:-hcst_1234567890}"
 export DEFAULT_HOST="${DEFAULT_HOST:-hst_1234567890}"
 export DEFAULT_USER="${DEFAULT_USER:-u_1234567890}"
 export DEFAULT_UNPRIVILEGED_USER="${DEFAULT_UNPRIVILEGED_USER:-u_0987654321}"
-export SKIP_FAILING_TESTS_IN_CI="${SKIP_FAILING_TESTS_IN_CI:-false}"
 
 function strip() {
   echo "$1" | tr -d '"'
@@ -32,4 +31,4 @@ function has_status_code() {
 
 diag() {
     echo "$@" | sed -e 's/^/# /' >&3 ;
-}
\ No newline at end of file
+}
diff --git a/internal/tests/cli/boundary/groups.bats b/internal/tests/cli/boundary/groups.bats
index dc139d62ce..656229f45f 100644
--- a/internal/tests/cli/boundary/groups.bats
+++ b/internal/tests/cli/boundary/groups.bats
@@ -42,24 +42,16 @@ export NEW_GROUP='test'
 }
 
 @test "boundary/group/add-members: can associate $NEW_GROUP group with default user" {
-  if [ "$SKIP_FAILING_TESTS_IN_CI" == "true" ]; then
-      skip
-  fi
   local gid=$(group_id $NEW_GROUP)
-  run assoc_group_acct 'u_1234567890' $gid
+  run assoc_group_acct $DEFAULT_USER $gid
   echo "$output"
-  diag "$output"
   [ "$status" -eq 0 ]
 }
 
 @test "boundary/group/add-members: $NEW_GROUP group contains default user" {
-  if [ "$SKIP_FAILING_TESTS_IN_CI" == "true" ]; then
-      skip
-  fi
   local gid=$(group_id $NEW_GROUP)
-  run group_has_member_id 'u_1234567890' $gid
+  run group_has_member_id $DEFAULT_USER $gid
   echo "$output"
-  diag "$output"
   [ "$status" -eq 0 ]
 }
 
diff --git a/internal/tests/cli/boundary/roles.bats b/internal/tests/cli/boundary/roles.bats
index e362e65577..f2286c7312 100644
--- a/internal/tests/cli/boundary/roles.bats
+++ b/internal/tests/cli/boundary/roles.bats
@@ -15,13 +15,13 @@ export NEW_GRANT='id=*;type=*;actions=create,read,update,delete,list'
 }
 
 @test "boundary/roles: can add $NEW_ROLE role to global scope granting rights in default org scope" {
-	run create_role 'global' $NEW_ROLE $DEFAULT_O_ID
+	run create_role $DEFAULT_GLOBAL $NEW_ROLE $DEFAULT_O_ID
   echo "$output"
 	[ "$status" -eq 0 ]
 }
 
 @test "boundary/roles: can not add already created $NEW_ROLE role" {
-	run create_role 'global' $NEW_ROLE $DEFAULT_O_ID
+	run create_role $DEFAULT_GLOBAL $NEW_ROLE $DEFAULT_O_ID
   echo "$output"
 	[ "$status" -eq 1 ]
 }
@@ -50,22 +50,15 @@ export NEW_GRANT='id=*;type=*;actions=create,read,update,delete,list'
 }
 
 @test "boundary/role/add-principals: $NEW_ROLE role contains default principal" {
-  if [ "$SKIP_FAILING_TESTS_IN_CI" == "true" ]; then
-      skip
-  fi
   local rid=$(role_id $NEW_ROLE $DEFAULT_GLOBAL)
   run role_has_principal_id $rid $DEFAULT_USER
   echo "$output"
-  diag "$output"
   [ "$status" -eq 0 ]
 }
 
 @test "boundary/role/remove-principals: can remove default principal from $NEW_ROLE role" {
   local rid=$(role_id $NEW_ROLE $DEFAULT_GLOBAL)
   run remove_role_principal $DEFAULT_USER $rid
-  if [ "$SKIP_FAILING_TESTS_IN_CI" == "true" ]; then
-      skip
-  fi
   echo "$output"
   [ "$status" -eq 0 ]
 }
@@ -87,11 +80,7 @@ export NEW_GRANT='id=*;type=*;actions=create,read,update,delete,list'
 @test "boundary/role/add-grantss: $NEW_ROLE role contains $NEW_GRANT grant" {
   local rid=$(role_id $NEW_ROLE $DEFAULT_GLOBAL)
   run role_has_grant $rid $NEW_GRANT
-  if [ "$SKIP_FAILING_TESTS_IN_CI" == "true" ]; then
-      skip
-  fi
   echo "$output"
-  diag "$output"
   [ "$status" -eq 0 ]
 }
 
@@ -103,13 +92,9 @@ export NEW_GRANT='id=*;type=*;actions=create,read,update,delete,list'
 }
 
 @test "boundary/role/remove-grants: $NEW_ROLE role no longer contains $NEW_GRANT grant" {
-  if [ "$SKIP_FAILING_TESTS_IN_CI" == "true" ]; then
-      skip
-  fi
   local rid=$(role_id $NEW_ROLE $DEFAULT_GLOBAL)
   run role_has_grant $rid $NEW_GRANT
   echo "$output"
-  diag "$output"
   [ "$status" -eq 1 ]
 }
 
@@ -122,12 +107,8 @@ export NEW_GRANT='id=*;type=*;actions=create,read,update,delete,list'
 }
 
 @test "boundary/roles: can not read deleted $NEW_ROLE role" {
-  if [ "$SKIP_FAILING_TESTS_IN_CI" == "true" ]; then
-      skip
-  fi
   local rid=$(role_id $NEW_ROLE $DEFAULT_GLOBAL)
 	run read_role $rid
   echo "$output"
-  diag "$output"
 	[ "$status" -eq 1 ]
 }
diff --git a/internal/tests/cli/boundary/sessions.bats b/internal/tests/cli/boundary/sessions.bats
index 356386f41a..bcdb142723 100644
--- a/internal/tests/cli/boundary/sessions.bats
+++ b/internal/tests/cli/boundary/sessions.bats
@@ -20,20 +20,17 @@ load _helpers
 }
 
 @test "boundary/session/connect: unpriv user can connect to default target" {
-  if [ "$SKIP_FAILING_TESTS_IN_CI" == "true" ]; then
-      skip
-  fi
-
   run login $DEFAULT_UNPRIVILEGED_LOGIN
+  echo $output
   [ "$status" -eq 0 ]
 
   run connect_nc $DEFAULT_TARGET
+  echo $output
   [ "$status" -eq 0 ]
 
   # Run twice so we have two values for later testing
   run connect_nc $DEFAULT_TARGET
   echo "$output"
-  diag "$output"
   [ "$status" -eq 0 ]
 }
 
@@ -85,14 +82,11 @@ load _helpers
 }
 
 @test "boundary/session: verify read and cancellation permissions on unpriv session" {
-  if [ "$SKIP_FAILING_TESTS_IN_CI" == "true" ]; then
-      skip
-  fi
-
   # Find an unpriv session
   run login $DEFAULT_UNPRIVILEGED_LOGIN
   [ "$status" -eq 0 ]
   run list_sessions $DEFAULT_P_ID
+  echo $output
   [ "$status" -eq 0 ]
   id=$(echo "$output" | jq -r "[.items[]|select(.user_id == \"$DEFAULT_UNPRIVILEGED_USER\")][0].id")
 
@@ -111,6 +105,4 @@ load _helpers
   [ "$status" -eq 0 ]
   run cancel_session $id
   [ "$status" -eq 0 ]
-
-  diag "$output"
 }
diff --git a/internal/tests/cli/boundary/target.bats b/internal/tests/cli/boundary/target.bats
index 969fc0e937..612d5d1490 100644
--- a/internal/tests/cli/boundary/target.bats
+++ b/internal/tests/cli/boundary/target.bats
@@ -27,13 +27,8 @@ load _helpers
 }
 
 @test "boundary/target/connect: unpriv user can connect to default target" {
-  if [ "$SKIP_FAILING_TESTS_IN_CI" == "true" ]; then
-      skip
-  fi
-
   run connect_nc $DEFAULT_TARGET
   [ "$status" -eq 0 ]
-  diag "$output"
 }
 
 @test "boundary/target: unpriv user can not read default target" {