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.
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 | ||
| // } |
There was a problem hiding this comment.
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 |
There was a problem hiding this comment.
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 |
There was a problem hiding this comment.
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 |
There was a problem hiding this comment.
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 |
There was a problem hiding this comment.
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"), | ||
| ) |
There was a problem hiding this comment.
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 | |||
There was a problem hiding this comment.
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