fix: missing mulumi tag #906
Conversation
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. 🗂️ Base branches to auto review (1)
Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the WalkthroughThis change introduces a comprehensive Pulumi-based infrastructure deployment program for the ledger system, adding new Go packages and components for API, storage, provisioning, generator, devbox, monitoring, and configuration management. It also updates the Earthfile, disables client generation in the pre-commit hook, and includes enhancements and new tests for internal ledger logic and storage querying. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant PulumiMain
participant ConfigLoader
participant LedgerComponent
participant StorageComponent
participant APIComponent
participant ProvisionComponent
participant GeneratorComponent
participant DevboxComponent
User->>PulumiMain: Run Pulumi deployment
PulumiMain->>ConfigLoader: Load configuration
ConfigLoader-->>PulumiMain: Return config
PulumiMain->>LedgerComponent: Create ledger component with config
LedgerComponent->>StorageComponent: Create storage (Postgres/RDS)
LedgerComponent->>APIComponent: Create API with storage
LedgerComponent->>ProvisionComponent: (If enabled) Create provisioner with API
LedgerComponent->>GeneratorComponent: (If enabled) Create generator with API
LedgerComponent->>DevboxComponent: (If enabled) Create devbox with API and storage
LedgerComponent-->>PulumiMain: Register outputs (API, storage, etc.)
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
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 (
|
gfyrag
force-pushed
the
fix/v2.1/pulumi-missing-yaml-tag
branch
from
e1fc27c to
71391de
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Inline review comments failed to post. This is likely due to GitHub's limits when posting large numbers of comments. If you are seeing this consistently it is likely a permissions issue. Please check "Moderation" -> "Code review limits" under your organization settings.
🛑 Comments failed to post (32)
internal/script.go (1)
35-40: 🛠️ Refactor suggestion
Improve type switch robustness for handling amount values
The new type switch for handling different amount types is a good improvement, but it lacks some safety checks:
- There's no validation that the map contains both "asset" and "amount" keys before accessing them
- Converting float64 to int could lose precision for values with fractional parts
- The type switch doesn't handle other common numeric types (int, int64)
Consider adding safety checks and handling more types:
case map[string]any: + asset, hasAsset := v["asset"] + amount, hasAmount := v["amount"] + if !hasAsset || !hasAmount { + s.Script.Vars[k] = fmt.Sprint(v) + continue + } + switch amount := v["amount"].(type) { case string: - s.Script.Vars[k] = fmt.Sprintf("%s %s", v["asset"], amount) + s.Script.Vars[k] = fmt.Sprintf("%v %s", asset, amount) case float64: - s.Script.Vars[k] = fmt.Sprintf("%s %d", v["asset"], int(amount)) + s.Script.Vars[k] = fmt.Sprintf("%v %d", asset, int(amount)) + case int: + s.Script.Vars[k] = fmt.Sprintf("%v %d", asset, amount) + case int64: + s.Script.Vars[k] = fmt.Sprintf("%v %d", asset, amount) + default: + s.Script.Vars[k] = fmt.Sprintf("%v %v", asset, amount) }Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents (early access)
In internal/script.go around lines 35 to 40, the type switch handling the "amount" value lacks checks for the presence of "asset" and "amount" keys, does not handle numeric types beyond string and float64, and converts float64 to int which may lose precision. To fix this, first verify that both "asset" and "amount" keys exist in the map before accessing them. Extend the type switch to handle additional numeric types like int and int64. Avoid converting float64 to int directly; instead, format float64 values preserving their decimal precision. Update the code to safely handle these cases and prevent runtime errors or data loss.internal/engine/import.go (1)
128-128:
⚠️ Potential issuePotential panic risk in final chain update
Be cautious with the final update to the chain using the last entry in the batch. If the batch is empty, this would cause a panic due to index out of bounds.
-l.chain.ReplaceLast(batch[len(batch)-1]) +if len(batch) > 0 { + l.chain.ReplaceLast(batch[len(batch)-1]) +}🤖 Prompt for AI Agents (early access)
In internal/engine/import.go at line 128, the code calls ReplaceLast on the last element of batch without checking if batch is empty, which can cause a panic. Add a check to ensure batch is not empty before accessing batch[len(batch)-1] and calling ReplaceLast to prevent an index out of bounds panic.deployments/pulumi/pkg/storage/migrate.go (1)
17-50: 🛠️ Refactor suggestion
Consider adding resource limits and backoff configuration
The migration job looks well-structured, but lacks resource constraints and timeout configurations that would be important in production environments.
Consider adding:
- Resource limits and requests to prevent resource exhaustion:
Containers: corev1.ContainerArray{ corev1.ContainerArgs{ Name: pulumi.String("migrate"), Args: pulumi.StringArray{ pulumi.String("migrate"), }, Image: utils.GetMainImage(args.Tag), ImagePullPolicy: args.ImagePullPolicy.ToOutput(ctx.Context()).Untyped().(pulumi.StringOutput), Env: envVars, + Resources: &corev1.ResourceRequirementsArgs{ + Limits: pulumi.StringMap{ + "cpu": pulumi.String("500m"), + "memory": pulumi.String("512Mi"), + }, + Requests: pulumi.StringMap{ + "cpu": pulumi.String("100m"), + "memory": pulumi.String("128Mi"), + }, + }, }, },
- Add backoff limits and active deadline seconds to the job spec:
Spec: batchv1.JobSpecArgs{ + BackoffLimit: pulumi.Int(6), + ActiveDeadlineSeconds: pulumi.Int(600), // 10 minutes timeout Template: corev1.PodTemplateSpecArgs{The DeleteBeforeReplace option is a good addition to ensure proper job replacement during updates.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.func runMigrateJob(ctx *pulumi.Context, args migrationArgs, opts ...pulumi.ResourceOption) (*batchv1.Job, error) { envVars := corev1.EnvVarArray{ corev1.EnvVarArgs{ Name: pulumi.String("DEBUG"), Value: utils.BoolToString(args.Debug).Untyped().(pulumi.StringOutput), }, } envVars = append(envVars, args.component.GetEnvVars()...) envVars = append(envVars, args.Monitoring.GetEnvVars(ctx)...) return batchv1.NewJob(ctx, "migrate", &batchv1.JobArgs{ Metadata: &metav1.ObjectMetaArgs{ Namespace: args.Namespace.ToOutput(ctx.Context()).Untyped().(pulumi.StringOutput), }, Spec: batchv1.JobSpecArgs{ BackoffLimit: pulumi.Int(6), ActiveDeadlineSeconds: pulumi.Int(600), // 10 minutes timeout Template: corev1.PodTemplateSpecArgs{ Spec: corev1.PodSpecArgs{ RestartPolicy: pulumi.String("OnFailure"), Containers: corev1.ContainerArray{ corev1.ContainerArgs{ Name: pulumi.String("migrate"), Args: pulumi.StringArray{ pulumi.String("migrate"), }, Image: utils.GetMainImage(args.Tag), ImagePullPolicy: args.ImagePullPolicy.ToOutput(ctx.Context()).Untyped().(pulumi.StringOutput), Env: envVars, Resources: &corev1.ResourceRequirementsArgs{ Limits: pulumi.StringMap{ "cpu": pulumi.String("500m"), "memory": pulumi.String("512Mi"), }, Requests: pulumi.StringMap{ "cpu": pulumi.String("100m"), "memory": pulumi.String("128Mi"), }, }, }, }, }, }, }, }, append(opts, pulumi.DeleteBeforeReplace(true))...) }🤖 Prompt for AI Agents (early access)
In deployments/pulumi/pkg/storage/migrate.go between lines 17 and 50, the migration job definition lacks resource limits and backoff configurations. To fix this, add resource requests and limits under the container spec to control CPU and memory usage, and include backoffLimit and activeDeadlineSeconds fields in the job spec to manage retry behavior and job timeout. This will ensure the job runs with proper constraints and resilience in production environments.deployments/pulumi/pkg/devbox/component.go (1)
44-59: 🛠️ Refactor suggestion
Set resource limits to prevent resource starvation
The pod specification is missing resource limits and requests for the containers, which could lead to resource starvation in a shared cluster.
Spec: corev1.PodSpecArgs{ TerminationGracePeriodSeconds: pulumi.IntPtr(0), Containers: containers, + Resources: &corev1.ResourceRequirementsArgs{ + Limits: pulumi.StringMap{ + "cpu": pulumi.String("500m"), + "memory": pulumi.String("512Mi"), + }, + Requests: pulumi.StringMap{ + "cpu": pulumi.String("100m"), + "memory": pulumi.String("128Mi"), + }, + }, },Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents (early access)
In deployments/pulumi/pkg/devbox/component.go between lines 44 and 59, the PodSpec for the deployment containers lacks resource limits and requests, which can cause resource starvation in a shared cluster. Add resource requests and limits fields to each container specification within the Containers array, specifying appropriate CPU and memory values to ensure fair resource allocation and prevent any container from consuming excessive resources.deployments/pulumi/pkg/storage/component_external.go (1)
53-72: 🛠️ Refactor suggestion
Add validation for required connection properties
The component constructor doesn't validate that critical connection properties like endpoint, username, and password are provided. Consider adding validation to prevent potential runtime errors.
func newExternalDatabaseComponent(ctx *pulumi.Context, name string, args ExternalDatabaseComponentArgs, opts ...pulumi.ResourceOption) (*externalDatabaseComponent, error) { cmp := &externalDatabaseComponent{} + + // Validate required inputs + if args.Endpoint == nil { + return nil, fmt.Errorf("endpoint is required") + } + if args.Username == nil { + return nil, fmt.Errorf("username is required") + } + if args.Password == nil { + return nil, fmt.Errorf("password is required") + } + if args.Port == nil { + return nil, fmt.Errorf("port is required") + } + if args.Database == nil { + return nil, fmt.Errorf("database is required") + } + err := ctx.RegisterComponentResource("Formance:Ledger:ExternalPostgres", name, cmp, opts...) if err != nil { return nil, err }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.func newExternalDatabaseComponent(ctx *pulumi.Context, name string, args ExternalDatabaseComponentArgs, opts ...pulumi.ResourceOption) (*externalDatabaseComponent, error) { cmp := &externalDatabaseComponent{} // Validate required inputs if args.Endpoint == nil { return nil, fmt.Errorf("endpoint is required") } if args.Username == nil { return nil, fmt.Errorf("username is required") } if args.Password == nil { return nil, fmt.Errorf("password is required") } if args.Port == nil { return nil, fmt.Errorf("port is required") } if args.Database == nil { return nil, fmt.Errorf("database is required") } err := ctx.RegisterComponentResource("Formance:Ledger:ExternalPostgres", name, cmp, opts...) if err != nil { return nil, err } cmp.Endpoint = args.Endpoint cmp.Username = args.Username cmp.Password = args.Password cmp.Port = args.Port cmp.Options = args.Options cmp.Database = args.Database if err := ctx.RegisterResourceOutputs(cmp, pulumi.Map{}); err != nil { return nil, fmt.Errorf("registering outputs: %w", err) } return cmp, nil }🤖 Prompt for AI Agents (early access)
In deployments/pulumi/pkg/storage/component_external.go around lines 53 to 72, the newExternalDatabaseComponent function lacks validation for essential connection properties such as endpoint, username, and password. Add checks after assigning args to cmp fields to verify these required properties are not empty or nil, and return an error if any are missing. This will prevent runtime errors by ensuring all critical connection details are provided before proceeding.deployments/pulumi/pkg/api/component.go (1)
20-40: 🛠️ Refactor suggestion
Avoid hardcoded image version
The
GetDevBoxContainermethod uses a hardcoded image version (alpine/httpie:3.2.4). Consider using a constant or configuration variable for better maintainability.func (cmp *Component) GetDevBoxContainer(_ context.Context) corev1.ContainerInput { return corev1.ContainerArgs{ Name: pulumi.String("api"), - Image: pulumi.String("alpine/httpie:3.2.4"), + Image: pulumi.String("alpine/httpie:" + HttpieVersion), Command: pulumi.StringArray{ pulumi.String("sleep"), }, Args: pulumi.StringArray{ pulumi.String("infinity"), }, Env: corev1.EnvVarArray{ corev1.EnvVarArgs{ Name: pulumi.String("API_URL"), Value: pulumi.Sprintf("http://%s.%s.svc.cluster.local:8080", cmp.Service.Metadata.Name().Elem(), cmp.Service.Metadata.Namespace().Elem(), ), }, }, } }Add this constant at the package level:
// HttpieVersion is the version of httpie to use for the devbox container const HttpieVersion = "3.2.4"🤖 Prompt for AI Agents (early access)
In deployments/pulumi/pkg/api/component.go around lines 20 to 40, the GetDevBoxContainer method uses a hardcoded image version "alpine/httpie:3.2.4". To improve maintainability, define a package-level constant named HttpieVersion with the value "3.2.4" and update the Image field to use this constant instead of the hardcoded string.deployments/pulumi/pkg/api/ingress.go (1)
23-49: 🛠️ Refactor suggestion
Use parameterized ingress name
The ingress name is hardcoded as "ledger" on line 24, which might cause conflicts if multiple ingresses need to be created in the same namespace.
-func createIngress(ctx *pulumi.Context, args createIngressArgs, opts ...pulumi.ResourceOption) (*networkingv1.Ingress, error) { - return networkingv1.NewIngress(ctx, "ledger", &networkingv1.IngressArgs{ +func createIngress(ctx *pulumi.Context, name string, args createIngressArgs, opts ...pulumi.ResourceOption) (*networkingv1.Ingress, error) { + return networkingv1.NewIngress(ctx, name, &networkingv1.IngressArgs{And update the caller in component.go:
-if _, err := createIngress(ctx, createIngressArgs{ +if _, err := createIngress(ctx, "ledger", createIngressArgs{📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.func createIngress(ctx *pulumi.Context, name string, args createIngressArgs, opts ...pulumi.ResourceOption) (*networkingv1.Ingress, error) { return networkingv1.NewIngress(ctx, name, &networkingv1.IngressArgs{ Metadata: &metav1.ObjectMetaArgs{ Namespace: args.Namespace.ToOutput(ctx.Context()).Untyped().(pulumi.StringOutput), }, Spec: &networkingv1.IngressSpecArgs{ Rules: networkingv1.IngressRuleArray{ networkingv1.IngressRuleArgs{ Host: args.Host.ToOutput(ctx.Context()).Untyped().(pulumi.StringOutput), Http: networkingv1.HTTPIngressRuleValueArgs{ Paths: networkingv1.HTTPIngressPathArray{ networkingv1.HTTPIngressPathArgs{ Backend: networkingv1.IngressBackendArgs{ Service: &networkingv1.IngressServiceBackendArgs{ Name: args.Service.Metadata.Name().ToOutput(ctx.Context()).Untyped().(pulumi.StringOutput), Port: networkingv1.ServiceBackendPortArgs{ Name: pulumi.String("http"), }, }, }, Path: pulumi.String("/"), PathType: pulumi.String("Prefix"), }, }, }, }, },🤖 Prompt for AI Agents (early access)
In deployments/pulumi/pkg/api/ingress.go around lines 23 to 49, the ingress resource name is hardcoded as "ledger", which can cause conflicts when creating multiple ingresses in the same namespace. Modify the createIngress function to accept an ingress name as a parameter and use it instead of the hardcoded string. Then update the caller in component.go to pass the desired ingress name when invoking createIngress.deployments/pulumi/pkg/storage/setup_postgres.go (4)
40-43:
⚠️ Potential issueNil-pointer dereference when
Installis omitted
If the caller supplies an (empty) URI but omitsInstall,a.Installisnil, yet it is dereferenced at line 42 – this panics during planning:PostgresInstallArgs: *a.Install,A safe guard (or default) must be set before dereferencing.
-PostgresInstallArgs: *a.Install, +PostgresInstallArgs: func() PostgresInstallArgs { + if a.Install == nil { + return PostgresInstallArgs{} + } + return *a.Install +}(),🤖 Prompt for AI Agents (early access)
In deployments/pulumi/pkg/storage/setup_postgres.go around lines 40 to 43, the code dereferences a.Install without checking if it is nil, which causes a panic if Install is omitted. To fix this, add a nil check for a.Install before dereferencing it; if it is nil, provide a default PostgresInstallArgs value or handle the nil case safely to avoid the panic during planning.
35-37: 🛠️ Refactor suggestion
Database name contains leading “/” from DSN path
dsn.GetPath()returns the raw path (e.g. “/ledger”), resulting in a database name “/ledger”. Trim the prefix:-Database: pulumix.Val(dsn.GetPath()), +Database: pulumix.Val(strings.TrimPrefix(dsn.GetPath(), "/")),📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.Options: pulumix.Val(dsn.GetParams()), - Database: pulumix.Val(dsn.GetPath()), + Database: pulumix.Val(strings.TrimPrefix(dsn.GetPath(), "/")), })🤖 Prompt for AI Agents (early access)
In deployments/pulumi/pkg/storage/setup_postgres.go around lines 35 to 37, the database name is set using dsn.GetPath(), which includes a leading slash (e.g., "/ledger"). To fix this, trim the leading "/" from the path before assigning it to the Database field, ensuring the database name does not contain the slash prefix.
30-37:
⚠️ Potential issue
options …pulumi.ResourceOptionsilently discarded for external DB
newExternalDatabaseComponentis called without forwardingoptions, breaking parent/aliasing settings applied by the caller.-return newExternalDatabaseComponent(ctx, "postgres", ExternalDatabaseComponentArgs{ +return newExternalDatabaseComponent(ctx, "postgres", ExternalDatabaseComponentArgs{ … -}) +}, options...)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.return newExternalDatabaseComponent(ctx, "postgres", ExternalDatabaseComponentArgs{ Endpoint: pulumix.Val(dsn.GetHost()), Username: pulumix.Val(dsn.GetUser()), Password: pulumix.Val(dsn.GetPassword()), Port: pulumix.Val(port), Options: pulumix.Val(dsn.GetParams()), Database: pulumix.Val(dsn.GetPath()), }, options...)🤖 Prompt for AI Agents (early access)
In deployments/pulumi/pkg/storage/setup_postgres.go around lines 30 to 37, the call to newExternalDatabaseComponent does not forward the variadic options parameter, causing parent and aliasing settings to be lost. Modify the function call to pass the options parameter along with the existing arguments to ensure these settings are preserved.
46-56:
⚠️ Potential issue
SetDefaultsleavesInstallnil in valid scenarios
WhenURIis provided (even as an empty string)Installmay staynil, contradicting later expectations. Add a second guard and set sensible defaults:func (a *PostgresDatabaseArgs) SetDefaults() { - if a.URI == nil && a.Install == nil { - a.Install = &PostgresInstallArgs{} - } + if a.Install == nil { + a.Install = &PostgresInstallArgs{} + } if a.URI == nil { a.URI = pulumix.Val("") } if a.Install != nil { a.Install.SetDefaults() } }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.func (a *PostgresDatabaseArgs) SetDefaults() { if a.Install == nil { a.Install = &PostgresInstallArgs{} } if a.URI == nil { a.URI = pulumix.Val("") } if a.Install != nil { a.Install.SetDefaults() } }🤖 Prompt for AI Agents (early access)
In deployments/pulumi/pkg/storage/setup_postgres.go around lines 46 to 56, the SetDefaults method does not ensure that Install is set when URI is provided, which can lead to Install remaining nil unexpectedly. Modify the method to add a second guard that sets Install to a default PostgresInstallArgs instance if it is nil, regardless of the URI value, so Install is always initialized with sensible defaults.deployments/pulumi/pkg/generator/component.go (3)
178-204: 🛠️ Refactor suggestion
Side-effects and shared
errvariable insideApplyclosure
The closure mutates the outererrvariable and creates resources – both anti-patterns for PulumiApply, and potential data races when multiple ledgers are configured. Prefer extracting the conditional resource creation outside ofApplyor return a struct from the closure and act on it afterward.🤖 Prompt for AI Agents (early access)
In deployments/pulumi/pkg/generator/component.go around lines 178 to 204, the closure passed to pulumix.Apply2Err mutates the outer err variable and creates resources inside the Apply, which is an anti-pattern and can cause data races. To fix this, refactor the code to avoid mutating the outer err by returning any error from the closure and handling it after Apply completes. Also, move the resource creation logic outside of the Apply closure to prevent side effects during Apply execution, ensuring resource creation happens in a controlled, sequential manner.
33-43:
⚠️ Potential issueMissing defaults for
SkipAwait,HTTPClientTimeout, etc.
LedgerConfiguration.SetDefaultsinitialises onlyVUs. WhenSkipAwaitisnilthe later call toApply2Errpanics. Initialise all optional inputs:func (a *LedgerConfiguration) SetDefaults() { if a.VUs == nil { a.VUs = pulumix.Val(0) } + if a.SkipAwait == nil { + a.SkipAwait = pulumix.Val(false) + } + if a.HTTPClientTimeout == nil { + a.HTTPClientTimeout = pulumix.Val(0) + } + if a.Script == nil { + a.Script = pulumix.Val("") + } + if a.ScriptFromFile == nil { + a.ScriptFromFile = pulumix.Val("") + } … }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.func (a *LedgerConfiguration) SetDefaults() { if a.VUs == nil { a.VUs = pulumix.Val(0) } if a.SkipAwait == nil { a.SkipAwait = pulumix.Val(false) } if a.HTTPClientTimeout == nil { a.HTTPClientTimeout = pulumix.Val(0) } if a.Script == nil { a.Script = pulumix.Val("") } if a.ScriptFromFile == nil { a.ScriptFromFile = pulumix.Val("") } a.VUs = pulumix.Apply(a.VUs, func(vus int) int { if vus == 0 { return 1 } return vus }) }🤖 Prompt for AI Agents (early access)
In deployments/pulumi/pkg/generator/component.go around lines 33 to 43, the SetDefaults method only initializes the VUs field, but other optional inputs like SkipAwait and HTTPClientTimeout are not set, causing panics later. Update SetDefaults to check if these fields are nil and assign appropriate default values to each to prevent nil pointer dereferences during Apply2Err calls.
61-66:
⚠️ Potential issueDefaults are not persisted back into the map
You iterate overa.Ledgers, mutate a copy, and drop the result; defaults are never stored.-for _, ledger := range a.Ledgers { - ledger.SetDefaults() +for k, v := range a.Ledgers { + v.SetDefaults() + a.Ledgers[k] = v }🤖 Prompt for AI Agents (early access)
In deployments/pulumi/pkg/generator/component.go around lines 61 to 66, the code iterates over the map a.Ledgers and calls SetDefaults on each ledger, but since map iteration returns copies of the values, the defaults set are not persisted back into the map. To fix this, retrieve each ledger by key, call SetDefaults on it, and then assign the updated ledger back to the map at the same key to ensure the changes are saved.deployments/pulumi/pkg/provision/component.go (2)
111-113:
⚠️ Potential issueWrong field name – should be
ServiceAccountName
corev1.PodSpecArgsusesServiceAccountName. UsingServiceAccountwill not compile.- ServiceAccount: serviceAccount.Metadata.Name(), + ServiceAccountName: serviceAccount.Metadata.Name(),🤖 Prompt for AI Agents (early access)
In deployments/pulumi/pkg/provision/component.go around lines 111 to 113, the field name used in corev1.PodSpecArgs is incorrect; it should be ServiceAccountName instead of ServiceAccount. Replace ServiceAccount with ServiceAccountName to match the expected field name and ensure the code compiles correctly.
27-29:
⚠️ Potential issue
ProvisionerVersionshould be an Input to compose withApply
Currently declared aspulumi.String, causing a type mismatch inApply2. Change topulumix.Input[string]:-type Args struct { - ProvisionerVersion pulumi.String +type Args struct { + ProvisionerVersion pulumix.Input[string]📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.type Args struct { ProvisionerVersion pulumix.Input[string] Ledgers map[string]LedgerConfigArgs `json:"ledgers"` }🤖 Prompt for AI Agents (early access)
In deployments/pulumi/pkg/provision/component.go at lines 27 to 29, change the type of ProvisionerVersion from pulumi.String to pulumi.Input[string] to ensure it can be composed with Apply and avoid type mismatches. Update the declaration accordingly to use the Input interface for string values.deployments/pulumi/pkg/storage/component_postgres.go (4)
118-146:
⚠️ Potential issueReliance on
pulumi/internalAPI is fragile
internals.UnsafeAwaitOutputis an internal symbol; it is not covered by Pulumi’s compatibility guarantees and may break on any SDK upgrade.You can achieve the same outcome without relying on internals:
service := pulumix.ApplyErr(cmp.Chart.Resources, func(res []any) (*corev1.Service, error) { for _, r := range res { if s, ok := r.(*corev1.Service); ok { return s, nil } } return nil, errors.New("service not found") })Keep it within the standard
pulumix.Apply*helpers and propagate theOutputdirectly.🤖 Prompt for AI Agents (early access)
In deployments/pulumi/pkg/storage/component_postgres.go around lines 118 to 146, the code uses the internal and unstable API internals.UnsafeAwaitOutput, which risks breaking on SDK upgrades. Refactor to avoid this by using pulumix.ApplyErr directly on cmp.Chart.Resources to find and return the desired *corev1.Service without awaiting the output internally. Return the Output from ApplyErr and propagate errors normally, eliminating the need for UnsafeAwaitOutput and ensuring compatibility with Pulumi's stable API.
90-103:
⚠️ Potential issue
Namespacemay benil→ panic onToOutput
newPostgresComponentdoes not validate thatargs.Namespaceis non-nil before invoking:args.Namespace.ToOutput(ctx.Context())When
Namespaceis omitted, this will panic at runtime. Add an explicit check and/or a sensible default.if args.Namespace == nil { return nil, fmt.Errorf("namespace is required") }🤖 Prompt for AI Agents (early access)
In deployments/pulumi/pkg/storage/component_postgres.go around lines 90 to 103, the code calls args.Namespace.ToOutput without checking if args.Namespace is nil, which can cause a runtime panic. To fix this, add an explicit nil check for args.Namespace before calling ToOutput, and provide a sensible default namespace value if it is nil to ensure safe execution.
152-154: 🛠️ Refactor suggestion
No meaningful outputs are exported
RegisterResourceOutputsreceives an empty map; consumers can’t reference the DB endpoint, credentials, or service.if err := ctx.RegisterResourceOutputs(cmp, pulumi.Map{ + "service": cmp.Service, + "username": cmp.Username, + "password": cmp.Password, }); err != nil {This improves composability with other components.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.if err := ctx.RegisterResourceOutputs(cmp, pulumi.Map{ "service": cmp.Service, "username": cmp.Username, "password": cmp.Password, }); err != nil { return nil, fmt.Errorf("registering outputs: %w", err) }🤖 Prompt for AI Agents (early access)
In deployments/pulumi/pkg/storage/component_postgres.go around lines 152 to 154, the call to ctx.RegisterResourceOutputs uses an empty map, so no meaningful outputs like DB endpoint, credentials, or service are exported. Update the map to include relevant output properties from the component, such as the database endpoint, username, password, and service details, so that consumers of this component can reference these outputs and improve composability.
63-82: 🛠️ Refactor suggestion
Default setters overwrite user-provided Inputs and allocate new Outputs
SetDefaultsmutatesargs.Username/args.Passwordunconditionally by wrapping them inpulumix.Apply.
If callers originally passed an already computedpulumix.Input[string](e.g. a secret reference), the extraApplylayer is unnecessary and may hinder downstream introspection or serialization.A cleaner pattern is:
func (args *PostgresInstallArgs) SetDefaults() { - if args.Username == nil { - args.Username = pulumix.Val("") - } - args.Username = pulumix.Apply(args.Username, func(username string) string { - if username == "" { - return "root" - } - return username - }) + if args.Username == nil { + args.Username = pulumix.Val("root") + } …Same for
Password.
This keeps the originalInputuntouched when it’s already set, only supplying a constant when it is nil.📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.func (args *PostgresInstallArgs) SetDefaults() { if args.Username == nil { args.Username = pulumix.Val("root") } if args.Password == nil { args.Password = pulumix.Val("") } args.Password = pulumix.Apply(args.Password, func(password string) string { if password == "" { return "password" } return password }) }🤖 Prompt for AI Agents (early access)
In deployments/pulumi/pkg/storage/component_postgres.go around lines 63 to 82, the SetDefaults method currently wraps args.Username and args.Password in pulumix.Apply unconditionally, which overwrites user-provided Inputs and adds unnecessary Apply layers. To fix this, modify SetDefaults to only assign default constant values using pulumix.Val when args.Username or args.Password are nil, and leave them unchanged if they are already set. This avoids wrapping existing Inputs and preserves their original structure for better downstream handling.deployments/pulumi/pkg/component.go (2)
29-36:
⚠️ Potential issue
InstallDevBoxlacks a default → runtime panic
SetDefaultsforgets to initialiseInstallDevBox.
Later, line 134:internals.UnsafeAwaitOutput(ctx.Context(), args.InstallDevBox.ToOutput(ctx.Context()))dereferences a
nilInput and crashes the program.func (args *ComponentArgs) SetDefaults() { … + if args.InstallDevBox == nil { + args.InstallDevBox = pulumix.Val(false) + } }🤖 Prompt for AI Agents (early access)
In deployments/pulumi/pkg/component.go around lines 29 to 36, the SetDefaults method does not initialize the InstallDevBox field, which leads to a nil pointer dereference and runtime panic later when calling ToOutput on it. To fix this, add initialization logic for InstallDevBox in SetDefaults to ensure it is not nil before use, typically by assigning it a default value or calling its own SetDefaults method if applicable.
85-92:
⚠️ Potential issue
DependsOnlist may include nil resourcesWhen the storage component uses an external DB,
cmp.Storage.DatabaseComponent(and possiblyService) isnil.
Passingnilinside apulumi.DependsOnslice results in:
interface conversion: <nil> is nil, not pulumi.ResourceFilter nils before appending:
deps := []pulumi.Resource{cmp.Storage.Credentials} if cmp.Storage.DatabaseComponent != nil { deps = append(deps, cmp.Storage.DatabaseComponent) } if cmp.Storage.Service != nil { deps = append(deps, cmp.Storage.Service) } options = append(options, pulumi.DependsOn(deps))📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.// ensure we don't pass nil into DependsOn deps := []pulumi.Resource{cmp.Storage.Credentials} if cmp.Storage.DatabaseComponent != nil { deps = append(deps, cmp.Storage.DatabaseComponent) } if cmp.Storage.Service != nil { deps = append(deps, cmp.Storage.Service) } options = append(options, pulumi.DependsOn(deps))🤖 Prompt for AI Agents (early access)
In deployments/pulumi/pkg/component.go around lines 85 to 92, the pulumi.DependsOn slice includes resources that may be nil, causing a runtime error. Before appending cmp.Storage.DatabaseComponent, cmp.Storage.Credentials, and cmp.Storage.Service to the DependsOn slice, check each for nil and only include non-nil resources to avoid passing nil values to pulumi.DependsOn.deployments/pulumi/pkg/storage/setup_rds.go (2)
168-185: 🛠️ Refactor suggestion
Missing
pulumi.Parent/pulumi.DependsOnfor snapshot resource
rds.NewClusterSnapshotis created without establishing an ownership edge to the parent component or the cluster itself.
Addingpulumi.Parent(cmp)(or at leastpulumi.DependsOn(clusterIdentifier)) ensures deletion order and keeps the snapshot under the same component tree.🤖 Prompt for AI Agents (early access)
In deployments/pulumi/pkg/storage/setup_rds.go around lines 168 to 185, the rds.NewClusterSnapshot resource is created without specifying pulumi.Parent or pulumi.DependsOn, which can cause issues with resource ownership and deletion order. To fix this, add pulumi.Parent(cmp) to the resource options to establish the parent-child relationship with the component, or at minimum add pulumi.DependsOn(clusterIdentifier) to ensure proper dependency tracking and lifecycle management.
51-59:
⚠️ Potential issueDo not default to a hard-coded master password
A universal, plaintext
"password"is a critical security vulnerability and will almost certainly be committed to state files or leaked to CI logs.Recommended approaches:
- ‑ Require the caller to provide a non-empty password and fail fast otherwise, or
- ‑ Generate a random secret via
random/random.RandomPasswordand store it in a secrets manager (e.g. AWS Secrets Manager).- if a.MasterPassword == nil { - a.MasterPassword = pulumix.Val("") - } - a.MasterPassword = pulumix.Apply(a.MasterPassword, func(password string) string { - if password == "" { - return "password" - } - return password - }) + // Do NOT set a default password; enforce explicit input + if a.MasterPassword == nil { + panic("MasterPassword must be provided – refusing to use insecure default") + }🤖 Prompt for AI Agents (early access)
In deployments/pulumi/pkg/storage/setup_rds.go around lines 51 to 59, the code defaults the master password to a hard-coded string "password", which is a security risk. Remove the default assignment of "password" and instead enforce that a non-empty password must be provided by the caller, returning an error or failing fast if it is missing. Alternatively, implement logic to generate a secure random password using the random/random.RandomPassword resource and store it securely in a secrets manager, avoiding any plaintext defaults.deployments/pulumi/pkg/monitoring/monitoring.go (1)
63-68: 🛠️ Refactor suggestion
Defaults are bypassed when
SetDefaults()is not invoked
GetEnvVarsrelies on fields such asServiceName, but the caller might forget to executeSetDefaults().
Invoking it at the top ofGetEnvVarsguarantees sane fallbacks.func (args *Args) GetEnvVars(ctx *pulumi.Context) corev1.EnvVarArray { - envVars := corev1.EnvVarArray{} + if args != nil { + args.SetDefaults() + } + envVars := corev1.EnvVarArray{}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.func (args *Args) GetEnvVars(ctx *pulumi.Context) corev1.EnvVarArray { if args != nil { args.SetDefaults() } envVars := corev1.EnvVarArray{} if args == nil { return envVars } if args.ServiceName != nil {🤖 Prompt for AI Agents (early access)
In deployments/pulumi/pkg/monitoring/monitoring.go around lines 63 to 68, the GetEnvVars method uses fields like ServiceName without ensuring defaults are set, which can cause issues if SetDefaults() was not called by the caller. To fix this, call args.SetDefaults() at the start of the GetEnvVars method to guarantee that all necessary default values are initialized before accessing any fields.deployments/pulumi/pkg/storage/component_rds.go (2)
79-80: 🛠️ Refactor suggestion
SkipFinalSnapshothard-coded totrue– risk of data lossDisabling the final snapshot removes the last-ditch backup when the cluster is destroyed.
Make this configurable or default tofalseso users must consciously opt-out.🤖 Prompt for AI Agents (early access)
In deployments/pulumi/pkg/storage/component_rds.go around lines 79 to 80, the SkipFinalSnapshot property is hard-coded to true, which risks data loss by disabling the final snapshot on cluster destruction. Modify this to be configurable via args or another input parameter, defaulting to false if not specified, so users must explicitly opt out of taking the final snapshot. Update the code to read the value from the configuration and use pulumi.Bool with that value instead of always true.
90-96:
⚠️ Potential issueGenerated cluster identifier may violate RDS naming rules
ctx.Organization(),ctx.Project()andctx.Stack()can include upper-case letters, underscores, or exceed 63 chars – all invalid for Aurora identifiers.Add a sanitiser:
sanitize := func(s string) string { s = strings.ToLower(s) s = strings.ReplaceAll(s, "_", "-") if len(s) > 20 { s = s[:20] } // keep overall length ≤ 63 return s } ClusterIdentifier: pulumi.Sprintf("%s-%s-%s", sanitize(ctx.Organization()), sanitize(ctx.Project()), sanitize(strings.ReplaceAll(ctx.Stack(), ".", "-")), ),🤖 Prompt for AI Agents (early access)
In deployments/pulumi/pkg/storage/component_rds.go around lines 90 to 96, the ClusterIdentifier is constructed using ctx.Organization(), ctx.Project(), and ctx.Stack() which may contain uppercase letters, underscores, or be longer than allowed by RDS naming rules. To fix this, create a sanitize function that converts strings to lowercase, replaces underscores with hyphens, and truncates them to a maximum length (e.g., 20 characters) to ensure the overall identifier length is within 63 characters. Then apply this sanitize function to each component before formatting the ClusterIdentifier string.deployments/pulumi/pkg/storage/component.go (3)
286-290:
⚠️ Potential issueStoring URL-escaped password breaks psql & any code that relies on the raw secret
url.QueryEscapeis great for the URI but should not overwrite the canonical secret value.
Down-stream consumers that readPOSTGRES_PASSWORD(e.g.psql, clients using
PGPASSWORD) will transmit the escaped password and fail authentication.Suggested pattern:
- Store the raw password in the secret.
- Encode only when constructing the DSN, e.g.
pulumix.Apply(password, url.QueryEscape)inside thepulumi.Sprintf.If backward compatibility is a concern, introduce a second key (
passwordUri) with the escaped value.🤖 Prompt for AI Agents (early access)
In deployments/pulumi/pkg/storage/component.go around lines 286 to 290, the password is being URL-escaped before storing, which breaks clients expecting the raw secret. To fix this, store the raw password value directly without applying url.QueryEscape. Perform URL escaping only when building the connection string (DSN), for example by applying url.QueryEscape inside pulumi.Sprintf at the point of DSN construction. If backward compatibility is needed, add a separate key like "passwordUri" to hold the escaped password instead of overwriting the original "password" key.
19-25: 🛠️ Refactor suggestion
Optionsshould be anInput, not anOutputAll the other connectivity knobs are
pulumix.Input, butOptionsis declared as
pulumix.Output[map[string]string], forcing callers to construct an output even
for static maps and making the API inconsistent.- Options pulumix.Output[map[string]string] + Options pulumix.Input[map[string]string]📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.type ConnectivityDatabaseArgs struct { AWSEnableIAM pulumix.Input[bool] MaxIdleConns pulumix.Input[*int] MaxOpenConns pulumix.Input[*int] ConnMaxIdleTime pulumix.Input[*time.Duration] - Options pulumix.Output[map[string]string] + Options pulumix.Input[map[string]string] }🤖 Prompt for AI Agents (early access)
In deployments/pulumi/pkg/storage/component.go lines 19 to 25, the field Options in ConnectivityDatabaseArgs is incorrectly declared as pulumix.Output[map[string]string] while it should be pulumix.Input[map[string]string] to be consistent with other fields and allow callers to pass static maps directly. Change the type of Options from pulumix.Output to pulumix.Input accordingly.
118-142: 🛠️ Refactor suggestion
Query-string builder drops additional options when default options are nil
options == nilcauses an early return, so any extracmp.Optionssupplied by the user are silently ignored.
This makes it impossible to pass custom DSN parameters unless both maps are non-nil.- if options == nil { - return "" - } - asSlice := make([]string, 0, len(options)) - for k, v := range options { - asSlice = append(asSlice, fmt.Sprintf("%s=%s", k, v)) - } + asSlice := make([]string, 0, + len(options)+len(additionalOptions)) + + if options != nil { + for k, v := range options { + asSlice = append(asSlice, + fmt.Sprintf("%s=%s", k, v)) + } + } for k, v := range additionalOptions { asSlice = append(asSlice, fmt.Sprintf("%s=%s", k, v)) } slices.Sort(asSlice) return strings.Join(asSlice, "&")🤖 Prompt for AI Agents (early access)
In deployments/pulumi/pkg/storage/component.go around lines 118 to 142, the query-string builder returns early when the default options map is nil, causing it to ignore any additional user-supplied options. To fix this, remove the early return when options is nil and instead initialize an empty slice, then append key-value pairs from both options and additionalOptions maps before sorting and joining them. This ensures that additional options are included even if the default options map is nil.deployments/pulumi/pkg/config/config.go (2)
172-176: 🛠️ Refactor suggestion
Using
panicfor missing configuration blocks propagates to the provider
PostgresDatabase.toInputpanics when bothURIandInstallare empty.
Prefer returningnilplus an error that can be surfaced by the Pulumi plan,
or validating earlier duringLoad. Panics abort the whole deployment and make
troubleshooting harder.🤖 Prompt for AI Agents (early access)
In deployments/pulumi/pkg/config/config.go around lines 172 to 176, replace the panic call when a.Install is nil with returning nil and an error indicating the missing URI or install configuration. Modify the function signature to return an error alongside the result, and ensure the error is properly propagated so Pulumi can handle it gracefully during the plan phase instead of aborting the deployment.
426-437: 🛠️ Refactor suggestion
Nil-pointer risk when OTLP metrics are omitted
m.MonitoringMetricsOTLP.toInput()is called unconditionally.
When the field isnil, this results in a panic before your guard ever runs.- OTLP: m.MonitoringMetricsOTLP.toInput(), + OTLP: func() *monitoring.EndpointArgs { + if m.MonitoringMetricsOTLP == nil { + return nil + } + return m.MonitoringMetricsOTLP.toInput() + }(),📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.func (m *MonitoringMetrics) toInput() *monitoring.MetricsArgs { if m == nil { return nil } return &monitoring.MetricsArgs{ PushInterval: pulumix.Val(m.PushInterval), Runtime: pulumix.Val(m.Runtime), MinimumReadMemStatsInterval: pulumix.Val(m.RuntimeMinimumReadMemStatsInterval), Exporter: pulumix.Val(m.Exporter), KeepInMemory: pulumix.Val(m.KeepInMemory), OTLP: func() *monitoring.EndpointArgs { if m.MonitoringMetricsOTLP == nil { return nil } return m.MonitoringMetricsOTLP.toInput() }(), } }🤖 Prompt for AI Agents (early access)
In deployments/pulumi/pkg/config/config.go around lines 426 to 437, the method calls m.MonitoringMetricsOTLP.toInput() without checking if m.MonitoringMetricsOTLP is nil, which can cause a panic. Add a nil check for m.MonitoringMetricsOTLP before calling toInput(), and only call it if it is non-nil; otherwise, set the OTLP field to nil in the returned struct.
No description provided.