refactor(operator): introduce product-neutral ClusterIntent seam (non-functional) - #446
Conversation
Introduce an internal product seam so the operator's reconciliation core can be shared across products without product-branding logic in the reconciler. - add internal/product: ProductProfile, Adapter, and ClusterIntent, with a DocumentDBAdapter as the first adapter - make util.ResolveComponentImage product-neutral (env version passed in) - render the CNPG cluster from ClusterIntent via GetCnpgClusterSpecFromIntent; the string-based GetCnpgClusterSpec is kept as a thin backward-compatible wrapper - route topology, storage, images, identity, and the pure-spec postgres and bootstrap inputs through the intent - reconciler builds the intent through the adapter No functional change; existing behavior and tests are preserved. Signed-off-by: Rayhan Hossain <hossain.rayhan@outlook.com>
There was a problem hiding this comment.
Pull request overview
Refactors the operator’s reconciliation/build path to introduce an internal, product-neutral “ClusterIntent” seam (via a new internal/product package) so the CNPG cluster rendering logic can be reused by future operator variants without embedding product-specific defaults and naming throughout the reconciler.
Changes:
- Added
internal/productpackage withProductProfile,ClusterIntent, and a firstDocumentDBAdapterthat mapsDocumentDBinto the neutral intent model. - Made image resolution logic product-neutral via
util.ResolveComponentImage(...), keeping existingDocumentDB-specific wrappers for compatibility. - Updated the reconciler and CNPG builder to render from
ClusterIntentviaGetCnpgClusterSpecFromIntent(...), retaining the originalGetCnpgClusterSpec(...)wrapper.
Reviewed changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| operator/src/internal/utils/util.go | Extracts a product-neutral image resolver and rewires existing DocumentDB image helpers to call it. |
| operator/src/internal/product/profile.go | Introduces product profile + adapter interface for the new internal seam. |
| operator/src/internal/product/profile_test.go | Pins DocumentDB profile values to existing canonical constants to prevent drift. |
| operator/src/internal/product/intent.go | Defines the product-neutral ClusterIntent model consumed by the CNPG builder/reconciler. |
| operator/src/internal/product/intent_test.go | Validates adapter-to-intent mapping for defaults and overrides (images, topology, identity, bootstrap, feature gates). |
| operator/src/internal/product/documentdb.go | Implements DocumentDBProfile and DocumentDBAdapter including image resolution + intent mapping. |
| operator/src/internal/product/documentdb_test.go | Drift-guard tests ensuring adapter image resolution matches existing util behavior (including env var fallback). |
| operator/src/internal/controller/documentdb_controller.go | Switches reconciliation to build intent via adapter and call the intent-based CNPG builder entry point. |
| operator/src/internal/cnpg/cnpg_intent_test.go | Adds intent-based CNPG rendering tests and asserts wrapper equivalence. |
| operator/src/internal/cnpg/cnpg_cluster.go | Adds GetCnpgClusterSpecFromIntent(...) and rewires product-varying values to come from intent. |
Suppressed comments (1)
operator/src/internal/utils/util.go:467
- The priority comment is now inaccurate: this function can return the change-stream override image when FeatureGateChangeStreams is enabled, but the documented priority order omits that case.
// GetDocumentDBImageForInstance returns the documentdb engine image.
// Priority: spec.image.documentDB > spec.documentDBVersion > env.DOCUMENTDB_VERSION > default
func GetDocumentDBImageForInstance(documentdb *dbpreview.DocumentDB) string {
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
|
🤖 Auto-triaged by documentdb-triage-tool. Applied: Reasoningcomponent from path globs (controllers, test); effort from diff stats (783+111 LOC, 10 files); LLM: Non-functional internal refactor introducing a product-neutral abstraction layer in the reconciler/builder; no runtime behavior change, multi-file across internal packages. If a label is wrong, remove it manually and ping |
Address PR review: remove the parallel util image resolvers now that the adapter owns resolution, and fix the Adapter interface doc. - delete util.GetGatewayImageForDocumentDB and util.GetDocumentDBImageForInstance (no production callers besides the webhook); the webhook now resolves via DocumentDBAdapter.ExtensionImage - move the resolution unit tests onto the adapter (direct expected-value cases) - keep the generic util.ResolveComponentImage as the shared priority helper - correct the Adapter interface doc to match its contract (exposes Profile only; ToClusterIntent is a product-specific method on the concrete adapter) No functional change; existing behavior and tests are preserved. Signed-off-by: Rayhan Hossain <hossain.rayhan@outlook.com> Co-authored-by: Copilot <198982749+Copilot@users.noreply.github.com>
Summary
Purely non-functional refactor. It introduces an internal, product-neutral seam so the operator's reconciliation core can be reused by future operator variants without carrying product-specific branding logic in the reconciler. There is no change to runtime behavior, the rendered CNPG
Cluster, the CRD/API, Helm values, env vars, or image defaults.Motivation
The reconciler previously read product-specific values (image names, credential secret, plugin names, spec fields) directly off the
DocumentDBtype throughout the CNPG cluster builder. This change extracts a smallinternal/productseam (ProductProfile+Adapter+ClusterIntent) withDocumentDBas the first adapter, so the shared rendering path consumes a neutral model. This is groundwork only; it does not add any new product.What changed
internal/productpackageProductProfile— product identity plus image repos, default images, credential secret, and CNPG plugin names (sourced from the existing operator constants).Adapterinterface andDocumentDBAdapter(first adapter).ClusterIntent— product-neutral desired state:Images{PostgresExtension, Gateway, Postgres, PullSecrets},Topology{Instances, Affinity},Storage{PvcSize},Identity{Name, UID, APIVersion, Kind},Postgres{UID, GID, PostInitSQL},FeatureGates{IOUring},Bootstrap{Recovery},CredentialSecret, plugin names, andProduct.DocumentDBAdapter.ToClusterIntent(*DocumentDB)resolves these via the profile.util.ResolveComponentImagemade product-neutral: the environment-resolved version is passed in as a parameter, so the shared resolver contains no product-specific env var or default names. TheDocumentDBwrappers keep the same behavior.ClusterIntentviaGetCnpgClusterSpecFromIntent(...). The originalGetCnpgClusterSpec(...)is retained as a thin backward-compatible wrapper, so existing call sites and tests are untouched.*DocumentDBwrappers kept for existing direct-call tests.Non-breaking guarantee
util.Get*functions by drift-guard unit tests (adapter output == util output across all cases).PostInitSQL) map 1:1 into the renderedCluster.cnpgv1.LocalObjectReferencehas onlyName).Testing
go build,go vet, andgofmtare clean.internal/controllerenvtest integration tests that render and assert the actual CNPG cluster spec.test/e2emodule does not import any changed internal symbols; it drives the operator through the Kubernetes API against the (byte-identical) renderedCluster. Recommend running the e2esmokelabel in CI as the final gate.Follow-ups (out of scope, not in this PR)