design: Guarantee module import boundaries (#6851)

* design: Guarantee module import boundaries

To prevent issues like the one solved in [1] in the future, we want to:
1) Clarify in-tree the design of our modules
2) Guarantee by CI that bad things won't happen.

[1]: https://github.com/external-secrets/external-secrets/pull/6846

Signed-off-by: Jean-Philippe Evrard <jean-philippe.evrard+rochepub@external.roche.com>

* Reintroduce (partial) linting of sprig

Signed-off-by: Jean-Philippe Evrard <jean-philippe.evrard+rochepub@external.roche.com>

* minor grammatical issues

Signed-off-by: Jean-Philippe Evrard <jean-philippe.evrard+rochepub@external.roche.com>

---------

Signed-off-by: Jean-Philippe Evrard <jean-philippe.evrard+rochepub@external.roche.com>

.golangci.yaml

@@ -9,6 +9,7 @@ linters:
   enable:
     - asciicheck
     - bodyclose
+    - depguard
     - dogsled
     - dupl
     - errcheck
@@ -34,6 +35,52 @@ linters:
     - whitespace
     - revive
   settings:
+    depguard:
+      rules:
+        apis:
+          files:
+            - "**/apis/**/*.go"
+          list-mode: strict
+          allow:
+            - $gostd
+            - github.com/external-secrets/external-secrets/apis
+            - github.com/google/go-cmp
+            - github.com/stretchr/testify
+            - k8s.io/api
+            - k8s.io/apiextensions-apiserver
+            - k8s.io/apimachinery
+            - sigs.k8s.io/controller-runtime
+        runtime:
+          files:
+            - "**/runtime/**/*.go"
+          list-mode: strict
+          allow:
+            - $gostd
+            - dario.cat/mergo
+            - github.com/Masterminds/goutils
+            - github.com/Masterminds/semver/v3
+            - github.com/external-secrets/external-secrets/apis
+            - github.com/external-secrets/external-secrets/runtime
+            - github.com/go-logr/logr
+            - github.com/google/go-cmp
+            - github.com/google/uuid
+            - github.com/hashicorp/golang-lru
+            - github.com/huandu/xstrings
+            - github.com/lestrrat-go/jwx/v2
+            - github.com/mitchellh/copystructure
+            - github.com/prometheus/client_golang
+            - github.com/shopspring/decimal
+            - github.com/spf13/cast
+            - github.com/spf13/pflag
+            - github.com/stretchr/testify
+            - golang.org/x/crypto
+            - k8s.io/api
+            - k8s.io/apiextensions-apiserver
+            - k8s.io/apimachinery
+            - k8s.io/client-go
+            - sigs.k8s.io/controller-runtime
+            - sigs.k8s.io/yaml
+            - software.sslmate.com/src/go-pkcs12
     gocritic:
       enabled-tags:
         - diagnostic
@@ -75,9 +122,37 @@ linters:
       disable:
         - omitzero
   exclusions:
-    paths:
-      - runtime/template/v2/sprig
     rules:
+      # The in-tree sprig copy is excluded from legacy checks, but module boundaries still apply.
+      - path: runtime/template/v2/sprig
+        linters:
+          - asciicheck
+          - bodyclose
+          - copyloopvar
+          - dogsled
+          - dupl
+          - errcheck
+          - errorlint
+          - exhaustive
+          - gocritic
+          - godot
+          - goheader
+          - goprintffuncname
+          - gosec
+          - govet
+          - ineffassign
+          - lll
+          - misspell
+          - modernize
+          - nakedret
+          - nolintlint
+          - prealloc
+          - revive
+          - unconvert
+          - unparam
+          - unused
+          - whitespace
+
       # Exclude some linters from running on tests files.
       - path: _test(ing)?\.go
         linters:
@@ -132,6 +207,9 @@ formatters:
     - gofmt
     - goimports
     - golines
+  exclusions:
+    paths:
+      - runtime/template/v2/sprig
   settings:
     goimports:
       local-prefixes:

design/015-module-import-boundaries.md

@@ -0,0 +1,48 @@
+# Module Import Boundaries
+
+## Intent
+
+https://github.com/external-secrets/external-secrets/issues/5494 introduced
+the split of our go module into multiple go modules.
+
+The module structure has been exercised and can now be moved to a design
+document.
+
+## Principles
+
+- Packages in `apis` are for CRDs and a limited set of interfaces.
+  They may import the standard library, packages in the `apis`
+  module and an explicit allowlist of API dependencies (for example, k8s.io
+  modules).
+- Packages in `runtime` are common utilities. They contain shared code
+  (validation utilities, webhook helpers, metrics/logging tooling, ...).
+  They may import the standard library, packages in `apis` or
+  `runtime`, and an explicit allowlist of runtime dependencies.
+  As the dependencies are included in provider modules, the addition of
+  a module into a `runtime` package needs to be carefully analysed.
+- Packages in `pkg` are for the core orchestration. It's where the
+  controllers and binary reside. They can depend on `apis` and `runtime`.
+- Packages in `e2e` are for the e2e testing, and can require any of the
+  above packages. They are built in isolation, so they should not leak
+  dependencies everywhere.
+- Transitive dependencies are not evaluated: If some module
+  imports the whole world, then we will have to deal with it.
+
+## Enforcement
+
+In a first stage, we use code reviews to highlight discrepancies and preserve
+the module layout expressed above.
+
+In a second stage, we enable golangci-lint's existing `depguard` linter for `apis` and `runtime`; `pkg` and `e2e` remain review-only.
+It defines strict rules selected by path like `**/apis/**/*.go` or `**/runtime/**/*.go`.
+Each rule allows `$gostd`, the module's own package prefix, and the package prefixes belonging to its allowed current direct dependencies.
+
+Depguard analyzes import declarations rather than the module graph, so it
+checks only direct imports. The file globs include tests. An import such as an
+AWS or Oracle SDK from a runtime test therefore fails the existing `make lint`
+and `make reviewable` gates without constraining dependencies loaded
+transitively by an allowed package.
+
+The existing `go mod tidy` and `make check-diff` flow remains responsible for
+removing stale `go.mod` requirements. No new command, dependency, or CI job is
+needed.