feat: feature cleanup #18944
feat: feature cleanup #18944
Conversation
WalkthroughThe change removes the text "Hey there test" from the welcome message in the unicorn app's main page. Additionally, a comment line Changes
Tip ⚡️ Faster reviews with caching
Enjoy the performance boost—your workflow just got faster. ✨ Finishing Touches
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
Documentation and Community
|
|
View your CI Pipeline Execution ↗ for commit 63b892f.
☁️ Nx Cloud last updated this comment at |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
apps/unicorn-app/src/app/page.tsx (1)
3-15: Enhance semantic HTML structure
Consider replacing the nested<div>wrappers with semantic elements (<main>,<section>) to improve accessibility, SEO, and reduce DOM complexity. For example:- return ( - <div> - <div className="wrapper"> - <div className="container"> - <div id="welcome"> - <h1> - <span role="img" aria-label="unicorn"> - Welcome unicorn 🦄 - </span> - </h1> - </div> - </div> - </div> - </div> - ) + return ( + <main className="wrapper container"> + <section id="welcome"> + <h1> + <span role="img" aria-label="unicorn"> + Welcome unicorn 🦄 + </span> + </h1> + </section> + </main> + )This change enhances semantics and keeps the styling intact by combining wrapper classes.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
apps/unicorn-app/src/app/page.tsx(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
`apps/**/*`: "Confirm that the code adheres to the following: - NextJS best practices, including file structure, API routes, and static generation methods. - Efficient state manage...
apps/**/*: "Confirm that the code adheres to the following:
- NextJS best practices, including file structure, API routes, and static generation methods.
- Efficient state management and server-side rendering techniques.
- Optimal use of TypeScript for component and utility type safety."
apps/unicorn-app/src/app/page.tsx
🔇 Additional comments (1)
apps/unicorn-app/src/app/page.tsx (1)
9-9: Clean up welcome message
The removal of the placeholder " Hey there test" simplifies the UI and aligns with the updated requirements.
Datadog ReportAll test runs ✅ 2 Total Test Services: 0 Failed, 2 Passed Test Services
|
robertaandersen
added
deploy-feature
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (4)
infra/src/dsl/output-generators/feature-jobs.ts (1)
101-163: Reduce duplication between job generation functionsThere's significant code duplication between
generateCleanUpForFeatureandgenerateJobsForFeature. This could lead to maintenance challenges.Consider refactoring to extract common functionality into a shared helper function:
const generateBaseJobForFeature = async ( image: string, services: ServiceDefinitionForEnv[], env: EnvironmentConfig, options: { jobPrefix: string, containerCommand: string | string[], includeEnv: boolean, annotations?: Record<string, string>, } ): Promise<FeatureKubeJob> => { const feature = env.feature if (typeof feature === 'undefined') { throw new Error('Feature jobs with a feature name not defined') } // ... rest of common logic } // Then use it in both functions: export const generateJobsForFeature = async (...) => { return generateBaseJobForFeature(..., { jobPrefix: 'create-db', containerCommand: ['/app/create-db.sh'], includeEnv: true }) } export const generateCleanUpForFeature = async (...) => { return generateBaseJobForFeature(..., { jobPrefix: 'destroy-fd', containerCommand: ['/app/destroy-dbs.sh', feature], includeEnv: false, annotations: { 'argocd.argoproj.io/hook': 'PostDelete' } }) }infra/src/feature-env.ts (3)
334-338: Unused parameter in cleanup commandThe
containerCommandparameter is defined but not used in the handler function.Either use this parameter in the handler function or remove it if it's not needed:
// If you want to use it, modify line 346: featureYaml = await renderCleanUpForFeature( env, typedArgv.cleanupImage!, affectedServices, typedArgv.containerCommand // Pass the parameter here ); // Or remove lines 334-338 if not needed
358-359: Fix typo in console log messageThere's a syntax error in the console.log message with an extra closing curly brace.
- console.log(`writing file to: ${writeDest}/${affectedServices[0].name()}/cleanup-fd-job.yaml}`) + console.log(`writing file to: ${writeDest}/${affectedServices[0].name()}/cleanup-fd-job.yaml`)
319-368: Reduce code duplication between jobs and cleanup commandsThe cleanup command implementation duplicates much of the jobs command logic, which could lead to maintenance challenges.
Consider extracting the common pattern into a reusable function:
const handleJobCommand = async (argv: unknown, jobType: 'bootstrap' | 'cleanup') => { const typedArgv = (argv as unknown) as Arguments const { affectedServices, env, writeDest } = parseArguments(typedArgv) // Determine which function and image to use based on jobType const renderFn = jobType === 'bootstrap' ? renderHelmJobForFeature : renderCleanUpForFeature const image = jobType === 'bootstrap' ? typedArgv.jobImage! : typedArgv.cleanupImage! const featureYaml = await renderFn(env, image, affectedServices) if (featureYaml.spec.template.spec.containers.length <= 0 || affectedServices.length <= 0) { return } const svcString = dumpJobYaml(featureYaml) const filename = jobType === 'bootstrap' ? 'bootstrap-fd-job.yaml' : 'cleanup-fd-job.yaml' if (writeDest != '') { try { fs.mkdirSync(`${writeDest}/${affectedServices[0].name()}`, { recursive: true }) console.log(`writing file to: ${writeDest}/${affectedServices[0].name()}/${filename}`) fs.writeFileSync(`${writeDest}/${affectedServices[0].name()}/${filename}`, svcString); } catch (error) { console.log(`Failed to write values file for ${affectedServices[0].name()}:`, error) throw new Error(`Failed to write values for ${affectedServices[0].name()}`); } } else { await writeToOutput(svcString, typedArgv.output) } } // Then use this in both command handlers
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
infra/scripts/container-scripts/destroy-dbs.sh(1 hunks)infra/scripts/generate-feature-values.sh(1 hunks)infra/src/dsl/exports/helm.ts(2 hunks)infra/src/dsl/output-generators/feature-jobs.ts(1 hunks)infra/src/dsl/types/output-types.ts(1 hunks)infra/src/feature-env.ts(8 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
`infra/src/dsl/**/*`: "Confirm that the code adheres to the following: - The clarity and expressiveness of the DSL syntax. - Integration with Helm charts and Kubernetes resources. ...
infra/src/dsl/**/*: "Confirm that the code adheres to the following:
- The clarity and expressiveness of the DSL syntax.
- Integration with Helm charts and Kubernetes resources.
- Documentation on how to use the DSL to create complex Helm values."
infra/src/dsl/types/output-types.tsinfra/src/dsl/exports/helm.tsinfra/src/dsl/output-generators/feature-jobs.ts
🧬 Code Graph Analysis (1)
infra/src/dsl/exports/helm.ts (2)
apps/native/app/src/stores/environment-store.ts (1)
EnvironmentConfig(7-14)infra/src/dsl/output-generators/feature-jobs.ts (1)
generateCleanUpForFeature(101-163)
🪛 Shellcheck (0.10.0)
infra/scripts/container-scripts/destroy-dbs.sh
[warning] 11-11: feature is referenced but not assigned.
(SC2154)
🔇 Additional comments (8)
infra/src/dsl/types/output-types.ts (1)
149-149: Well-structured metadata extension for Kubernetes annotations supportThe addition of the optional
annotationsfield to theFeatureKubeJobinterface's metadata property is a well-structured change that aligns with standard Kubernetes resource definitions. This enhancement properly enables annotation support for the cleanup job functionality, which is essential for features like the ArgoCD PostDelete hook integration shown in the related code snippets.infra/scripts/generate-feature-values.sh (1)
34-41: Good integration of cleanup stepThe addition of the cleanup command follows the existing pattern of the script and integrates well with the current workflow. It correctly passes all the necessary parameters to generate cleanup job manifests.
infra/scripts/container-scripts/destroy-dbs.sh (1)
13-21: Confirm intentionally commented-out database cleanup logicThe database and user cleanup commands have been commented out, which means the cleanup job will not perform any actual cleanup operations. Please verify if this is intentional or if this is meant to be a temporary change during development.
If the commented code is intentional as a safety measure, consider adding an explanatory comment about why these operations are disabled and when they should be re-enabled.
infra/src/dsl/exports/helm.ts (1)
14-14: Clean import additionGood addition of the import for
generateCleanUpForFeaturefrom the feature-jobs module.infra/src/dsl/output-generators/feature-jobs.ts (2)
110-113: LGTM: Appropriate security context settingsThe security context settings with
privileged: falseandallowPrivilegeEscalation: falseare correctly maintained from the original function, which follows security best practices.
149-151: Great integration with ArgoCD lifecyclesThe addition of the ArgoCD PostDelete hook annotation is an excellent practice that ensures proper cleanup after environment deletion.
infra/src/feature-env.ts (2)
320-322: Clear and descriptive command designThe command name and description clearly communicate its purpose for cleaning up feature environments.
19-19: Maintain proper import formattingThe new import is correctly added to the existing import list from the same module.
| export const renderCleanUpForFeature = async ( | ||
| env: EnvironmentConfig, | ||
| image: string, | ||
| services: ServiceBuilder<any>[], | ||
| ) => { | ||
| const result = prepareServicesForEnv({ | ||
| services: services, | ||
| env: env, | ||
| outputFormat: renderers.helm, | ||
| }) | ||
| return generateCleanUpForFeature(image, result, env) | ||
| } |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Well-structured cleanup function implementation
The new renderCleanUpForFeature function follows the same pattern as the existing renderHelmJobForFeature function, which maintains consistency in the codebase. The implementation correctly prepares services for the environment before generating the cleanup job.
I noticed that generateCleanUpForFeature expects a feature property on the EnvironmentConfig object, but this property isn't explicitly shown in the interface definition from the relevant code snippets. Verify that this property is correctly defined and always available when needed to avoid potential runtime errors.
🏁 Script executed:
#!/bin/bash
# Check if the EnvironmentConfig interface includes the 'feature' property
rg -A 10 "interface EnvironmentConfig" --type tsLength of output: 1430
🏁 Script executed:
#!/bin/bash
# Locate definition and usage of generateCleanUpForFeature to verify its expected parameters
rg -n "generateCleanUpForFeature" --type ts .Length of output: 351
🏁 Script executed:
#!/bin/bash
# Show context around generateCleanUpForFeature definition
rg -n -A 20 "export const generateCleanUpForFeature" infra/src/dsl/output-generators/feature-jobs.tsLength of output: 935
🏁 Script executed:
#!/bin/bash
# Display import statements and context for EnvironmentConfig in helm.ts
sed -n '1,50p' infra/src/dsl/exports/helm.tsLength of output: 1428
🏁 Script executed:
#!/bin/bash
# Search for 'feature' mentions in the EnvironmentConfig type definition
rg -n "feature" infra/src/dsl/types/charts.tsLength of output: 144
Ensure feature Is Defined for Cleanup Jobs
The EnvironmentConfig interface (infra/src/dsl/types/charts.ts) declares
feature?: stringbut generateCleanUpForFeature immediately does:
const feature = env.feature
if (typeof feature === 'undefined') {
throw new Error('Feature jobs with a feature name not defined')
}This means calling renderCleanUpForFeature(env, …) with an env lacking feature will throw at runtime.
To address this:
- Either make
featurea required property onEnvironmentConfig(remove?) for contexts that need it. - Or tighten the signature of
renderCleanUpForFeatureto acceptenv: EnvironmentConfig & { feature: string }. - Alternatively, add an explicit precondition check/clear error in
renderCleanUpForFeatureso missingfeatureis caught earlier and with a more descriptive message.
Locations needing updates:
- infra/src/dsl/types/charts.ts – change
feature?: stringtofeature: string(or introduce a narrower subtype). - infra/src/dsl/exports/helm.ts – update
renderCleanUpForFeature’senvparameter type or add an explicit check before callinggenerateCleanUpForFeature.
🤖 Prompt for AI Agents (early access)
In infra/src/dsl/exports/helm.ts lines 84 to 95, the renderCleanUpForFeature
function uses env.feature which is optional in EnvironmentConfig, causing
potential runtime errors. Fix this by either updating the EnvironmentConfig
interface in infra/src/dsl/types/charts.ts to make feature a required string
property or by narrowing the env parameter type in renderCleanUpForFeature to
EnvironmentConfig & { feature: string }. Alternatively, add an explicit check in
renderCleanUpForFeature to throw a clear error if env.feature is undefined
before calling generateCleanUpForFeature.
| command: [`/app/destroy-dbs.sh ${feature}`], | ||
| image, | ||
| name: `${info!.username!.replace(/_/g, '-').substr(0, 60)}1`, | ||
| securityContext, | ||
| env: [], | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Consider environment variables for database connection in cleanup containers
While the creation job properly sets environment variables for database connections, the cleanup containers have an empty environment array.
The cleanup script likely needs at least some environment variables to connect to the database. Consider adding the essential database connection variables:
env: [],
+env: [
+ {
+ name: 'PGHOST',
+ value: host.writer,
+ },
+ {
+ name: 'PGDATABASE',
+ value: 'postgres',
+ },
+ {
+ name: 'PGUSER',
+ value: 'root',
+ },
+ {
+ name: 'PGPASSWORD_KEY',
+ value: '/rds/vidspyrna/masterpassword',
+ }
+],📝 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.
| command: [`/app/destroy-dbs.sh ${feature}`], | |
| image, | |
| name: `${info!.username!.replace(/_/g, '-').substr(0, 60)}1`, | |
| securityContext, | |
| env: [], | |
| } | |
| command: [`/app/destroy-dbs.sh ${feature}`], | |
| image, | |
| name: `${info!.username!.replace(/_/g, '-').substr(0, 60)}1`, | |
| securityContext, | |
| env: [ | |
| { | |
| name: 'PGHOST', | |
| value: host.writer, | |
| }, | |
| { | |
| name: 'PGDATABASE', | |
| value: 'postgres', | |
| }, | |
| { | |
| name: 'PGUSER', | |
| value: 'root', | |
| }, | |
| { | |
| name: 'PGPASSWORD_KEY', | |
| value: '/rds/vidspyrna/masterpassword', | |
| }, | |
| ], | |
| } |
🤖 Prompt for AI Agents (early access)
In infra/src/dsl/output-generators/feature-jobs.ts around lines 124 to 129, the
cleanup container's env array is empty, but it needs essential environment
variables for database connections similar to the creation job. Fix this by
adding the necessary database connection environment variables to the env array
in the cleanup container configuration to ensure the cleanup script can connect
to the database properly.
| export const generateCleanUpForFeature = async ( | ||
| image: string, | ||
| services: ServiceDefinitionForEnv[], | ||
| env: EnvironmentConfig, | ||
| ): Promise<FeatureKubeJob> => { | ||
| const feature = env.feature | ||
| if (typeof feature === 'undefined') { | ||
| throw new Error('Feature jobs with a feature name not defined') | ||
| } | ||
| const securityContext = { | ||
| privileged: false, | ||
| allowPrivilegeEscalation: false, | ||
| } | ||
| const containers = Object.values(services) | ||
| .map((service) => | ||
| [service.postgres, service.initContainers?.postgres] | ||
| .filter((id) => id) | ||
| .map((info) => { | ||
| const host = resolveDbHost(service, env, info?.host) | ||
| const extensions = getPostgresExtensions( | ||
| service.initContainers?.postgres?.extensions, | ||
| ) | ||
| return { | ||
| command: [`/app/destroy-dbs.sh ${feature}`], | ||
| image, | ||
| name: `${info!.username!.replace(/_/g, '-').substr(0, 60)}1`, | ||
| securityContext, | ||
| env: [], | ||
| } | ||
| }), | ||
| ) | ||
| .reduce((acc, cur) => { | ||
| let result = acc | ||
| cur.forEach((c) => { | ||
| if (result.map((a) => a.name).indexOf(c.name) === -1) { | ||
| result = result.concat([c]) | ||
| } | ||
| }) | ||
| return result | ||
| }, []) | ||
| return { | ||
| apiVersion: 'batch/v1', | ||
| kind: 'Job', | ||
| metadata: { | ||
| name: resolveWithMaxLength( | ||
| `destroy-fd-${feature}-${new Date().getTime()}`, | ||
| 62, | ||
| ), | ||
| annotations: { | ||
| 'argocd.argoproj.io/hook': 'PostDelete' | ||
| } | ||
| }, | ||
| spec: { | ||
| template: { | ||
| spec: { | ||
| serviceAccountName: 'feature-deployment', | ||
| containers, | ||
| restartPolicy: 'Never', | ||
| }, | ||
| }, | ||
| }, | ||
| } | ||
| } |
There was a problem hiding this comment.
Command format inconsistency between creation and cleanup jobs
The implementation of the generateCleanUpForFeature function has a command format inconsistency compared to the original generateJobsForFeature function. In Kubernetes, container commands should be specified as arrays, not strings.
At line 124, you're using a string for the command:
- command: [`/app/destroy-dbs.sh ${feature}`],
+ command: ['/app/destroy-dbs.sh', feature],This is inconsistent with line 30 in the original function which correctly uses an array format. The current implementation might cause shell parsing issues or even potential shell injection vulnerabilities.
📝 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.
| export const generateCleanUpForFeature = async ( | |
| image: string, | |
| services: ServiceDefinitionForEnv[], | |
| env: EnvironmentConfig, | |
| ): Promise<FeatureKubeJob> => { | |
| const feature = env.feature | |
| if (typeof feature === 'undefined') { | |
| throw new Error('Feature jobs with a feature name not defined') | |
| } | |
| const securityContext = { | |
| privileged: false, | |
| allowPrivilegeEscalation: false, | |
| } | |
| const containers = Object.values(services) | |
| .map((service) => | |
| [service.postgres, service.initContainers?.postgres] | |
| .filter((id) => id) | |
| .map((info) => { | |
| const host = resolveDbHost(service, env, info?.host) | |
| const extensions = getPostgresExtensions( | |
| service.initContainers?.postgres?.extensions, | |
| ) | |
| return { | |
| command: [`/app/destroy-dbs.sh ${feature}`], | |
| image, | |
| name: `${info!.username!.replace(/_/g, '-').substr(0, 60)}1`, | |
| securityContext, | |
| env: [], | |
| } | |
| }), | |
| ) | |
| .reduce((acc, cur) => { | |
| let result = acc | |
| cur.forEach((c) => { | |
| if (result.map((a) => a.name).indexOf(c.name) === -1) { | |
| result = result.concat([c]) | |
| } | |
| }) | |
| return result | |
| }, []) | |
| return { | |
| apiVersion: 'batch/v1', | |
| kind: 'Job', | |
| metadata: { | |
| name: resolveWithMaxLength( | |
| `destroy-fd-${feature}-${new Date().getTime()}`, | |
| 62, | |
| ), | |
| annotations: { | |
| 'argocd.argoproj.io/hook': 'PostDelete' | |
| } | |
| }, | |
| spec: { | |
| template: { | |
| spec: { | |
| serviceAccountName: 'feature-deployment', | |
| containers, | |
| restartPolicy: 'Never', | |
| }, | |
| }, | |
| }, | |
| } | |
| } | |
| export const generateCleanUpForFeature = async ( | |
| image: string, | |
| services: ServiceDefinitionForEnv[], | |
| env: EnvironmentConfig, | |
| ): Promise<FeatureKubeJob> => { | |
| const feature = env.feature | |
| if (typeof feature === 'undefined') { | |
| throw new Error('Feature jobs with a feature name not defined') | |
| } | |
| const securityContext = { | |
| privileged: false, | |
| allowPrivilegeEscalation: false, | |
| } | |
| const containers = Object.values(services) | |
| .map((service) => | |
| [service.postgres, service.initContainers?.postgres] | |
| .filter((id) => id) | |
| .map((info) => { | |
| const host = resolveDbHost(service, env, info?.host) | |
| const extensions = getPostgresExtensions( | |
| service.initContainers?.postgres?.extensions, | |
| ) | |
| return { | |
| command: ['/app/destroy-dbs.sh', feature], | |
| image, | |
| name: `${info!.username!.replace(/_/g, '-').substr(0, 60)}1`, | |
| securityContext, | |
| env: [], | |
| } | |
| }), | |
| ) | |
| .reduce((acc, cur) => { | |
| let result = acc | |
| cur.forEach((c) => { | |
| if (result.map((a) => a.name).indexOf(c.name) === -1) { | |
| result = result.concat([c]) | |
| } | |
| }) | |
| return result | |
| }, []) | |
| return { | |
| apiVersion: 'batch/v1', | |
| kind: 'Job', | |
| metadata: { | |
| name: resolveWithMaxLength( | |
| `destroy-fd-${feature}-${new Date().getTime()}`, | |
| 62, | |
| ), | |
| annotations: { | |
| 'argocd.argoproj.io/hook': 'PostDelete' | |
| } | |
| }, | |
| spec: { | |
| template: { | |
| spec: { | |
| serviceAccountName: 'feature-deployment', | |
| containers, | |
| restartPolicy: 'Never', | |
| }, | |
| }, | |
| }, | |
| } | |
| } |
🤖 Prompt for AI Agents (early access)
In infra/src/dsl/output-generators/feature-jobs.ts between lines 101 and 163,
the command for containers in generateCleanUpForFeature is incorrectly specified
as a single string instead of an array of strings. To fix this, change the
command property from a single string like "/app/destroy-dbs.sh ${feature}" to
an array format such as ["/app/destroy-dbs.sh", feature]. This aligns with
Kubernetes best practices and matches the format used in generateJobsForFeature,
preventing shell parsing issues and improving security.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (4)
infra/src/secrets.ts (4)
114-114: Avoid usinganytype when possible.Using
anybypasses TypeScript's type checking, which can lead to runtime errors. Consider using a more specific type forParameterListbased on the AWS SDK type definitions.-let ParameterList: any = [] +let ParameterList: AWS.SSM.ParameterMetadataList = []
117-135: Improve code readability and fix indentation issues.The indentation inside the while loop is inconsistent. Also, the condition check for
NextTokenis overly verbose and redundant.while (true) { let response = await ssm .describeParameters({ ParameterFilters: [ { Key: 'Name', Option: 'BeginsWith', Values: [prefix] }, ], NextToken, }) .promise() - NextToken = response.NextToken + NextToken = response.NextToken if (response.Parameters && response.Parameters.length > 0) { ParameterList = ParameterList.concat(response.Parameters) } - if (!NextToken || NextToken == undefined || NextToken == null || NextToken == '') { + if (!NextToken) { break } - }
117-117: Useconstfor variables that aren't reassigned.Since
responseis only assigned once within each loop iteration, it's better to useconstinstead oflet.-let response = await ssm +const response = await ssm
136-146: Consider batching delete operations for large parameter lists.AWS has API rate limits. For large numbers of parameters, consider implementing batching to avoid hitting these limits.
if (ParameterList && ParameterList.length > 0) { logger.debug( `Parameters to destroy: ${ParameterList.map(({ Name }) => Name)}`, ) + // Process in batches of 10 to avoid API rate limits + const batchSize = 10; + for (let i = 0; i < ParameterList.length; i += batchSize) { + const batch = ParameterList.slice(i, i + batchSize); await Promise.all( - ParameterList.map(({ Name }) => + batch.map(({ Name }) => Name ? ssm.deleteParameter({ Name }).promise() : new Promise((resolve) => resolve(true)), ), ) + } }
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
infra/package.json(1 hunks)infra/src/secrets.ts(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- infra/package.json
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: helm-values-validation
🔇 Additional comments (1)
infra/src/secrets.ts (1)
113-135: Good implementation of paginated parameter retrieval.The implementation effectively handles paginated responses from AWS SSM, ensuring all parameters matching the prefix are collected before deletion. This is an important improvement for handling large sets of parameters.
...
Attach a link to issue if relevant
What
Specify what you're trying to achieve
Why
Specify why you need to achieve this
Screenshots / Gifs
Attach Screenshots / Gifs to help reviewers understand the scope of the pull request
Checklist:
Summary by CodeRabbit