Fix linter error #32
Open
Fix linter error #32
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Signed-off-by: Stany MARCEL <[email protected]>
This commit resolves a variety of issues reported by golangci-lint, leading to significant improvements in code quality, error handling, and maintainability. Key changes include: - **Error Handling:** Added checks for previously unhandled errors, particularly from `io.ReadAll` and `strconv.Atoi`, to make the application more robust. - **Code Complexity:** Refactored several complex functions (`object.get`, `object.put`, `router.getHandler`) to reduce their cognitive and cyclomatic complexity, making them easier to understand and maintain. - **Logging:** Standardized the logging mechanism to use `logrus` consistently across all packages by updating the S3 client initialization and middleware. - **Security:** Addressed `gosec` warnings by adding comments to acknowledge the necessary use of MD5 for S3 compatibility. - **Code Cleanup:** Removed unused variables, parameters, and empty code blocks to improve code clarity. - **Static Analysis:** Fixed various static analysis warnings related to redundant selectors and variable declarations. Signed-off-by: Stany MARCEL <[email protected]>
This commit enhances code quality by implementing improvements identified during review, resolving all golangci-lint issues. Key improvements: - Add configuration validation module with startup checks * Validate S3 bucket names according to AWS naming rules * Validate object keys with path traversal protection * Implement content length validation (5GB limit) * Add host configuration format validation - Improve error handling consistency across the codebase * Fix error propagation in S3 client (errors now properly returned) * Add proper error wrapping with context using fmt.Errorf * Ensure all errors result in appropriate HTTP responses * Improve error messages to avoid exposing internal details - Enhance request validation and security * Validate bucket/key names before processing requests * Add content length validation for PUT requests - Fix resource management issues * Add proper defer statements for response body closure * Fix missing KEK (Key Encryption Key) initialization * Improve handling of client disconnections * Add versionID support throughout request flow Affected files: - NEW: s3proxy/internal/config/validation.go - s3proxy/cmd/main.go: Add startup validation - s3proxy/internal/s3/s3.go: Fix error handling - s3proxy/internal/router/router.go: Add input validation - s3proxy/internal/router/object.go: Improve error handling and cleanup - s3proxy/internal/router/handler.go: Fix KEK initialization and cleanup Signed-off-by: Stany MARCEL <[email protected]>
This commit introduces comprehensive technical documentation to the S3Proxy project. __README.md updates:__ - Added a detailed "Technical Details" section covering the architecture, encryption/decryption flows (PUT/GET), key components (`main`, `router`, `s3`, `crypto`), and default blocking of multipart uploads. - Documented the main Go libraries used for configuration (`koanf`), logging (`logrus`), AWS S3 interaction (`aws-sdk-go-v2/service/s3`), and cryptography (`tink-go/v2`). - Provided an in-depth explanation of the Helm chart deployment, including configurable parameters from `values.yaml` and its role in Kubernetes deployments. __CONTRIBUTING.md updates:__ - Added a "Linting" subsection, detailing the use of `golangci-lint` for static code analysis. - Listed the enabled linters from `.golangci.yml` and provided instructions on how to run the linter. - Clarified the recommended development environment setup using Docker and VSCode's "Attach to Running Container" feature. Signed-off-by: Stany MARCEL <[email protected]>
ynsta
force-pushed
the
fix-linter-error
branch
from
054c3ea to
d402fbb
Compare
The is* validation functions already check HTTP methods internally, making the switch statement unnecessary. Added comments for clarity. Signed-off-by: Stany MARCEL <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Add linter and propose fixes by flash pro and improvement from a ai assisted review with opus 4.1.