-
Notifications
You must be signed in to change notification settings - Fork 1
feat(go): License Updates and Test Migrations #57
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(go): License Updates and Test Migrations #57
Conversation
* chore(feature/go): Init Signed-off-by: Julio Jimenez <julio@clickhouse.com> * go Signed-off-by: Julio Jimenez <julio@clickhouse.com> * Dockerfile Signed-off-by: Julio Jimenez <julio@clickhouse.com> * .golangci Signed-off-by: Julio Jimenez <julio@clickhouse.com> * validation not defined Signed-off-by: Julio Jimenez <julio@clickhouse.com> * regexp and strings not used Signed-off-by: Julio Jimenez <julio@clickhouse.com> * io undefined Signed-off-by: Julio Jimenez <julio@clickhouse.com> * fix: pre-commit Signed-off-by: Julio Jimenez <julio@clickhouse.com> * fix: pre-commit Signed-off-by: Julio Jimenez <julio@clickhouse.com> * fix: pre-commit Signed-off-by: Julio Jimenez <julio@clickhouse.com> * fix: pre-commit Signed-off-by: Julio Jimenez <julio@clickhouse.com> * fix: pre-commit Signed-off-by: Julio Jimenez <julio@clickhouse.com> * fix: integration test Signed-off-by: Julio Jimenez <julio@clickhouse.com> * fix: integration test Signed-off-by: Julio Jimenez <julio@clickhouse.com> * fix: integration test Signed-off-by: Julio Jimenez <julio@clickhouse.com> * fix: integration test Signed-off-by: Julio Jimenez <julio@clickhouse.com> * fix: integration test Signed-off-by: Julio Jimenez <julio@clickhouse.com> * fix: integration test Signed-off-by: Julio Jimenez <julio@clickhouse.com> * fix: integration test Signed-off-by: Julio Jimenez <julio@clickhouse.com> * fix: lint Signed-off-by: Julio Jimenez <julio@clickhouse.com> * fix: docker build Signed-off-by: Julio Jimenez <julio@clickhouse.com> * test: e2e Signed-off-by: Julio Jimenez <julio@clickhouse.com> * test: fix benchmark Signed-off-by: Julio Jimenez <julio@clickhouse.com> * test: fix benchmark Signed-off-by: Julio Jimenez <julio@clickhouse.com> * test: fix security check Signed-off-by: Julio Jimenez <julio@clickhouse.com> --------- Signed-off-by: Julio Jimenez <julio@clickhouse.com>
Signed-off-by: Julio Jimenez <julio@clickhouse.com>
* chore(feature/go): Trivy Integration Signed-off-by: Julio Jimenez <julio@clickhouse.com> * fix(debug): extract from wrapper function Signed-off-by: Julio Jimenez <julio@clickhouse.com> * fix(debug): extract json from zip Signed-off-by: Julio Jimenez <julio@clickhouse.com> * fix(debug): remove debug print of sbom Signed-off-by: Julio Jimenez <julio@clickhouse.com> * fix(aws): Some inputs are not longer required Signed-off-by: Julio Jimenez <julio@clickhouse.com> * fix(aws): Some inputs are not longer required Signed-off-by: Julio Jimenez <julio@clickhouse.com> * fix: add trivy to config validation Signed-off-by: Julio Jimenez <julio@clickhouse.com> * fix: add trivy to config validation Signed-off-by: Julio Jimenez <julio@clickhouse.com> * fix: add trivy to config validation Signed-off-by: Julio Jimenez <julio@clickhouse.com> * fix: ecr auth Signed-off-by: Julio Jimenez <julio@clickhouse.com> * fix: trivy clickhouse table name Signed-off-by: Julio Jimenez <julio@clickhouse.com> * feat: ability to do application scope reports Signed-off-by: Julio Jimenez <julio@clickhouse.com> * fix: i don't think org uuid is always required Signed-off-by: Julio Jimenez <julio@clickhouse.com> * fix: if no projectUuids are provided Signed-off-by: Julio Jimenez <julio@clickhouse.com> * fix: mend-project-uuids Signed-off-by: Julio Jimenez <julio@clickhouse.com> * fix: maxDepthLevel Signed-off-by: Julio Jimenez <julio@clickhouse.com> * fix: maxDepthLevel Signed-off-by: Julio Jimenez <julio@clickhouse.com> * fix: maxDepthLevel Signed-off-by: Julio Jimenez <julio@clickhouse.com> * fix: maxDepthLevel Signed-off-by: Julio Jimenez <julio@clickhouse.com> * fix: maxDepthLevel Signed-off-by: Julio Jimenez <julio@clickhouse.com> * fix: maxDepthLevel Signed-off-by: Julio Jimenez <julio@clickhouse.com> * fix: maxDepthLevel Signed-off-by: Julio Jimenez <julio@clickhouse.com> * fix: stuff Signed-off-by: Julio Jimenez <julio@clickhouse.com> * feat: add merge Signed-off-by: Julio Jimenez <julio@clickhouse.com> * feat: add merge Signed-off-by: Julio Jimenez <julio@clickhouse.com> * feat: add merge Signed-off-by: Julio Jimenez <julio@clickhouse.com> * feat: add merge Signed-off-by: Julio Jimenez <julio@clickhouse.com> * fix: lint Signed-off-by: Julio Jimenez <julio@clickhouse.com> --------- Signed-off-by: Julio Jimenez <julio@clickhouse.com>
Signed-off-by: Julio Jimenez <julio@clickhouse.com>
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.
Pull request overview
This PR migrates the codebase from a bash-based entrypoint script to a Go-based implementation, removes extensive BATS test files, and adds new license mappings. The migration introduces a new Go package structure with proper logging, validation, and storage integration, while removing the shell script testing infrastructure.
Key Changes
- Complete migration from bash scripts to Go implementation with new packages for logger, validation, storage, and SBOM handling
- Removal of all BATS test files and test infrastructure (simple.bats, advanced.bats, setup-bats.sh, run-tests.sh)
- Addition of four new license mappings in license-mappings.json
Reviewed changes
Copilot reviewed 45 out of 49 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| test/simple.bats | Removed 832-line BATS test suite for bash entrypoint |
| test/advanced.bats | Removed 2326-line advanced BATS test suite |
| setup-bats.sh | Removed BATS installation and setup script |
| run-tests.sh | Removed test runner script with 311 lines |
| lib/*.sh | Removed all bash library files (wiz.sh, validation.sh, sbom-processing.sh, etc.) |
| pkg/logger/logger.go | New Go logger package with debug, info, success, warning, error, and fatal methods |
| internal/validation/sanitize.go | New Go validation package replacing bash sanitization functions |
| internal/storage/s3.go | New S3 client implementation with upload, download, and list operations |
| internal/storage/clickhouse.go | New ClickHouse client with table setup, migration, and data insertion |
| internal/sbom/*.go | New SBOM processing packages for GitHub, Mend, Wiz, Trivy, merging, and filtering |
| license-mappings.json | Added 4 new license mappings for HyperDX packages |
Comments suppressed due to low confidence (3)
internal/storage/s3.go:1
- Unused import should be removed. This commented-out import statement is not being used and should be deleted to keep the code clean.
internal/sbom/processing.go:1 - The nolint directive suggests this string is repeated elsewhere. Consider defining string constants for SBOM format literals to avoid duplication and potential typos.
// Package sbom provides functionalities to interact with Software Bill of Materials (SBOM).
internal/storage/s3_integration_test.go:1
- The NewS3Client function is being called with three parameters (access key, secret key, region) but according to internal/storage/s3.go lines 27-35, the actual implementation only takes a context parameter. This will cause a compilation error.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| // uuids := strings.Split(m.projectUUIDs, ",") | ||
| // payload["projectUuids"] = uuids | ||
| url = fmt.Sprintf("%s/api/v3.0/projects/%s/dependencies/reports/SBOM", m.baseURL, m.projectUUID) | ||
| case m.productUUID != "": | ||
| // if len(m.projectUUIDs) != 0 { | ||
| // uuids := strings.Split(m.projectUUIDs, ",") | ||
| // payload["projectUuids"] = uuids | ||
| // } |
Copilot
AI
Dec 15, 2025
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.
Dead code should be removed. These commented-out lines appear to be alternative implementations that should either be uncommented and used, or deleted entirely to improve code maintainability.
| // uuids := strings.Split(m.projectUUIDs, ",") | |
| // payload["projectUuids"] = uuids | |
| url = fmt.Sprintf("%s/api/v3.0/projects/%s/dependencies/reports/SBOM", m.baseURL, m.projectUUID) | |
| case m.productUUID != "": | |
| // if len(m.projectUUIDs) != 0 { | |
| // uuids := strings.Split(m.projectUUIDs, ",") | |
| // payload["projectUuids"] = uuids | |
| // } | |
| url = fmt.Sprintf("%s/api/v3.0/projects/%s/dependencies/reports/SBOM", m.baseURL, m.projectUUID) | |
| case m.productUUID != "": |
| payload["scopeType"] = "project" | ||
| payload["scopeUuid"] = m.projectUUID | ||
| // uuids := strings.Split(m.projectUUIDs, ",") | ||
| // payload["projectUuids"] = uuids |
Copilot
AI
Dec 15, 2025
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.
Dead code should be removed. This commented-out code block appears to be unused and should be either implemented or deleted.
| // payload["projectUuids"] = uuids |
| for _, comp := range components { | ||
| name, _ := comp["name"].(string) | ||
| if name == "" { | ||
| name = "unknown" // nolint:goconst |
Copilot
AI
Dec 15, 2025
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.
The nolint directive suggests this 'unknown' string is repeated throughout the codebase. Consider defining it as a constant (e.g., const UnknownValue = \"unknown\") to ensure consistency and make updates easier.
| // MapLicense maps an unknown license to a known one, or returns the original | ||
| func (m *LicenseMapper) MapLicense(componentName, license string) string { | ||
| // If license is already known, return it | ||
| if license != "" && license != "unknown" && license != "null" { // nolint:goconst |
Copilot
AI
Dec 15, 2025
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.
The nolint directive suggests these string literals are repeated. Consider defining constants for 'unknown' and 'null' to avoid duplication and improve maintainability.
| func getStringField(m map[string]interface{}, key, defaultVal string) string { //nolint:unparam | ||
| if val, ok := m[key]; ok { | ||
| if str, ok := val.(string); ok { | ||
| return str | ||
| } | ||
| } | ||
| return defaultVal |
Copilot
AI
Dec 15, 2025
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.
The unparam directive suggests the defaultVal parameter is always passed with the same value. Consider removing the parameter and using a constant, or document why this flexibility is maintained for future use.
| func getStringField(m map[string]interface{}, key, defaultVal string) string { //nolint:unparam | |
| if val, ok := m[key]; ok { | |
| if str, ok := val.(string); ok { | |
| return str | |
| } | |
| } | |
| return defaultVal | |
| func getStringField(m map[string]interface{}, key string) string { | |
| if val, ok := m[key]; ok { | |
| if str, ok := val.(string); ok { | |
| return str | |
| } | |
| } | |
| return "unknown" |
| s3Client, err := storage.NewS3Client( | ||
| ctx, | ||
| os.Getenv("AWS_ACCESS_KEY_ID"), | ||
| os.Getenv("AWS_SECRET_ACCESS_KEY"), | ||
| os.Getenv("AWS_DEFAULT_REGION"), | ||
| ) |
Copilot
AI
Dec 15, 2025
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.
The NewS3Client function is being called with three parameters but the actual implementation in s3.go only accepts a context parameter. This will cause a compilation error.
| s3Client, err := storage.NewS3Client( | |
| ctx, | |
| os.Getenv("AWS_ACCESS_KEY_ID"), | |
| os.Getenv("AWS_SECRET_ACCESS_KEY"), | |
| os.Getenv("AWS_DEFAULT_REGION"), | |
| ) | |
| s3Client, err := storage.NewS3Client(ctx) |
| @@ -0,0 +1,117 @@ | |||
| //go:build integration | |||
|
|
|||
| package storage | |||
Copilot
AI
Dec 15, 2025
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.
Package declaration is incorrect. This file is in the internal/sbom/ directory but declares package storage. It should declare package sbom to match the directory structure.
| package storage | |
| package sbom |
https://github.com/ClickHouse/security/issues/6562