From fc98b5795b98997c0781f4cc3ab72e990065f8fa Mon Sep 17 00:00:00 2001 From: Tim Ross Date: Mon, 13 Jan 2025 15:58:56 -0500 Subject: [PATCH] Update depguard rules to prevent importing test code outside tests There is a lot of shared test code defined outside of _test.go files scattered through out the code base. Dead code detection in Go is not that great and cannot detect that these test helpers are only consumed in tests. This results in testify code being included in production binaries: ```bash strings build/teleport | rg testify github.com/stretchr/testify/assert.init github.com/stretchr/testify/assert.uint32Type go:link.pkghashbytes.github.com/stretchr/testify/assert github.com/stretchr/testify/assert.int64Type github.com/stretchr/testify/assert.uintptrType go:link.pkghashbytes.github.com/stretchr/testify/require github.com/stretchr/testify/assert.stringType github.com/stretchr/testify/assert.float32Type github.com/stretchr/testify/assert.bytesType github.com/stretchr/testify/assert.intType go:link.pkghashbytes.github.com/stretchr/testify/assert/yaml github.com/stretchr/testify/assert.float64Type github.com/stretchr/testify/assert..inittask github.com/stretchr/testify/assert.int32Type github.com/stretchr/testify/assert.uint16Type github.com/stretchr/testify/assert.int8Type github.com/stretchr/testify/assert.uint64Type github.com/stretchr/testify/assert.uintType github.com/stretchr/testify/assert.uint8Type go:link.pkghash.github.com/stretchr/testify/assert/yaml github.com/stretchr/testify/assert.int16Type go:link.pkghash.github.com/stretchr/testify/require go:link.pkghash.github.com/stretchr/testify/assert github.com/stretchr/testify/assert.timeType dep github.com/stretchr/testify v1.10.0 h1:Xv5erBjTwe/5IxqUQTdXv5kgmIvbHo3QQyRwhJsOfJA= github.com/stretchr/testify/assert.init github.com/stretchr/testify@v1.10.0/assert/assertion_compare.go dep github.com/stretchr/testify v1.10.0 h1:Xv5erBjTwe/5IxqUQTdXv5kgmIvbHo3QQyRwhJsOfJA= ``` While this doesn't remediate anything, it should hopeful avoid any future code from making it harder to separate test and production code. --- .golangci.yml | 129 ++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 129 insertions(+) diff --git a/.golangci.yml b/.golangci.yml index ecc5e7c8e253f..7b4ee9f6512d3 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -128,6 +128,135 @@ linters-settings: desc: 'use "log/slog" instead' - pkg: golang.org/x/exp/slog desc: 'use "log/slog" instead' + # Prevent importing testify in production code. + testify: + files: + # Tests can import testify + - '!$test' + # Execptions + # Remove these once they are complaint. + - '!**/api/testhelpers/**' + - '!**/e/lib/auth/ssotestlib.go' + - '!**/e/lib/aws/identitycenter/test/**' + - '!**/e/lib/idp/saml/testenv/**' + - '!**/e/lib/operatortest/**' + - '!**/e/tests/**' + - '!**/lib/automaticupgrades/basichttp/servermock.go' + - '!**/lib/auth/helpers.go' + - '!**/lib/auth/keystore/testhelpers.go' + - '!**/lib/auth/test/**' + - '!**/lib/backend/test/**' + - '!**/lib/events/athena/test.go' + - '!**/lib/events/test/**' + - '!**/lib/kube/proxy/utils_testing.go' + - '!**/lib/services/suite/**' + - '!**/lib/srv/mock.go' + - '!**/lib/srv/db/redis/test.go' + - '!**/lib/teleterm/gatewaytest/**' + - '!**/lib/utils/testhelpers.go' + - '!**/lib/utils/testutils/**' + - '!**/integration/appaccess/fixtures.go' + - '!**/integration/appaccess/jwt.go' + - '!**/integration/appaccess/pack.go' + - '!**/integration/db/fixture.go' + - '!**/integration/hsm/helpers.go' + - '!**/integration/helpers/**' + - '!**/integration/proxy/proxy_helpers.go' + - '!**/integrations/access/email/testlib/**' + - '!**/integrations/access/datadog/testlib/**' + - '!**/integrations/access/discord/testlib/**' + - '!**/integrations/access/jira/testlib/**' + - '!**/integrations/access/mattermost/testlib/**' + - '!**/integrations/access/msteams/testlib/**' + - '!**/integrations/access/opsgenie/testlib/**' + - '!**/integrations/access/pagerduty/testlib/**' + - '!**/integrations/access/servicenow/testlib/**' + - '!**/integrations/access/slack/testlib/**' + - '!**/integrations/lib/testing/integration/accessrequestsuite.go' + - '!**/integrations/lib/testing/integration/app.go' + - '!**/integrations/lib/testing/integration/authhelper.go' + - '!**/integrations/lib/testing/integration/suite.go' + - '!**/integrations/operator/controllers/resources/testlib/**' + - '!**/tool/teleport/testenv/**' + deny: + - pkg: github.com/stretchr/testify + desc: 'testify should not be imported outside of test code' + # Prevent importing integration test helpers in production code. + integration: + files: + # Tests can do anything + - '!$test' + - '!**/integration/**' + - '!**/e/tests/**' + - '!**/integrations/operator/controllers/resources/testlib/**' + deny: + - pkg: github.com/gravitational/teleport/integration$ + desc: 'integration test should not be imported outside of intergation tests' + - pkg: github.com/gravitational/teleport/integration/appaccess + desc: 'integration test should not be imported outside of intergation tests' + - pkg: github.com/gravitational/teleport/integration/autoupdate + desc: 'integration test should not be imported outside of intergation tests' + - pkg: github.com/gravitational/teleport/integration/conntest + desc: 'integration test should not be imported outside of intergation tests' + - pkg: github.com/gravitational/teleport/integration/db + desc: 'integration test should not be imported outside of intergation tests' + - pkg: github.com/gravitational/teleport/integration/helpers + desc: 'integration test should not be imported outside of intergation tests' + - pkg: github.com/gravitational/teleport/integration/integrations + desc: 'integration test should not be imported outside of intergation tests' + - pkg: github.com/gravitational/teleport/integration/kube + desc: 'integration test should not be imported outside of intergation tests' + - pkg: github.com/gravitational/teleport/integration/proxy + desc: 'integration test should not be imported outside of intergation tests' + - pkg: github.com/gravitational/teleport/integration/testdata + desc: 'integration test should not be imported outside of intergation tests' + # Prevent importing testing in production code. + testing: + files: + # Tests can do anything + - '!$test' + # Execptions + # Remove these once they are complaint. + - '!**/api/testhelpers/**' + - '!**/e/lib/auth/ssotestlib.go' + - '!**/e/lib/aws/identitycenter/test/**' + - '!**/e/lib/devicetrust/testenv/**' + - '!**/e/lib/devicetrust/storage/storage.go' + - '!**/e/lib/idp/saml/testenv/**' + - '!**/e/lib/jamf/testenv/**' + - '!**/e/lib/okta/api/oktaapitest/**' + - '!**/e/lib/operatortest/**' + - '!**/e/tests/**' + - '!**/integration/**' + - '!**/integrations/access/email/testlib/**' + - '!**/integrations/access/msteams/testlib/**' + - '!**/integrations/access/slack/testlib/**' + - '!**/integrations/operator/controllers/resources/testlib/**' + - '!**/lib/auth/helpers.go' + - '!**/lib/auth/keystore/testhelpers.go' + - '!**/lib/auth/test/**' + - '!**/lib/automaticupgrades/basichttp/servermock.go' + - '!**/lib/backend/test/**' + - '!**/lib/cryptosuites/precompute.go' + - '!**/lib/cryptosuites/internal/rsa/rsa.go' + - '!**/lib/events/test/**' + - '!**/lib/events/athena/test.go' + - '!**/lib/fixtures/**' + - '!**/lib/kube/proxy/utils_testing.go' + - '!**/lib/modules/test.go' + - '!**/lib/service/service.go' + - '!**/lib/services/local/users.go' + - '!**/lib/services/suite/**' + - '!**/lib/srv/mock.go' + - '!**/lib/srv/db/redis/test.go' + - '!**/lib/teleterm/gatewaytest/**' + - '!**/lib/utils/cli.go' + - '!**/lib/utils/testhelpers.go' + - '!**/lib/utils/testutils/**' + - '!**/tool/teleport/testenv/**' + deny: + - pkg: testing + desc: 'testing should not be imported outside of tests' # Prevent importing internal packages in client tools or packages containing # common interfaces consumed by them that are known to bloat binaries or break builds # because they only support a single platform.