From aabe4eaa9bc50c72f14668995ad1cbd04a596000 Mon Sep 17 00:00:00 2001 From: Thibault VINCENT Date: Wed, 6 May 2026 04:36:53 +0200 Subject: [PATCH] refactor(pkg): promote fileglob and source/* from internal --- CLAUDE.md | 4 +++- CONTRIBUTING.md | 17 ++++++++++------- TODO.md | 10 +++++----- Taskfile.yml | 2 +- cmd/x509-certificate-exporter/main.go | 8 ++++---- internal/config/config.go | 2 +- {internal => pkg}/fileglob/fileglob.go | 15 +++++++++------ .../fileglob/fileglob_fuzz_test.go | 0 {internal => pkg}/fileglob/fileglob_test.go | 0 {internal => pkg}/fileglob/inode_other.go | 0 {internal => pkg}/fileglob/inode_unix.go | 0 {internal => pkg}/source/file/file.go | 11 +++++++---- {internal => pkg}/source/file/file_test.go | 2 +- {internal => pkg}/source/k8s/k8s.go | 3 +++ {internal => pkg}/source/k8s/k8s_test.go | 0 {internal => pkg}/source/k8s/metrics.go | 0 .../source/kubeconfig/kubeconfig.go | 15 +++++++++------ .../source/kubeconfig/kubeconfig_test.go | 0 18 files changed, 53 insertions(+), 36 deletions(-) rename {internal => pkg}/fileglob/fileglob.go (97%) rename {internal => pkg}/fileglob/fileglob_fuzz_test.go (100%) rename {internal => pkg}/fileglob/fileglob_test.go (100%) rename {internal => pkg}/fileglob/inode_other.go (100%) rename {internal => pkg}/fileglob/inode_unix.go (100%) rename {internal => pkg}/source/file/file.go (95%) rename {internal => pkg}/source/file/file_test.go (99%) rename {internal => pkg}/source/k8s/k8s.go (99%) rename {internal => pkg}/source/k8s/k8s_test.go (100%) rename {internal => pkg}/source/k8s/metrics.go (100%) rename {internal => pkg}/source/kubeconfig/kubeconfig.go (94%) rename {internal => pkg}/source/kubeconfig/kubeconfig_test.go (100%) diff --git a/CLAUDE.md b/CLAUDE.md index 05f3f8a..c88a107 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -5,7 +5,9 @@ - `cmd/x509-certificate-exporter/` — binary entrypoint - `pkg/cert/` — public API for certificate parsing (`pem`, `pkcs12` subpackages) - `pkg/registry/` — public Prometheus collector + label registry -- `internal/` — everything else (config, log, source/{file,k8s}, server, fileglob, product) +- `pkg/fileglob/` — public glob/walk engine (EXPERIMENTAL — promoted in v4 RC) +- `pkg/source/{file,k8s,kubeconfig}/` — public Source implementations (EXPERIMENTAL) +- `internal/` — wiring & process-lifecycle: config, log, server, product - `chart/` — Helm chart The v3 code was deleted; v4 is at the repo root. The Go module path uses `/v4` diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 57fd4ee..8a0bf46 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -311,15 +311,16 @@ direnv. ├── cmd/x509-certificate-exporter/ Binary entrypoint (main.go and CLI flags) ├── pkg/ Public Go API │ ├── cert/ Certificate parsing (PEM, PKCS#12) -│ └── registry/ Prometheus collector + label registry -├── internal/ Implementation details +│ ├── registry/ Prometheus collector + label registry +│ ├── fileglob/ Glob expansion helpers (EXPERIMENTAL) +│ └── source/ Source implementations (EXPERIMENTAL) +│ ├── file/ File / directory globbing +│ ├── k8s/ Kubernetes Secret/ConfigMap LIST+WATCH +│ └── kubeconfig/ kubeconfig embedded-cert extraction +├── internal/ Wiring & process lifecycle │ ├── config/ YAML config loader │ ├── log/ Structured logging setup -│ ├── source/ The "source" abstraction -│ │ ├── file/ File / directory globbing -│ │ └── k8s/ Kubernetes Secret/ConfigMap LIST+WATCH │ ├── server/ HTTP server, metrics endpoint -│ ├── fileglob/ Glob expansion helpers │ └── product/ Build-time injected version metadata ├── chart/ Helm chart (THE primary deploy method) ├── dev/ @@ -829,7 +830,9 @@ into implementation. ### Code style - **Public API in `pkg/`, internals in `internal/`.** Don't promote - without a real consumer. + without a real consumer. Packages flagged EXPERIMENTAL in their + godoc were promoted ahead of a settled API contract — pin a + specific version if you depend on them. - **Tests colocated with code** as `*_test.go`. - **Table-driven tests** for parsers, registries, anything with many similar cases. diff --git a/TODO.md b/TODO.md index feeadf4..2bac07e 100644 --- a/TODO.md +++ b/TODO.md @@ -13,7 +13,7 @@ filesystem operation** — `open`, `Stat`, `Lstat`, `ReadDir`, `Readlink`. Today (post-symlink-mapping change) the chart writes **in-pod** paths into the configmap (`/mnt/watch/file-/var/lib/kubelet/pki/...`), and `PathMapping` is consulted only when an absolute symlink target is -read back — i.e. only inside `internal/fileglob.handleSymlink`. The +read back — i.e. only inside `pkg/fileglob.handleSymlink`. The proposal is to flip the polarity: have the chart write raw host paths, and let the runtime translate transparently for every FS op. @@ -27,7 +27,7 @@ and let the runtime translate transparently for every FS op. the runtime knows the host path natively. 2. **One translation point, not two.** Right now - `internal/fileglob.handleSymlink` has bespoke translation + + `pkg/fileglob.handleSymlink` has bespoke translation + containment logic. With a universal translator, the same logic covers every code path that touches the FS, so any new FS-using feature is covered for free. @@ -39,7 +39,7 @@ and let the runtime translate transparently for every FS op. ### Why this is *not* done as part of the symlink-mapping change - **Blast radius.** It touches every FS op in `fileglob`, not just - symlink resolution. The cache key in `internal/source/file/file.go` + symlink resolution. The cache key in `pkg/source/file/file.go` becomes the host path (today it's the walker's `Path` = in-pod), affecting cache invariants and the `SkipUnchanged` machinery. @@ -70,12 +70,12 @@ and let the runtime translate transparently for every FS op. ### Sketch of the change -- `internal/fileglob/walkfs.go` (new): a small `WalkFS` decorator that +- `pkg/fileglob/walkfs.go` (new): a small `WalkFS` decorator that takes `[]PathMapping` and a base `WalkFS`, applies the longest-prefix `From → To` rewrite to every method's `name` argument, and forwards. The walker becomes oblivious to the translation — it sees host paths everywhere. -- `internal/source/file/file.go`: drop the special-case +- `pkg/source/file/file.go`: drop the special-case `if e.LinkTo != "" { readPath = e.LinkTo }` (no longer needed — `Reader` would also be wrapped to apply the translation). - `chart/templates/configmap.yaml`: write host paths in diff --git a/Taskfile.yml b/Taskfile.yml index c4ec856..12ea9ec 100644 --- a/Taskfile.yml +++ b/Taskfile.yml @@ -218,7 +218,7 @@ tasks: cmds: - go test -run=^$ -fuzz=FuzzParse -fuzztime=5s ./pkg/cert/pem/ - go test -run=^$ -fuzz=FuzzParse -fuzztime=5s ./pkg/cert/pkcs12/ - - go test -run=^$ -fuzz=FuzzCompile -fuzztime=5s ./internal/fileglob/ + - go test -run=^$ -fuzz=FuzzCompile -fuzztime=5s ./pkg/fileglob/ test:helm-examples: desc: "Assert every docs/examples/**/*.values.yaml is accepted by the chart" diff --git a/cmd/x509-certificate-exporter/main.go b/cmd/x509-certificate-exporter/main.go index d435983..2bac63b 100644 --- a/cmd/x509-certificate-exporter/main.go +++ b/cmd/x509-certificate-exporter/main.go @@ -26,17 +26,17 @@ import ( "go.uber.org/automaxprocs/maxprocs" "github.com/enix/x509-certificate-exporter/v4/internal/config" - "github.com/enix/x509-certificate-exporter/v4/internal/fileglob" xlog "github.com/enix/x509-certificate-exporter/v4/internal/log" "github.com/enix/x509-certificate-exporter/v4/internal/product" "github.com/enix/x509-certificate-exporter/v4/internal/server" - filesource "github.com/enix/x509-certificate-exporter/v4/internal/source/file" - k8ssource "github.com/enix/x509-certificate-exporter/v4/internal/source/k8s" - kcsource "github.com/enix/x509-certificate-exporter/v4/internal/source/kubeconfig" "github.com/enix/x509-certificate-exporter/v4/pkg/cert" pemparser "github.com/enix/x509-certificate-exporter/v4/pkg/cert/pem" pkcs12parser "github.com/enix/x509-certificate-exporter/v4/pkg/cert/pkcs12" + "github.com/enix/x509-certificate-exporter/v4/pkg/fileglob" "github.com/enix/x509-certificate-exporter/v4/pkg/registry" + filesource "github.com/enix/x509-certificate-exporter/v4/pkg/source/file" + k8ssource "github.com/enix/x509-certificate-exporter/v4/pkg/source/k8s" + kcsource "github.com/enix/x509-certificate-exporter/v4/pkg/source/kubeconfig" "k8s.io/client-go/kubernetes" "k8s.io/client-go/rest" diff --git a/internal/config/config.go b/internal/config/config.go index 16d4ecf..4cdd053 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -17,8 +17,8 @@ import ( "gopkg.in/yaml.v3" - "github.com/enix/x509-certificate-exporter/v4/internal/fileglob" "github.com/enix/x509-certificate-exporter/v4/pkg/cert" + "github.com/enix/x509-certificate-exporter/v4/pkg/fileglob" ) // Canonical values for Source.Kind in the YAML config. These strings are diff --git a/internal/fileglob/fileglob.go b/pkg/fileglob/fileglob.go similarity index 97% rename from internal/fileglob/fileglob.go rename to pkg/fileglob/fileglob.go index 8eaab98..21c3998 100644 --- a/internal/fileglob/fileglob.go +++ b/pkg/fileglob/fileglob.go @@ -1,6 +1,9 @@ // Package fileglob implements a small, dedicated glob/walk engine for // certificate discovery. It is NOT a general-purpose glob library. // +// EXPERIMENTAL: this package was promoted from internal/ to enable external +// reuse but its API surface may still change without notice in v5. +// // Supported pattern tokens: // // - match within a single segment, never across "/" @@ -515,11 +518,11 @@ func (w *walker) descend(ctx context.Context, dir string, depth int, seenInodes entries, err := w.fsys.ReadDir(dir) if err != nil { // Report the dir-level error and stop descending here. - reason := "walk_error" + reason := cert.ReasonWalkError if os.IsPermission(err) { - reason = "permission_denied" + reason = cert.ReasonPermissionDenied } else if os.IsNotExist(err) { - reason = "not_found" + reason = cert.ReasonNotFound } w.emit(Result{Err: &Error{Path: dir, Reason: reason, Err: err}}) return @@ -542,7 +545,7 @@ func (w *walker) descend(ctx context.Context, dir string, depth int, seenInodes } info, err := w.fsys.Lstat(full) if err != nil { - w.emit(Result{Err: &Error{Path: full, Reason: "walk_error", Err: err}}) + w.emit(Result{Err: &Error{Path: full, Reason: cert.ReasonWalkError, Err: err}}) continue } // Determine whether this entry can match any include (file or dir). @@ -585,7 +588,7 @@ func (w *walker) descend(ctx context.Context, dir string, depth int, seenInodes func (w *walker) handleSymlink(ctx context.Context, full string, info fs.FileInfo, fileMatched *Pattern, canDescend bool, depth int, seen map[uint64]struct{}) { target, err := w.fsys.Readlink(full) if err != nil { - w.emit(Result{Err: &Error{Path: full, Reason: "broken_symlink", Err: err}}) + w.emit(Result{Err: &Error{Path: full, Reason: cert.ReasonBrokenSymlink, Err: err}}) return } resolved := target @@ -610,7 +613,7 @@ func (w *walker) handleSymlink(ctx context.Context, full string, info fs.FileInf tinfo, err := w.fsys.Stat(resolved) if err != nil { - w.emit(Result{Err: &Error{Path: full, Reason: "broken_symlink", Err: err}}) + w.emit(Result{Err: &Error{Path: full, Reason: cert.ReasonBrokenSymlink, Err: err}}) return } if tinfo.IsDir() { diff --git a/internal/fileglob/fileglob_fuzz_test.go b/pkg/fileglob/fileglob_fuzz_test.go similarity index 100% rename from internal/fileglob/fileglob_fuzz_test.go rename to pkg/fileglob/fileglob_fuzz_test.go diff --git a/internal/fileglob/fileglob_test.go b/pkg/fileglob/fileglob_test.go similarity index 100% rename from internal/fileglob/fileglob_test.go rename to pkg/fileglob/fileglob_test.go diff --git a/internal/fileglob/inode_other.go b/pkg/fileglob/inode_other.go similarity index 100% rename from internal/fileglob/inode_other.go rename to pkg/fileglob/inode_other.go diff --git a/internal/fileglob/inode_unix.go b/pkg/fileglob/inode_unix.go similarity index 100% rename from internal/fileglob/inode_unix.go rename to pkg/fileglob/inode_unix.go diff --git a/internal/source/file/file.go b/pkg/source/file/file.go similarity index 95% rename from internal/source/file/file.go rename to pkg/source/file/file.go index 4505ee2..6e94774 100644 --- a/internal/source/file/file.go +++ b/pkg/source/file/file.go @@ -1,5 +1,8 @@ // Package file implements a Source that scans local files using the // custom fileglob engine and caches parse results between walks. +// +// EXPERIMENTAL: this package was promoted from internal/ to enable external +// reuse but its API surface may still change without notice in v5. package file import ( @@ -12,8 +15,8 @@ import ( "sync" "time" - "github.com/enix/x509-certificate-exporter/v4/internal/fileglob" "github.com/enix/x509-certificate-exporter/v4/pkg/cert" + "github.com/enix/x509-certificate-exporter/v4/pkg/fileglob" ) // Reader fetches the bytes of one path. The default implementation reads @@ -160,7 +163,7 @@ func (s *Source) runOnce(ctx context.Context, sink cert.Sink, isFirst bool) { // Emit a synthetic bundle so the registry can count error reasons. sink.Upsert(cert.Bundle{ Source: cert.SourceRef{ - Kind: cert.KindFile, Format: "pem", + Kind: cert.KindFile, Format: cert.FormatPEM, Location: r.Err.Path, SourceName: s.opts.Name, }, Errors: []cert.ItemError{{Index: -1, Reason: r.Err.Reason, Err: r.Err.Err}}, @@ -186,7 +189,7 @@ func (s *Source) runOnce(ctx context.Context, sink cert.Sink, isFirst bool) { s.mu.Unlock() for _, p := range stale { ref := cert.SourceRef{ - Kind: cert.KindFile, Location: p, SourceName: s.opts.Name, Format: "pem", + Kind: cert.KindFile, Location: p, SourceName: s.opts.Name, Format: cert.FormatPEM, } sink.Delete(ref) s.log.Debug("file disappeared", "path", p) @@ -246,7 +249,7 @@ func (s *Source) processEntry(ctx context.Context, sink cert.Sink, e fileglob.En s.log.Warn("read error", "path", path, "error", err) sink.Upsert(cert.Bundle{ Source: cert.SourceRef{ - Kind: cert.KindFile, Format: "pem", + Kind: cert.KindFile, Format: cert.FormatPEM, Location: path, SourceName: s.opts.Name, }, Errors: []cert.ItemError{{Index: -1, Reason: reason, Err: err}}, diff --git a/internal/source/file/file_test.go b/pkg/source/file/file_test.go similarity index 99% rename from internal/source/file/file_test.go rename to pkg/source/file/file_test.go index 4b6db9a..1e41c4d 100644 --- a/internal/source/file/file_test.go +++ b/pkg/source/file/file_test.go @@ -19,9 +19,9 @@ import ( "testing" "time" - "github.com/enix/x509-certificate-exporter/v4/internal/fileglob" "github.com/enix/x509-certificate-exporter/v4/pkg/cert" "github.com/enix/x509-certificate-exporter/v4/pkg/cert/pem" + "github.com/enix/x509-certificate-exporter/v4/pkg/fileglob" ) func nopLogger() *slog.Logger { return slog.New(slog.NewTextHandler(io.Discard, nil)) } diff --git a/internal/source/k8s/k8s.go b/pkg/source/k8s/k8s.go similarity index 99% rename from internal/source/k8s/k8s.go rename to pkg/source/k8s/k8s.go index 8f1ed99..ef4caec 100644 --- a/internal/source/k8s/k8s.go +++ b/pkg/source/k8s/k8s.go @@ -1,6 +1,9 @@ // Package k8s implements a Source that watches Kubernetes Secrets (and // optionally ConfigMaps). // +// EXPERIMENTAL: this package was promoted from internal/ to enable external +// reuse but its API surface may still change without notice in v5. +// // Architecture: Secrets and ConfigMaps are observed via a direct paginated // LIST + WATCH loop, not a client-go SharedInformer. The reason is that // client-go's pager.List accumulates every page into a single in-memory diff --git a/internal/source/k8s/k8s_test.go b/pkg/source/k8s/k8s_test.go similarity index 100% rename from internal/source/k8s/k8s_test.go rename to pkg/source/k8s/k8s_test.go diff --git a/internal/source/k8s/metrics.go b/pkg/source/k8s/metrics.go similarity index 100% rename from internal/source/k8s/metrics.go rename to pkg/source/k8s/metrics.go diff --git a/internal/source/kubeconfig/kubeconfig.go b/pkg/source/kubeconfig/kubeconfig.go similarity index 94% rename from internal/source/kubeconfig/kubeconfig.go rename to pkg/source/kubeconfig/kubeconfig.go index cafa35a..5e40599 100644 --- a/internal/source/kubeconfig/kubeconfig.go +++ b/pkg/source/kubeconfig/kubeconfig.go @@ -2,6 +2,9 @@ // embedded in kubeconfig files (or referenced via certificate-authority/ // client-certificate file paths). Mirrors the existing exporter's // behaviour: only the four canonical JSONPath-like locations are read. +// +// EXPERIMENTAL: this package was promoted from internal/ to enable external +// reuse but its API surface may still change without notice in v5. package kubeconfig import ( @@ -147,7 +150,7 @@ func (s *Source) scan(path string, sink cert.Sink, seen map[string]struct{}, all if err != nil { *allOK = false s.log.Warn("read kubeconfig", "path", path, "error", err) - ref := cert.SourceRef{Kind: cert.KindKubeconfig, Location: path, SourceName: s.opts.Name, Format: "pem"} + ref := cert.SourceRef{Kind: cert.KindKubeconfig, Location: path, SourceName: s.opts.Name, Format: cert.FormatPEM} sink.Upsert(cert.Bundle{ Source: ref, Errors: []cert.ItemError{{Index: -1, Reason: cert.ReasonReadFailed, Err: err}}, @@ -159,7 +162,7 @@ func (s *Source) scan(path string, sink cert.Sink, seen map[string]struct{}, all var doc kubeconfigDoc if err := yaml.Unmarshal(data, &doc); err != nil { *allOK = false - ref := cert.SourceRef{Kind: cert.KindKubeconfig, Location: path, SourceName: s.opts.Name, Format: "pem"} + ref := cert.SourceRef{Kind: cert.KindKubeconfig, Location: path, SourceName: s.opts.Name, Format: cert.FormatPEM} sink.Upsert(cert.Bundle{ Source: ref, Errors: []cert.ItemError{{Index: -1, Reason: cert.ReasonDecodeFailed, Err: err}}, @@ -186,7 +189,7 @@ func (s *Source) emit(path, kind, key, b64Data, refPath string, sink cert.Sink, Source: cert.SourceRef{ Kind: cert.KindKubeconfig, Location: path, Key: fmt.Sprintf("%s/%s", kind, key), - Format: "pem", SourceName: s.opts.Name, + Format: cert.FormatPEM, SourceName: s.opts.Name, Attributes: map[string]string{"embedded_kind": kind, "embedded_key": key}, }, Errors: []cert.ItemError{{Index: -1, Reason: cert.ReasonDecodeFailed, Err: err}}, @@ -208,7 +211,7 @@ func (s *Source) emit(path, kind, key, b64Data, refPath string, sink cert.Sink, Source: cert.SourceRef{ Kind: cert.KindKubeconfig, Location: path, Key: fmt.Sprintf("%s/%s", kind, key), - Format: "pem", SourceName: s.opts.Name, + Format: cert.FormatPEM, SourceName: s.opts.Name, Attributes: map[string]string{"embedded_kind": kind, "embedded_key": key}, }, Errors: []cert.ItemError{{Index: -1, Reason: cert.ReasonReadFailed, Err: err}}, @@ -225,7 +228,7 @@ func (s *Source) emit(path, kind, key, b64Data, refPath string, sink cert.Sink, parsed := s.parser.Parse(data, cert.SourceRef{ Kind: cert.KindKubeconfig, Location: path, Key: fmt.Sprintf("%s/%s", kind, key), - Format: "pem", SourceName: s.opts.Name, + Format: cert.FormatPEM, SourceName: s.opts.Name, Attributes: map[string]string{"embedded_kind": kind, "embedded_key": key}, }, cert.ParseOptions{}) sink.Upsert(parsed) @@ -264,7 +267,7 @@ func decodeKey(k, sourceName string) cert.SourceRef { _ = i r := cert.SourceRef{ Kind: cert.KindKubeconfig, Location: path, - Format: "pem", SourceName: sourceName, + Format: cert.FormatPEM, SourceName: sourceName, } if kind != "" || key != "" { r.Key = fmt.Sprintf("%s/%s", kind, key) diff --git a/internal/source/kubeconfig/kubeconfig_test.go b/pkg/source/kubeconfig/kubeconfig_test.go similarity index 100% rename from internal/source/kubeconfig/kubeconfig_test.go rename to pkg/source/kubeconfig/kubeconfig_test.go