chore: address code review findings
- Rename shadowed err variable in resolveTargetMetadata - Lower registry routing log from Info to Debug - Rename misleading MaxBodySize test - Clarify limitedReadCloser.Read slice capping - Fix Go version in h2c transport comment (1.24+)
- commit
- 278618d3ed197a8f4dd70b790b8bf2a035e2dbee
- author
- brice · · 2026-03-16 00:17 UTC
- parent
- 01ffd59
- signature
- unsigned
4 files · +10 -9
M internal/adapters/in/http/proxy/forwarder.go +5 -4
| @@ -35,7 +35,7 @@ func newAppTransport() *http.Transport { | |||
| 35 | 35 | ||
| 36 | 36 | // newH2CTransport creates a dedicated cleartext HTTP/2 transport for containers | |
| 37 | 37 | // that opt in via the gordon.proxy.protocol=h2c label. | |
| 38 | - | // Uses Go 1.26 native UnencryptedHTTP2 protocol support. | |
| 38 | + | // Uses Go 1.24+ native UnencryptedHTTP2 protocol support. | |
| 39 | 39 | // HTTP/1 is disabled so http:// URLs use HTTP/2 prior-knowledge. | |
| 40 | 40 | func newH2CTransport() *http.Transport { | |
| 41 | 41 | var protos http.Protocols | |
| @@ -248,10 +248,11 @@ func (l *limitedReadCloser) Read(p []byte) (int, error) { | |||
| 248 | 248 | } | |
| 249 | 249 | return 0, fmt.Errorf("response body exceeded maximum size limit") | |
| 250 | 250 | } | |
| 251 | - | if int64(len(p)) > l.remaining { | |
| 252 | - | p = p[:l.remaining] | |
| 251 | + | toRead := len(p) | |
| 252 | + | if int64(toRead) > l.remaining { | |
| 253 | + | toRead = int(l.remaining) | |
| 253 | 254 | } | |
| 254 | - | n, err := l.ReadCloser.Read(p) | |
| 255 | + | n, err := l.ReadCloser.Read(p[:toRead]) | |
| 255 | 256 | l.remaining -= int64(n) | |
| 256 | 257 | return n, err | |
| 257 | 258 | } | |
| @@ -35,7 +35,7 @@ func newAppTransport() *http.Transport { | |||||
| 35 | 35 | ||||
| 36 | // newH2CTransport creates a dedicated cleartext HTTP/2 transport for containers | 36 | // newH2CTransport creates a dedicated cleartext HTTP/2 transport for containers | ||
| 37 | // that opt in via the gordon.proxy.protocol=h2c label. | 37 | // that opt in via the gordon.proxy.protocol=h2c label. | ||
| 38 | - | // Uses Go 1.26 native UnencryptedHTTP2 protocol support. | 38 | + | // Uses Go 1.24+ native UnencryptedHTTP2 protocol support. |
| 39 | // HTTP/1 is disabled so http:// URLs use HTTP/2 prior-knowledge. | 39 | // HTTP/1 is disabled so http:// URLs use HTTP/2 prior-knowledge. | ||
| 40 | func newH2CTransport() *http.Transport { | 40 | func newH2CTransport() *http.Transport { | ||
| 41 | var protos http.Protocols | 41 | var protos http.Protocols | ||
| @@ -248,10 +248,11 @@ func (l *limitedReadCloser) Read(p []byte) (int, error) { | |||||
| 248 | } | 248 | } | ||
| 249 | return 0, fmt.Errorf("response body exceeded maximum size limit") | 249 | return 0, fmt.Errorf("response body exceeded maximum size limit") | ||
| 250 | } | 250 | } | ||
| 251 | - | if int64(len(p)) > l.remaining { | 251 | + | toRead := len(p) |
| 252 | - | p = p[:l.remaining] | 252 | + | if int64(toRead) > l.remaining { |
| 253 | + | toRead = int(l.remaining) | |||
| 253 | } | 254 | } | ||
| 254 | - | n, err := l.ReadCloser.Read(p) | 255 | + | n, err := l.ReadCloser.Read(p[:toRead]) |
| 255 | l.remaining -= int64(n) | 256 | l.remaining -= int64(n) | ||
| 256 | return n, err | 257 | return n, err | ||
| 257 | } | 258 | } | ||
M internal/adapters/in/http/proxy/handler.go +1 -1
| @@ -68,7 +68,7 @@ func (h *Handler) ServeHTTP(w http.ResponseWriter, r *http.Request) { | |||
| 68 | 68 | ||
| 69 | 69 | // Check if this is the registry domain | |
| 70 | 70 | if h.proxySvc.IsRegistryDomain(r.Host) { | |
| 71 | - | log.Info().Msg("routing request to registry") | |
| 71 | + | log.Debug().Msg("routing request to registry") | |
| 72 | 72 | h.forwardToRegistry(w, r, cfg.RegistryPort) | |
| 73 | 73 | return | |
| 74 | 74 | } | |
| @@ -68,7 +68,7 @@ func (h *Handler) ServeHTTP(w http.ResponseWriter, r *http.Request) { | |||||
| 68 | 68 | ||||
| 69 | // Check if this is the registry domain | 69 | // Check if this is the registry domain | ||
| 70 | if h.proxySvc.IsRegistryDomain(r.Host) { | 70 | if h.proxySvc.IsRegistryDomain(r.Host) { | ||
| 71 | - | log.Info().Msg("routing request to registry") | 71 | + | log.Debug().Msg("routing request to registry") |
| 72 | h.forwardToRegistry(w, r, cfg.RegistryPort) | 72 | h.forwardToRegistry(w, r, cfg.RegistryPort) | ||
| 73 | return | 73 | return | ||
| 74 | } | 74 | } | ||
M internal/adapters/in/http/proxy/handler_test.go +1 -1
| @@ -110,7 +110,7 @@ func TestHandler_ProxiesToTarget(t *testing.T) { | |||
| 110 | 110 | assert.Equal(t, "ok", w.Body.String()) | |
| 111 | 111 | } | |
| 112 | 112 | ||
| 113 | - | func TestHandler_MaxBodySize_Applied(t *testing.T) { | |
| 113 | + | func TestHandler_ReadsMaxBodySizeConfig(t *testing.T) { | |
| 114 | 114 | proxySvc := inmocks.NewMockProxyService(t) | |
| 115 | 115 | ||
| 116 | 116 | proxySvc.EXPECT().ProxyConfig().Return(in.ProxyServiceConfig{ | |
| @@ -110,7 +110,7 @@ func TestHandler_ProxiesToTarget(t *testing.T) { | |||||
| 110 | assert.Equal(t, "ok", w.Body.String()) | 110 | assert.Equal(t, "ok", w.Body.String()) | ||
| 111 | } | 111 | } | ||
| 112 | 112 | ||||
| 113 | - | func TestHandler_MaxBodySize_Applied(t *testing.T) { | 113 | + | func TestHandler_ReadsMaxBodySizeConfig(t *testing.T) { |
| 114 | proxySvc := inmocks.NewMockProxyService(t) | 114 | proxySvc := inmocks.NewMockProxyService(t) | ||
| 115 | 115 | ||||
| 116 | proxySvc.EXPECT().ProxyConfig().Return(in.ProxyServiceConfig{ | 116 | proxySvc.EXPECT().ProxyConfig().Return(in.ProxyServiceConfig{ | ||
M internal/usecase/proxy/target_metadata.go +3 -3
| @@ -46,9 +46,9 @@ func (s *Service) resolveTargetMetadata(ctx context.Context, imageRef string) (T | |||
| 46 | 46 | } | |
| 47 | 47 | ||
| 48 | 48 | if port == 0 { | |
| 49 | - | exposedPorts, err := s.runtime.GetImageExposedPorts(ctx, imageRef) | |
| 50 | - | if err != nil { | |
| 51 | - | return TargetMetadata{}, err | |
| 49 | + | exposedPorts, portsErr := s.runtime.GetImageExposedPorts(ctx, imageRef) | |
| 50 | + | if portsErr != nil { | |
| 51 | + | return TargetMetadata{}, portsErr | |
| 52 | 52 | } | |
| 53 | 53 | if len(exposedPorts) == 0 { | |
| 54 | 54 | return TargetMetadata{}, fmt.Errorf("no exposed ports found for image %s", imageRef) | |
| @@ -46,9 +46,9 @@ func (s *Service) resolveTargetMetadata(ctx context.Context, imageRef string) (T | |||||
| 46 | } | 46 | } | ||
| 47 | 47 | ||||
| 48 | if port == 0 { | 48 | if port == 0 { | ||
| 49 | - | exposedPorts, err := s.runtime.GetImageExposedPorts(ctx, imageRef) | 49 | + | exposedPorts, portsErr := s.runtime.GetImageExposedPorts(ctx, imageRef) |
| 50 | - | if err != nil { | 50 | + | if portsErr != nil { |
| 51 | - | return TargetMetadata{}, err | 51 | + | return TargetMetadata{}, portsErr |
| 52 | } | 52 | } | ||
| 53 | if len(exposedPorts) == 0 { | 53 | if len(exposedPorts) == 0 { | ||
| 54 | return TargetMetadata{}, fmt.Errorf("no exposed ports found for image %s", imageRef) | 54 | return TargetMetadata{}, fmt.Errorf("no exposed ports found for image %s", imageRef) | ||