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 12, 2023
1 parent 6c7f26e commit 134f95a
Show file tree
Hide file tree
Showing 8 changed files with 259 additions and 157 deletions.
14 changes: 7 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,15 @@ lint/%:
@echo "--> Running enumcover ($*)"
@cd $* && GOWORK=off enumcover ./...

# check that the test-container module only imports allowlisted packages
# from the root consul 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 134f95a

Please sign in to comment.