Skip to content

Remove sub-path from container registry realm #31293

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

Merged
merged 3 commits into from
Jun 9, 2024
Merged

Remove sub-path from container registry realm #31293

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
29d15f3
fix
wxiaoguang Jun 7, 2024
a62ec02
try to remove RegistryHost
wxiaoguang Jun 7, 2024
d54bc4e
fine tune tests
wxiaoguang Jun 7, 2024
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
5 changes: 0 additions & 5 deletions modules/setting/packages.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@ package setting
import (
"fmt"
"math"
"net/url"
"os"
"path/filepath"

Expand All @@ -19,7 +18,6 @@ var (
Storage *Storage
Enabled bool
ChunkedUploadPath string
RegistryHost string

LimitTotalOwnerCount int64
LimitTotalOwnerSize int64
Expand Down Expand Up @@ -66,9 +64,6 @@ func loadPackagesFrom(rootCfg ConfigProvider) (err error) {
return err
}

appURL, _ := url.Parse(AppURL)
Packages.RegistryHost = appURL.Host

Packages.ChunkedUploadPath = filepath.ToSlash(sec.Key("CHUNKED_UPLOAD_PATH").MustString("tmp/package-upload"))
if !filepath.IsAbs(Packages.ChunkedUploadPath) {
Packages.ChunkedUploadPath = filepath.ToSlash(filepath.Join(AppDataPath, Packages.ChunkedUploadPath))
Expand Down
6 changes: 4 additions & 2 deletions modules/test/utils.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,8 +34,10 @@ func IsNormalPageCompleted(s string) bool {
return strings.Contains(s, `<footer class="page-footer"`) && strings.Contains(s, `</html>`)
}

func MockVariableValue[T any](p *T, v T) (reset func()) {
func MockVariableValue[T any](p *T, v ...T) (reset func()) {
old := *p
*p = v
if len(v) > 0 {
*p = v[0]
}
return func() { *p = old }
}
6 changes: 3 additions & 3 deletions routers/api/packages/container/container.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -116,9 +116,9 @@ func apiErrorDefined(ctx *context.Context, err *namedError) {
}

func apiUnauthorizedError(ctx *context.Context) {
// TODO: it doesn't seem quite right but it doesn't really cause problem at the moment.
// container registry requires that the "/v2" must be in the root, so the sub-path in AppURL should be removed, ideally.
ctx.Resp.Header().Add("WWW-Authenticate", `Bearer realm="`+httplib.GuessCurrentAppURL(ctx)+`v2/token",service="container_registry",scope="*"`)
// container registry requires that the "/v2" must be in the root, so the sub-path in AppURL should be removed
realmURL := strings.TrimSuffix(httplib.GuessCurrentAppURL(ctx), setting.AppSubURL+"/") + "/v2/token"
ctx.Resp.Header().Add("WWW-Authenticate", `Bearer realm="`+realmURL+`",service="container_registry",scope="*"`)
apiErrorDefined(ctx, errUnauthorized)
}

Expand Down
8 changes: 7 additions & 1 deletion routers/web/user/package.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ package user

import (
"net/http"
"net/url"

"code.gitea.io/gitea/models/db"
org_model "code.gitea.io/gitea/models/organization"
Expand All @@ -15,6 +16,7 @@ import (
repo_model "code.gitea.io/gitea/models/repo"
"code.gitea.io/gitea/modules/base"
"code.gitea.io/gitea/modules/container"
"code.gitea.io/gitea/modules/httplib"
"code.gitea.io/gitea/modules/log"
"code.gitea.io/gitea/modules/optional"
alpine_module "code.gitea.io/gitea/modules/packages/alpine"
Expand Down Expand Up @@ -178,7 +180,11 @@ func ViewPackageVersion(ctx *context.Context) {

switch pd.Package.Type {
case packages_model.TypeContainer:
ctx.Data["RegistryHost"] = setting.Packages.RegistryHost
registryAppURL, err := url.Parse(httplib.GuessCurrentAppURL(ctx))
if err != nil {
registryAppURL, _ = url.Parse(setting.AppURL)
}
ctx.Data["RegistryHost"] = registryAppURL.Host
case packages_model.TypeAlpine:
branches := make(container.Set[string])
repositories := make(container.Set[string])
Expand Down
12 changes: 9 additions & 3 deletions tests/integration/api_packages_container_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -84,15 +84,15 @@ func TestPackageContainer(t *testing.T) {
Token string `json:"token"`
}

authenticate := []string{`Bearer realm="` + setting.AppURL + `v2/token",service="container_registry",scope="*"`}
defaultAuthenticateValues := []string{`Bearer realm="` + setting.AppURL + `v2/token",service="container_registry",scope="*"`}

t.Run("Anonymous", func(t *testing.T) {
defer tests.PrintCurrentTest(t)()

req := NewRequest(t, "GET", fmt.Sprintf("%sv2", setting.AppURL))
resp := MakeRequest(t, req, http.StatusUnauthorized)

assert.ElementsMatch(t, authenticate, resp.Header().Values("WWW-Authenticate"))
assert.ElementsMatch(t, defaultAuthenticateValues, resp.Header().Values("WWW-Authenticate"))

req = NewRequest(t, "GET", fmt.Sprintf("%sv2/token", setting.AppURL))
resp = MakeRequest(t, req, http.StatusOK)
Expand All @@ -115,6 +115,12 @@ func TestPackageContainer(t *testing.T) {

req = NewRequest(t, "GET", fmt.Sprintf("%sv2/token", setting.AppURL))
MakeRequest(t, req, http.StatusUnauthorized)

defer test.MockVariableValue(&setting.AppURL, "https://domain:8443/sub-path/")()
defer test.MockVariableValue(&setting.AppSubURL, "/sub-path")()
req = NewRequest(t, "GET", "/v2")
resp = MakeRequest(t, req, http.StatusUnauthorized)
assert.Equal(t, `Bearer realm="https://domain:8443/v2/token",service="container_registry",scope="*"`, resp.Header().Get("WWW-Authenticate"))
})

t.Run("User", func(t *testing.T) {
Expand All @@ -123,7 +129,7 @@ func TestPackageContainer(t *testing.T) {
req := NewRequest(t, "GET", fmt.Sprintf("%sv2", setting.AppURL))
resp := MakeRequest(t, req, http.StatusUnauthorized)

assert.ElementsMatch(t, authenticate, resp.Header().Values("WWW-Authenticate"))
assert.ElementsMatch(t, defaultAuthenticateValues, resp.Header().Values("WWW-Authenticate"))

req = NewRequest(t, "GET", fmt.Sprintf("%sv2/token", setting.AppURL)).
AddBasicAuth(user.Name)
Expand Down