feat: add name option to customize the encrypted environment file name
#911
Conversation
- `.env` -> support.EnvPath - `.env.encrypted` -> support.EnvEncryptPath - unified unit test format
…ialize path variables
WalkthroughThe changes include the removal of two AES-related error constants, modifications to command handling for environment encryption and decryption, and extensive test refactoring. New command-line flags have been added to allow custom file naming while updating file path usage to reference support constants. Several test suites have been restructured using the testify suite pattern with improved variable naming and clarity. Additionally, two new support constants have been introduced and existing ones initialized to default values. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Command
participant FS as "File System"
participant Support
User->>Command: Invoke encrypt/decrypt with --name flag
Command->>Support: Retrieve EnvPath and EnvEncryptPath
Command->>FS: Read/Write file using provided flag value
FS-->>Command: Return file content/result
Command->>User: Output result message with cipher info and file name
Suggested Reviewers
Warning There were issues while running some tools. Please review the errors and either fix the tool’s configuration or disable the tool if it’s a critical failure. 🔧 golangci-lint (1.62.2)Error: can't load config: the Go language version (go1.23) used to build golangci-lint is lower than the targeted Go version (1.24.0) 📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
⏰ Context from checks skipped due to timeout of 300000ms (1)
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 (
|
kuafuRace
changed the title
name option to customize the encrypted environment file name
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #911 +/- ##
==========================================
+ Coverage 68.01% 68.86% +0.84%
==========================================
Files 154 154
Lines 10192 10196 +4
==========================================
+ Hits 6932 7021 +89
+ Misses 2934 2852 -82
+ Partials 326 323 -3 ☔ View full report in Codecov by Sentry. |
|
Ready Review |
![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
🔭 Outside diff range comments (1)
foundation/console/env_encrypt_command.go (1)
104-104:⚠️ Potential issueSecurity concern: Predictable IV generation.
Using the key as the IV is not recommended as it makes the encryption more predictable. Consider using a cryptographically secure random IV.
- iv := key[:aes.BlockSize] + iv := make([]byte, aes.BlockSize) + if _, err := rand.Read(iv); err != nil { + return nil, fmt.Errorf("failed to generate IV: %w", err) + }
🧹 Nitpick comments (5)
foundation/console/env_decrypt_command.go (2)
81-85: Enhance error handling with specific error types.The current error handling could be more informative by providing specific error types for different failure scenarios (e.g., decryption failures vs. file system errors).
plaintext, err := r.decrypt(ciphertext, []byte(key)) if err != nil { - ctx.Error(fmt.Sprintf("Decrypt error: %v", err)) + switch { + case errors.Is(err, base64.CorruptInputError(0)): + ctx.Error("Invalid base64 encoding in encrypted file") + case errors.Is(err, aes.KeySizeError(0)): + ctx.Error("Invalid key size for decryption") + default: + ctx.Error(fmt.Sprintf("Decrypt error: %v", err)) + } return nil }
87-87: Consider using a constant for file permissions.The file permission
0644is hardcoded. Consider defining it as a constant for better maintainability and reusability.+const envFilePermissions = 0644 - err = os.WriteFile(support.EnvPath, plaintext, 0644) + err = os.WriteFile(support.EnvPath, plaintext, envFilePermissions)foundation/console/test_make_command_test.go (1)
71-100: Consider adding cleanup in a TearDown method.The file cleanup at the end of TestTestHandle could be moved to a suite-level TearDown method to ensure cleanup happens even if tests fail.
+func (s *TestMakeCommandTestSuite) TearDownTest() { + if file.Exists("tests") { + s.NoError(file.Remove("tests")) + } +} func (s *TestMakeCommandTestSuite) TestTestHandle() { // ... existing test code ... - s.NoError(file.Remove("tests")) }foundation/console/vendor_publish_command_test.go (2)
39-72: Consider simplifying theTestExtendmethod.While the test is thorough, it could be simplified:
- The type assertion error message uses an incorrect format string (
%Twithout arguments).- The deep equality comparison could be simplified by directly using
s.Equalinstead of manual reflection.Consider this simplified version:
func (s *VendorPublishCommandTestSuite) TestExtend() { cmd := &VendorPublishCommand{} got := cmd.Extend() s.Run("should return correct category", func() { expected := "vendor" s.Require().Equal(expected, got.Category) }) if len(got.Flags) > 0 { s.Run("should have correctly configured StringFlag", func() { flag, ok := got.Flags[0].(*command.BoolFlag) - if !ok { - s.Fail("existing flag is not BoolFlag (got type: %T)", got.Flags[0]) - } + s.Require().True(ok, "existing flag is not BoolFlag (got type: %T)", got.Flags[0]) - testCases := []struct { - name string - got interface{} - expected interface{} - }{ - {"Name", flag.Name, "existing"}, - {"Aliases", flag.Aliases, []string{"e"}}, - {"Usage", flag.Usage, "Publish and overwrite only the files that have already been published"}, - } - - for _, tc := range testCases { - if !reflect.DeepEqual(tc.got, tc.expected) { - s.Require().Equal(tc.expected, tc.got) - } - } + s.Equal("existing", flag.Name, "flag name should match") + s.Equal([]string{"e"}, flag.Aliases, "flag aliases should match") + s.Equal("Publish and overwrite only the files that have already been published", flag.Usage, "flag usage should match") }) } }
310-424: Improve error handling and cleanup in file operation tests.While the tests are comprehensive, consider these improvements:
- Use
t.TempDir()instead ofos.MkdirTemp()for automatic cleanup- Replace
s.Nil(err)with more descriptives.NoError(err)- Use
deferfor cleanup immediately after resource creationExample improvement for the
TestPublishmethod:func (s *VendorPublishCommandTestSuite) TestPublish() { cmd := &VendorPublishCommand{} - sourceDir, err := os.MkdirTemp("", "source") - s.Require().Nil(err) - defer func(path string) { - if err := file.Remove(path); err != nil { - panic(err) - } - }(sourceDir) + sourceDir := s.T().TempDir() + targetDir := s.T().TempDir() - targetDir, err := os.MkdirTemp("", "target") - s.Require().Nil(err) - sourceFile := filepath.Join(sourceDir, "test.txt") - s.Require().Nil(file.PutContent(sourceFile, "test")) + s.Require().NoError(file.PutContent(sourceFile, "test"), "Failed to create test file")
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (10)
errors/list.go(0 hunks)foundation/console/about_command_test.go(3 hunks)foundation/console/env_decrypt_command.go(3 hunks)foundation/console/env_decrypt_command_test.go(2 hunks)foundation/console/env_encrypt_command.go(3 hunks)foundation/console/env_encrypt_command_test.go(2 hunks)foundation/console/package_make_command_test.go(4 hunks)foundation/console/test_make_command_test.go(1 hunks)foundation/console/vendor_publish_command_test.go(13 hunks)support/constant.go(1 hunks)
💤 Files with no reviewable changes (1)
- errors/list.go
🔇 Additional comments (21)
support/constant.go (2)
14-15: LGTM! Well-defined constants for environment encryption.The new constants follow a consistent naming convention and provide clear defaults for the encryption feature.
17-18: Good practice: Initializing path variables with empty strings.This change prevents potential nil pointer dereferences and makes the behavior more predictable.
foundation/console/env_decrypt_command.go (1)
45-50: LGTM! Well-structured flag definition.The new flag follows the consistent pattern with other flags and provides clear usage information.
foundation/console/env_encrypt_command.go (2)
47-52: LGTM! Consistent flag definition with decrypt command.The flag definition matches the decrypt command, maintaining symmetry between the two commands.
84-84: Consider using a constant for file permissions.Similar to the decrypt command, the file permission
0644should be defined as a constant.foundation/console/test_make_command_test.go (2)
16-22: LGTM! Well-structured test suite setup.The conversion to a testify suite improves test organization and follows the project's testing patterns.
36-69: LGTM! Comprehensive flag testing.The test cases thoroughly verify the flag configuration using table-driven tests and proper type assertions.
foundation/console/package_make_command_test.go (3)
16-22: LGTM! Well-structured test suite implementation.The test suite is properly structured using testify's suite pattern, which improves test organization and maintainability.
34-68: LGTM! Comprehensive test coverage for command flags.The test implementation thoroughly validates the command's flags using structured test cases and proper type assertions. The use of
reflect.DeepEqualfor comparisons is a good practice.
70-113: LGTM! Well-organized test cases with proper cleanup.The test cases are well-structured with clear setup and assertions. The cleanup of test files is properly handled using deferred functions.
foundation/console/about_command_test.go (3)
20-29: LGTM! Clean test suite implementation.The test suite is properly structured and follows the same pattern as other command tests.
31-41: LGTM! Clear and focused test methods.The test methods are well-focused and validate specific aspects of the command (signature and description).
43-70: LGTM! Thorough flag validation.The test implementation properly validates the command's flags using structured test cases and type assertions.
foundation/console/env_decrypt_command_test.go (3)
81-90: LGTM! Proper implementation of the newnameoption.The test cases correctly validate the new
nameoption functionality while maintaining existing behavior.
92-103: LGTM! Good error handling test coverage.The test cases properly validate error scenarios with the new option.
173-185: LGTM! Thorough encryption validation.The test cases properly validate both successful and failed encryption scenarios.
foundation/console/env_encrypt_command_test.go (3)
76-86: LGTM! Consistent implementation with decrypt command.The test cases for the encrypt command mirror those of the decrypt command, ensuring consistent behavior.
120-144: LGTM! Comprehensive success scenario testing.The test cases thoroughly validate successful encryption scenarios with proper file handling and cleanup.
167-179: LGTM! Good encryption validation.The test cases properly validate both successful and failed encryption operations.
foundation/console/vendor_publish_command_test.go (2)
27-37: LGTM! Well-structured basic tests.The
TestSignatureandTestDescriptionmethods are well-implemented with clear assertions and good error messages.
75-76: LGTM! Consistent variable naming.The renaming of
commandtocmdacross all test methods improves consistency and follows Go's idiomatic practices.Also applies to: 105-106, 221-222, 304-305, 311-312, 382-383
hwbrzzl
left a comment
hwbrzzl
left a comment
There was a problem hiding this comment.
Thanks, great PR, just some nitpicks.
| mockConfig := mocksconfig.NewConfig(t) | ||
|
|
||
| cmd := NewAboutCommand(mockApp) | ||
| ctx := &mocksconsole.Context{} |
There was a problem hiding this comment.
| ctx := &mocksconsole.Context{} | |
| mockContext := mocksconsole.NewContext(t) |
| } | ||
| } | ||
|
|
||
| func TestAboutCommand(t *testing.T) { |
There was a problem hiding this comment.
Merge this function to the suite.
| } | ||
|
|
||
| if file.Exists(".env") { | ||
| if file.Exists(support.EnvPath) { |
| envDecryptCommand := NewEnvDecryptCommand() | ||
| mockContext := mocksconsole.NewContext(s.T()) | ||
| cmd := NewEnvDecryptCommand() | ||
| ctx := mocksconsole.NewContext(s.T()) |
There was a problem hiding this comment.
| ctx := mocksconsole.NewContext(s.T()) | |
| mockContext := mocksconsole.NewContext(s.T()) |
| envEncryptCommand := NewEnvEncryptCommand() | ||
| mockContext := mocksconsole.NewContext(s.T()) | ||
| cmd := NewEnvEncryptCommand() | ||
| ctx := mocksconsole.NewContext(s.T()) |
There was a problem hiding this comment.
| ctx := mocksconsole.NewContext(s.T()) | |
| mockContext := mocksconsole.NewContext(s.T()) |
| assert.True(t, file.Contain("packages/sms/facades/sms.go", "goravel/packages/sms")) | ||
| assert.True(t, file.Contain("packages/sms/facades/sms.go", "goravel/packages/sms/contracts")) | ||
| assert.NoError(t, file.Remove("packages")) | ||
| s.NoError(NewPackageMakeCommand().Handle(mockContext)) |
|
|
||
| func (s *TestMakeCommandTestSuite) TestTestHandle() { | ||
| cmd := NewTestMakeCommand() | ||
| ctx := mocksconsole.NewContext(s.T()) |
There was a problem hiding this comment.
| ctx := mocksconsole.NewContext(s.T()) | |
| mockContext := mocksconsole.NewContext(s.T()) |
|
And about the Test External CI, I'm changing some logic, it's expected to fail. |
…this is a simulated object
…this is a simulated object
…this is a simulated object
There was a problem hiding this comment.
Actionable comments posted: 1
🔭 Outside diff range comments (1)
foundation/console/about_command_test.go (1)
71-139:⚠️ Potential issueFix test expectations to match method calls.
The pipeline failures indicate that the test expectations for
TwoColumnDetailandNewLinemethods don't match the actual calls.The test is failing because:
- The order of sections in the test output doesn't match the expectations
- The number of
NewLinecalls might be incorrectVerify and fix the test expectations:
- Ensure the sections are added in the correct order
- Verify the number of
NewLinecalls- Double-check the content of each
TwoColumnDetailcall🧰 Tools
🪛 GitHub Actions: Codecov
[error] 138-138: TestAboutCommandTestSuite failed: Unexpected Method Call for TwoColumnDetail. Expected '<fg=green;op=bold>Environment</>', got '<fg=green;op=bold>Test</>'.
[error] 99-99: NewLine() method failed.
[error] 130-130: TwoColumnDetail(string,string) method failed.
[error] 137-137: TwoColumnDetail(string,string) method failed.
🧹 Nitpick comments (3)
foundation/console/test_make_command_test.go (2)
36-69: Improve test efficiency and error handling.Consider the following improvements:
- The type assertion error message uses incorrect formatting.
- The DeepEqual check followed by Equal assertion is redundant.
Apply this diff to improve the code:
- s.Fail("First flag is not BoolFlag (got type: %T)", got.Flags[0]) + s.Failf("First flag is not BoolFlag", "got type: %T", got.Flags[0]) - for _, tc := range testCases { - if !reflect.DeepEqual(tc.got, tc.expected) { - s.Require().Equal(tc.expected, tc.got) - } - } + for _, tc := range testCases { + s.Require().Equal(tc.expected, tc.got, tc.name) + }
75-100: Consider using subtests for better organization.While the test cases are comprehensive, they could be more readable if organized into subtests using
s.Run(). This would make it easier to identify which scenario failed and provide better test output.Example structure:
func (s *TestMakeCommandTestSuite) TestTestHandle() { cmd := NewTestMakeCommand() s.Run("should handle empty test name", func() { mockContext := mocksconsole.NewContext(s.T()) mockContext.EXPECT().Argument(0).Return("").Once() mockContext.EXPECT().Ask("Enter the test name", mock.Anything).Return("", errors.New("the test name cannot be empty")).Once() mockContext.EXPECT().Error("the test name cannot be empty").Once() s.Nil(cmd.Handle(mockContext)) }) s.Run("should create test successfully", func() { mockContext := mocksconsole.NewContext(s.T()) mockContext.EXPECT().Argument(0).Return("UserTest").Once() mockContext.EXPECT().OptionBool("force").Return(false).Once() mockContext.EXPECT().Success("Test created successfully").Once() s.NoError(cmd.Handle(mockContext)) s.True(file.Exists("tests/user_test.go")) }) // ... more subtests ... s.NoError(file.Remove("tests")) }foundation/console/about_command_test.go (1)
42-69: Simplify flag testing using testify's assertions.The flag testing logic using reflection can be simplified using testify's built-in assertions.
- testCases := []struct { - name string - got interface{} - expected interface{} - }{ - {"Name", flag.Name, "only"}, - {"Usage", flag.Usage, "The section to display"}, - } - - for _, tc := range testCases { - if !reflect.DeepEqual(tc.got, tc.expected) { - s.Require().Equal(tc.expected, tc.got) - } - } + s.Equal("only", flag.Name, "Flag name should be 'only'") + s.Equal("The section to display", flag.Usage, "Flag usage should match expected")
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
foundation/console/about_command_test.go(3 hunks)foundation/console/env_decrypt_command_test.go(2 hunks)foundation/console/env_encrypt_command_test.go(2 hunks)foundation/console/test_make_command_test.go(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- foundation/console/env_decrypt_command_test.go
- foundation/console/env_encrypt_command_test.go
🧰 Additional context used
🪛 GitHub Actions: Codecov
foundation/console/about_command_test.go
[error] 138-138: TestAboutCommandTestSuite failed: Unexpected Method Call for TwoColumnDetail. Expected '<fg=green;op=bold>Environment</>', got '<fg=green;op=bold>Test</>'.
[error] 99-99: NewLine() method failed.
[error] 130-130: TwoColumnDetail(string,string) method failed.
[error] 137-137: TwoColumnDetail(string,string) method failed.
⏰ Context from checks skipped due to timeout of 300000ms (2)
- GitHub Check: test / windows (1.24)
- GitHub Check: test / windows (1.23)
🔇 Additional comments (5)
foundation/console/test_make_command_test.go (4)
3-22: LGTM! Well-structured test suite setup.The imports are well-organized, and the test suite is properly structured using testify's suite pattern.
24-28: LGTM! Clear and focused test.The test follows the Arrange-Act-Assert pattern and uses appropriate assertions.
30-34: LGTM! Well-structured test.The test follows the Arrange-Act-Assert pattern and uses appropriate assertions.
71-74: LGTM! Correct mock context setup.The mock context creation follows the suggested pattern from past reviews.
foundation/console/about_command_test.go (1)
19-24: LGTM! Test suite setup follows best practices.The test suite structure is well-organized using testify's suite package. Consider adding common setup logic in the
SetupTestmethod if needed in the future.
| AddAboutInformation("Custom", foundation.AboutItem{Key: "Test Info", Value: "<fg=cyan>OK</>"}) | ||
| assert.Contains(t, color.CaptureOutput(func(w io.Writer) { | ||
| assert.Nil(t, aboutCommand.Handle(mockContext)) | ||
| s.Contains(color.CaptureOutput(func(w io.Writer) { | ||
| s.Nil(cmd.Handle(mockContext)) | ||
| }), strings.Join(expected, "")) |
There was a problem hiding this comment.
Address pipeline failures in test assertions.
The test is failing due to mismatched expectations. The pipeline failures indicate:
TestAboutCommandTestSuite failed: Unexpected Method Call for TwoColumnDetail. Expected '<fg=green;op=bold>Environment</>', got '<fg=green;op=bold>Test</>'
Fix the test by:
- Moving the
AddAboutInformationcall before setting up the expectations - Including the custom section in the expectations array
+ AddAboutInformation("Custom", foundation.AboutItem{Key: "Test Info", Value: "<fg=cyan>OK</>"})
var expected []string
for _, ex := range [][2]string{
{"<fg=green;op=bold>Environment</>", ""},
// ... other sections ...
} {
mockContext.EXPECT().TwoColumnDetail(ex[0], ex[1]).
Run(func(first string, second string, _ ...rune) {
expected = append(expected, color.Default().Sprintf("%s %s\n", first, second))
color.Default().Printf("%s %s\n", first, second)
}).Return().Once()
}
- AddAboutInformation("Custom", foundation.AboutItem{Key: "Test Info", Value: "<fg=cyan>OK</>"})📝 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.
| AddAboutInformation("Custom", foundation.AboutItem{Key: "Test Info", Value: "<fg=cyan>OK</>"}) | |
| assert.Contains(t, color.CaptureOutput(func(w io.Writer) { | |
| assert.Nil(t, aboutCommand.Handle(mockContext)) | |
| s.Contains(color.CaptureOutput(func(w io.Writer) { | |
| s.Nil(cmd.Handle(mockContext)) | |
| }), strings.Join(expected, "")) | |
| // Move the AddAboutInformation call before setting up expectations | |
| AddAboutInformation("Custom", foundation.AboutItem{Key: "Test Info", Value: "<fg=cyan>OK</>"}) | |
| var expected []string | |
| for _, ex := range [][2]string{ | |
| {"<fg=green;op=bold>Environment</>", ""}, | |
| // ... other sections, including expected custom section if applicable ... | |
| } { | |
| mockContext.EXPECT().TwoColumnDetail(ex[0], ex[1]). | |
| Run(func(first string, second string, _ ...rune) { | |
| expected = append(expected, color.Default().Sprintf("%s %s\n", first, second)) | |
| color.Default().Printf("%s %s\n", first, second) | |
| }).Return().Once() | |
| } | |
| s.Contains(color.CaptureOutput(func(w io.Writer) { | |
| s.Nil(cmd.Handle(mockContext)) | |
| }), strings.Join(expected, "")) | |
| // Removed duplicate AddAboutInformation call here |
🧰 Tools
🪛 GitHub Actions: Codecov
[error] 138-138: TestAboutCommandTestSuite failed: Unexpected Method Call for TwoColumnDetail. Expected '<fg=green;op=bold>Environment</>', got '<fg=green;op=bold>Test</>'.
[error] 137-137: TwoColumnDetail(string,string) method failed.
…ColumnDetail. Expected '<fg=green;op=bold>Environment</>', got '<fg=green;op=bold>Test</>'
…replace `.env.encrypted`
2 participants
📑 Description
nameoption to customize the encrypted environment file name, such asIf no name is specified, the default encrypted environment file name is
.env.encryptedpackage_make_command.go,test_make_command.go,vendor_publish_command.go, etc.envwithsupport.EnvPath.env.encryptedwithsupport.EnvEncryptPathSummary by CodeRabbit
New Features
Bug Fixes
Refactor
✅ Checks