Skip to content

Commit

Permalink
Implement the Catalog V2 controller integration container tests
Browse files Browse the repository at this point in the history
This now allows the container tests to import things from the root module. However for now we want to be very restrictive about which packages we allow importing.
  • Loading branch information
mkeeler committed Jun 14, 2023
1 parent aef6583 commit e4dffa5
Show file tree
Hide file tree
Showing 8 changed files with 261 additions and 157 deletions.
16 changes: 9 additions & 7 deletions GNUmakefile
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@

SHELL = bash


GO_MODULES := $(shell find . -name go.mod -exec dirname {} \; | grep -v "proto-gen-rpc-glue/e2e" | sort)

###
Expand Down Expand Up @@ -351,16 +352,17 @@ lint/%:
@echo "--> Running enumcover ($*)"
@cd $* && GOWORK=off enumcover ./...

# check that the test-container module only imports allowlisted packages
# from the root consul module. Generally we don't want to allow these imports.
# In a few specific instances though it is okay to import test definitions and
# helpers from some of the packages in the root module.
.PHONY: lint-container-test-deps
lint-container-test-deps:
@echo "--> Checking container tests for bad dependencies"
@cd test/integration/consul-container && ( \
found="$$(go list -m all | grep -c '^github.com/hashicorp/consul ')" ; \
if [[ "$$found" != "0" ]]; then \
echo "test/integration/consul-container: This project should not depend on the root consul module" >&2 ; \
exit 1 ; \
fi \
)
@cd test/integration/consul-container && \
$(CURDIR)/build-support/scripts/check-allowed-imports.sh \
github.com/hashicorp/consul \
internal/catalog/catalogtest

# Build the static web ui inside a Docker container. For local testing only; do not commit these assets.
ui: ui-docker
Expand Down
118 changes: 118 additions & 0 deletions build-support/scripts/check-allowed-imports.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,118 @@
#!/usr/bin/env bash
# Copyright (c) HashiCorp, Inc.
# SPDX-License-Identifier: MPL-2.0


readonly SCRIPT_NAME="$(basename ${BASH_SOURCE[0]})"
readonly SCRIPT_DIR="$(dirname "${BASH_SOURCE[0]}")"
readonly SOURCE_DIR="$(dirname "$(dirname "${SCRIPT_DIR}")")"
readonly FN_DIR="$(dirname "${SCRIPT_DIR}")/functions"

source "${SCRIPT_DIR}/functions.sh"


set -uo pipefail

usage() {
cat <<-EOF
Usage: ${SCRIPT_NAME} <module root> [<allowed relative package path>...]
Description:
Verifies that only the specified packages may be imported from the given module
Options:
-h | --help Print this help text.
EOF
}

function err_usage {
err "$1"
err ""
err "$(usage)"
}

function main {
local module_root=""
declare -a allowed_packages=()
while test $# -gt 0
do
case "$1" in
-h | --help )
usage
return 0
;;
* )
if test -z "$module_root"
then
module_root="$1"
else
allowed_packages+="$1"
fi
shift
esac
done

check_imports "$module_root" ${allowed_packages[@]+"${allowed_packages[@]}"}
return $?
}

function check_imports {
local module_root="$1"
shift
local allowed_packages="$@"

module_imports=$( go list -test -f '{{join .TestImports "\n"}}' ./... | grep "$module_root" | sort | uniq)
module_test_imports=$( go list -test -f '{{join .TestImports "\n"}}' ./... | grep "$module_root" | sort | uniq)

any_error=0

for imp in $module_imports
do
is_import_allowed "$imp" "$module_root" $allowed_packages
allowed=$?

if test $any_error -ne 1
then
any_error=$allowed
fi
done

if test $any_error -eq 1
then
echo "Only the following direct imports are allowed from module $module_root:"
for pkg in $allowed_packages
do
echo " * $pkg"
done
fi

return $any_error
}

function is_import_allowed {
local pkg_import=$1
shift
local module_root=$1
shift
local allowed_packages="$@"

# check if the import path is a part of the module we are restricting imports for
if test "$( go list -f '{{.Module.Path}}' $pkg_import)" != "$module_root"
then
return 0
fi

for pkg in $allowed_packages
do
if test "${module_root}/$pkg" == "$pkg_import"
then
return 0
fi
done

err "Import of package $pkg_import is not allowed"
return 1
}

main "$@"
exit $?
35 changes: 24 additions & 11 deletions internal/resource/resourcetest/builder.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,11 @@ package resourcetest

import (
"context"
"strings"

"github.com/hashicorp/consul/internal/resource"
"github.com/hashicorp/consul/internal/storage"
"github.com/hashicorp/consul/proto-public/pbresource"
"github.com/hashicorp/consul/sdk/testutil/retry"
"github.com/oklog/ulid/v2"
"github.com/stretchr/testify/require"
"google.golang.org/grpc/codes"
Expand Down Expand Up @@ -121,24 +123,35 @@ func (b *resourceBuilder) Write(t T, client pbresource.ResourceServiceClient) *p

res := b.resource

rsp, err := client.Write(context.Background(), &pbresource.WriteRequest{
Resource: res,
var rsp *pbresource.WriteResponse
var err error

// Retry any writes where the error is a UID mismatch and the UID was not specified. This is indicative
// of using a follower to rewrite an object who is not perfectly in-sync with the leader.
retry.Run(t, func(r *retry.R) {
rsp, err = client.Write(context.Background(), &pbresource.WriteRequest{
Resource: res,
})

if err == nil || res.Id.Uid != "" || status.Code(err) != codes.FailedPrecondition {
return
}

if strings.Contains(err.Error(), storage.ErrWrongUid.Error()) {
r.Fatalf("resource write failed due to uid mismatch - most likely a transient issue when talking to a non-leader")
}
// other failed precondition errors will be checked outside of the retry
})

require.NoError(t, err)

if !b.dontCleanup {
cleaner, ok := t.(CleanupT)
require.True(t, ok, "T does not implement a Cleanup method and cannot be used with automatic resource cleanup")
id := proto.Clone(rsp.Resource.Id).(*pbresource.ID)
id.Uid = ""
cleaner.Cleanup(func() {
_, err := client.Delete(context.Background(), &pbresource.DeleteRequest{
Id: rsp.Resource.Id,
})

// ignore not found errors
if err != nil && status.Code(err) != codes.NotFound {
t.Fatalf("Failed to delete resource %s of type %s: %v", rsp.Resource.Id.Name, resource.ToGVK(rsp.Resource.Id.Type), err)
}
NewClient(client).MustDelete(t, id)
})
}

Expand Down
36 changes: 29 additions & 7 deletions internal/resource/resourcetest/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package resourcetest

import (
"context"
"fmt"
"math/rand"
"time"

Expand All @@ -12,6 +13,7 @@ import (
"golang.org/x/exp/slices"
"google.golang.org/grpc/codes"
"google.golang.org/grpc/status"
"google.golang.org/protobuf/proto"
)

type Client struct {
Expand Down Expand Up @@ -75,12 +77,20 @@ func (client *Client) PublishResources(t T, resources []*pbresource.Resource) {
}

t.Logf("Writing resource %s with type %s", res.Id.Name, resource.ToGVK(res.Id.Type))
_, err := client.Write(context.Background(), &pbresource.WriteRequest{
rsp, err := client.Write(context.Background(), &pbresource.WriteRequest{
Resource: res,
})
require.NoError(t, err)

// track the number o
cleaner, ok := t.(CleanupT)
if ok {
id := proto.Clone(rsp.Resource.Id).(*pbresource.ID)
cleaner.Cleanup(func() {
client.MustDelete(t, id)
})
}

// track the number of resources published
published += 1
written = append(written, res.Id)
}
Expand Down Expand Up @@ -227,10 +237,22 @@ func (client *Client) ResolveResourceID(t T, id *pbresource.ID) *pbresource.ID {
}

func (client *Client) MustDelete(t T, id *pbresource.ID) {
_, err := client.Delete(context.Background(), &pbresource.DeleteRequest{Id: id})
if status.Code(err) == codes.NotFound {
return
}
client.retry(t, func(r *retry.R) {
_, err := client.Delete(context.Background(), &pbresource.DeleteRequest{Id: id})
if status.Code(err) == codes.NotFound {
return
}

if err != nil && status.Code(err) != codes.Aborted {
r.Stop(fmt.Errorf("failed to delete the resource: %w", err))
}

require.NoError(r, err)
})
// _, err := client.Delete(context.Background(), &pbresource.DeleteRequest{Id: id})
// if status.Code(err) == codes.NotFound {
// return
// }

require.NoError(t, err)
// require.NoError(t, err)
}
12 changes: 9 additions & 3 deletions test/integration/consul-container/go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ require (
github.com/avast/retry-go v3.0.0+incompatible
github.com/docker/docker v23.0.6+incompatible
github.com/docker/go-connections v0.4.0
github.com/hashicorp/consul v0.0.0-00010101000000-000000000000
github.com/hashicorp/consul/api v1.20.0
github.com/hashicorp/consul/envoyextensions v0.1.2
github.com/hashicorp/consul/proto-public v0.2.1
Expand All @@ -25,7 +26,6 @@ require (
github.com/testcontainers/testcontainers-go v0.20.1
golang.org/x/mod v0.10.0
google.golang.org/grpc v1.55.0
google.golang.org/protobuf v1.30.0
)

require (
Expand All @@ -36,6 +36,7 @@ require (
github.com/Azure/go-ansiterm v0.0.0-20230124172434-306776ec8161 // indirect
github.com/Microsoft/go-winio v0.6.1 // indirect
github.com/armon/go-metrics v0.4.1 // indirect
github.com/armon/go-radix v1.0.0 // indirect
github.com/cenkalti/backoff/v4 v4.2.1 // indirect
github.com/cncf/xds/go v0.0.0-20230310173818-32f1caf87195 // indirect
github.com/containerd/containerd v1.7.1 // indirect
Expand All @@ -49,6 +50,7 @@ require (
github.com/gogo/protobuf v1.3.2 // indirect
github.com/golang/protobuf v1.5.3 // indirect
github.com/google/btree v1.0.1 // indirect
github.com/google/go-cmp v0.5.9 // indirect
github.com/google/uuid v1.3.0 // indirect
github.com/hashicorp/errwrap v1.1.0 // indirect
github.com/hashicorp/go-hclog v1.5.0 // indirect
Expand All @@ -57,6 +59,7 @@ require (
github.com/hashicorp/go-rootcerts v1.0.2 // indirect
github.com/hashicorp/go-sockaddr v1.0.2 // indirect
github.com/hashicorp/golang-lru v0.5.4 // indirect
github.com/hashicorp/hcl v1.0.0 // indirect
github.com/hashicorp/memberlist v0.5.0 // indirect
github.com/imdario/mergo v0.3.15 // indirect
github.com/itchyny/timefmt-go v0.1.4 // indirect
Expand All @@ -72,24 +75,27 @@ require (
github.com/moby/sys/sequential v0.5.0 // indirect
github.com/moby/term v0.5.0 // indirect
github.com/morikuni/aec v1.0.0 // indirect
github.com/oklog/ulid/v2 v2.1.0 // indirect
github.com/opencontainers/go-digest v1.0.0 // indirect
github.com/opencontainers/image-spec v1.1.0-rc3 // indirect
github.com/opencontainers/runc v1.1.7 // indirect
github.com/pmezard/go-difflib v1.0.1-0.20181226105442-5d4384ee4fb2 // indirect
github.com/sean-/seed v0.0.0-20170313163322-e2103e2c3529 // indirect
github.com/sirupsen/logrus v1.9.0 // indirect
github.com/stretchr/objx v0.5.0 // indirect
golang.org/x/exp v0.0.0-20230321023759-10a507213a29 // indirect
golang.org/x/net v0.10.0 // indirect
golang.org/x/sync v0.2.0 // indirect
golang.org/x/sys v0.8.0 // indirect
golang.org/x/text v0.9.0 // indirect
golang.org/x/time v0.3.0 // indirect
golang.org/x/tools v0.9.1 // indirect
google.golang.org/genproto v0.0.0-20230410155749-daa745c078e1 // indirect
google.golang.org/protobuf v1.30.0 // indirect
gopkg.in/yaml.v3 v3.0.1 // indirect
gotest.tools/v3 v3.4.0 // indirect
)

replace (
github.com/hashicorp/consul => ../../..
github.com/hashicorp/consul/api => ../../../api
github.com/hashicorp/consul/envoyextensions => ../../../envoyextensions
github.com/hashicorp/consul/proto-public => ../../../proto-public
Expand Down
Loading

0 comments on commit e4dffa5

Please sign in to comment.