From 5ca48e7f59a990af2bdfbeb980499616abeef5eb Mon Sep 17 00:00:00 2001 From: Johannes Batzill Date: Mon, 25 Sep 2023 22:34:45 +0000 Subject: [PATCH] [MISC] Replace `BIND` with `PORT`, fix space_path for postgres (#607) --- cli/server/config.go | 11 +---------- cli/server/config_test.go | 20 +++----------------- cli/server/server.go | 2 +- gitrpc/server/config.go | 20 ++++++++++---------- gitrpc/server/http.go | 2 +- gitrpc/server/server.go | 6 +++--- http/server.go | 9 ++++++--- internal/server/wire.go | 2 +- internal/store/database/space_path.go | 12 ++++++++---- types/config.go | 2 +- 10 files changed, 35 insertions(+), 51 deletions(-) diff --git a/cli/server/config.go b/cli/server/config.go index 748b7ae6c..b1b6feff1 100644 --- a/cli/server/config.go +++ b/cli/server/config.go @@ -61,16 +61,7 @@ func LoadConfig() (*types.Config, error) { func backfillURLs(config *types.Config) error { // default base url - scheme, host, port, path := "http", "localhost", "3000", "" - - // override default with whatever server bind provided - bindHost, bindPort, _ := strings.Cut(config.Server.HTTP.Bind, ":") - if bindHost != "" { - host = bindHost - } - if bindPort != "" { - port = bindPort - } + scheme, host, port, path := "http", "localhost", fmt.Sprint(config.Server.HTTP.Port), "" // TODO: handle https of server bind diff --git a/cli/server/config_test.go b/cli/server/config_test.go index 64b0dd980..8fbf499a7 100644 --- a/cli/server/config_test.go +++ b/cli/server/config_test.go @@ -23,7 +23,7 @@ import ( func TestBackfilURLsPortBind(t *testing.T) { config := &types.Config{} - config.Server.HTTP.Bind = ":1234" + config.Server.HTTP.Port = 1234 err := backfillURLs(config) require.NoError(t, err) @@ -35,23 +35,9 @@ func TestBackfilURLsPortBind(t *testing.T) { require.Equal(t, "http://host.docker.internal:1234/git", config.URL.GitContainer) } -func TestBackfilURLsHostBind(t *testing.T) { - config := &types.Config{} - config.Server.HTTP.Bind = "abc:1234" - - err := backfillURLs(config) - require.NoError(t, err) - - require.Equal(t, "http://abc:1234/api", config.URL.API) - require.Equal(t, "http://abc:1234/git", config.URL.Git) - require.Equal(t, "http://abc: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) { config := &types.Config{} - config.Server.HTTP.Bind = "abc:1234" + config.Server.HTTP.Port = 1234 config.URL.Base = "https://xyz:4321/test" err := backfillURLs(config) @@ -66,7 +52,7 @@ func TestBackfilURLsBase(t *testing.T) { func TestBackfilURLsCustom(t *testing.T) { config := &types.Config{} - config.Server.HTTP.Bind = "abc:1234" + 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" diff --git a/cli/server/server.go b/cli/server/server.go index 659b9a07b..4b4d00ead 100644 --- a/cli/server/server.go +++ b/cli/server/server.go @@ -119,7 +119,7 @@ func (c *command) run(*kingpin.ParseContext) error { } log.Info(). - Str("port", config.Server.HTTP.Bind). + Int("port", config.Server.HTTP.Port). Str("revision", version.GitCommit). Str("repository", version.GitRepository). Stringer("version", version.Version). diff --git a/gitrpc/server/config.go b/gitrpc/server/config.go index 64e7a9dd8..52fcb9b95 100644 --- a/gitrpc/server/config.go +++ b/gitrpc/server/config.go @@ -27,8 +27,8 @@ const ( // Config represents the configuration for the gitrpc server. type Config struct { - // Bind specifies the addr used to bind the grpc server. - Bind string `envconfig:"GITRPC_SERVER_BIND" default:":3001"` + // Port specifies the port used to bind the grpc server. + Port int `envconfig:"GITRPC_SERVER_PORT" default:"3001"` // GitRoot specifies the directory containing git related data (e.g. repos, ...) GitRoot string `envconfig:"GITRPC_SERVER_GIT_ROOT"` // TmpDir (optional) specifies the directory for temporary data (e.g. repo clones, ...) @@ -37,7 +37,7 @@ type Config struct { GitHookPath string `envconfig:"GITRPC_SERVER_GIT_HOOK_PATH"` HTTP struct { - Bind string `envconfig:"GITRPC_SERVER_HTTP_BIND" default:":4001"` + Port int `envconfig:"GITRPC_SERVER_HTTP_PORT" default:"4001"` } MaxConnAge time.Duration `envconfig:"GITRPC_SERVER_MAX_CONN_AGE" default:"630720000s"` MaxConnAgeGrace time.Duration `envconfig:"GITRPC_SERVER_MAX_CONN_AGE_GRACE" default:"630720000s"` @@ -66,23 +66,23 @@ func (c *Config) Validate() error { if c == nil { return errors.New("config is required") } - if c.Bind == "" { - return errors.New("config.Bind is required") + if c.Port < 0 { + return errors.New("Port is required") } if c.GitRoot == "" { - return errors.New("config.GitRoot is required") + return errors.New("GitRoot is required") } if c.GitHookPath == "" { - return errors.New("config.GitHookPath is required") + return errors.New("GitHookPath is required") } if c.MaxConnAge == 0 { - return errors.New("config.MaxConnAge is required") + return errors.New("MaxConnAge is required") } if c.MaxConnAgeGrace == 0 { - return errors.New("config.MaxConnAgeGrace is required") + return errors.New("MaxConnAgeGrace is required") } if m := c.LastCommitCache.Mode; m != "" && m != ModeInMemory && m != ModeRedis && m != ModeNone { - return errors.New("config.LastCommitCache.Mode has unsupported value") + return errors.New("LastCommitCache.Mode has unsupported value") } return nil diff --git a/gitrpc/server/http.go b/gitrpc/server/http.go index 8010be547..256758817 100644 --- a/gitrpc/server/http.go +++ b/gitrpc/server/http.go @@ -57,7 +57,7 @@ func NewHTTPServer(config Config) (*HTTPServer, error) { return &HTTPServer{ gitnesshttp.NewServer( gitnesshttp.Config{ - Addr: config.HTTP.Bind, + Port: config.HTTP.Port, }, handleHTTP(reposRoot), ), diff --git a/gitrpc/server/server.go b/gitrpc/server/server.go index 0025075d6..02caf4b9e 100644 --- a/gitrpc/server/server.go +++ b/gitrpc/server/server.go @@ -39,7 +39,7 @@ const ( type GRPCServer struct { *grpc.Server - Bind string + Port int } func NewServer(config Config, adapter service.GitAdapter) (*GRPCServer, error) { @@ -124,12 +124,12 @@ func NewServer(config Config, adapter service.GitAdapter) (*GRPCServer, error) { return &GRPCServer{ Server: s, - Bind: config.Bind, + Port: config.Port, }, nil } func (s *GRPCServer) Start() error { - lis, err := net.Listen("tcp", s.Bind) + lis, err := net.Listen("tcp", fmt.Sprintf(":%d", s.Port)) if err != nil { return err } diff --git a/http/server.go b/http/server.go index 294336ffd..27e057fde 100644 --- a/http/server.go +++ b/http/server.go @@ -17,7 +17,9 @@ package http import ( "context" "crypto/tls" + "fmt" "net/http" + "strings" "time" "golang.org/x/crypto/acme/autocert" @@ -33,7 +35,7 @@ const ( // TODO: expose via options? type Config struct { Acme bool - Addr string + Port int Cert string Key string AcmeHost string @@ -74,7 +76,7 @@ func (s *Server) ListenAndServe() (*errgroup.Group, ShutdownFunction) { func (s *Server) listenAndServe() (*errgroup.Group, ShutdownFunction) { var g errgroup.Group s1 := &http.Server{ - Addr: s.config.Addr, + Addr: fmt.Sprintf(":%d", s.config.Port), ReadHeaderTimeout: s.config.ReadHeaderTimeout, Handler: s.handler, } @@ -161,6 +163,7 @@ func (s Server) listenAndServeAcme() (*errgroup.Group, ShutdownFunction) { } func redirect(w http.ResponseWriter, req *http.Request) { - target := "https://" + req.Host + req.URL.Path + // TODO: in case of reverse-proxy the host might be not the external host. + target := "https://" + req.Host + "/" + strings.TrimPrefix(req.URL.Path, "/") http.Redirect(w, req, target, http.StatusTemporaryRedirect) } diff --git a/internal/server/wire.go b/internal/server/wire.go index 70a3955cd..7fc5feb93 100644 --- a/internal/server/wire.go +++ b/internal/server/wire.go @@ -30,7 +30,7 @@ func ProvideServer(config *types.Config, router *router.Router) *Server { return &Server{ http.NewServer( http.Config{ - Addr: config.Server.HTTP.Bind, + Port: config.Server.HTTP.Port, Acme: config.Server.Acme.Enabled, AcmeHost: config.Server.Acme.Host, }, diff --git a/internal/store/database/space_path.go b/internal/store/database/space_path.go index 8f1272c90..c9fbbe687 100644 --- a/internal/store/database/space_path.go +++ b/internal/store/database/space_path.go @@ -143,8 +143,8 @@ func (s *SpacePathStore) FindPrimaryBySpaceID(ctx context.Context, spaceID int64 }, nil } func (s *SpacePathStore) FindByPath(ctx context.Context, path string) (*types.SpacePath, error) { - const sqlQueryParent = spacePathSelectBase + ` WHERE space_path_uid_unique = $1 AND space_path_parent_id = $2` const sqlQueryNoParent = spacePathSelectBase + ` WHERE space_path_uid_unique = $1 AND space_path_parent_id IS NULL` + const sqlQueryParent = spacePathSelectBase + ` WHERE space_path_uid_unique = $1 AND space_path_parent_id = $2` db := dbtx.GetAccessor(ctx, s.db) segment := new(spacePathSegment) @@ -154,13 +154,18 @@ func (s *SpacePathStore) FindByPath(ctx context.Context, path string) (*types.Sp return nil, fmt.Errorf("path with no segments was passed '%s'", path) } + var err error var parentID int64 - sqlquery := sqlQueryNoParent originalPath := "" isPrimary := true for i, segmentUID := range segmentUIDs { uniqueSegmentUID := s.spacePathTransformation(segmentUID, i == 0) - err := db.GetContext(ctx, segment, sqlquery, uniqueSegmentUID, parentID) + + if parentID == 0 { + err = db.GetContext(ctx, segment, sqlQueryNoParent, uniqueSegmentUID) + } else { + err = db.GetContext(ctx, segment, sqlQueryParent, uniqueSegmentUID, parentID) + } if err != nil { return nil, database.ProcessSQLErrorf(err, "Failed to find segment for '%s' in '%s'", uniqueSegmentUID, path) } @@ -168,7 +173,6 @@ func (s *SpacePathStore) FindByPath(ctx context.Context, path string) (*types.Sp originalPath = paths.Concatinate(originalPath, segment.UID) parentID = segment.SpaceID isPrimary = isPrimary && segment.IsPrimary.ValueOrZero() - sqlquery = sqlQueryParent } return &types.SpacePath{ diff --git a/types/config.go b/types/config.go index d2e066082..499bedce6 100644 --- a/types/config.go +++ b/types/config.go @@ -93,7 +93,7 @@ type Config struct { Server struct { // HTTP defines the http configuration parameters HTTP struct { - Bind string `envconfig:"GITNESS_HTTP_BIND" default:":3000"` + Port int `envconfig:"GITNESS_HTTP_PORT" default:"3000"` Proto string `envconfig:"GITNESS_HTTP_PROTO" default:"http"` }