Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Replace http util reverese proxy with custom request forwarding #436

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Replace http util reverese proxy with custom request forwarding #436

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0

### Changed

- [#436](https://github.com/spegel-org/spegel/pull/436) Replace http util reverese proxy with custom request forwarding.

### Deprecated

### Removed
Expand Down
56 changes: 31 additions & 25 deletions pkg/registry/registry.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@ import (
"io"
"net"
"net/http"
"net/http/httputil"
"net/url"
"path"
"strconv"
Expand All @@ -32,7 +31,7 @@ type Registry struct {
throttler *throttle.Throttler
ociClient oci.Client
router routing.Router
transport http.RoundTripper
httpClient *http.Client
localAddr string
resolveRetries int
resolveTimeout time.Duration
Expand Down Expand Up @@ -61,7 +60,7 @@ func WithResolveTimeout(resolveTimeout time.Duration) Option {

func WithTransport(transport http.RoundTripper) Option {
return func(r *Registry) {
r.transport = transport
r.httpClient.Transport = transport
}
}

Expand All @@ -87,6 +86,7 @@ func NewRegistry(ociClient oci.Client, router routing.Router, opts ...Option) *R
r := &Registry{
ociClient: ociClient,
router: router,
httpClient: &http.Client{},
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we initialize it with some sane default?

	httpClient := &http.Client{
		Transport: &http.Transport{
			MaxIdleConns:        20,
			MaxIdleConnsPerHost: 20,
		},
	}

resolveRetries: 3,
resolveTimeout: 1 * time.Second,
resolveLatestTag: true,
Expand Down Expand Up @@ -184,10 +184,8 @@ func (r *Registry) registryHandler(rw mux.ResponseWriter, req *http.Request) {
}
}

// Request with mirror header are proxied.
// Requests without mirror header set will be mirrored
if req.Header.Get(MirroredHeaderKey) != "true" {
// Set mirrored header in request to stop infinite loops
req.Header.Set(MirroredHeaderKey, "true")
key := dgst.String()
if key == "" {
key = ref
Expand Down Expand Up @@ -241,6 +239,7 @@ func (r *Registry) handleMirror(rw mux.ResponseWriter, req *http.Request, key st
rw.WriteError(http.StatusInternalServerError, err)
return
}

// TODO: Refactor context cancel and mirror channel closing
for {
select {
Expand All @@ -255,36 +254,43 @@ func (r *Registry) handleMirror(rw mux.ResponseWriter, req *http.Request, key st
return
}

// Modify response returns and error on non 200 status code and NOP error handler skips response writing.
// If proxy fails no response is written and it is tried again against a different mirror.
// If the response writer has been written to it means that the request was properly proxied.
succeeded := false
scheme := "http"
if req.TLS != nil {
scheme = "https"
}
u := &url.URL{
u := url.URL{
Scheme: scheme,
Host: ipAddr.String(),
Path: req.URL.Path,
// TODO: Should this error early if not set?
RawQuery: fmt.Sprintf("ns=%s", req.URL.Query().Get("ns")),
}
proxy := httputil.NewSingleHostReverseProxy(u)
proxy.Transport = r.transport
proxy.ErrorHandler = func(_ http.ResponseWriter, _ *http.Request, err error) {
log.Error(err, "proxy failed attempting next")
forwardReq, err := http.NewRequestWithContext(req.Context(), req.Method, u.String(), nil)
if err != nil {
rw.WriteError(http.StatusInternalServerError, err)
return
}
proxy.ModifyResponse = func(resp *http.Response) error {
if resp.StatusCode != http.StatusOK {
err := fmt.Errorf("expected mirror to respond with 200 OK but received: %s", resp.Status)
log.Error(err, "mirror failed attempting next")
return err
}
succeeded = true
return nil
forwardReq.Header.Add(MirroredHeaderKey, "true")
resp, err := r.httpClient.Do(forwardReq)
if err != nil {
log.Error(err, "mirror failed attempting next")
break
}
proxy.ServeHTTP(rw, req)
if !succeeded {
defer resp.Body.Close()
if resp.StatusCode != http.StatusOK {
log.Error(fmt.Errorf("expected mirror to respond with 200 OK but received: %s", resp.Status), "mirror failed attempting next")
break
}
for k, v := range resp.Header {
for _, vv := range v {
rw.Header().Add(k, vv)
}
}
_, err = io.Copy(rw, resp.Body)
if err != nil {
rw.WriteError(http.StatusInternalServerError, err)
return
}
log.V(5).Info("mirrored request", "url", u.String())
return
}
Expand Down
Loading