feat: Resolve sharded URL at startup#293
Conversation
c83a34e to
d9c0174
Compare
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
d9c0174 to
0b6bed1
Compare
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #293 +/- ##
==========================================
+ Coverage 85.24% 85.40% +0.15%
==========================================
Files 36 36
Lines 2657 2686 +29
==========================================
+ Hits 2265 2294 +29
Misses 264 264
Partials 128 128
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
6c125cf to
8685a29
Compare
8685a29 to
54a06b0
Compare
There was a problem hiding this comment.
Pull request overview
This PR prepares the gateway for sharded Twingate hosts by resolving the effective host during configuration loading and allowing JWT issuer validation for sharded hostnames.
Changes:
- Adds config-time hostname resolution via HEAD request to the JWKS endpoint.
- Adds host suffix matching for JWT issuer selection.
- Adds tests for hostname stripping/resolution and sharded issuer handling.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
internal/config/config.go |
Adds sharded hostname resolution during config load. |
internal/config/config_test.go |
Adds tests for hostname stripping and redirect resolution. |
internal/token/parser.go |
Adds issuer lookup helper with suffix matching for sharded hosts. |
internal/token/parser_test.go |
Adds tests for sharded issuer lookup and parser behavior. |
go.mod |
Promotes go-retryablehttp to a direct dependency. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
… test sync - Skip sharded host resolution when Twingate network is empty - Replace plain bool with buffered channel for race-safe test assertion - Add edge case test for empty network in stripNetworkPrefix Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
a6ffbba to
3edce14
Compare
495f001 to
4e60e31
Compare
|
|
||
| func (c *Config) ResolveTwingateHost() { | ||
| targetURL := fmt.Sprintf("https://%s.%s/api/v1/jwk/ec", c.Twingate.Network, c.Twingate.Host) | ||
| resolvedHostname := resolveTwingateHostname(targetURL, c.Twingate.Host, 3) |
There was a problem hiding this comment.
With this configuration we get:
| Step | What | Duration |
|---|---|---|
| Attempt 0 | HEAD → timeout | 1s |
| Backoff 0 | 2^0 × 1s | 1s |
| Attempt 1 | HEAD → timeout | 1s |
| Backoff 1 | 2^1 × 1s | 2s |
| Attempt 2 | HEAD → timeout | 1s |
| Backoff 2 | 2^2 × 1s | 4s |
| Attempt 3 | HEAD → timeout | 1s |
Total = 11s
I think 11s is fine? Alternatively, we can shorten the backoff to 0.5s or set the max attempt to 2, to squeeze within the 10s window
There was a problem hiding this comment.
- Yes, I would lower the max retries to 2. Maybe also increase the timeout to 2s.
- Why is the number of retries passed in as an argument but the 1s timeout is hard coded here?
There was a problem hiding this comment.
Why is the number of retries passed in as an argument but the 1s timeout is hard coded here?
We are setting the same value (1 second) for the test as well, so Go lint complains: internal/config/config.go:196:61: resolveTwingateHostname - timeout always receives 1 * time.Second (1000000000) (unparam), whereas retryMax is set to 0 for all tests
|
|
||
| func (c *Config) ResolveTwingateHost() { | ||
| targetURL := fmt.Sprintf("https://%s.%s/api/v1/jwk/ec", c.Twingate.Network, c.Twingate.Host) | ||
| resolvedHostname := resolveTwingateHostname(targetURL, c.Twingate.Host, 3) |
There was a problem hiding this comment.
- Yes, I would lower the max retries to 2. Maybe also increase the timeout to 2s.
- Why is the number of retries passed in as an argument but the 1s timeout is hard coded here?
Issue: #292
Changes
HEADrequest to the JWKS endpoint and extract theLocationheaderresolveTwingateHostnameextract theLocationheader and don't follow the redirectstripNetworkPrefixstrips the network prefix from the resolved hostname (e.g.,acme.us1.twingate.com→us1.twingate.com)getIssuer()Note: Sharded host is not supported in production yet. This is preparation work, so the operator is ready when sharding is enabled in production.