feat: deprecate module-specific settings, expose them on module CRDs#473
Draft
gfyrag wants to merge 2 commits into
Draft
feat: deprecate module-specific settings, expose them on module CRDs#473gfyrag wants to merge 2 commits into
gfyrag wants to merge 2 commits into
Conversation
Cross-cutting Settings (postgres, broker, temporal, otel, etc.) remain; module-specific tuning knobs now live on the corresponding module Spec with a dual-read fallback that logs a deprecation warning when the legacy Settings path is still in use. - New settings.PreferSpec* helpers + LogDeprecation in dual_read.go - Ledger: experimental flags, api.*, schema-enforcement-mode, worker.* - Payments: encryption-key (also fixes a bug where Spec value was dropped), clear-temporal, worker.temporal-* tunings - Orchestration: max-parallel-activities - TransactionPlane: worker-enabled - Auth: stack-level issuers (Auth.Spec.Issuers and per-module AuthConfig.Issuers); check-scopes setting now logs deprecation - Gateway: caddyfile.*, config.idle-timeout, ingress.labels, ingress.tls.enabled, ingress.hosts, dns.<type>.* via Spec.DNS list CRDs regenerated via controller-gen. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Contributor
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
…elm CRDs withAnnotations and withLabels must still merge settings ∪ spec (spec wins on key collision), not short-circuit to spec when present; the previous refactor broke the existing GatewayController test by dropping settings-sourced entries when the spec had any annotations. Also regenerate the helm/crds/* copies and the docs/09-Configuration reference/* artifacts that derive from the v1beta1 types via crd-ref-docs and the settings-catalog tool. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
The Settings system historically mixed cross-cutting concerns (postgres, broker, temporal, otel, …) with module-specific tuning knobs (ledger experimental flags, payments worker temporal tunings, gateway/caddyfile/ingress/DNS, …). This PR keeps Settings for cross-cutting only and exposes ~36 module-specific keys as typed fields on the corresponding module CRD specs, with a dual-read fallback that prefers the spec value and logs a deprecation warning when the legacy Settings path resolves a value.
No setting is removed yet — this is the additive half of the deprecation. Hard removal is intentionally deferred to a future major.
Changes
internal/resources/settings/dual_read.go—PreferSpecBool/Int/String/StringSlice/Duration/Maphelpers (and*OrDefaultvariants) +LogDeprecation.Spec.API.*,Spec.Worker.{AsyncBlockHasher,BucketCleanup,Pipelines}, experimental flags,SchemaEnforcementModeSpec.Worker.*temporal tunings,Spec.ClearTemporalSpec.MaxParallelActivitiesSpec.WorkerEnabledSpec.Issuers, sharedAuthConfig.Issuersfor downstream modulesSpec.Caddyfile,Spec.Config,Spec.DNS[],Spec.Ingress.LabelsDEPRECATION:logzz_generated.deepcopy.goregenerated via controller-genpayments/deployments.go:getEncryptionKeywas returning""whenSpec.EncryptionKeywas set — now correctly propagates the valueTest plan
go build ./...cleango vet ./...cleango test -short ./internal/resources/settings/...passesspec.experimentalFeatures: true→ confirmEXPERIMENTAL_FEATURES=truein the pod env; then drop the spec and add a matchingSettings→ same env +DEPRECATIONlog on the operator🤖 Generated with Claude Code