fix: [CODE-2684]: Move go-get to git URL and support submodules and major versioning (#2914)

This commit is contained in:
Johannes Batzill 2024-11-05 17:13:26 +00:00 committed by Harness
parent 2f45f99f8b
commit 9b89229a35
7 changed files with 265 additions and 44 deletions

View File

@ -184,7 +184,7 @@ func pathTerminatedWithMarkerAndURL(
}
// if marker was found - convert to escaped version (skip first character in case path starts with '/').
escapedPath := path[0:1] + strings.ReplaceAll(path[1:], types.PathSeparator, EncodedPathSeparator)
escapedPath := path[0:1] + strings.ReplaceAll(path[1:], types.PathSeparatorAsString, EncodedPathSeparator)
prefixWithPath := prefix + path + marker
prefixWithEscapedPath := prefix + escapedPath + markerReplacement

View File

@ -15,21 +15,28 @@
package goget
import (
"errors"
"fmt"
"html/template"
"net/http"
"regexp"
"strings"
"github.com/harness/gitness/app/api/auth"
"github.com/harness/gitness/app/api/controller/repo"
"github.com/harness/gitness/app/api/render"
"github.com/harness/gitness/app/api/request"
"github.com/harness/gitness/app/paths"
"github.com/harness/gitness/app/url"
"github.com/harness/gitness/types"
"github.com/harness/gitness/store"
"github.com/rs/zerolog/log"
)
var goGetTmpl *template.Template
var httpRegex = regexp.MustCompile("https?://")
var (
goGetTmpl *template.Template
httpRegex = regexp.MustCompile("https?://")
)
func init() {
var err error
@ -49,8 +56,10 @@ func init() {
}
// Middleware writes to response with html meta tags go-import and go-source.
//
//nolint:gocognit
func Middleware(
config *types.Config,
maxRepoPathDepth int,
repoCtrl *repo.Controller,
urlProvider url.Provider,
) func(http.Handler) http.Handler {
@ -64,27 +73,64 @@ func Middleware(
ctx := r.Context()
session, _ := request.AuthSessionFrom(ctx)
repoRef, err := request.GetRepoRefFromPath(r)
importPath, err := request.GetRepoRefFromPath(r)
if err != nil {
render.TranslatedUserError(ctx, w, err)
return
}
repository, err := repoCtrl.Find(ctx, session, repoRef)
if err != nil {
render.TranslatedUserError(ctx, w, err)
// go get can also be used with (sub)module suffixes (e.g 127.0.0.1/my-project/my-repo/v2)
// for which go expects us to return the import path corresponding to the repository root
// (e.g. 127.0.0.1/my-project/my-repo).
//
// WARNING: This can lead to ambiguities as we allow matching paths between spaces and repos:
// 1. (space)foo + (repo)bar + (sufix)v2 = 127.0.0.1/my-project/my-repo/v2
// 2. (space)foo/bar + (repo)v2 = 127.0.0.1/my-project/my-repo/v2
//
// We handle ambiguities by always choosing the repo with the longest path (e.g. 2. case above).
// To solve go get related ambiguities users would have to move their repositories.
var repository *repo.RepositoryOutput
var repoRef string
pathSegments := paths.Segments(importPath)
if len(pathSegments) > maxRepoPathDepth {
pathSegments = pathSegments[:maxRepoPathDepth]
}
for l := len(pathSegments); l >= 2; l-- {
repoRef = paths.Concatenate(pathSegments[:l]...)
repository, err = repoCtrl.Find(ctx, session, repoRef)
if err == nil {
break
}
if errors.Is(err, store.ErrResourceNotFound) {
log.Ctx(ctx).Debug().Err(err).
Msgf("repository %q doesn't exist, assume submodule and try again", repoRef)
continue
}
if errors.Is(err, auth.ErrNotAuthorized) {
// To avoid leaking information about repos' existence we continue as if it wasn't found.
// WARNING: This can lead to different import results depending on access (very unlikely)
log.Ctx(ctx).Debug().Err(err).
Msgf("user has no access on repository %q, assume submodule and try again", repoRef)
continue
}
render.TranslatedUserError(ctx, w, fmt.Errorf("failed to find repo for path %q: %w", repoRef, err))
return
}
defaultBranch := config.Git.DefaultBranch
if repository.DefaultBranch != "" {
defaultBranch = repository.DefaultBranch
if repository == nil {
render.NotFound(ctx, w)
return
}
uiRepoURL := urlProvider.GenerateUIRepoURL(ctx, repoRef)
goImportURL := httpRegex.ReplaceAllString(uiRepoURL, "")
cloneURL := urlProvider.GenerateGITCloneURL(ctx, repoRef)
prefix := fmt.Sprintf("%s/files/%s/~", uiRepoURL, defaultBranch)
goImportURL := httpRegex.ReplaceAllString(cloneURL, "")
goImportURL = strings.TrimSuffix(goImportURL, ".git")
prefix := fmt.Sprintf("%s/files/%s/~", uiRepoURL, repository.DefaultBranch)
insecure := ""
if strings.HasPrefix(uiRepoURL, "http:") {

View File

@ -19,6 +19,8 @@ import (
"strings"
"github.com/harness/gitness/types"
"github.com/gotidy/ptr"
)
var (
@ -28,13 +30,13 @@ var (
// DisectLeaf splits a path into its parent path and the leaf name
// e.g. space1/space2/space3 -> (space1/space2, space3, nil).
func DisectLeaf(path string) (string, string, error) {
path = strings.Trim(path, types.PathSeparator)
path = strings.Trim(path, types.PathSeparatorAsString)
if path == "" {
return "", "", ErrPathEmpty
}
i := strings.LastIndex(path, types.PathSeparator)
i := strings.LastIndex(path, types.PathSeparatorAsString)
if i == -1 {
return "", path, nil
}
@ -45,13 +47,13 @@ func DisectLeaf(path string) (string, string, error) {
// DisectRoot splits a path into its root space and sub-path
// e.g. space1/space2/space3 -> (space1, space2/space3, nil).
func DisectRoot(path string) (string, string, error) {
path = strings.Trim(path, types.PathSeparator)
path = strings.Trim(path, types.PathSeparatorAsString)
if path == "" {
return "", "", ErrPathEmpty
}
i := strings.Index(path, types.PathSeparator)
i := strings.Index(path, types.PathSeparatorAsString)
if i == -1 {
return path, "", nil
}
@ -63,38 +65,62 @@ func DisectRoot(path string) (string, string, error) {
* Concatenate two paths together (takes care of leading / trailing '/')
* e.g. (space1/, /space2/) -> space1/space2
*
* NOTE: "//" is not a valid path, so all '/' will be trimmed.
* NOTE: All leading, trailing, and consecutive '/' will be trimmed to ensure correct paths.
*/
func Concatenate(path1 string, path2 string) string {
path1 = strings.Trim(path1, types.PathSeparator)
path2 = strings.Trim(path2, types.PathSeparator)
if path1 == "" {
return path2
} else if path2 == "" {
return path1
func Concatenate(paths ...string) string {
if len(paths) == 0 {
return ""
}
return path1 + types.PathSeparator + path2
sb := strings.Builder{}
for i := 0; i < len(paths); i++ {
// remove all leading, trailing, and consecutive '/'
var nextRune *rune
for _, r := range paths[i] {
// skip '/' if we already have '/' as next rune or we don't have anything other than '/' yet
if (nextRune == nil || *nextRune == types.PathSeparator) && r == types.PathSeparator {
continue
}
// first time we take a rune we have to make sure we add a separator if needed (guaranteed no '/').
if nextRune == nil && sb.Len() > 0 {
sb.WriteString(types.PathSeparatorAsString)
}
// flush the previous rune before storing the next rune
if nextRune != nil {
sb.WriteRune(*nextRune)
}
nextRune = ptr.Of(r)
}
// flush the final rune
if nextRune != nil && *nextRune != types.PathSeparator {
sb.WriteRune(*nextRune)
}
}
return sb.String()
}
// Segments returns all segments of the path
// e.g. /space1/space2/space3 -> [space1, space2, space3].
func Segments(path string) []string {
path = strings.Trim(path, types.PathSeparator)
return strings.Split(path, types.PathSeparator)
path = strings.Trim(path, types.PathSeparatorAsString)
return strings.Split(path, types.PathSeparatorAsString)
}
// IsAncesterOf returns true iff 'path' is an ancestor of 'other' or they are the same.
// e.g. other = path(/.*).
func IsAncesterOf(path string, other string) bool {
path = strings.Trim(path, types.PathSeparator)
other = strings.Trim(other, types.PathSeparator)
path = strings.Trim(path, types.PathSeparatorAsString)
other = strings.Trim(other, types.PathSeparatorAsString)
// add "/" to both to handle space1/inner and space1/in
return strings.Contains(
other+types.PathSeparator,
path+types.PathSeparator,
other+types.PathSeparatorAsString,
path+types.PathSeparatorAsString,
)
}

141
app/paths/paths_test.go Normal file
View File

@ -0,0 +1,141 @@
// Copyright 2023 Harness, Inc.
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.
package paths
import (
"testing"
"github.com/stretchr/testify/assert"
)
func Test_Concatenate(t *testing.T) {
type testCase struct {
in []string
want string
}
tests := []testCase{
{
in: nil,
want: "",
},
{
in: []string{},
want: "",
},
{
in: []string{
"",
},
want: "",
},
{
in: []string{
"",
"",
},
want: "",
},
{
in: []string{
"a",
},
want: "a",
},
{
in: []string{
"",
"a",
},
want: "a",
},
{
in: []string{
"a",
"",
},
want: "a",
},
{
in: []string{
"",
"a",
"",
},
want: "a",
},
{
in: []string{
"a",
"b",
},
want: "a/b",
},
{
in: []string{
"a",
"b",
"c",
},
want: "a/b/c",
},
{
in: []string{
"seg1",
"seg2/seg3",
"seg4",
},
want: "seg1/seg2/seg3/seg4",
},
{
in: []string{
"/",
"/",
"/",
},
want: "",
},
{
in: []string{
"//",
},
want: "",
},
{
in: []string{
"/a/",
},
want: "a",
},
{
in: []string{
"/a/",
"/b/",
},
want: "a/b",
},
{
in: []string{
"/a/b/",
"//c//d//",
"///e///f///",
},
want: "a/b/c/d/e/f",
},
}
for _, tt := range tests {
got := Concatenate(tt.in...)
assert.Equal(t, tt.want, got, "path isn't matching for %v", tt.in)
}
}

View File

@ -29,6 +29,7 @@ import (
"github.com/harness/gitness/app/auth/authn"
"github.com/harness/gitness/app/url"
"github.com/harness/gitness/types"
"github.com/harness/gitness/types/check"
"github.com/harness/gitness/types/enum"
"github.com/go-chi/chi"
@ -43,6 +44,12 @@ func NewGitHandler(
authenticator authn.Authenticator,
repoCtrl *repo.Controller,
) http.Handler {
// maxRepoDepth depends on config
maxRepoDepth := check.MaxRepoPathDepth
if !config.NestedSpacesEnabled {
maxRepoDepth = 2
}
// Use go-chi router for inner routing.
r := chi.NewRouter()
@ -60,7 +67,7 @@ func NewGitHandler(
r.Use(middlewareauthn.Attempt(authenticator))
r.Route(fmt.Sprintf("/{%s}", request.PathParamRepoRef), func(r chi.Router) {
r.Use(goget.Middleware(config, repoCtrl, urlProvider))
r.Use(goget.Middleware(maxRepoDepth, repoCtrl, urlProvider))
// routes that aren't coming from git
r.Group(func(r chi.Router) {
// redirect to repo (meant for UI, in case user navigates to clone url in browser)

View File

@ -22,8 +22,8 @@ import (
)
const (
maxPathSegmentsForSpace = 9
maxPathSegments = 10
MaxSpacePathDepth = 9
MaxRepoPathDepth = 10
)
var (
@ -32,13 +32,13 @@ var (
}
ErrPathInvalidDepth = &ValidationError{
fmt.Sprintf("A path can have at most %d segments (%d for spaces).",
maxPathSegments, maxPathSegmentsForSpace),
MaxRepoPathDepth, MaxSpacePathDepth),
}
ErrEmptyPathSegment = &ValidationError{
"Empty segments are not allowed.",
}
ErrPathCantBeginOrEndWithSeparator = &ValidationError{
fmt.Sprintf("Path can't start or end with the separator ('%s').", types.PathSeparator),
fmt.Sprintf("Path can't start or end with the separator ('%s').", types.PathSeparatorAsString),
}
)
@ -49,7 +49,7 @@ func Path(path string, isSpace bool, identifierCheck SpaceIdentifier) error {
}
// ensure path doesn't begin or end with /
if path[:1] == types.PathSeparator || path[len(path)-1:] == types.PathSeparator {
if path[:1] == types.PathSeparatorAsString || path[len(path)-1:] == types.PathSeparatorAsString {
return ErrPathCantBeginOrEndWithSeparator
}
@ -59,7 +59,7 @@ func Path(path string, isSpace bool, identifierCheck SpaceIdentifier) error {
}
// ensure all segments of the path are valid identifiers
segments := strings.Split(path, types.PathSeparator)
segments := strings.Split(path, types.PathSeparatorAsString)
for i, s := range segments {
if s == "" {
return ErrEmptyPathSegment
@ -83,6 +83,6 @@ func PathDepth(path string, isSpace bool) error {
// IsPathTooDeep Checks if the provided path is too long.
// NOTE: A repository path can be one deeper than a space path (as otherwise the space would be useless).
func IsPathTooDeep(path string, isSpace bool) bool {
l := strings.Count(path, types.PathSeparator) + 1
return (!isSpace && l > maxPathSegments) || (isSpace && l > maxPathSegmentsForSpace)
l := strings.Count(path, types.PathSeparatorAsString) + 1
return (!isSpace && l > MaxRepoPathDepth) || (isSpace && l > MaxSpacePathDepth)
}

View File

@ -17,7 +17,8 @@ package types
import "encoding/json"
const (
PathSeparator = "/"
PathSeparatorAsString = string(PathSeparator)
PathSeparator = '/'
)
// SpacePath represents a full path to a space.