feat!: remove Stack v1 support and all v1 components #294
Conversation
- Remove deprecated DeploymentStrategy and Locking properties from Ledger CRD - Remove all v1-specific logic and conditional code paths - Simplify ledger deployment to always use v2 behavior - Remove v1.0.0 reindex assets - Remove unused Caddyfile template for ledger - Update documentation to remove references to deployment strategies - Clean up imports and remove unused dependencies BREAKING CHANGE: This removes support for Ledger v1 deployments. All ledgers will now use v2 behavior by default.
WalkthroughThis change removes all code related to multiple ledger deployment strategies and version-conditional logic, including types, fields, and branching for deployment modes and version flags. It also eliminates the embedded Caddyfile template and its associated deployment, simplifying the ledger deployment to a single, unified approach without legacy or version-dependent pathways. Changes
Sequence Diagram(s)sequenceDiagram
participant Operator
participant LedgerDeployment
participant Database
Operator->>LedgerDeployment: Install single-instance ledger (no strategy/version branching)
LedgerDeployment->>Database: Connect and configure
Operator->>LedgerDeployment: Set standard environment variables
Operator->>LedgerDeployment: Complete deployment (no Caddy gateway)
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms (1)
🔇 Additional comments (5)
✨ Finishing Touches
🧪 Generate Unit Tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
flemzord
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🔭 Outside diff range comments (2)
api/formance.com/v1beta1/ledger_types.go (1)
20-20: Remove unused import.The
timepackage is imported but not used in the file, causing a compilation error.Apply this diff to fix the compilation error:
-import ( - "time" - - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" -) +import ( + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" +)internal/resources/ledgers/deployments.go (1)
70-71: Fix function signature to use image parameter.The function should receive
imageinstead ofversionto be consistent with other deployment functions and fix the compilation error.Apply this diff to fix the function signature:
func installLedgerStateless(ctx core.Context, stack *v1beta1.Stack, - ledger *v1beta1.Ledger, database *v1beta1.Database, version string) error { + ledger *v1beta1.Ledger, database *v1beta1.Database, image string) error {
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (6)
config/crd/bases/formance.com_ledgers.yamlis excluded by!**/*.yamlinternal/resources/ledgers/assets/reindex/v1.0.0/ledger_reindex.yamlis excluded by!**/*.yamlinternal/resources/ledgers/assets/reindex/v1.0.0/ledger_reindex_accounts.yamlis excluded by!**/*.yamlinternal/resources/ledgers/assets/reindex/v1.0.0/ledger_reindex_all.yamlis excluded by!**/*.yamlinternal/resources/ledgers/assets/reindex/v1.0.0/ledger_reindex_transactions.yamlis excluded by!**/*.yamlinternal/resources/ledgers/assets/reindex/v1.0.0/ledger_reindex_volumes.yamlis excluded by!**/*.yaml
📒 Files selected for processing (6)
api/formance.com/v1beta1/ledger_types.go(1 hunks)api/formance.com/v1beta1/zz_generated.deepcopy.go(0 hunks)internal/resources/ledgers/assets.go(0 hunks)internal/resources/ledgers/assets/Caddyfile.gotpl(0 hunks)internal/resources/ledgers/deployments.go(8 hunks)internal/resources/ledgers/init.go(3 hunks)
💤 Files with no reviewable changes (3)
- internal/resources/ledgers/assets/Caddyfile.gotpl
- internal/resources/ledgers/assets.go
- api/formance.com/v1beta1/zz_generated.deepcopy.go
🧰 Additional context used
🪛 GitHub Actions: Default
api/formance.com/v1beta1/ledger_types.go
[error] 20-20: GolangCI-Lint typecheck error: "time" imported and not used.
🔇 Additional comments (5)
api/formance.com/v1beta1/ledger_types.go (1)
40-41: LGTM!The simplified description accurately reflects the ledger's purpose after removing multiple deployment strategies.
internal/resources/ledgers/init.go (1)
86-106: LGTM!The simplified migration logic correctly handles version changes while maintaining backward compatibility for versions >= v2.2.0-alpha.
internal/resources/ledgers/deployments.go (3)
30-49: LGTM!The simplified deployment logic correctly handles different ledger versions while removing v1-specific code paths.
290-306: LGTM!The consistent removal of environment variable prefixes standardizes the configuration approach across all ledger deployments.
260-285: LGTM!The simplified deployment creation correctly removes v1-specific volume logic, making the code cleaner and more maintainable.
- Simplified the description of the ledger as a stateful application managing financial transactions and maintaining an immutable audit trail. - Removed deprecated properties related to deployment strategies and locking from the Ledger CRD and documentation.
…ge parameter - Changed the parameter name from 'version' to 'image' in the installLedgerStateless function to better reflect its purpose. - Updated the call to setCommonAPIContainerConfiguration to use the new image parameter.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
docs/09-Configuration reference/02-Custom Resource Definitions.md (1)
744-745: Add an explicit migration note for removed v1-only fieldsThe new description is concise, but existing users upgrading from v1 will likely look here for guidance. A short, in-place note that
deploymentStrategyandlockingfields are no longer supported would prevent silent confusion.The ledger is a stateful application that manages financial transactions and maintains an immutable audit trail. + +> **Important:** Starting with Stack v2, the legacy `deploymentStrategy` +> and `locking` fields have been removed. Manifests that still declare +> these properties will be rejected by the operator.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (2)
config/crd/bases/formance.com_ledgers.yamlis excluded by!**/*.yamlhelm/crds/templates/crds/apiextensions.k8s.io_v1_customresourcedefinition_ledgers.formance.com.yamlis excluded by!**/*.yaml
📒 Files selected for processing (3)
api/formance.com/v1beta1/ledger_types.go(1 hunks)docs/09-Configuration reference/02-Custom Resource Definitions.md(1 hunks)internal/resources/ledgers/init.go(3 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- api/formance.com/v1beta1/ledger_types.go
- internal/resources/ledgers/init.go
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Tests
- Changed the embedded asset path to include versioning for reindex YAML files.
flemzord
changed the title
BREAKING CHANGE: This removes support for Ledger v1 deployments. All ledgers will now use v2 behavior by default.