Skip to content
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

Merged
merged 1 commit into from
Nov 24, 2024

Conversation

GaoNeng-wWw
Copy link
Collaborator

@GaoNeng-wWw GaoNeng-wWw commented Nov 23, 2024

PR

PR Checklist

Please check if your PR fulfills the following requirements:

  • The commit message follows our Commit Message Guidelines
  • Tests for the changes have been added (for bug fixes / features)
  • Docs have been added / updated (for bug fixes / features)

PR Type

What kind of change does this PR introduce?

  • Bugfix
  • Feature
  • Code style update (formatting, local variables)
  • Refactoring (no functional changes, no api changes)
  • Build related changes
  • CI related changes
  • Documentation content changes
  • Other... Please describe:

What is the current behavior?

Issue Number: N/A

What is the new behavior?

Does this PR introduce a breaking change?

  • Yes
  • No

Other information

  • MYSQL_ROOT_PASSWORD (This variable is mandatory and specifies the password that will be set for the MySQL root superuser account. see here)
  • MYSQL_DATABASE
  • MYSQL_USER
  • MYSQL_PASSWORD

Summary by CodeRabbit

  • New Features

    • Added a new dependency for YAML support, enhancing future configuration capabilities.
    • Improved project setup and configuration for server projects, particularly with Docker integration.
  • Bug Fixes

    • Enhanced error logging for project creation and configuration processes, improving overall reliability.

Copy link

coderabbitai bot commented Nov 23, 2024

Walkthrough

The pull request introduces updates to the @opentiny/tiny-toolkit-pro package, specifically enhancing the package.json and init.ts files. A new dependency, "yaml": "^2.6.1", is added to the package.json, while the init.ts file sees modifications that improve project setup and Docker configuration handling. Key changes include updated file handling methods, enhanced error logging during project creation, and modifications to the Docker Compose configuration process.

Changes

File Path Change Summary
packages/toolkits/pro/package.json Added new dependency: "yaml": "^2.6.1"
packages/toolkits/pro/src/lib/init.ts Updated fs import to use readFileSync, enhanced createServerSync and createProjectSync functions with improved error logging and Docker configuration handling.

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
Loading

Poem

🐇 In the meadow where bunnies play,
A new tool comes to light today.
With YAML's charm and Docker's grace,
Our projects now find a happier place.
Errors logged, no more dismay,
Hooray for changes, hip-hip-hooray! 🎉


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?

❤️ Share
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Generate unit testing code for this file.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai generate unit testing code for this file.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and generate unit testing code.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

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)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

Copy link

@coderabbitai coderabbitai bot left a 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 messages

The 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 manager

The current implementation directly modifies the Docker Compose configuration. Consider extracting this logic into a dedicated configuration manager class that can:

  1. Handle different service types (not just MySQL)
  2. Provide validation and schema checking
  3. Manage configuration backups
  4. 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

📥 Commits

Reviewing files that changed from the base of the PR and between a0dfef3 and 06e1db2.

📒 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:

  1. When username is 'root', both MYSQL_USER and MYSQL_PASSWORD are set to undefined
  2. 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:

  1. The code uses the yaml package which properly handles undefined values by omitting them from the output
  2. This behavior is confirmed by the template docker-compose.yml which shows only defined environment variables
  3. 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

Comment on lines +214 to +221
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));
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

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.

Suggested change
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;
}

@kagol kagol merged commit c9c5688 into opentiny:dev Nov 24, 2024
@kagol kagol added enhancement New feature or request toolkit-pro toolkit-pro labels Nov 24, 2024
@coderabbitai coderabbitai bot mentioned this pull request Nov 28, 2024
13 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request toolkit-pro toolkit-pro
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants