[MISC] Make `Internal`/`Container` URL Generic (and fix `GetTreeNode`) (#621)

pull/3393/head
Johannes Batzill 2023-09-28 03:33:45 +00:00 committed by Harness
parent c3032c90e8
commit f51f97adb2
13 changed files with 92 additions and 76 deletions

View File

@ -114,7 +114,7 @@ func CreateRPCWriteParams(ctx context.Context, urlProvider url.Provider,
// generate envars (add everything githook CLI needs for execution)
envVars, err := githook.GenerateEnvironmentVariables(
ctx,
urlProvider.GetAPIBaseURLInternal(),
urlProvider.GetInternalAPIURL(),
repo.ID,
session.Principal.ID,
false,

View File

@ -181,7 +181,7 @@ func (c *Controller) createGitRPCRepository(ctx context.Context, session *auth.S
// generate envars (add everything githook CLI needs for execution)
envVars, err := githook.GenerateEnvironmentVariables(
ctx,
c.urlProvider.GetAPIBaseURLInternal(),
c.urlProvider.GetInternalAPIURL(),
0,
session.Principal.ID,
true,

View File

@ -37,7 +37,7 @@ func CreateRPCWriteParams(ctx context.Context, urlProvider url.Provider,
// generate envars (add everything githook CLI needs for execution)
envVars, err := githook.GenerateEnvironmentVariables(
ctx,
urlProvider.GetAPIBaseURLInternal(),
urlProvider.GetInternalAPIURL(),
repo.ID,
session.Principal.ID,
false,

View File

@ -297,7 +297,7 @@ func (m *Manager) Details(ctx context.Context, stageID int64) (*ExecutionContext
return nil, err
}
// Backfill clone URL
repo.GitURL = m.urlProvider.GenerateGITCloneURLContainer(repo.Path)
repo.GitURL = m.urlProvider.GenerateContainerGITCloneURL(repo.Path)
stages, err := m.Stages.List(noContext, stage.ExecutionID)
if err != nil {

View File

@ -449,7 +449,7 @@ func (r *Repository) createEnvVars(ctx context.Context,
) (map[string]string, error) {
envVars, err := githook.GenerateEnvironmentVariables(
ctx,
r.urlProvider.GetAPIBaseURLInternal(),
r.urlProvider.GetInternalAPIURL(),
repoID,
principal.ID,
false,

View File

@ -264,7 +264,7 @@ func createSystemRPCWriteParams(
// generate envars (add everything githook CLI needs for execution)
envVars, err := githook.GenerateEnvironmentVariables(
ctx,
urlProvider.GetAPIBaseURLInternal(),
urlProvider.GetInternalAPIURL(),
repoID,
principal.ID,
false,

View File

@ -21,21 +21,32 @@ import (
"strings"
)
const (
// GITSuffix is the suffix used to terminate repo paths for git apis.
GITSuffix = ".git"
// APIMount is the prefix path for the api endpoints.
APIMount = "api"
// GITMount is the prefix path for the git endpoints.
GITMount = "git"
)
// Provider is an abstraction of a component that provides system related URLs.
// NOTE: Abstract to allow for custom implementation for more complex routing environments.
type Provider interface {
// GetAPIBaseURLInternal returns the internally reachable base url of the api server.
// GetInternalAPIURL returns the internally reachable base url of the server.
// NOTE: url is guaranteed to not have any trailing '/'.
GetAPIBaseURLInternal() string
GetInternalAPIURL() string
// GenerateContainerGITCloneURL generates a URL that can be used by CI container builds to
// interact with gitness and clone a repo.
GenerateContainerGITCloneURL(repoPath string) string
// GenerateGITCloneURL generates the public git clone URL for the provided repo path.
// NOTE: url is guaranteed to not have any trailing '/'.
GenerateGITCloneURL(repoPath string) string
// GenerateGITCloneURLContainer generates a URL that can be used by CI container builds to
// interact with gitness and clone a repo.
GenerateGITCloneURLContainer(repoPath string) string
// GenerateUIPRURL returns the url for the UI screen of an existing pr.
GenerateUIPRURL(repoPath string, prID int64) string
@ -51,95 +62,95 @@ type Provider interface {
// Provider provides the URLs of the gitness system.
type provider struct {
// internalURL stores the URL via which the service is reachable at internally
// (no need for internal services to go via public route).
internalURL *url.URL
// containerURL stores the URL that can be used to communicate with gitness from inside a
// build container.
containerURL *url.URL
// apiURL stores the raw URL the api endpoints are reachable at publicly.
apiURL *url.URL
// apiURLInternalRaw stores the raw URL the api endpoints are reachable at internally
// (no need for internal services to go via public route).
// NOTE: url is guaranteed to not have any trailing '/'.
apiURLInternalRaw string
// gitURL stores the URL the git endpoints are available at.
// NOTE: we store it as url.URL so we can derive clone URLS without errors.
gitURL *url.URL
// gitURLContainer stores the rawURL that can be used to communicate with gitness from inside a
// build container.
gitURLContainer *url.URL
// uiURL stores the raw URL to the ui endpoints.
uiURL *url.URL
}
func NewProvider(
internalURLRaw,
containerURLRaw string,
apiURLRaw string,
apiURLInternalRaw,
gitURLRaw,
gitURLContainerRaw string,
uiURLRaw string,
) (Provider, error) {
// remove trailing '/' to make usage easier
internalURLRaw = strings.TrimRight(internalURLRaw, "/")
containerURLRaw = strings.TrimRight(containerURLRaw, "/")
apiURLRaw = strings.TrimRight(apiURLRaw, "/")
apiURLInternalRaw = strings.TrimRight(apiURLInternalRaw, "/")
gitURLRaw = strings.TrimRight(gitURLRaw, "/")
gitURLContainerRaw = strings.TrimRight(gitURLContainerRaw, "/")
uiURLRaw = strings.TrimRight(uiURLRaw, "/")
// parseAPIURL
internalURL, err := url.Parse(internalURLRaw)
if err != nil {
return nil, fmt.Errorf("provided internalURLRaw '%s' is invalid: %w", internalURLRaw, err)
}
containerURL, err := url.Parse(containerURLRaw)
if err != nil {
return nil, fmt.Errorf("provided containerURLRaw '%s' is invalid: %w", containerURLRaw, err)
}
apiURL, err := url.Parse(apiURLRaw)
if err != nil {
return nil, fmt.Errorf("provided apiURLRaw '%s' is invalid: %w", apiURLRaw, err)
}
// parse gitURL
gitURL, err := url.Parse(gitURLRaw)
if err != nil {
return nil, fmt.Errorf("provided gitURLRaw '%s' is invalid: %w", gitURLRaw, err)
}
// parse gitURLContainer
gitURLContainer, err := url.Parse(gitURLContainerRaw)
if err != nil {
return nil, fmt.Errorf("provided gitURLContainerRaw '%s' is invalid: %w", gitURLContainerRaw, err)
}
// parse uiURL
uiURL, err := url.Parse(uiURLRaw)
if err != nil {
return nil, fmt.Errorf("provided uiURLRaw '%s' is invalid: %w", uiURLRaw, err)
}
return &provider{
apiURL: apiURL,
apiURLInternalRaw: apiURLInternalRaw,
gitURL: gitURL,
gitURLContainer: gitURLContainer,
uiURL: uiURL,
internalURL: internalURL,
containerURL: containerURL,
apiURL: apiURL,
gitURL: gitURL,
uiURL: uiURL,
}, nil
}
func (p *provider) GetAPIBaseURLInternal() string {
return p.apiURLInternalRaw
func (p *provider) GetInternalAPIURL() string {
return p.internalURL.JoinPath(APIMount).String()
}
func (p *provider) GenerateContainerGITCloneURL(repoPath string) string {
repoPath = path.Clean(repoPath)
if !strings.HasSuffix(repoPath, GITSuffix) {
repoPath += GITSuffix
}
return p.containerURL.JoinPath(GITMount, repoPath).String()
}
func (p *provider) GenerateGITCloneURL(repoPath string) string {
repoPath = path.Clean(repoPath)
if !strings.HasSuffix(repoPath, ".git") {
repoPath += ".git"
if !strings.HasSuffix(repoPath, GITSuffix) {
repoPath += GITSuffix
}
return p.gitURL.JoinPath(repoPath).String()
}
func (p *provider) GenerateGITCloneURLContainer(repoPath string) string {
repoPath = path.Clean(repoPath)
if !strings.HasSuffix(repoPath, ".git") {
repoPath += ".git"
}
return p.gitURLContainer.JoinPath(repoPath).String()
}
func (p *provider) GenerateUIPRURL(repoPath string, prID int64) string {
return p.uiURL.JoinPath(repoPath, "pulls", fmt.Sprint(prID)).String()
}

View File

@ -25,10 +25,10 @@ var WireSet = wire.NewSet(ProvideURLProvider)
func ProvideURLProvider(config *types.Config) (Provider, error) {
return NewProvider(
config.URL.Internal,
config.URL.Container,
config.URL.API,
config.URL.APIInternal,
config.URL.Git,
config.URL.GitContainer,
config.URL.UI,
)
}

View File

@ -66,11 +66,11 @@ func backfillURLs(config *types.Config) error {
// TODO: handle https of server bind
// backfil internal URLS before continuing override with user provided base (which is external facing)
if config.URL.APIInternal == "" {
config.URL.APIInternal = fmt.Sprintf("%s://localhost:%s/api", scheme, port)
if config.URL.Internal == "" {
config.URL.Internal = fmt.Sprintf("%s://localhost:%s", scheme, port)
}
if config.URL.GitContainer == "" {
config.URL.GitContainer = fmt.Sprintf("%s://host.docker.internal:%s/git", scheme, port)
if config.URL.Container == "" {
config.URL.Container = fmt.Sprintf("%s://host.docker.internal:%s", scheme, port)
}
// override base with whatever user explicit override

View File

@ -29,11 +29,12 @@ func TestBackfilURLsPortBind(t *testing.T) {
err := backfillURLs(config)
require.NoError(t, err)
require.Equal(t, "http://localhost:1234", config.URL.Internal)
require.Equal(t, "http://host.docker.internal:1234", config.URL.Container)
require.Equal(t, "http://localhost:1234/api", config.URL.API)
require.Equal(t, "http://localhost:1234/git", config.URL.Git)
require.Equal(t, "http://localhost:1234", config.URL.UI)
require.Equal(t, "http://localhost:1234/api", config.URL.APIInternal)
require.Equal(t, "http://host.docker.internal:1234/git", config.URL.GitContainer)
}
func TestBackfilURLsBase(t *testing.T) {
@ -44,11 +45,12 @@ func TestBackfilURLsBase(t *testing.T) {
err := backfillURLs(config)
require.NoError(t, err)
require.Equal(t, "http://localhost:1234", config.URL.Internal)
require.Equal(t, "http://host.docker.internal:1234", config.URL.Container)
require.Equal(t, "https://xyz:4321/test/api", config.URL.API)
require.Equal(t, "https://xyz:4321/test/git", config.URL.Git)
require.Equal(t, "https://xyz:4321/test", config.URL.UI)
require.Equal(t, "http://localhost:1234/api", config.URL.APIInternal)
require.Equal(t, "http://host.docker.internal:1234/git", config.URL.GitContainer)
}
func TestBackfilURLsCustom(t *testing.T) {
@ -56,17 +58,18 @@ func TestBackfilURLsCustom(t *testing.T) {
config.Server.HTTP.Port = 1234
config.URL.Base = "https://xyz:4321/test"
config.URL.API = "http://API:1111/API/p"
config.URL.APIInternal = "http://APIInternal:1111/APIInternal/p"
config.URL.Internal = "http://APIInternal:1111/APIInternal/p"
config.URL.Git = "http://Git:1111/Git/p"
config.URL.GitContainer = "http://GitContainer:1111/GitContainer/p"
config.URL.Container = "http://GitContainer:1111/GitContainer/p"
config.URL.UI = "http://UI:1111/UI/p"
err := backfillURLs(config)
require.NoError(t, err)
require.Equal(t, "http://APIInternal:1111/APIInternal/p", config.URL.Internal)
require.Equal(t, "http://GitContainer:1111/GitContainer/p", config.URL.Container)
require.Equal(t, "http://API:1111/API/p", config.URL.API)
require.Equal(t, "http://Git:1111/Git/p", config.URL.Git)
require.Equal(t, "http://UI:1111/UI/p", config.URL.UI)
require.Equal(t, "http://APIInternal:1111/APIInternal/p", config.URL.APIInternal)
require.Equal(t, "http://GitContainer:1111/GitContainer/p", config.URL.GitContainer)
}

View File

@ -49,8 +49,12 @@ func (g Adapter) PathsDetails(ctx context.Context,
results := make([]types.PathDetails, len(paths))
for i, path := range paths {
// store original path in output in case caller is doing exact matching
results[i].Path = path
// use cleaned-up path for calculations to avoid not-founds.
path = cleanTreePath(path)
//nolint:nestif
if len(path) > 0 {
entry, err := tree.FindEntry(path)

View File

@ -76,7 +76,7 @@ func (s RepositoryService) GetTreeNode(ctx context.Context,
gitNode, err := s.adapter.GetTreeNode(ctx, repoPath, request.GitRef, request.Path)
if err != nil {
return nil, processGitErrorf(err, "no such path '%s' in '%s'", request.Path, request.GetGitRef())
return nil, processGitErrorf(err, "failed to find node '%s' in '%s'", request.Path, request.GitRef)
}
res := &rpc.GetTreeNodeResponse{
@ -95,7 +95,7 @@ func (s RepositoryService) GetTreeNode(ctx context.Context,
pathDetails, err := s.adapter.PathsDetails(ctx, repoPath, request.GitRef, []string{request.Path})
if err != nil {
return nil, err
return nil, processGitErrorf(err, "failed to get path details for '%s' in '%s'", request.Path, request.GitRef)
}
if len(pathDetails) != 1 || pathDetails[0].LastCommit == nil {

View File

@ -64,18 +64,16 @@ type Config struct {
// Value is derived from Base unless explicitly specified (e.g. http://localhost:3000).
UI string `envconfig:"GITNESS_URL_UI"`
// APIInternal defines the internal URL via which the rest API is reachable.
// NOTE: for routing to work properly, the request path reaching gitness has to end with `/api`
// (this could be after proxy path rewrite).
// Value is derived from HTTP.Server unless explicitly specified (e.g. http://localhost:3000/api).
APIInternal string `envconfig:"GITNESS_URL_API_INTERNAL"`
// Internal defines the internal URL via which the service is reachable.
// Value is derived from HTTP.Server unless explicitly specified (e.g. http://localhost:3000).
Internal string `envconfig:"GITNESS_URL_INTERNAL"`
// GitContainer is the endpoint that can be used by running container builds to communicate
// Container is the endpoint that can be used by running container builds to communicate
// with gitness (for example while performing a clone on a local repo).
// host.docker.internal allows a running container to talk to services exposed on the host
// (either running directly or via a port exposed in a docker container).
// Value is derived from HTTP.Server unless explicitly specified (e.g. http://host.docker.internal:3000/git).
GitContainer string `envconfig:"GITNESS_URL_GIT_CONTAINER"`
// Value is derived from HTTP.Server unless explicitly specified (e.g. http://host.docker.internal:3000).
Container string `envconfig:"GITNESS_URL_CONTAINER"`
}
// Git defines the git configuration parameters