Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Tests: Fix/improve tests with Restricted PSA enforcement #2780

Merged
merged 2 commits into from
Sep 11, 2023

Conversation

pglass
Copy link
Contributor

@pglass pglass commented Aug 15, 2023

Changes proposed in this PR:

In the acceptance tests:

  • fix: Deploy apps to a separate namespace in ConnectHelper when not on OpenShift and -enable-restricted-psa-enforcement is set
  • improvement: Auto-configure the restricted PSA enforcement label when -enable-restricted-psa-enforcement is set

How I've tested this PR:

Run the following commands:

$ make kind-cni
$ ./test-psa-kind.sh -cni

Where the ./test-psa-kind.sh script is the following

Test script
#!/usr/bin/env bash

set -euo pipefail

SCRIPT_DIR=$( cd -- "$( dirname -- "${BASH_SOURCE[0]}" )" &> /dev/null && pwd )

export CONSUL_LICENSE=$(cat ~/.consul-ent-license)
export CONSUL_ENT_LICENSE=$CONSUL_LICENSE

# Cleanup old namespaces
# for context in $(kubectl config get-contexts -o name | grep '^kind-') ; do
for context in kind-dc1 kind-dc2 ; do
    kubectl --context $context get ns \
		| grep ^acceptance | awk '{print $1}' \
		| xargs -n 1 -I '{}' kubectl --context $context delete ns '{}' || true
done

EXTRA_FLAGS=""

while [[ $# -gt 0 ]]; do
    case $1 in
        -tproxy)
            EXTRA_FLAGS+=" -enable-transparent-proxy"
            shift;
            ;;
        -cni)
            EXTRA_FLAGS+=" -enable-cni -enable-transparent-proxy"
            shift;
            ;;
        *)
            echo "Unrecognized argument: '$1'"
            exit 1
    esac
done

function runtest() {
    local testdir=$1
    local runtest=$2

    if [ -n "$runtest" ]; then
        runtest="-run $runtest"
    fi

    # Create consul namespaces with restricted PSA enformcement.
    set -xeuo pipefail

    local ns_base="acceptance-$1-$RANDOM"
    local contexts=""
    local namespaces=""
    #for context in $(kubectl config get-contexts -o name | grep '^kind-') ; do
    for context in kind-dc1 kind-dc2 ; do
        local consul_namespace="${ns_base}-$context"
        if [ -n "$contexts" ]; then
            contexts+=","
            namespaces+=","
        fi
        contexts+="$context"
        namespaces+="$consul_namespace"
    done

    # Grab the default image versions from the helm values.
    imageK8S=$(cat ../charts/consul/values.yaml | yq -r '.global.imageK8S')
    imageConsul=$(cat ../charts/consul/values.yaml | yq -r '.global.image' | sed 's/consul:/consul-enterprise:/')
    imageDataplane=$(cat ../charts/consul/values.yaml | yq -r '.global.imageConsulDataplane')

    cd "${SCRIPT_DIR}/tests/$testdir"
    rm -rf ./_debug
    mkdir ./_debug
    go test  -v -p 1 -timeout 15m -failfast \
        -consul-k8s-image "$imageK8S" \
        -consul-image "$imageConsul" \
        -consul-dataplane-image "$imageDataplane" \
        -debug-directory ./_debug \
        -enable-enterprise \
        -kube-contexts "$contexts" \
        -kube-namespaces "$namespaces" \
        -enable-multi-cluster -use-kind \
        -enable-restricted-psa-enforcement \
        $EXTRA_FLAGS $runtest \
        ./...
}

runtest "connect" 'TestConnectInject$'

How I expect reviewers to test this PR:

👀 or try to run the tests if you want

Checklist:

@pglass pglass added backport/1.0.x backport/1.1.x Backport to release/1.1.x branch pr/no-changelog PR does not need a corresponding .changelog entry backport/1.2.x This release branch is no longer active. labels Aug 15, 2023
if updateErr == nil {
logger.Logf(t, "Updated namespace %s", namespace)
return
}
Copy link
Contributor Author

@pglass pglass Aug 15, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This felt odd after writing it, but I wanted to make sure the label is set whether the namespace exists or not. The namespace is passed in as a test flag, but it's not clear whether that namespace should/must exist (or at least, it isn't clear to me).

I was hoping the Update() would be a "update or create if not exist", but it isn't.

@david-yu
Copy link
Contributor

david-yu commented Sep 8, 2023

@missylbytes Should we go ahead and merge this in?

@missylbytes missylbytes merged commit da909d7 into main Sep 11, 2023
@missylbytes missylbytes deleted the pglass/fix-kind-psa-use-app-namespace branch September 11, 2023 14:43
missylbytes pushed a commit that referenced this pull request Sep 11, 2023
* tests: Respect UseAppNamespace in ConnectHelper

* tests: Auto-configure restricted PSA enforcement when enabled

---------

Co-authored-by: Paul Glass <pglass@hashicorp.com>
missylbytes pushed a commit that referenced this pull request Sep 11, 2023
* tests: Respect UseAppNamespace in ConnectHelper

* tests: Auto-configure restricted PSA enforcement when enabled

---------

Co-authored-by: Paul Glass <pglass@hashicorp.com>
missylbytes pushed a commit that referenced this pull request Sep 11, 2023
…into release/1.0.x (#2935)

Tests: Fix/improve tests with Restricted PSA enforcement (#2780)

* tests: Respect UseAppNamespace in ConnectHelper

* tests: Auto-configure restricted PSA enforcement when enabled

---------

Co-authored-by: Paul Glass <pnglass@gmail.com>
Co-authored-by: Paul Glass <pglass@hashicorp.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport/1.1.x Backport to release/1.1.x branch backport/1.2.x This release branch is no longer active. pr/no-changelog PR does not need a corresponding .changelog entry
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants