refactor: improve fuzzers - #2173
Conversation
|
/agentic_review |
PR Summary by QodoImprove conversion fuzzers for v1alpha1 roundtrip tests
AI Description
Diagram
High-Level Assessment
Files changed (1)
|
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #2173 +/- ##
==========================================
- Coverage 57.12% 56.99% -0.14%
==========================================
Files 286 287 +1
Lines 16129 16241 +112
==========================================
+ Hits 9214 9256 +42
- Misses 5970 6033 +63
- Partials 945 952 +7
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Code Review by Qodo
1. ADMIN_SERVER trailing colon
|
b272ef4 to
67bd0a0
Compare
67bd0a0 to
3c67099
Compare
3c67099 to
c971e11
Compare
c971e11 to
534becf
Compare
serviceReferenceToAddressPort, splitURLPath, and buildURL dropped userinfo/query/fragment and mis-bracketed IPv6 hosts. Rebuilt on url.URL mutation and JoinPath instead of hand-assembled strings. Anchored the gRPC target port regex to end-of-string so it can't match a port in the resolver authority instead of the target. Added internal/testing/fuzzer with wider URL/host/port generators so round-trip fuzz tests exercise these paths. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
buildURL now produces //[:port][/path] when address is empty, preserving port and prefix through the v1alpha1↔v1 round-trip. Previously it dropped both, losing user configuration on upgrade. Relax TUF CEL validation to accept schemeless authority URIs alongside scheme://host URLs. Extract shared mergeURLs helper so both ResolveInternalServiceUrl and ResolveExternalServiceUrl merge user port/path overrides with the autodiscovered host. Switch tree action to ResolveInternalGrpcService for Trillian, matching the gRPC protocol Trillian actually uses. Remove unused objectMetaFuzzerFuncs. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
b0d5a41 to
f0f3805
Compare
|
/agentic_review |
| if err != nil { | ||
| return i.Error(ctx, fmt.Errorf("could not resolve trillian service: %w", err), instance) | ||
| } | ||
| trillUrl = fmt.Sprintf("%s:%s", trillHost, trillPort) |
There was a problem hiding this comment.
1. Admin_server trailing colon 🐞 Bug ≡ Correctness
Multiple call sites (the create-tree job and CTLog server config generation) build Trillian endpoints by always concatenating host and port with ":", so when the gRPC resolver returns an empty port for a portless user URL (e.g. dns:///custom-host), they produce malformed backend addresses ending in a trailing colon (e.g. dns:///custom-host:) that are then used as ADMIN_SERVER and written into CTLog config.
Agent Prompt
## Issue description
The create-tree action (`internal/action/tree/action.go`) and CTLog server config generation (`internal/controller/ctlog/actions/server_config.go`) build Trillian endpoints with `fmt.Sprintf("%s:%s", host, port)` without guarding against an empty port, which can produce malformed addresses like `dns:///custom-host:` when the user supplies a portless URL and the resolver returns an empty `port`.
## Issue Context
`ResolveInternalGrpcService` can return an empty `port` when the user provides a URL without a trailing `:port` (it returns early for a non-empty `userAddress`, yielding `(address, "")`), and gRPC resolution can legally result in an empty port in this case. The malformed concatenated endpoint is then propagated into critical configuration: the create-tree job receives it as `ADMIN_SERVER`, and CTLog server config writes it as the Trillian backend address used for CTLog config generation. Fixes should ensure callers don’t emit trailing-colon endpoints by either rejecting empty ports at call sites or adjusting `ResolveInternalGrpcService` to fall back to a resolved/default port when the user omits it.
## Fix Focus Areas
- internal/action/tree/action.go[219-226]
- internal/action/tree/action.go[381-404]
- internal/controller/ctlog/actions/server_config.go[80-85]
- internal/utils/service_ref_resolver.go[108-134]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
|
Code review by qodo was updated up to the latest commit f0f3805 |
No description provided.