Conversation
- setup the workspace - create a new packages - structuring the codebase
- create new packages `bx`, `socket` and `cli` - implementing new features - fixing some bugs - refactoring the codebase
Reviewer's Guide by SourceryThis pull request involves significant refactoring of the project structure, introduces a new 'bx' build tool with advanced capabilities and various output targets, implements a WebSocket communication layer for build triggering and monitoring, and adds foundational backend services for database, authentication (JWT), and file handling. It also updates frontend tooling and enhances development workflows. No diagrams generated as the changes look simple and do not need a visual representation. File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey @DoniLite - I've reviewed your changes and found some issues that need to be addressed.
Blocking issues:
- Hardcoded database credentials may expose sensitive information. (link)
Overall Comments:
- The scope of this PR is very large, involving significant restructuring into workspace projects and adding major features like a new build system and WebSocket communication; the title 'Chore setup the workspace projects' may not fully capture the extent of these changes.
- This PR introduces a new multi-project structure (
server,services,bx,frontend, etc.); consider adding a brief overview of the responsibilities of each new top-level directory in the description or related documentation.
Here's what I looked at during the review
- 🟡 General issues: 2 issues found
- 🔴 Security: 1 blocking issue
- 🟡 Testing: 3 issues found
- 🟡 Complexity: 2 issues found
- 🟢 Documentation: all looks good
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
|
|
||
| func SaveEncryptedFile(filename string, data []byte) error { | ||
| return os.WriteFile("/path/to/storage/"+filename, data, 0644) | ||
| storagePath := "/path" |
There was a problem hiding this comment.
suggestion: Use a platform‐independent method to join file paths.
Instead of concatenating storagePath and filename with '+', consider using filepath.Join(storagePath, filename) to ensure correct path formatting on all platforms.
| Components []Component `json:"components"` | ||
| } | ||
|
|
||
| const lockFilePath = "../../frontend/components.lock.json" |
There was a problem hiding this comment.
suggestion (bug_risk): Revisit hardcoded file paths.
Using relative paths like '../../frontend/components.lock.json' may lead to issues if the working directory changes. Consider making these paths configurable or deriving them from a centralized base path.
Suggested implementation:
var lockFilePath string
var componentsDir string
func init() {
basePath := os.Getenv("FRONTEND_PATH")
if basePath == "" {
workingDir, err := os.Getwd()
if err != nil {
workingDir = "."
}
basePath = filepath.Join(workingDir, "frontend")
}
lockFilePath = filepath.Join(basePath, "components.lock.json")
componentsDir = filepath.Join(basePath, "components")
}Ensure that the FRONTEND_PATH environment variable is documented for users/developers and reviewed within the broader configuration strategy in the codebase.
|
|
||
| // Contient la base de données du projet 'cloudbeast' gérée par `gorm ORM` | ||
| func Setup() (*gorm.DB, error) { | ||
| dsn := "host=localhost user=doni password=DoniLite13 dbname=anexis port=5432 sslmode=disable" |
There was a problem hiding this comment.
🚨 suggestion (security): Hardcoded database credentials may expose sensitive information.
It is advisable to extract credentials from environment variables or a secure configuration to reduce the chances of accidental exposure.
Suggested implementation:
import (
"fmt"
"os"
"gorm.io/driver/postgres"
"gorm.io/gorm"
) host := os.Getenv("DB_HOST")
user := os.Getenv("DB_USER")
password := os.Getenv("DB_PASSWORD")
dbname := os.Getenv("DB_NAME")
port := os.Getenv("DB_PORT")
dsn := fmt.Sprintf("host=%s user=%s password=%s dbname=%s port=%s sslmode=disable", host, user, password, dbname, port)Make sure that the respective environment variables (DB_HOST, DB_USER, DB_PASSWORD, DB_NAME, and DB_PORT) are set in your deployment or development environment.
| OUTPUTDIR = "../tests/output" | ||
| ) | ||
|
|
||
| func TestCompressionService(t *testing.T) { |
There was a problem hiding this comment.
issue (testing): Test lacks assertions to verify behavior.
This test compresses data and writes it to a file, but it doesn't assert that the compression actually worked or that the output has expected properties (e.g., smaller size, specific format). Consider adding assertions, perhaps by decompressing the result within the test and comparing it to the original.
| } | ||
|
|
||
|
|
||
| func TestDecompressionService(t *testing.T) { |
There was a problem hiding this comment.
issue (testing): Test lacks assertions and depends on external state.
This test reads a file presumably created by TestCompressionService, decompresses it, and writes the output. It lacks assertions to verify if the decompressed data matches the original input. Furthermore, tests should be independent; this test relies on TestCompressionService running successfully first and creating the 'output' file. Consider making tests self-contained, possibly using t.Run for subtests with shared setup or using in-memory data.
|
|
||
| const USER = "doni" | ||
|
|
||
| func TestGenerator(t *testing.T) { |
There was a problem hiding this comment.
issue (testing): Test lacks assertions to verify key generation.
This test calls GenerateRSAKeys but doesn't assert anything about the result. It should verify that a non-nil private key is returned and potentially check that the corresponding public/private key files were created in the expected location (though file system interactions might be better mocked or handled with cleanup).
|
|
||
|
|
||
| // runBuildLogic contient la logique de build principale, adaptée pour les notifications. | ||
| // ATTENTION: Cette fonction est maintenant longue et complexe. Envisager de la découper. |
There was a problem hiding this comment.
issue (complexity): Consider decomposing the runBuildLogic function into smaller, focused helper functions for environment setup, secret loading, and build process steps.
The main build logic function is now handling too many responsibilities. Consider decomposing it into focused helper functions, for example:
- Setup Environment
Extract environment preparation into its own function:func (s *BuildService) setupBuildDirectory(buildID string) (string, error) { buildDir := filepath.Join(s.workDir, buildID) if err := os.MkdirAll(buildDir, 0755); err != nil { return "", fmt.Errorf("cannot create build directory '%s': %w", buildDir, err) } return buildDir, nil }
- Load Environment & Secrets
Separate these responsibilities:func (s *BuildService) loadEnvironment(spec *BuildSpec) map[string]string { env := make(map[string]string) for k, v := range spec.Env { env[k] = v } // Optionally merge additional environment setup here... return env } func (s *BuildService) fetchSecrets(ctx context.Context, spec *BuildSpec, notifier socket.BuildNotifier, buildLogger *log.Logger) (map[string]string, error) { secrets := make(map[string]string) if s.secretFetcher != nil && len(spec.Secrets) > 0 { buildLogger.Println("Fetching secrets...") for _, secretSpec := range spec.Secrets { secretValue, err := s.GetSecret(ctx, secretSpec.Source) if err != nil { return nil, fmt.Errorf("failed to fetch secret '%s': %w", secretSpec.Name, err) } secrets[secretSpec.Name] = secretValue buildLogger.Printf("Secret '%s' fetched successfully.\n", secretSpec.Name) } } return secrets, nil }
- Build Process Steps
Similarly, extract download, codebase preparation, build execution, and output handling into their own helper functions.
By splitting these concerns into individually testable functions, you not only reduce the cognitive complexity but also improve overall maintainability without risking a change in functionality.
| // --- Core Build Logic --- | ||
|
|
||
| // Running the build based on the provided spec | ||
| func (s *BuildService) Build(ctx context.Context, spec *BuildSpec) (*BuildResult, error) { |
There was a problem hiding this comment.
issue (complexity): Consider splitting the Build function into smaller, well-defined modules for environment setup, codebase fetching, resource handling, Dockerfile builds, Compose builds, and helper utilities to improve maintainability.
The changes introduce a very long “Build” function and mix many responsibilities. To improve maintainability while keeping functionality, consider splitting the code into clearly defined modules. For example:
-
Extract Environment Setup
Move setup of working directory, env file loading, and secret fetching into its own module/file (e.g.,environment.go):// environment.go package build func SetupEnvironment(ctx context.Context, spec *BuildSpec, workDir string, inMemory bool, secretFetcher SecretFetcher) (buildDir string, finalEnv map[string]string, logs strings.Builder, err error) { var overallLogs strings.Builder // Step 1: Create working directory (temp if inMemory...) // Step 2: Load env files // Step 3: Fetch secrets // Return buildDir, merged environment and the logs collected. return buildDir, finalEnv, overallLogs, nil }
-
Separate Codebase Fetching and Resource Handling
Create files likecodebases.goandresources.goto encapsulate fetching from Git/local/archive/buffer and downloading/extracting resources. For instance:// codebases.go package build func FetchCodebases(ctx context.Context, spec *BuildSpec, buildDir string) (map[string]CodebaseConfig, error) { // Loop through spec.Codebases and fetch each into buildDir // Return a map keyed on codebase name. return nil, nil }
// resources.go package build func DownloadAndExtractResources(ctx context.Context, spec *BuildSpec, buildDir string, logs *strings.Builder) error { // Loop through spec.Resources and download/extract as needed. return nil }
-
Split Docker and Compose Build Logic
Instead of one Build() method handling both Dockerfile and Compose logic, create separate functions:// docker_build.go package build func BuildWithDockerfile(ctx context.Context, spec *BuildSpec, buildDir string, logs *strings.Builder) (string, error) { // Build image using Dockerfile return "", nil }
// compose_build.go package build func BuildWithCompose(ctx context.Context, spec *BuildSpec, buildDir string, logs *strings.Builder) error { // Build using Docker Compose project return nil }
-
Extract Helper Utilities
Functions like archive extraction, container creation and image save can be moved to a dedicatedutils.gofile.
By splitting these responsibilities, you make each module easier to follow, test, and maintain while preserving existing functionality.
1 participant
Summary by Sourcery
Introduce a new build system (
bx/build) with ecosystem detection, Dockerfile templating, and support for various sources (Git, local) and outputs (Docker, local tar, B2). Add a WebSocket API (socket/) for remote build triggering and status streaming. Reorganize the project structure intoserver/,builder/,bx/,services/,frontend/,cli/directories.New Features:
bx/build) to manage artifact creation.socket/) for remote build management.cli/components/run.go).Build:
golangci-lintconfiguration to v2 format.linttarget to the Makefile.testtarget to pass Docker socket path.CI:
Tests:
bx/build).Chores:
gossr) dependency and usage.