diff --git a/smells/anti-patterns.md b/smells/anti-patterns.md index 4b08721..ad3e6bd 100644 --- a/smells/anti-patterns.md +++ b/smells/anti-patterns.md @@ -1,355 +1,473 @@ -# Anti-Patterns: What Kubernetes Avoids (and Why) +# Anti-Patterns: What Go's Stdlib Avoids (and Why) -## 1. Never Mutate Shared Cache Objects +Patterns the Go standard library team actively avoids, extracted +from studying what they DON'T do in their source code. -**What they avoid:** Modifying objects returned by Listers/Informers without deep-copying first. +**Source:** [golang/go](https://github.com/golang/go) at commit +[`17bd5ab`](https://github.com/golang/go/tree/17bd5ab8c650155dd2bd09f7005726552639eea0) -**Why:** The informer cache is shared across all controllers. Mutating a cached object corrupts state for every other consumer. +--- + +## 1. Returning Errors and Values Simultaneously + +**What they avoid:** Functions that return a valid value alongside +a non-nil error. + +**Source evidence:** The entire stdlib follows: if error is non-nil, +other return values are undefined/zero. `io.Reader.Read` is the ONE +exception (documented as "n > 0 AND err possible"). + +**Why it's bad:** Callers check the error OR use the value — not both. +If you return data with an error, callers might ignore the error +because they got "valid" data. -**The pattern K8s enforces:** ```go -// WRONG — mutates shared cache -deployment, _ := dc.dLister.Deployments(ns).Get(name) -deployment.Spec.Replicas = ptr.To[int32](3) // CORRUPTION! +// BAD — ambiguous: is result valid when err != nil? +func fetch(url string) ([]byte, error) { + resp, err := http.Get(url) + if err != nil { + return partialData, err // caller might use partialData without checking err + } + return io.ReadAll(resp.Body) +} -// RIGHT — deep copy before mutating -deployment, _ := dc.dLister.Deployments(ns).Get(name) -deploymentCopy := deployment.DeepCopy() -deploymentCopy.Spec.Replicas = ptr.To[int32](3) +// GOOD — nil data when error is non-nil +func fetch(url string) ([]byte, error) { + resp, err := http.Get(url) + if err != nil { + return nil, err + } + return io.ReadAll(resp.Body) +} ``` - ### When to Apply This Rule **Triggers:** -- You're reading from any shared data structure (cache, registry, concurrent map) -- Your function modifies a struct that was returned by a Lister, cache, or lookup -- You see code like `obj.Field = newValue` where `obj` came from a shared source +- Returning non-zero values alongside non-nil errors +- Callers that use the value without checking the error -**Example — detecting the smell:** -```go -// This line is the red flag: modifying something obtained from a shared cache -deployment, _ := deploymentLister.Get(name) -deployment.Spec.Replicas = ptr.To[int32](5) // SMELL: mutating cached object -client.Update(ctx, deployment) -``` +### Exceptions -**Example — fixed:** -```go -deployment, _ := deploymentLister.Get(name) -copy := deployment.DeepCopy() // isolate your mutation -copy.Spec.Replicas = ptr.To[int32](5) -client.Update(ctx, copy) -``` - -**Evidence:** The `runtime.Object` interface *mandates* `DeepCopyObject()`. Every API type has generated deep copy methods. The entire architecture assumes immutable reads. - -### Exceptions (When This Rule Doesn't Apply) - -**Acceptable to skip deep copy when:** -- You're only *reading* fields, never mutating the object -- The object was just created locally (not from a shared cache) and hasn't been shared yet -- You're inside a test where the cache is a mock/fake with no other consumers -- Performance profiling shows deep copy is a measurable bottleneck AND you can prove no concurrent readers exist - -**Acceptable-use example:** -```go -// Reading only — no mutation, no copy needed -deployment, _ := dc.dLister.Deployments(ns).Get(name) -if deployment.Spec.Replicas != nil && *deployment.Spec.Replicas == 0 { - logger.Info("deployment scaled to zero", "name", name) - return nil // just reading, never modified the object -} -``` - -**Why this is fine:** Deep copy has a cost (allocation + GC pressure). If you're only reading fields to make decisions and never writing to the object, the cache stays unmodified and other consumers are unaffected. +- `io.Reader.Read()` — explicitly documented as "n > 0 AND err == io.EOF" case +- Functions that return "best effort" partial results (document this clearly) --- -## 2. Never Process the Same Key Concurrently +## 2. Large Interfaces (Java-Style) -**What they avoid:** Multiple goroutines syncing the same object simultaneously. +**What they avoid:** Interfaces with more than 1-3 methods. -**Why:** Two goroutines reading the same Deployment, each computing a different desired state, then both writing → conflict errors and potential state corruption. +**Source evidence:** `io.Reader` (1 method), `io.Writer` (1 method), +`fmt.Stringer` (1 method), `sort.Interface` (3 methods). The largest +stdlib interface is `net.Conn` at 8 methods — and it's widely +considered too large. -**The pattern K8s enforces:** The workqueue's `processing` set ensures a key is only handed to one worker at a time. If an item is added while being processed, it's re-queued *after* `Done()` is called: +**Why it's bad:** Large interfaces are hard to implement (fewer types +satisfy the contract). Small interfaces compose: `io.ReadWriter` is +just `Reader + Writer`. The bigger the interface, the tighter the +coupling. ```go -// From queue.go — the processing set blocks concurrent access -func (q *Typed[T]) Get() (item T, shutdown bool) { - // ... - q.processing.Insert(item) // Mark as being worked on - q.dirty.Delete(item) - return item, false +// BAD — Java-style "service" interface +type UserService interface { + Create(ctx context.Context, u *User) error + Get(ctx context.Context, id string) (*User, error) + Update(ctx context.Context, u *User) error + Delete(ctx context.Context, id string) error + List(ctx context.Context, filter Filter) ([]*User, error) + Count(ctx context.Context) (int, error) + Search(ctx context.Context, q string) ([]*User, error) } +// Only ONE implementation will ever satisfy this. It's a class in disguise. + +// GOOD — small, composable interfaces +type UserReader interface { + GetUser(ctx context.Context, id string) (*User, error) +} +type UserWriter interface { + SaveUser(ctx context.Context, u *User) error +} +// Functions accept the smallest interface they need +func sendWelcome(ctx context.Context, r UserReader, id string) error { ... } ``` ---- - -## 3. Never Use Edge-Triggered Logic - -**What they avoid:** Controllers that react to *what changed* rather than *what the current state is*. - -**Why:** Events can be missed (watch disconnects), delivered out of order, or duplicated. If your controller says "a pod was deleted, so decrement counter" rather than "count current pods and compare to desired", you'll drift. - -**The pattern K8s enforces:** Level-triggered reconciliation. The `syncHandler` reads *current state from the cache*, computes *desired state from the spec*, and makes the world match: - - ### When to Apply This Rule **Triggers:** -- Your handler says "X happened, so do Y" instead of "the world should look like Z, make it so" -- You're incrementing/decrementing counters based on events instead of counting actual state -- A missed event (network blip, restart) would cause permanent drift +- Interface with 4+ methods +- Only one implementation exists +- Interface defined in the same package as the implementation -**Example — detecting the smell:** -```go -func onPodDeleted(pod Pod) { - deployment.Status.Replicas-- // edge-triggered: if we miss a delete, count is wrong forever -} -``` +### Exceptions -**Example — fixed:** -```go -func reconcile(deployment Deployment) { - actualPods := listPodsForDeployment(deployment) - deployment.Status.Replicas = int32(len(actualPods)) // level-triggered: always correct -} -``` - -```go -// The sync function always reads current state, never relies on "what happened" -func (dc *DeploymentController) syncDeployment(ctx context.Context, key string) error { - deployment, err := dc.dLister.Deployments(namespace).Get(name) - // Compute desired state from deployment.Spec - // Read actual state from replicaset lister - // Reconcile difference -} -``` - -### Exceptions (When This Rule Doesn't Apply) - -**Edge-triggered logic is acceptable when:** -- You're implementing audit logging or event sourcing (recording *what happened* is the goal, not reconciling state) -- The event carries information that isn't available from current state (e.g., "who deleted this" from the delete event's user context) -- You're optimizing a hot path where full reconciliation is too expensive, AND you have a periodic full-sync as a safety net - -**Acceptable-use example:** -```go -// Audit logging — we WANT to record the specific event, not just current state -func onPodDeleted(pod *v1.Pod) { - auditLog.Record(AuditEvent{ - Action: "delete", - Resource: pod.Name, - DeletedBy: pod.Annotations["deleted-by"], - Timestamp: time.Now(), - }) -} -// This is fine because: -// 1. The goal is recording history, not maintaining correct state -// 2. A missed event means a missing audit entry (acceptable, not corrupt state) -// 3. The actual replica count is still reconciled level-triggered elsewhere -``` - -**Why this is fine:** Level-triggered reconciliation is about *correctness of state*. Audit logs, metrics counters, and notification systems have different requirements — they genuinely need to know *what happened*, not just *what is*. +- Interfaces matching an external protocol (net.Conn — mirrors TCP API) +- Well-known stdlib patterns (sort.Interface — exactly 3 methods) --- -## 4. Never Forget to Call queue.Forget() on Success +## 3. Package-Level init() for Complex Logic -**What they avoid:** Letting the rate limiter track items that succeeded. +**What they avoid:** Using `init()` for anything beyond simple +registration or flag setup. -**Why:** The rate limiter is per-item. If you never call `Forget()`, the next time the same key needs processing (even for a new event), it will be rate-limited as if it previously failed. +**Source evidence:** Stdlib init() functions are exclusively used for: +registering encodings, initializing hash algorithms, setting up flags. +Never for: opening files, making network calls, or complex computation. + +**Why it's bad:** init() runs at import time — you can't control when. +Can't pass parameters. Can't return errors. Can't skip in tests. +Hard to debug (runs before main). Creates import side effects. -**Source:** The rate_limiting_queue.go comments explicitly warn: ```go -// NewTypedRateLimitingQueue constructs a new workqueue with rateLimited queuing ability -// Remember to call Forget! If you don't, you may end up tracking failures forever. -``` - -**The pattern K8s enforces:** -```go -func (dc *DeploymentController) handleErr(ctx context.Context, err error, key string) { - if err == nil { - dc.queue.Forget(key) // CRITICAL: clear the failure counter - return +// BAD — complex logic in init +func init() { + db, err := sql.Open("postgres", os.Getenv("DATABASE_URL")) + if err != nil { + log.Fatal(err) // crashes the program at import time } - if dc.queue.NumRequeues(key) < maxRetries { - dc.queue.AddRateLimited(key) - return + globalDB = db +} + +// GOOD — explicit initialization +func NewApp(dsn string) (*App, error) { + db, err := sql.Open("postgres", dsn) + if err != nil { + return nil, fmt.Errorf("opening database: %w", err) } - dc.queue.Forget(key) // Also forget when giving up + return &App{db: db}, nil } ``` +### When to Apply This Rule + +**Triggers:** +- init() that opens files, network connections, or databases +- init() with error handling (log.Fatal/panic) +- init() that takes >5 lines +- init() that depends on environment variables + +### Exceptions + +- Registering encodings: `encoding/gob`, `image/png` init registration +- Seeding randomness or initializing lookup tables +- Flag registration (`flag.String(...)` in init) + --- -## 5. Never Hit the API Server in a Tight Loop +## 4. Stuttering Names -**What they avoid:** Direct API calls for reads. List/Get calls in hot paths. +**What they avoid:** Package-qualified names that repeat the package +name: `http.HTTPServer`, `user.UserService`, `json.JSONEncoder`. -**Why:** The API server is a shared resource. If 100 controllers each make 10 API calls per reconciliation at 1 sync/second, that's 1000 req/s to the API server per controller manager instance. +**Source evidence:** The stdlib uses: `http.Server` (not HTTPServer), +`json.Encoder` (not JSONEncoder), `bytes.Buffer` (not BytesBuffer). +The package name provides context. + +**Why it's bad:** When you import and use `http.HTTPServer`, you say +"HTTP" twice. Go's package system already provides namespace context. +Stuttering is visual noise that adds no information. -**The pattern K8s enforces:** Read from Listers (local cache), write to API server: ```go -// READ from cache (free, local, fast) -deployment, err := dc.dLister.Deployments(namespace).Get(name) +// BAD — stuttering +package user -// WRITE to API server (expensive, remote, rate-limited) -_, err = dc.client.AppsV1().Deployments(namespace).Update(ctx, deployment, ...) +type UserService struct { ... } // user.UserService +type UserRepository interface { ... } // user.UserRepository +func NewUserService() *UserService { ... } + +// GOOD — the package name provides context +package user + +type Service struct { ... } // user.Service +type Repository interface { ... } // user.Repository +func NewService() *Service { ... } ``` ---- +### When to Apply This Rule -## 6. Never Sync Before Caches Are Warm +**Triggers:** +- Type name starts with the package name +- `New` constructor pattern +- Reading the qualified name aloud sounds redundant -**What they avoid:** Processing items before the informer has done its initial List. +### Exceptions -**Why:** With an empty cache, a controller might think "no pods exist, must create all of them" — causing a thundering herd of duplicate creates. - -**The pattern K8s enforces:** -```go -// pkg/controller/deployment/deployment_controller.go:189 -if !cache.WaitForNamedCacheSyncWithContext(ctx, - dc.dListerSynced, dc.rsListerSynced, dc.podListerSynced) { - return // Don't start workers until all caches are populated -} -``` +- When omitting the prefix would be ambiguous (`error.Error` — Go uses this) +- Single-package binaries where qualification doesn't apply --- -## 7. Never Ignore Tombstones in Delete Handlers +## 5. Naked Returns in Long Functions -**What they avoid:** Assuming delete handlers always receive the concrete type. +**What they avoid:** Using named return values with bare `return` +in functions longer than a few lines. -**Why:** If a watch disconnects and reconnects, missed deletes arrive as `DeletedFinalStateUnknown` (tombstones). Ignoring them means your controller never learns about those deletions. +**Source evidence:** The stdlib uses naked returns only in very short +functions (1-5 lines). Longer functions always return explicit values. +The `go vet` tool warns about this. + +**Why it's bad:** In a 50-line function with naked return, you have +to trace every assignment to the named returns to understand what's +being returned. It's like having invisible variables. -**The pattern K8s enforces:** ```go -// Every delete handler must check for tombstones -func (dc *DeploymentController) deleteDeployment(logger klog.Logger, obj interface{}) { - d, ok := obj.(*apps.Deployment) - if !ok { - tombstone, ok := obj.(cache.DeletedFinalStateUnknown) - if !ok { - utilruntime.HandleError(...) - return - } - d, ok = tombstone.Obj.(*apps.Deployment) - // ... +// BAD — naked return in long function +func process(data []byte) (result int, err error) { + // ... 40 lines of code ... + if something { + result = compute() + return // what is err here? was it set earlier? who knows } + // ... more code ... + return // mystery values } -``` ---- - -## 8. Never Use ResourceVersion for Equality - -**What they avoid:** Comparing ResourceVersion to check if an object changed. - -**Why:** ResourceVersion is opaque (currently etcd's mod_revision, but this is an implementation detail). The only valid operation is "!=" to detect change. - -**The pattern K8s uses:** -```go -// pkg/controller/deployment/deployment_controller.go:284-288 -func (dc *DeploymentController) updateReplicaSet(logger klog.Logger, old, cur interface{}) { - curRS := cur.(*apps.ReplicaSet) - oldRS := old.(*apps.ReplicaSet) - if curRS.ResourceVersion == oldRS.ResourceVersion { - return // Periodic resync, nothing actually changed +// GOOD — explicit returns +func process(data []byte) (int, error) { + // ... 40 lines of code ... + if something { + return compute(), nil // clear what's being returned } - // ... process the real update + return 0, fmt.Errorf("processing: %w", err) } ``` +### When to Apply This Rule + +**Triggers:** +- Function body > 10 lines with naked returns +- Multiple return points with naked returns +- Named returns used ONLY for naked return (not for documentation) + +### Exceptions + +- Short functions (3-5 lines) where named returns add clarity +- Defer-based error wrapping: `defer func() { err = wrap(err) }()` + (this is the primary legitimate use of named returns) + --- -## 9. Never Panic in Production Goroutines (Without Recovery) +## 6. Error String Formatting (Capitalization/Punctuation) -**What they avoid:** Unhandled panics killing the entire controller manager. +**What they avoid:** Error messages that start with capitals or end +with punctuation. -**Why:** A single nil pointer in one controller's sync loop would crash all 30+ controllers running in the same process. +**Source evidence:** Every error string in the stdlib is lowercase, +no trailing period. `fmt.Errorf("open %s: %w", path, err)` — never +`"Failed to open file."`. + +**Why it's bad:** Errors compose. `fmt.Errorf("connect: %w", err)` +produces `connect: open /etc/hosts: permission denied`. If inner +errors are capitalized or punctuated, the chain looks broken: +`connect: Failed to open file.` -**The pattern K8s enforces:** ```go -// Every goroutine gets crash protection -func (dc *DeploymentController) Run(ctx context.Context, workers int) { - defer utilruntime.HandleCrash() // Top-level recovery - // ... -} +// BAD — capitalized, punctuated +return fmt.Errorf("Failed to open configuration file: %s.", path) -// And in every polling loop: -func BackoffUntilWithContext(ctx context.Context, f func(ctx context.Context), ...) { - func() { - defer runtime.HandleCrashWithContext(ctx) // Per-iteration recovery - f(ctx) - }() -} +// GOOD — lowercase, no punctuation, wraps cleanly +return fmt.Errorf("open config %s: %w", path, err) ``` +### When to Apply This Rule + +**Triggers:** +- Error strings starting with uppercase +- Error strings ending with `.` or `!` +- Error messages that don't compose well with wrapping + +### Exceptions + +- Proper nouns: `"Google API returned 503"` +- Acronyms at the start: `"DNS lookup failed"` (some teams accept this) + --- -## 10. Never Block Workers Indefinitely +## 7. Overusing Channels (When Mutex Suffices) -**What they avoid:** Unbounded blocking in a sync handler (e.g., waiting for a condition that may never occur). +**What they avoid:** Using channels for simple mutual exclusion +where `sync.Mutex` would be clearer. -**Why:** Workers are a finite pool. If one blocks forever, that's one fewer worker processing the queue. At scale, this cascades. +**Source evidence:** The stdlib uses Mutex 540 times vs channels +for synchronization ~50 times. Channels are for communication +between goroutines. Mutex is for protecting shared state. + +**Why it's bad:** A channel of capacity 1 as a mutex is clever +but obscure. It adds cognitive load, makes locking/unlocking +asymmetric, and doesn't have `defer mu.Unlock()` ergonomics. -**The pattern K8s enforces:** All API calls take context (with timeouts), all waits are bounded: ```go -// Timeouts on API calls -ctx, cancel := context.WithTimeout(ctx, 30*time.Second) -defer cancel() -_, err := client.CoreV1().Pods(ns).Create(ctx, pod, metav1.CreateOptions{}) +// BAD — channel as mutex (clever but obscure) +sem := make(chan struct{}, 1) +sem <- struct{}{} // "lock" +// ... critical section ... +<-sem // "unlock" + +// GOOD — mutex is clear and idiomatic +var mu sync.Mutex +mu.Lock() +defer mu.Unlock() +// ... critical section ... ``` +### When to Apply This Rule + +**Triggers:** +- `make(chan struct{}, 1)` used as a lock +- Channel send/receive pairs that don't actually communicate data +- Complex select statements where a simple lock would work + +### Exceptions + +- Rate limiting (semaphore with capacity > 1) +- Signaling between goroutines (cancellation, done channels) +- Fan-out/fan-in patterns (actual communication) + --- -## 11. Never Use sync.Mutex Where sync.Once Suffices +## 8. Accepting Interfaces, Returning Interfaces -**What they avoid:** Full mutual exclusion for one-shot operations. +**What they avoid:** Returning interfaces from functions (except +well-known ones like `error`). -**Why:** `sync.Once` is semantically clearer and avoids the bug where you forget to check a "done" flag under the mutex. +**Source evidence:** The Go proverb: "Accept interfaces, return structs." +The stdlib returns concrete types: `http.NewServeMux()` returns +`*ServeMux`, not `http.Handler`. This allows callers to access +all methods. + +**Why it's bad:** Returning an interface hides the concrete type. +Callers can't access methods that aren't in the interface. Makes +testing harder (can't construct the concrete type). Prevents +future method additions without breaking the interface. -**The pattern K8s uses:** ```go -// pkg/controller/controller_ref_manager.go:43-49 -type BaseControllerRefManager struct { - canAdoptErr error - canAdoptOnce sync.Once // One-shot lazy evaluation - CanAdoptFunc func(ctx context.Context) error +// BAD — returns interface (hides concrete type) +func NewCache() Cacher { + return &redisCache{...} +} +// Callers can't access redis-specific methods or fields + +// GOOD — returns concrete type +func NewCache() *RedisCache { + return &RedisCache{...} +} +// Callers get the full type. They can still use it as Cacher where needed. +``` + +### When to Apply This Rule + +**Triggers:** +- Factory functions returning interfaces +- Single-implementation interfaces returned from constructors +- Interface return types that prevent callers from accessing useful methods + +### Exceptions + +- Standard error returns (`error` interface) +- When there genuinely are multiple implementations chosen at runtime +- Plugin systems where the concrete type must be hidden + +--- + +## 9. Panic in Library Code + +**What they avoid:** Using `panic()` for conditions callers could +handle. + +**Source evidence:** stdlib panic usage is exclusively for: (1) index +out of bounds, (2) nil pointer on documented non-nil parameter, +(3) impossible states (unreachable code). Never for "file not found" +or "invalid input" style errors. + +**Why it's bad:** panic kills the goroutine (and the program unless +recovered). Library code shouldn't decide that an error is fatal — +that's the caller's decision. + +```go +// BAD — panicking on recoverable error +func MustParse(s string) Config { + cfg, err := Parse(s) + if err != nil { + panic(err) // caller has no way to handle bad input + } + return cfg } -func (m *BaseControllerRefManager) CanAdopt(ctx context.Context) error { - m.canAdoptOnce.Do(func() { - if m.CanAdoptFunc != nil { - m.canAdoptErr = m.CanAdoptFunc(ctx) - } - }) - return m.canAdoptErr +// GOOD — return error, let caller decide +func Parse(s string) (Config, error) { + // ... parse logic ... + if invalid { + return Config{}, fmt.Errorf("parse config: %w", err) + } + return cfg, nil } ``` ---- +### When to Apply This Rule -## 12. Never Expose Mutable State Through Interfaces +**Triggers:** +- `panic()` in exported functions on user input +- `Must*` functions without a non-panicking alternative +- `log.Fatal` in library code (same as panic — kills the process) -**What they avoid:** Returning pointers to internal state through public interfaces. +### Exceptions -**Why:** Callers can accidentally mutate internal state, creating subtle bugs that only manifest under concurrency. - -**The pattern K8s enforces:** Listers return objects from the read-only cache. The `DeepCopy()` pattern ensures mutation safety is the caller's responsibility, not the cache's. +- `Must*` pattern for compile-time constants: `regexp.MustCompile("^\\d+$")` +- Truly impossible states after exhaustive validation +- Test helpers that panic on setup failure --- -## Summary: The Philosophy +## 10. Unexported Interface Satisfaction -Kubernetes avoids these anti-patterns because of one fundamental truth: **in a distributed system, every assumption you make about state being consistent is wrong.** +**What they avoid:** Defining interfaces in the implementor's package +rather than the consumer's package. -The patterns exist because: -1. **Events are unreliable** → level-triggered reconciliation -2. **Reads are stale** → always compare desired vs actual -3. **Concurrent access is inevitable** → deep copy, queue serialization -4. **Failures are normal** → retry with backoff, graceful degradation -5. **Resources are shared** → cache reads, rate-limit writes -6. **Systems outlive their authors** → code generation, type registries, feature gates +**Source evidence:** The Go wiki explicitly says: "Define interfaces +in the package that USES them, not the package that implements them." +The stdlib follows this — `io.Reader` is in `io` (consumer), not +in `os` or `net` (implementors). + +**Why it's bad:** Pre-declaring interfaces in the implementor package +couples consumer to producer. It's Java-style "implements Readable" +thinking. In Go, satisfaction is implicit — the consumer defines +what it needs. + +```go +// BAD — interface in the implementation package +package storage + +type Store interface { // defined where it's implemented + Get(key string) ([]byte, error) + Set(key string, val []byte) error +} + +type RedisStore struct { ... } +// Now everyone who imports storage gets this interface forced on them + +// GOOD — interface in the consumer package +package handler + +type Getter interface { // defined where it's used + Get(key string) ([]byte, error) +} + +func NewHandler(store Getter) *Handler { ... } +// handler only depends on what it actually needs +``` + +### When to Apply This Rule + +**Triggers:** +- Interface defined next to its only implementation +- Interface in the same package as the struct that satisfies it +- Interface with methods matching exactly one struct + +### Exceptions + +- Well-known "standard" interfaces (io.Reader, sort.Interface) +- Interfaces that define a protocol many packages implement +- Plugin/driver interfaces where the contract IS the package's purpose + +