Skip to content

Commit c53ce48

Browse files
authored
Merge pull request kubernetes#92217 from p0lyn0mial/proxy-transport-pass-err
Transport.RoundTrip should return a non-nil for failure to obtain a response.
2 parents c352cf0 + c0593b6 commit c53ce48

File tree

5 files changed

+58
-12
lines changed

5 files changed

+58
-12
lines changed

staging/src/k8s.io/apimachinery/pkg/util/proxy/transport.go

Lines changed: 1 addition & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -101,15 +101,7 @@ func (t *Transport) RoundTrip(req *http.Request) (*http.Response, error) {
101101
resp, err := rt.RoundTrip(req)
102102

103103
if err != nil {
104-
message := fmt.Sprintf("Error trying to reach service: '%v'", err.Error())
105-
resp = &http.Response{
106-
Header: http.Header{},
107-
StatusCode: http.StatusServiceUnavailable,
108-
Body: ioutil.NopCloser(strings.NewReader(message)),
109-
}
110-
resp.Header.Set("Content-Type", "text/plain; charset=utf-8")
111-
resp.Header.Set("X-Content-Type-Options", "nosniff")
112-
return resp, nil
104+
return nil, fmt.Errorf("error trying to reach service: %w", err)
113105
}
114106

115107
if redirect := resp.Header.Get("Location"); redirect != "" {

staging/src/k8s.io/apimachinery/pkg/util/proxy/upgradeaware.go

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -232,6 +232,12 @@ func (h *UpgradeAwareHandler) ServeHTTP(w http.ResponseWriter, req *http.Request
232232
proxy.Transport = h.Transport
233233
proxy.FlushInterval = h.FlushInterval
234234
proxy.ErrorLog = log.New(noSuppressPanicError{}, "", log.LstdFlags)
235+
if h.Responder != nil {
236+
// if an optional error interceptor/responder was provided wire it
237+
// the custom responder might be used for providing a unified error reporting
238+
// or supporting retry mechanisms by not sending non-fatal errors to the clients
239+
proxy.ErrorHandler = h.Responder.Error
240+
}
235241
proxy.ServeHTTP(w, newReq)
236242
}
237243

staging/src/k8s.io/apimachinery/pkg/util/proxy/upgradeaware_test.go

Lines changed: 49 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -901,7 +901,8 @@ func TestFlushIntervalHeaders(t *testing.T) {
901901
t.Fatal(err)
902902
}
903903

904-
proxyHandler := NewUpgradeAwareHandler(backendURL, nil, false, false, nil)
904+
responder := &fakeResponder{t: t}
905+
proxyHandler := NewUpgradeAwareHandler(backendURL, nil, false, false, responder)
905906

906907
frontend := httptest.NewServer(proxyHandler)
907908
defer frontend.Close()
@@ -924,6 +925,53 @@ func TestFlushIntervalHeaders(t *testing.T) {
924925
}
925926
}
926927

928+
type fakeRT struct {
929+
err error
930+
}
931+
932+
func (frt *fakeRT) RoundTrip(*http.Request) (*http.Response, error) {
933+
return nil, frt.err
934+
}
935+
936+
// TestErrorPropagation checks if the default transport doesn't swallow the errors by providing a fakeResponder that intercepts and stores the error.
937+
func TestErrorPropagation(t *testing.T) {
938+
backend := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
939+
panic("unreachable")
940+
}))
941+
defer backend.Close()
942+
943+
backendURL, err := url.Parse(backend.URL)
944+
if err != nil {
945+
t.Fatal(err)
946+
}
947+
948+
responder := &fakeResponder{t: t}
949+
expectedErr := errors.New("nasty error")
950+
proxyHandler := NewUpgradeAwareHandler(backendURL, &fakeRT{err: expectedErr}, true, false, responder)
951+
952+
frontend := httptest.NewServer(proxyHandler)
953+
defer frontend.Close()
954+
955+
req, _ := http.NewRequest("GET", frontend.URL, nil)
956+
req.Close = true
957+
958+
ctx, cancel := context.WithTimeout(req.Context(), 10*time.Second)
959+
defer cancel()
960+
req = req.WithContext(ctx)
961+
962+
res, err := frontend.Client().Do(req)
963+
if err != nil {
964+
t.Fatalf("Get: %v", err)
965+
}
966+
defer res.Body.Close()
967+
if res.StatusCode != fakeStatusCode {
968+
t.Fatalf("unexpected HTTP status code returned: %v, expected: %v", res.StatusCode, fakeStatusCode)
969+
}
970+
if !strings.Contains(responder.err.Error(), expectedErr.Error()) {
971+
t.Fatalf("responder got unexpected error: %v, expected the error to contain %q", responder.err.Error(), expectedErr.Error())
972+
}
973+
}
974+
927975
// exampleCert was generated from crypto/tls/generate_cert.go with the following command:
928976
// go run generate_cert.go --rsa-bits 1024 --host example.com --ca --start-date "Jan 1 00:00:00 1970" --duration=1000000h
929977
var exampleCert = []byte(`-----BEGIN CERTIFICATE-----

staging/src/k8s.io/kube-aggregator/pkg/apiserver/handler_proxy.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -237,7 +237,7 @@ func (r *responder) Object(statusCode int, obj runtime.Object) {
237237
}
238238

239239
func (r *responder) Error(_ http.ResponseWriter, _ *http.Request, err error) {
240-
http.Error(r.w, err.Error(), http.StatusInternalServerError)
240+
http.Error(r.w, err.Error(), http.StatusServiceUnavailable)
241241
}
242242

243243
// these methods provide locked access to fields

staging/src/k8s.io/kube-aggregator/pkg/apiserver/handler_proxy_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -561,7 +561,7 @@ func TestGetContextForNewRequest(t *testing.T) {
561561
if err != nil {
562562
t.Fatal(err)
563563
}
564-
if !strings.Contains(string(body), "Error trying to reach service: 'context deadline exceeded'") {
564+
if !strings.Contains(string(body), "context deadline exceeded") {
565565
t.Error(string(body))
566566
}
567567

0 commit comments

Comments
 (0)