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

[image-builder-bob] Introduce URL processing for non docker api urls #10266

Merged
merged 1 commit into from
Jun 1, 2022
Merged

[image-builder-bob] Introduce URL processing for non docker api urls #10266

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
47 changes: 44 additions & 3 deletions components/image-builder-bob/pkg/proxy/proxy.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ type Repo struct {
Auth func() docker.Authorizer
}

func rewriteURL(u *url.URL, fromRepo, toRepo, host, tag string) {
func rewriteDockerAPIURL(u *url.URL, fromRepo, toRepo, host, tag string) {
var (
from = "/v2/" + strings.Trim(fromRepo, "/") + "/"
to = "/v2/" + strings.Trim(toRepo, "/") + "/"
Expand All @@ -79,6 +79,27 @@ func rewriteURL(u *url.URL, fromRepo, toRepo, host, tag string) {
u.Host = host
}

// rewriteNonDockerAPIURL is used when a url has to be rewritten but the url
// contains a non docker api path
func rewriteNonDockerAPIURL(u *url.URL, fromPrefix, toPrefix, host string) {
var (
from = "/" + strings.Trim(fromPrefix, "/") + "/"
to = "/" + strings.Trim(toPrefix, "/") + "/"
)
if fromPrefix == "" {
from = "/"
}
if toPrefix == "" {
to = "/"
}
u.Path = to + strings.TrimPrefix(u.Path, from)

// we reset the escaped encoding hint, because EscapedPath will produce a valid encoding.
u.RawPath = ""

u.Host = host
}

// ServeHTTP serves the proxy
func (proxy *Proxy) ServeHTTP(w http.ResponseWriter, r *http.Request) {
ctx := r.Context()
Expand All @@ -88,9 +109,20 @@ func (proxy *Proxy) ServeHTTP(w http.ResponseWriter, r *http.Request) {
alias string
)
for k, v := range proxy.Aliases {
// Docker api request
if strings.HasPrefix(r.URL.Path, "/v2/"+k+"/") {
repo = &v
alias = k
rewriteDockerAPIURL(r.URL, alias, repo.Repo, repo.Host, repo.Tag)
break
}
// Non-Docker api request
if strings.HasPrefix(r.URL.Path, "/"+k+"/") {
// We will use the same repo/alias and its credentials but we will set target
// repo as empty
repo = &v
alias = k
rewriteNonDockerAPIURL(r.URL, alias, "", repo.Host)
break
}
}
Expand All @@ -99,7 +131,6 @@ func (proxy *Proxy) ServeHTTP(w http.ResponseWriter, r *http.Request) {
return
}

rewriteURL(r.URL, alias, repo.Repo, repo.Host, repo.Tag)
r.Host = r.URL.Host

auth := repo.Auth()
Expand Down Expand Up @@ -186,13 +217,23 @@ func (proxy *Proxy) reverse(alias string) *httputil.ReverseProxy {
rp.ModifyResponse = func(r *http.Response) error {
// Some registries return a Location header which we must rewrite to still push
// through this proxy.
// We support only relative URLs and not absolute URLs.
if loc := r.Header.Get("Location"); loc != "" {
lurl, err := url.Parse(loc)
if err != nil {
return err
}

rewriteURL(lurl, repo.Repo, alias, proxy.Host.Host, "")
if strings.HasPrefix(loc, "/v2/") {
rewriteDockerAPIURL(lurl, repo.Repo, alias, proxy.Host.Host, "")
} else {
// since this is a non docker api location we
// do not need to process the path.
// All docker api URLs always start with /v2/. See spec
// https://github.com/opencontainers/distribution-spec/blob/main/spec.md#endpoints
rewriteNonDockerAPIURL(lurl, "", alias, repo.Host)
}

lurl.Host = proxy.Host.Host
// force scheme to http assuming this proxy never runs as https
lurl.Scheme = proxy.Host.Scheme
Expand Down
188 changes: 188 additions & 0 deletions components/image-builder-bob/pkg/proxy/proxy_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,188 @@
// Copyright (c) 2022 Gitpod GmbH. All rights reserved.
// Licensed under the GNU Affero General Public License (AGPL).
// See License-AGPL.txt in the project root for license information.

package proxy

import (
"net/url"
"testing"
)

func TestRewriteNonDockerAPIURL(t *testing.T) {
type input struct {
u url.URL
fromPrefix string
toPrefix string
host string
}
tests := []struct {
Name string
in input
u url.URL
}{
{
Name: "toPrefix is empty",
in: input{
fromPrefix: "base",
toPrefix: "",
host: "europe-docker.pkg.dev",
u: url.URL{
Host: "localhost.com",
Path: "/base/artifacts-uploads/namespaces/prince-tf-experiments/repositories/dazzle/uploads/somedata",
},
},
u: url.URL{
Host: "europe-docker.pkg.dev",
Path: "/artifacts-uploads/namespaces/prince-tf-experiments/repositories/dazzle/uploads/somedata",
},
},
{
Name: "fromPrefix is empty",
in: input{
fromPrefix: "",
toPrefix: "base",
host: "localhost.com",
u: url.URL{
Host: "europe-docker.pkg.dev",
Path: "/artifacts-uploads/namespaces/prince-tf-experiments/repositories/dazzle/uploads/somedata",
},
},
u: url.URL{
Host: "localhost.com",
Path: "/base/artifacts-uploads/namespaces/prince-tf-experiments/repositories/dazzle/uploads/somedata",
},
},
{
Name: "fromPrefix and toPrefix are not empty",
in: input{
fromPrefix: "from",
toPrefix: "to",
host: "localhost.com",
u: url.URL{
Host: "example.com",
Path: "/from/some/random/path",
},
},
u: url.URL{
Host: "localhost.com",
Path: "/to/some/random/path",
},
},
{
Name: "fromPrefix and toPrefix are empty",
in: input{
fromPrefix: "",
toPrefix: "",
host: "localhost.com",
u: url.URL{
Host: "example.com",
Path: "/some/random/path",
},
},
u: url.URL{
Host: "localhost.com",
Path: "/some/random/path",
},
},
}

for _, test := range tests {
t.Run(test.Name, func(t *testing.T) {
rewriteNonDockerAPIURL(&test.in.u, test.in.fromPrefix, test.in.toPrefix, test.in.host)
if test.in.u.Path != test.u.Path {
t.Errorf("expected path: %s but got %s", test.u.Path, test.in.u.Path)
}
if test.in.u.Host != test.u.Host {
t.Errorf("expected Host: %s but got %s", test.u.Host, test.in.u.Host)
}
if test.in.u.RawPath != test.u.RawPath {
t.Errorf("expected RawPath: %s but got %s", test.u.RawPath, test.in.u.RawPath)
}
})
}

}

func TestRewriteDockerAPIURL(t *testing.T) {
type input struct {
u url.URL
fromRepo string
toRepo string
host string
tag string
}
tests := []struct {
Name string
in input
u url.URL
}{
{
Name: "remote to localhost",
in: input{
fromRepo: "base-images",
toRepo: "base",
host: "localhost.com",
tag: "",
u: url.URL{
Host: "prince.azurecr.io",
Path: "/v2/base-images/some/random/path",
},
},
u: url.URL{
Host: "localhost.com",
Path: "/v2/base/some/random/path",
},
},
{
Name: "localhost to remote",
in: input{
fromRepo: "base",
toRepo: "base-images",
host: "prince.azurecr.io",
tag: "",
u: url.URL{
Host: "localhost.com",
Path: "/v2/base/some/random/path",
},
},
u: url.URL{
Host: "prince.azurecr.io",
Path: "/v2/base-images/some/random/path",
},
},
{
Name: "manifest reference update with tag",
in: input{
fromRepo: "base",
toRepo: "base-images",
host: "prince.azurecr.io",
tag: "tag12345",
u: url.URL{
Host: "localhost.com",
Path: "/v2/base/uploads/manifests/manifest12345",
},
},
u: url.URL{
Host: "prince.azurecr.io",
Path: "/v2/base-images/uploads/manifests/tag12345",
},
},
}

for _, test := range tests {
t.Run(test.Name, func(t *testing.T) {
rewriteDockerAPIURL(&test.in.u, test.in.fromRepo, test.in.toRepo, test.in.host, test.in.tag)
if test.in.u.Path != test.u.Path {
Copy link
Contributor

Choose a reason for hiding this comment

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

minor nit: most tests use cmp.Diff to verify the result, e.g. here.

Makes for an easier read and improved consistency.

t.Errorf("expected path: %s but got %s", test.u.Path, test.in.u.Path)
}
if test.in.u.Host != test.u.Host {
t.Errorf("expected Host: %s but got %s", test.u.Host, test.in.u.Host)
}
if test.in.u.RawPath != test.u.RawPath {
t.Errorf("expected RawPath: %s but got %s", test.u.RawPath, test.in.u.RawPath)
}
})
}

}