-
Notifications
You must be signed in to change notification settings - Fork 38
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat(cli): auto overwrite docker env when create nestjs backend #226
feat(cli): auto overwrite docker env when create nestjs backend #226
Conversation
WalkthroughThe pull request introduces updates to the Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Init
participant Docker
participant FileSystem
User->>Init: Start project creation
Init->>FileSystem: Read package.json
alt Read success
Init->>Docker: Read docker-compose.yml
Docker->>Init: Return docker-compose.yml
Init->>Init: Modify MySQL environment variables
Init->>FileSystem: Write updated docker-compose.yml
else Read failure
Init->>Init: Log error
end
Init->>User: Project setup complete
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
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 (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🧹 Outside diff range and nitpick comments (3)
packages/toolkits/pro/package.json (1)
Line range hint
69-73
: Consider adding Docker-related type definitions.Since this PR involves Docker environment manipulation, consider adding
@types/dockerode
to devDependencies for better TypeScript support when handling Docker-related operations."devDependencies": { "@bitjson/npm-scripts-info": "^1.0.0", "@istanbuljs/nyc-config-typescript": "^1.0.2", + "@types/dockerode": "^3.3.23", "@types/fs-extra": "^11.0.4", "@types/inquirer": "^9.0.3",
packages/toolkits/pro/src/lib/init.ts (2)
348-349
: Enhance error logging with specific messagesThe error logging has been improved, but consider enhancing it further with more specific error messages and recovery suggestions.
Here's a suggested improvement:
- log.error(e); - log.error('配置项目信息创建失败'); + log.error('Failed to update package.json configuration:', e); + log.error('Project configuration failed. Please ensure you have write permissions and try again.'); - log.error(e); - log.error('开启mock模式失败'); - log.info('请手动配置env信息'); + log.error('Failed to enable mock mode:', e); + log.error('Mock mode configuration failed. Manual configuration required.'); + log.info('Please configure environment variables manually in the .env file.'); - log.error(e); - log.error('项目模板创建失败'); + log.error('Failed to create project template:', e); + log.error('Project creation failed. Please ensure you have necessary permissions and try again.');Also applies to: 370-373, 471-473
200-221
: Consider implementing a dedicated Docker configuration managerThe current implementation directly modifies the Docker Compose configuration. Consider extracting this logic into a dedicated configuration manager class that can:
- Handle different service types (not just MySQL)
- Provide validation and schema checking
- Manage configuration backups
- Implement retry mechanisms for file operations
This would improve maintainability and make it easier to add support for other services in the future.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (2)
packages/toolkits/pro/package.json
(1 hunks)packages/toolkits/pro/src/lib/init.ts
(6 hunks)
🔇 Additional comments (3)
packages/toolkits/pro/package.json (1)
64-65
: Verify the yaml package version for security and compatibility.
The newly added yaml package uses caret versioning (^2.6.1
), which allows minor and patch updates. Let's verify if this is the latest stable version.
✅ Verification successful
The yaml package version 2.6.1 is secure and up-to-date
The verification shows that:
- 2.6.1 is currently the latest stable version
- The only known high-severity vulnerability affects versions < 2.2.2
- The specified version ^2.6.1 is well above the vulnerable range
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check latest version and security advisories for yaml package
# Get latest version from npm
echo "Latest yaml version:"
curl -s https://registry.npmjs.org/yaml/latest | jq '.version'
# Check for known vulnerabilities
echo "Checking for vulnerabilities:"
gh api graphql -f query='
{
securityVulnerabilities(first: 5, ecosystem: NPM, package: "yaml") {
nodes {
advisory {
summary
severity
publishedAt
}
vulnerableVersionRange
firstPatchedVersion {
identifier
}
}
}
}'
Length of output: 728
packages/toolkits/pro/src/lib/init.ts (2)
1-1
: LGTM! Necessary imports added for YAML handling
The addition of YAML parsing utilities and file system operations is appropriate for managing Docker Compose configurations.
Also applies to: 22-22
200-211
: Validate MySQL environment variables configuration
The configuration correctly handles the mandatory MySQL environment variables. However, consider the following:
- When username is 'root', both
MYSQL_USER
andMYSQL_PASSWORD
are set to undefined - This might cause issues if the Docker Compose YAML serializer doesn't handle undefined values properly
Let's verify how undefined values are handled in the YAML output:
✅ Verification successful
Let me gather more information about how the YAML is being processed in the codebase.
Let me check how the YAML is being processed by looking for the stringify
function and its imports.
MySQL environment configuration is correctly handled
The implementation is safe because:
- The code uses the
yaml
package which properly handles undefined values by omitting them from the output - This behavior is confirmed by the template docker-compose.yml which shows only defined environment variables
- When username is 'root', omitting MYSQL_USER and MYSQL_PASSWORD is the correct approach as per MySQL Docker image specification, since MYSQL_ROOT_PASSWORD is used instead
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check if the docker-compose.yml file exists and how undefined values are handled
fd -e yml -e yaml docker-compose --exec sh -c 'echo "Checking {}" && cat {} | grep -A 5 "environment:"'
Length of output: 301
Script:
#!/bin/bash
# Find the code that processes the docker-compose configuration
ast-grep --pattern 'overwriteDockerComposeConfig' -A 10
Length of output: 1901
Script:
#!/bin/bash
# Find the stringify import and implementation
rg "stringify.*yaml" -A 2
rg "from.*yaml" packages/toolkits/pro/src/lib/init.ts
Length of output: 541
const dockerComposeYaml = readFileSync( | ||
path.join(serverTo, 'docker-compose.yml') | ||
).toString(); | ||
const yaml = parse(dockerComposeYaml); | ||
const { services } = yaml; | ||
const mysql = services.mysql; | ||
mysql.environment = overwriteDockerComposeConfig; | ||
writeFileSync(path.join(serverTo, 'docker-compose.yml'), stringify(yaml)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add error handling for Docker Compose configuration
The code lacks error handling for file operations and YAML parsing. Consider adding try-catch blocks and validation.
Here's a suggested improvement:
- const dockerComposeYaml = readFileSync(
- path.join(serverTo, 'docker-compose.yml')
- ).toString();
- const yaml = parse(dockerComposeYaml);
- const { services } = yaml;
- const mysql = services.mysql;
- mysql.environment = overwriteDockerComposeConfig;
- writeFileSync(path.join(serverTo, 'docker-compose.yml'), stringify(yaml));
+ try {
+ const dockerComposePath = path.join(serverTo, 'docker-compose.yml');
+ const dockerComposeYaml = readFileSync(dockerComposePath).toString();
+ const yaml = parse(dockerComposeYaml);
+
+ if (!yaml.services?.mysql) {
+ throw new Error('MySQL service configuration not found in docker-compose.yml');
+ }
+
+ // Create backup of original configuration
+ writeFileSync(`${dockerComposePath}.backup`, dockerComposeYaml);
+
+ // Update MySQL environment
+ yaml.services.mysql.environment = overwriteDockerComposeConfig;
+
+ // Remove undefined values
+ Object.keys(yaml.services.mysql.environment).forEach(key => {
+ if (yaml.services.mysql.environment[key] === undefined) {
+ delete yaml.services.mysql.environment[key];
+ }
+ });
+
+ writeFileSync(dockerComposePath, stringify(yaml));
+ } catch (error) {
+ log.error('Failed to update Docker Compose configuration:', error);
+ throw error;
+ }
📝 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.
const dockerComposeYaml = readFileSync( | |
path.join(serverTo, 'docker-compose.yml') | |
).toString(); | |
const yaml = parse(dockerComposeYaml); | |
const { services } = yaml; | |
const mysql = services.mysql; | |
mysql.environment = overwriteDockerComposeConfig; | |
writeFileSync(path.join(serverTo, 'docker-compose.yml'), stringify(yaml)); | |
try { | |
const dockerComposePath = path.join(serverTo, 'docker-compose.yml'); | |
const dockerComposeYaml = readFileSync(dockerComposePath).toString(); | |
const yaml = parse(dockerComposeYaml); | |
if (!yaml.services?.mysql) { | |
throw new Error('MySQL service configuration not found in docker-compose.yml'); | |
} | |
// Create backup of original configuration | |
writeFileSync(`${dockerComposePath}.backup`, dockerComposeYaml); | |
// Update MySQL environment | |
yaml.services.mysql.environment = overwriteDockerComposeConfig; | |
// Remove undefined values | |
Object.keys(yaml.services.mysql.environment).forEach(key => { | |
if (yaml.services.mysql.environment[key] === undefined) { | |
delete yaml.services.mysql.environment[key]; | |
} | |
}); | |
writeFileSync(dockerComposePath, stringify(yaml)); | |
} catch (error) { | |
log.error('Failed to update Docker Compose configuration:', error); | |
throw error; | |
} |
PR
PR Checklist
Please check if your PR fulfills the following requirements:
PR Type
What kind of change does this PR introduce?
What is the current behavior?
Issue Number: N/A
What is the new behavior?
Does this PR introduce a breaking change?
Other information
Summary by CodeRabbit
New Features
Bug Fixes