Conversation
4e86a45 to
314d86b
Compare
d9dea1a to
e1a7dff
Compare
There was a problem hiding this comment.
Pull request overview
This PR introduces a Go-based mock implementation of the THOR Thunderstorm API, generated from the Thunderstorm OpenAPI spec, including request logging, tests, CI, and Docker packaging.
Changes:
- Add a generated Go server implementation (routers/controllers/models) plus minimal service logic to mock responses.
- Add request logging middleware with configurable output destination, and add unit + subprocess-based integration tests.
- Add OpenAPI generation configs, CI workflow, Dockerfile, and documentation for building/running/regenerating.
Reviewed changes
Copilot reviewed 36 out of 38 changed files in this pull request and generated 11 comments.
Show a summary per file
| File | Description |
|---|---|
| openapitools.json | Pins OpenAPI generator CLI version/config schema. |
| openapi-genconfig.yaml | Generator config for producing the Go server from Thunderstorm OpenAPI spec. |
| main.go | CLI entrypoint: flag parsing, log output selection, router setup, server start. |
| main_test.go | Black-box integration tests that build/run the binary and exercise HTTP endpoints + stdout logs. |
| go/state.go | Mock state + scan tracking + history/status/info generation. |
| go/routers.go | Generated Gorilla/mux router wiring + middleware attachment. |
| go/model_timestamp_map.go | Generated model type for timestamp history responses. |
| go/model_thunderstorm_version_info.go | Generated version-info model. |
| go/model_thunderstorm_status.go | Generated status model + required-field assertions. |
| go/model_thunderstorm_info.go | Generated info model + required-field assertions. |
| go/model_thor_report.go | Generated ThorReport model. |
| go/model_thor_finding.go | Customized ThorFinding model for mock findings. |
| go/model_sample_id_obj.go | Generated response model for async sample ID object. |
| go/model_sample_id.go | Generated model representing the async ID schema. |
| go/model_file_object.go | Generated multipart upload wrapper model. |
| go/model_error.go | Generated error model (message field). |
| go/model_async_result.go | Generated async result model. |
| go/logger.go | Request/response JSON logging middleware + configurable output writer. |
| go/imported.go | Internal status enum and human-readable strings for async states. |
| go/impl.go | Generated implementation response container. |
| go/helpers.go | Generated helpers for parameter parsing + multipart upload temp-file handling. |
| go/error.go | Generated error types and default error handler. |
| go/api_test.go | Router-level tests using httptest.Server (workflow + error triggers + concurrency). |
| go/api_scan_service.go | Mock scan service logic + special source triggers. |
| go/api_scan.go | Generated scan controller parsing multipart + query params. |
| go/api_results_service.go | Mock results service + special ID triggers. |
| go/api_results.go | Generated results controller parsing query params. |
| go/api_info_service.go | Mock info service for status/info/history endpoints. |
| go/api_info.go | Generated info controller parsing query params. |
| go/api.go | Generated router/servicer interfaces for the API surface. |
| go.sum | Go dependency lockfile for faker, mux, etc. |
| go.mod | Go module definition and dependencies. |
| api/openapi.yaml | Vendored OpenAPI spec used/validated by generation and tests. |
| README.md | Usage, options, Docker instructions, and generation notes. |
| Dockerfile | Multi-stage Docker build packaging the mock server binary. |
| .openapi-generator-ignore | Controls which files are preserved vs regenerated. |
| .gitignore | Ignores OpenAPI generator output directory. |
| .github/workflows/ci.yml | CI: gofmt, lint, unit tests, integration tests, build + release steps. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| func StoreScanRequest(synchronous bool, file *os.File, source string) (ScanRequest, error) { | ||
| submissionTime := time.Now() | ||
|
|
||
| h := sha256.New() | ||
| f, err := os.Open(file.Name()) | ||
| if err != nil { | ||
| return ScanRequest{}, err | ||
| } | ||
| defer func() { _ = f.Close() }() | ||
| if _, err := io.Copy(h, f); err != nil { | ||
| return ScanRequest{}, err | ||
| } | ||
| fileHash := fmt.Sprintf("%x", h.Sum(nil)) | ||
|
|
||
| scanID := NextScanID() | ||
|
|
||
| scanRequest := ScanRequest{ | ||
| ID: scanID, | ||
| Synchronous: synchronous, | ||
| FileHash: fileHash, | ||
| Source: source, | ||
| SubmissionTime: submissionTime, | ||
| } | ||
|
|
||
| scanRequests.Store(scanID, scanRequest) | ||
| return scanRequest, nil | ||
| } |
There was a problem hiding this comment.
StoreScanRequest re-opens the uploaded temp file by name to hash it but never deletes that temp file afterward. Since each request writes a new temp file in ReadFormFileToTempFile, the mock server will steadily consume disk space in long-running scenarios. Remove the temp file after hashing (and consider closing/removing even on error paths).
| @@ -0,0 +1,15 @@ | |||
| FROM golang:1.19 AS build | |||
There was a problem hiding this comment.
The Docker build stage uses golang:1.19, but go.mod declares go 1.24.0 (and CI uses Go 1.24). This will fail to build in Docker due to an unsupported Go toolchain. Update the Docker base image to a Go version compatible with go.mod (or lower the go directive if 1.24 is not required).
| FROM golang:1.19 AS build | |
| FROM golang:1.24 AS build |
| defer file.Close() | ||
|
|
||
| _, err = io.Copy(file, formFile) | ||
| if err != nil { | ||
| return nil, err | ||
| } | ||
|
|
There was a problem hiding this comment.
readFileHeaderToTempFile defers file.Close() and then returns file, so callers receive a closed *os.File. This is surprising for an API returning a file handle and can break any service implementation that expects to read from the file directly. Also, the created temp file is never removed, so the server will leak temp files over time; prefer returning an open file (caller closes) and ensure the temp file is deleted after it’s no longer needed.
| defer file.Close() | |
| _, err = io.Copy(file, formFile) | |
| if err != nil { | |
| return nil, err | |
| } | |
| _, err = io.Copy(file, formFile) | |
| if err != nil { | |
| return nil, err | |
| } | |
| // Reset the offset so callers can read from the beginning of the file. | |
| if _, err := file.Seek(0, io.SeekStart); err != nil { | |
| return nil, err | |
| } |
| const ( | ||
| serverAddr = "http://localhost:8080" | ||
| apiBase = serverAddr + "/api/v1" | ||
| startupDelay = 2 * time.Second | ||
| shutdownDelay = 500 * time.Millisecond |
There was a problem hiding this comment.
These integration tests hard-code localhost:8080 for the server. This can make CI/local runs flaky if the port is already in use, and prevents running tests in parallel. Consider selecting a free port dynamically and starting the subprocess with --port / THUNDERSTORM_MOCK_PORT so the tests don’t rely on a fixed port.
No description provided.