From a426cdd69b0b21d55ab8547e9b2dec1587e726a7 Mon Sep 17 00:00:00 2001 From: Johannes Batzill Date: Tue, 10 Jan 2023 14:35:09 -0800 Subject: [PATCH] [GIT] Add Support for Other Git Clients (#171) --- cli/server/harness.wire_gen.go | 2 +- cli/server/standalone.wire_gen.go | 2 +- internal/api/controller/repo/controller.go | 2 +- internal/githook/cli.go | 2 +- internal/githook/client.go | 2 +- internal/githook/env.go | 4 +- internal/router/router.go | 65 ++++++++++++++++++---- internal/router/wire.go | 4 +- internal/url/provider.go | 16 +++++- types/check/space.go | 2 +- types/config.go | 22 +++++++- 11 files changed, 98 insertions(+), 25 deletions(-) diff --git a/cli/server/harness.wire_gen.go b/cli/server/harness.wire_gen.go index a4ce71b44..6cedee83f 100644 --- a/cli/server/harness.wire_gen.go +++ b/cli/server/harness.wire_gen.go @@ -137,7 +137,7 @@ func initSystem(ctx context.Context, config *types.Config) (*system, error) { apiHandler := router.ProvideAPIHandler(config, authenticator, accountClient, controller, spaceController, repoController, pullreqController, webhookController, githookController) gitHandler := router.ProvideGitHandler(config, provider, repoStore, authenticator, authorizer, gitrpcInterface) webHandler := router2.ProvideWebHandler(config) - routerRouter := router2.ProvideRouter(apiHandler, gitHandler, webHandler) + routerRouter := router2.ProvideRouter(config, apiHandler, gitHandler, webHandler) serverServer := server.ProvideServer(config, routerRouter) serverConfig := ProvideGitRPCServerConfig(config) server3, err := server2.ProvideServer(serverConfig) diff --git a/cli/server/standalone.wire_gen.go b/cli/server/standalone.wire_gen.go index fb995023b..40ad16c5c 100644 --- a/cli/server/standalone.wire_gen.go +++ b/cli/server/standalone.wire_gen.go @@ -100,7 +100,7 @@ func initSystem(ctx context.Context, config *types.Config) (*system, error) { apiHandler := router.ProvideAPIHandler(config, authenticator, repoController, spaceController, pullreqController, webhookController, githookController, serviceaccountController, controller) gitHandler := router.ProvideGitHandler(config, provider, repoStore, authenticator, authorizer, gitrpcInterface) webHandler := router.ProvideWebHandler(config) - routerRouter := router.ProvideRouter(apiHandler, gitHandler, webHandler) + routerRouter := router.ProvideRouter(config, apiHandler, gitHandler, webHandler) serverServer := server.ProvideServer(config, routerRouter) serverConfig := ProvideGitRPCServerConfig(config) server3, err := server2.ProvideServer(serverConfig) diff --git a/internal/api/controller/repo/controller.go b/internal/api/controller/repo/controller.go index 5f9b8f63f..5c772e3ca 100644 --- a/internal/api/controller/repo/controller.go +++ b/internal/api/controller/repo/controller.go @@ -66,7 +66,7 @@ func CreateRPCWriteParams(ctx context.Context, urlProvider *url.Provider, // generate envars (add everything githook CLI needs for execution) envVars, err := githook.GenerateEnvironmentVariables(&githook.Payload{ - BaseURL: urlProvider.GetAPIBaseURLInternal(), + APIBaseURL: urlProvider.GetAPIBaseURLInternal(), RepoID: repo.ID, PrincipalID: session.Principal.ID, RequestID: requestID, diff --git a/internal/githook/cli.go b/internal/githook/cli.go index 987714351..a1caa3379 100644 --- a/internal/githook/cli.go +++ b/internal/githook/cli.go @@ -36,7 +36,7 @@ func NewCLI() (*CLI, error) { payload: payload, client: &client{ httpClient: http.DefaultClient, - baseURL: payload.BaseURL, + baseURL: payload.APIBaseURL, requestPreparation: func(r *http.Request) *http.Request { // TODO: reference single constant (together with gitness middleware) r.Header.Add("X-Request-Id", payload.RequestID) diff --git a/internal/githook/client.go b/internal/githook/client.go index eb92e847e..65cab408c 100644 --- a/internal/githook/client.go +++ b/internal/githook/client.go @@ -53,7 +53,7 @@ func (c *client) PostReceive(ctx context.Context, // githook executes the requested githook type using the provided input. func (c *client) githook(ctx context.Context, githookType string, in interface{}) (*githook.ServerHookOutput, error) { - uri := fmt.Sprintf("%s/api/v1/internal/git-hooks/%s", c.baseURL, githookType) + uri := fmt.Sprintf("%s/v1/internal/git-hooks/%s", c.baseURL, githookType) bodyBytes, err := json.Marshal(in) if err != nil { return nil, fmt.Errorf("failed to serialize input: %w", err) diff --git a/internal/githook/env.go b/internal/githook/env.go index 60f8f57b7..bc89d6d36 100644 --- a/internal/githook/env.go +++ b/internal/githook/env.go @@ -21,7 +21,7 @@ const ( // Payload defines the Payload the githook binary is initiated with when executing the git hooks. type Payload struct { - BaseURL string + APIBaseURL string RepoID int64 PrincipalID int64 RequestID string @@ -83,7 +83,7 @@ func validatePayload(payload *Payload) error { if payload == nil { return errors.New("payload is empty") } - if payload.BaseURL == "" { + if payload.APIBaseURL == "" { return errors.New("payload doesn't contain a base url") } if payload.PrincipalID <= 0 { diff --git a/internal/router/router.go b/internal/router/router.go index c2c39525b..4cabe89f1 100644 --- a/internal/router/router.go +++ b/internal/router/router.go @@ -19,14 +19,18 @@ import ( ) const ( - APIMount = "/api" - gitUserAgentPrefix = "git/" + APIMount = "/api" + GitMount = "/git" ) type Router struct { api APIHandler git GitHandler web WebHandler + + // gitHost describes the optional host via which git traffic is identified. + // Note: always stored as lowercase. + gitHost string } // NewRouter returns a new http.Handler that routes traffic @@ -35,11 +39,14 @@ func NewRouter( api APIHandler, git GitHandler, web WebHandler, + gitHost string, ) *Router { return &Router{ api: api, git: git, web: web, + + gitHost: strings.ToLower(gitHost), } } @@ -57,15 +64,19 @@ func (r *Router) ServeHTTP(w http.ResponseWriter, req *http.Request) { * 1. GIT * * All Git originating traffic starts with "/space1/space2/repo.git". - * This can collide with other API endpoints and thus has to be checked first. - * To avoid any false positives, we use the user-agent header to identify git agents. */ - ua := req.Header.Get("user-agent") - if strings.HasPrefix(ua, gitUserAgentPrefix) { + if r.isGitTraffic(req) { log.UpdateContext(func(c zerolog.Context) zerolog.Context { return c.Str("http.handler", "git") }) + // remove matched prefix to simplify API handlers (only if it's there) + if err = stripPrefix(GitMount, req); err != nil { + hlog.FromRequest(req).Err(err).Msgf("Failed striping of prefix for git request.") + render.InternalError(w) + return + } + r.git.ServeHTTP(w, req) return } @@ -75,8 +86,7 @@ func (r *Router) ServeHTTP(w http.ResponseWriter, req *http.Request) { * * All Rest API calls start with "/api/", and thus can be uniquely identified. */ - p := req.URL.Path - if strings.HasPrefix(p, APIMount) { + if r.isAPITraffic(req) { log.UpdateContext(func(c zerolog.Context) zerolog.Context { return c.Str("http.handler", "api") }) @@ -104,7 +114,40 @@ func (r *Router) ServeHTTP(w http.ResponseWriter, req *http.Request) { r.web.ServeHTTP(w, req) } -// stripPrefix removes the prefix from the request path (expected to be there). -func stripPrefix(prefix string, r *http.Request) error { - return request.ReplacePrefix(r, r.URL.Path[:len(prefix)], "") +// stripPrefix removes the prefix from the request path (or noop if it's not there). +func stripPrefix(prefix string, req *http.Request) error { + p := req.URL.Path + if !strings.HasPrefix(p, prefix) { + return nil + } + return request.ReplacePrefix(req, req.URL.Path[:len(prefix)], "") +} + +// isGitTraffic returns true iff the request is identified as part of the git http protocol. +func (r *Router) isGitTraffic(req *http.Request) bool { + // git traffic is always reachable via the git mounting path. + p := req.URL.Path + if strings.HasPrefix(p, GitMount) { + return true + } + + // otherwise check if the request came in via the configured git host (if enabled) + if len(r.gitHost) > 0 { + // cut (optional) port off the host + h, _, _ := strings.Cut(req.Host, ":") + + // check if request host matches the configured git host (case insensitive) + if r.gitHost == strings.ToLower(h) { + return true + } + } + + // otherwise we don't treat it as git traffic + return false +} + +// isAPITraffic returns true iff the request is identified as part of our rest API. +func (r *Router) isAPITraffic(req *http.Request) bool { + p := req.URL.Path + return strings.HasPrefix(p, APIMount) } diff --git a/internal/router/wire.go b/internal/router/wire.go index 2f149d378..8f7b17811 100644 --- a/internal/router/wire.go +++ b/internal/router/wire.go @@ -31,11 +31,13 @@ var WireSet = wire.NewSet( ) func ProvideRouter( + config *types.Config, api APIHandler, git GitHandler, web WebHandler, ) *Router { - return NewRouter(api, git, web) + return NewRouter(api, git, web, + config.Server.HTTP.GitHost) } func ProvideGitHandler( diff --git a/internal/url/provider.go b/internal/url/provider.go index b9858d4f1..a3a0e6289 100644 --- a/internal/url/provider.go +++ b/internal/url/provider.go @@ -14,9 +14,12 @@ import ( // Provider provides the URLs of the gitness system. type Provider struct { // apiURLRaw stores the raw URL the api endpoints are reachable at publicly. + // NOTE: url is guaranteed to not have any trailing '/'. apiURLRaw string - // apiURLInternalRaw stores the raw URL the api endpoints are reachable at internally. - // NOTE: no need for internal services to go via public route. + + // 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. @@ -25,6 +28,12 @@ type Provider struct { } func NewProvider(apiURLRaw string, apiURLInternalRaw, gitURLRaw string) (*Provider, error) { + // remove trailing '/' to make usage easier + apiURLRaw = strings.TrimRight(apiURLRaw, "/") + apiURLInternalRaw = strings.TrimRight(apiURLInternalRaw, "/") + gitURLRaw = strings.TrimRight(gitURLRaw, "/") + + // parse gitURL gitURL, err := url.Parse(gitURLRaw) if err != nil { return nil, fmt.Errorf("provided gitURLRaw '%s' is invalid: %w", gitURLRaw, err) @@ -38,16 +47,19 @@ func NewProvider(apiURLRaw string, apiURLInternalRaw, gitURLRaw string) (*Provid } // GetAPIBaseURL returns the publicly reachable base url of the api server. +// NOTE: url is guaranteed to not have any trailing '/'. func (p *Provider) GetAPIBaseURL() string { return p.apiURLRaw } // GetAPIBaseURLInternal returns the internally reachable base url of the api server. +// NOTE: url is guaranteed to not have any trailing '/'. func (p *Provider) GetAPIBaseURLInternal() string { return p.apiURLInternalRaw } // GenerateRepoCloneURL generates the public git clone URL for the provided repo path. +// NOTE: url is guaranteed to not have any trailing '/'. func (p *Provider) GenerateRepoCloneURL(repoPath string) string { repoPath = path.Clean(repoPath) if !strings.HasSuffix(repoPath, ".git") { diff --git a/types/check/space.go b/types/check/space.go index 172026701..4668ad55f 100644 --- a/types/check/space.go +++ b/types/check/space.go @@ -12,7 +12,7 @@ import ( ) var ( - illegalRootSpaceNames = []string{"api"} + illegalRootSpaceNames = []string{"api", "git"} ErrRootSpaceNameNotAllowed = &ValidationError{ fmt.Sprintf("The following names are not allowed for a root space: %v", illegalRootSpaceNames), diff --git a/types/config.go b/types/config.go index 730d4037b..e0cf5db5e 100644 --- a/types/config.go +++ b/types/config.go @@ -18,9 +18,23 @@ type Config struct { // URL defines the URLs via which the different parts of the service are reachable by. URL struct { - Git string `envconfig:"GITNESS_URL_GIT" default:"http://localhost:3000"` - API string `envconfig:"GITNESS_URL_API" default:"http://localhost:3000"` - APIInternal string `envconfig:"GITNESS_URL_API_INTERNAL" default:"http://localhost:3000"` + // Git defines the external URL via which the GIT API is reachable. + // NOTE: for routing to work properly, the request path & hostname reaching gitness + // have to statisfy at least one of the following two conditions: + // - Path ends with `/git` + // - Hostname matches Config.Server.HTTP.GitHost + // (this could be after proxy path / header rewrite). + Git string `envconfig:"GITNESS_URL_GIT" default:"http://localhost:3000/git"` + + // API defines the external 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). + API string `envconfig:"GITNESS_URL_API" default:"http://localhost:3000/api"` + + // 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). + APIInternal string `envconfig:"GITNESS_URL_API_INTERNAL" default:"http://localhost:3000/api"` } // Git defines the git configuration parameters @@ -38,6 +52,8 @@ type Config struct { Bind string `envconfig:"GITNESS_HTTP_BIND" default:":3000"` Proto string `envconfig:"GITNESS_HTTP_PROTO"` Host string `envconfig:"GITNESS_HTTP_HOST"` + // GitHost is the host used to identify git traffic (OPTIONAL). + GitHost string `envconfig:"GITNESS_HTTP_GIT_HOST" default:"git.localhost"` } // GRPC defines the grpc configuration parameters