feat(router): add support for compressed config files #2316
Conversation
endigma
requested review from
Noroth,
StarpTech,
devsergiy and
jensneuse
as code owners
WalkthroughRemoved the Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
Comment |
Router-nonroot image scan passed✅ No security vulnerabilities found in image: |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
router/pkg/routerconfig/s3/client.go(3 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-08-26T17:19:28.178Z
Learnt from: SkArchon
Repo: wundergraph/cosmo PR: 2167
File: router/internal/expr/retry_context.go:3-10
Timestamp: 2025-08-26T17:19:28.178Z
Learning: In the Cosmo router codebase, prefer using netErr.Timeout() checks over explicit context.DeadlineExceeded checks for timeout detection in retry logic, as Go's networking stack typically wraps context deadline exceeded errors in proper net.Error implementations.
Applied to files:
router/pkg/routerconfig/s3/client.go
🧬 Code graph analysis (1)
router/pkg/routerconfig/s3/client.go (1)
router/pkg/routerconfig/client.go (2)
Client(16-20)Response(11-14)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (11)
- GitHub Check: build-router
- GitHub Check: build_test
- GitHub Check: integration_test (./events)
- GitHub Check: integration_test (./telemetry)
- GitHub Check: integration_test (./. ./fuzzquery ./lifecycle ./modules)
- GitHub Check: image_scan
- GitHub Check: image_scan (nonroot)
- GitHub Check: build_push_image
- GitHub Check: build_push_image (nonroot)
- GitHub Check: Analyze (javascript-typescript)
- GitHub Check: Analyze (go)
Compression methods speeds and ratiosOriginal size: 16711107 bytes (15.94 MB), large JSON execution config used in production Speeds measured on an M4 Pro, averaged over 10 runs Compression Results
Ranked by Compression Savings
Ranked by Decompression Time
SummaryUsing |
endigma
force-pushed
the
jesse/eng-8425-router-execution-config-compression
branch
from
4b356d1 to
cb674f7
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
router/pkg/routerconfig/s3/client.go (1)
100-122: Consider extracting extension constants for maintainability.The hardcoded extensions
".gz"and".zst"could be extracted as package-level constants if you anticipate adding more compression formats or referencing these values elsewhere. However, the current inline approach is perfectly clear and maintainable for two extensions.Example (if desired):
const ( extGzip = ".gz" extZstd = ".zst" )
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
router/pkg/config/config.schema.json(1 hunks)router/pkg/routerconfig/s3/client.go(3 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- router/pkg/config/config.schema.json
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-08-26T17:19:28.178Z
Learnt from: SkArchon
Repo: wundergraph/cosmo PR: 2167
File: router/internal/expr/retry_context.go:3-10
Timestamp: 2025-08-26T17:19:28.178Z
Learning: In the Cosmo router codebase, prefer using netErr.Timeout() checks over explicit context.DeadlineExceeded checks for timeout detection in retry logic, as Go's networking stack typically wraps context deadline exceeded errors in proper net.Error implementations.
Applied to files:
router/pkg/routerconfig/s3/client.go
🧬 Code graph analysis (1)
router/pkg/routerconfig/s3/client.go (1)
router/pkg/routerconfig/client.go (2)
Client(16-20)Response(11-14)
🔇 Additional comments (3)
router/pkg/routerconfig/s3/client.go (3)
75-130: Excellent resource management and decompression implementation!The streaming decompression logic is well-structured:
- Correct defer order: Decompressors close before
minioReadervia LIFO, ensuring proper cleanup.- Case-insensitive extension matching:
strings.ToLowerat line 100 correctly addresses the previous review feedback.- Graceful fallback: Unknown extensions fall through to uncompressed handling.
- Consistent error wrapping: All errors include helpful context.
The defer placement at lines 96, 107, and 117 ensures cleanup even on early returns, and the order guarantees decompressors close before the underlying reader closes.
132-151: Correct interface implementation with version parameter removal.The method correctly implements the
routerconfig.Clientinterface by accepting but ignoring theprevVersionparameter (line 132). For S3-based config retrieval, usingmodifiedSincefor conditional fetching makes sense, and version tracking is unnecessary.The error handling at lines 138-143 properly translates MinIO-specific errors to the appropriate sentinel errors expected by the config poller.
13-14: No issues found—klauspost/compress libraries are standard-compliant.The klauspost/compress packages are implemented as drop-in replacements that produce and read standard gzip and zstd streams. The imports at lines 13-14 are appropriate and will maintain interoperability with standard compression tools.
endigma
deleted the
jesse/eng-8425-router-execution-config-compression
branch
Summary by CodeRabbit
gzipandzstdcompressed router configurations in S3.gzor.zst, these are the defaults when using CLI tools to compress.config.json.gz, a reasonably helpful error is emittedChecklist