From 846b80e8a5a4f47c4d84182ab422e1891e9032c1 Mon Sep 17 00:00:00 2001
From: "R.B. Boyer" <rb@hashicorp.com>
Date: Thu, 22 Oct 2020 14:30:28 -0500
Subject: [PATCH] fix flaky envoy integration tests involving intentions
 (#8996)

There is a delay between an intentions change being made, and it being
reflected in the Envoy runtime configuration. Now that the enforcement
happens inside of Envoy instead of over in the agent, our tests need to
explicitly wait until the xDS reconfiguration is complete before
attempting to assert intentions worked.

Also remove a few double retry loops.
---
 .../connect/envoy/case-badauthz/capture.sh    |  4 ++
 .../connect/envoy/case-badauthz/verify.bats   |  4 ++
 .../envoy/case-http-badauthz/capture.sh       |  4 ++
 .../envoy/case-http-badauthz/verify.bats      | 12 ++--
 test/integration/connect/envoy/helpers.bash   | 59 +++++++++++--------
 5 files changed, 54 insertions(+), 29 deletions(-)
 create mode 100644 test/integration/connect/envoy/case-badauthz/capture.sh
 create mode 100644 test/integration/connect/envoy/case-http-badauthz/capture.sh

diff --git a/test/integration/connect/envoy/case-badauthz/capture.sh b/test/integration/connect/envoy/case-badauthz/capture.sh
new file mode 100644
index 000000000..af3245f34
--- /dev/null
+++ b/test/integration/connect/envoy/case-badauthz/capture.sh
@@ -0,0 +1,4 @@
+#!/bin/bash
+
+snapshot_envoy_admin localhost:19000 s1 || true
+snapshot_envoy_admin localhost:19001 s2 || true
diff --git a/test/integration/connect/envoy/case-badauthz/verify.bats b/test/integration/connect/envoy/case-badauthz/verify.bats
index e8586d7b7..78154fa3a 100644
--- a/test/integration/connect/envoy/case-badauthz/verify.bats
+++ b/test/integration/connect/envoy/case-badauthz/verify.bats
@@ -26,6 +26,10 @@ load helpers
   assert_upstream_has_endpoints_in_status 127.0.0.1:19000 s2.default.primary HEALTHY 1
 }
 
+@test "s2 should have network rbac rules loaded from xDS" {
+  retry_default assert_envoy_network_rbac_policy_count localhost:19001 1
+}
+
 @test "s1 upstream should NOT be able to connect to s2" {
   run retry_default must_fail_tcp_connection localhost:5000
 
diff --git a/test/integration/connect/envoy/case-http-badauthz/capture.sh b/test/integration/connect/envoy/case-http-badauthz/capture.sh
new file mode 100644
index 000000000..af3245f34
--- /dev/null
+++ b/test/integration/connect/envoy/case-http-badauthz/capture.sh
@@ -0,0 +1,4 @@
+#!/bin/bash
+
+snapshot_envoy_admin localhost:19000 s1 || true
+snapshot_envoy_admin localhost:19001 s2 || true
diff --git a/test/integration/connect/envoy/case-http-badauthz/verify.bats b/test/integration/connect/envoy/case-http-badauthz/verify.bats
index 825a3ea0e..b1bd1d9e4 100644
--- a/test/integration/connect/envoy/case-http-badauthz/verify.bats
+++ b/test/integration/connect/envoy/case-http-badauthz/verify.bats
@@ -27,10 +27,10 @@ load helpers
   assert_upstream_has_endpoints_in_status 127.0.0.1:19000 1a47f6e1~s2.default.primary HEALTHY 1
 }
 
-@test "s1 upstream should NOT be able to connect to s2" {
-  run retry_default must_fail_http_connection localhost:5000
-
-  echo "OUTPUT $output"
-
-  [ "$status" == "0" ]
+@test "s2 should have http rbac rules loaded from xDS" {
+  retry_default assert_envoy_http_rbac_policy_count localhost:19001 1
+}
+
+@test "s1 upstream should NOT be able to connect to s2" {
+  retry_default must_fail_http_connection localhost:5000
 }
diff --git a/test/integration/connect/envoy/helpers.bash b/test/integration/connect/envoy/helpers.bash
index b4dcda90b..5b6c11025 100755
--- a/test/integration/connect/envoy/helpers.bash
+++ b/test/integration/connect/envoy/helpers.bash
@@ -110,7 +110,6 @@ function assert_proxy_presents_cert_uri {
   local DC=${3:-primary}
   local NS=${4:-default}
 
-
   CERT=$(retry_default get_cert $HOSTPORT)
 
   echo "WANT SERVICE: ${NS}/${SERVICENAME}"
@@ -153,36 +152,48 @@ function assert_envoy_version {
   echo $VERSION | grep "/$ENVOY_VERSION/"
 }
 
+function assert_envoy_http_rbac_policy_count {
+  local HOSTPORT=$1
+  local EXPECT_COUNT=$2
+
+  GOT_COUNT=$(get_envoy_http_rbac_once $HOSTPORT | jq '.rules.policies | length')
+  [ "${GOT_COUNT:-0}" -eq $EXPECT_COUNT ]
+}
+
+function get_envoy_http_rbac_once {
+  local HOSTPORT=$1
+  run curl -s -f $HOSTPORT/config_dump
+  [ "$status" -eq 0 ]
+  echo "$output" | jq --raw-output '.configs[2].dynamic_listeners[].active_state.listener.filter_chains[0].filters[0].config.http_filters[] | select(.name == "envoy.filters.http.rbac") | .config'
+}
+
+function assert_envoy_network_rbac_policy_count {
+  local HOSTPORT=$1
+  local EXPECT_COUNT=$2
+
+  GOT_COUNT=$(get_envoy_network_rbac_once $HOSTPORT | jq '.rules.policies | length')
+  [ "${GOT_COUNT:-0}" -eq $EXPECT_COUNT ]
+}
+
+function get_envoy_network_rbac_once {
+  local HOSTPORT=$1
+  run curl -s -f $HOSTPORT/config_dump
+  [ "$status" -eq 0 ]
+  echo "$output" | jq --raw-output '.configs[2].dynamic_listeners[].active_state.listener.filter_chains[0].filters[] | select(.name == "envoy.filters.network.rbac") | .config'
+}
+
 function get_envoy_listener_filters {
   local HOSTPORT=$1
   run retry_default curl -s -f $HOSTPORT/config_dump
   [ "$status" -eq 0 ]
-  local ENVOY_VERSION=$(echo $output | jq --raw-output '.configs[0].bootstrap.node.metadata.envoy_version')
-  local QUERY=''
-  # from 1.13.0 on the config json looks slightly different
-  # 1.10.x, 1.11.x, 1.12.x are not affected
-  if [[ "$ENVOY_VERSION" =~ ^1\.1[012]\. ]]; then
-    QUERY='.configs[2].dynamic_active_listeners[].listener | "\(.name) \( .filter_chains[0].filters | map(.name) | join(","))"'
-  else
-    QUERY='.configs[2].dynamic_listeners[].active_state.listener | "\(.name) \( .filter_chains[0].filters | map(.name) | join(","))"'
-  fi
-  echo "$output" | jq --raw-output "$QUERY"
+  echo "$output" | jq --raw-output '.configs[2].dynamic_listeners[].active_state.listener | "\(.name) \( .filter_chains[0].filters | map(.name) | join(","))"'
 }
 
 function get_envoy_http_filters {
   local HOSTPORT=$1
   run retry_default curl -s -f $HOSTPORT/config_dump
   [ "$status" -eq 0 ]
-  local ENVOY_VERSION=$(echo $output | jq --raw-output '.configs[0].bootstrap.node.metadata.envoy_version')
-  local QUERY=''
-  # from 1.13.0 on the config json looks slightly different
-  # 1.10.x, 1.11.x, 1.12.x are not affected
-  if [[ "$ENVOY_VERSION" =~ ^1\.1[012]\. ]]; then
-      QUERY='.configs[2].dynamic_active_listeners[].listener | "\(.name) \( .filter_chains[0].filters[] | select(.name == "envoy.http_connection_manager") | .config.http_filters | map(.name) | join(","))"'
-  else
-      QUERY='.configs[2].dynamic_listeners[].active_state.listener | "\(.name) \( .filter_chains[0].filters[] | select(.name == "envoy.http_connection_manager") | .config.http_filters | map(.name) | join(","))"'
-  fi
-  echo "$output" | jq --raw-output "$QUERY"
+  echo "$output" | jq --raw-output '.configs[2].dynamic_listeners[].active_state.listener | "\(.name) \( .filter_chains[0].filters[] | select(.name == "envoy.http_connection_manager") | .config.http_filters | map(.name) | join(","))"'
 }
 
 function get_envoy_cluster_config {
@@ -241,7 +252,7 @@ function get_upstream_endpoint_in_status_count {
   local HOSTPORT=$1
   local CLUSTER_NAME=$2
   local HEALTH_STATUS=$3
-  run retry_default curl -s -f "http://${HOSTPORT}/clusters?format=json"
+  run curl -s -f "http://${HOSTPORT}/clusters?format=json"
   [ "$status" -eq 0 ]
   # echo "$output" >&3
   echo "$output" | jq --raw-output "
@@ -364,7 +375,7 @@ function get_healthy_service_count {
   local DC=$2
   local NS=$3
 
-  run retry_default curl -s -f ${HEADERS} "127.0.0.1:8500/v1/health/connect/${SERVICE_NAME}?dc=${DC}&passing&ns=${NS}"
+  run curl -s -f ${HEADERS} "127.0.0.1:8500/v1/health/connect/${SERVICE_NAME}?dc=${DC}&passing&ns=${NS}"
   [ "$status" -eq 0 ]
   echo "$output" | jq --raw-output '. | length'
 }
@@ -549,6 +560,8 @@ function must_fail_http_connection {
 
   echo "OUTPUT $output"
 
+  [ "$status" == "0" ]
+
   local expect_response="${2:-403 Forbidden}"
   # Should fail request with 503
   echo "$output" | grep "${expect_response}"