diff --git a/API.md b/API.md index 72c8949..00df429 100644 --- a/API.md +++ b/API.md @@ -345,8 +345,12 @@ Generated by `go run ./tools/api_inventory`. This is the public BucketGit Go SDK ## `github.com/bucketgit/bgit/broker/local` +- `const TargetLogicalAlias` +- `const TargetStorageExplicit` +- `const TargetStorageShorthand` - `func IsZeroOID` - `func New` +- `func ParseTarget` - `method Broker.ArchiveIssue` - `method Broker.AssignIssue` - `method Broker.Authorize` @@ -395,6 +399,7 @@ Generated by `go run ./tools/api_inventory`. This is the public BucketGit Go SDK - `method RepositoryStore.Read` - `method RepositoryStore.Write` - `method ResolverFunc.Resolve` +- `method Target.Provider` - `type Broker` - `type IdentityVerifier` - `type IdentityVerifierFunc` @@ -407,6 +412,8 @@ Generated by `go run ./tools/api_inventory`. This is the public BucketGit Go SDK - `type Resolver` - `type ResolverFunc` - `type StoryMove` +- `type Target` +- `type TargetKind` ## `github.com/bucketgit/bgit/transport` diff --git a/CHANGELOG.md b/CHANGELOG.md index 78c9a45..07d1bd1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,15 @@ All notable changes to `bgit` are documented in this file. This project follows semantic versioning. +## 1.4.1 + +Fixed + +- Native Git remote-helper sessions now resolve `file://`, `s3://`, and `gs://` + local-broker shorthand consistently with `bgit clone`, rehydrate existing + bucket-backed broker state without creating repositories, and resolve bare + logical aliases only from unambiguous checkout or `BGIT_HOME` state. + ## 1.4.0 Added diff --git a/README.md b/README.md index aa0bd0a..67c5ff5 100644 --- a/README.md +++ b/README.md @@ -137,14 +137,21 @@ BucketGit remote helper for `bgit://` and `bgit::` remotes: ```bash git clone bgit::https://broker.example.com/demo.git -git remote add origin bgit::demo.git +git clone bgit::gs://demo.git +git clone bgit::s3://demo.git +git clone bgit::file://demo.git +git remote add origin bgit://demo.git git fetch origin git push origin main ``` `bgit::https://broker.example.com/demo.git` carries the broker URL explicitly. -`bgit::demo.git` and `bgit://demo.git` resolve through the current checkout's -BucketGit broker configuration. +`bgit::gs://demo.git`, `bgit::s3://demo.git`, and `bgit::file://demo.git` use +the same local-broker shorthand as `bgit clone`, making them suitable for a +fresh checkout or stateless workload. They open existing broker state and do +not create a missing repository. `bgit://demo.git` is a logical alias resolved +from the current checkout or an unambiguous repository mapping in `BGIT_HOME`; +use an explicit storage or broker URL when no such mapping exists. ## Custom Domains diff --git a/SDK.md b/SDK.md index 1dce670..c2a5b8a 100644 --- a/SDK.md +++ b/SDK.md @@ -38,7 +38,8 @@ See [API.md](API.md) for the generated public surface, - `broker/capability`: broker-authorized object access through S3 STS, GCS signed URLs, or local capabilities, including supported legacy reads. - `broker/local`: in-process local broker persistence, scoped repository stores, - ref CAS, and issue/task-board services. + ref CAS, issue/task-board services, and provider-neutral classification of + logical, shorthand, and explicit storage targets. - `transport`: pkt-line framing, advertised capabilities, upload-pack, receive-pack, and Git remote-helper protocol handling. diff --git a/broker/local/target.go b/broker/local/target.go new file mode 100644 index 0000000..2dd0246 --- /dev/null +++ b/broker/local/target.go @@ -0,0 +1,103 @@ +package local + +import ( + "errors" + "net/url" + "strings" + + "github.com/bucketgit/bgit/protocol" +) + +// TargetKind identifies how a BucketGit repository address must be resolved. +type TargetKind string + +const ( + TargetLogicalAlias TargetKind = "logical-alias" + TargetStorageShorthand TargetKind = "storage-shorthand" + TargetStorageExplicit TargetKind = "storage-explicit" +) + +// Target is a provider-neutral classification of a local-broker repository +// address. Shorthand targets have no Prefix and require identity-based bucket +// resolution; explicit targets name a physical bucket and repository prefix. +type Target struct { + Kind TargetKind + Scheme string + Logical string + Bucket string + Prefix string + Original string +} + +// ParseTarget classifies logical aliases and file, S3, or GCS repository +// addresses without performing credential lookup or provisioning resources. +func ParseTarget(raw string) (Target, error) { + original := strings.TrimSpace(strings.TrimPrefix(strings.TrimSpace(raw), "bgit::")) + if original == "" { + return Target{}, errors.New("repository target is required") + } + if !strings.Contains(original, "://") { + logical, err := normalizeTargetLogical(original) + if err != nil { + return Target{}, err + } + return Target{Kind: TargetLogicalAlias, Logical: logical, Original: original}, nil + } + parsed, err := url.Parse(original) + if err != nil { + return Target{}, err + } + scheme := strings.ToLower(strings.TrimSpace(parsed.Scheme)) + if scheme == "gcs" { + scheme = "gs" + } + if scheme != "file" && scheme != "s3" && scheme != "gs" { + return Target{}, errors.New("repository target must use file://, s3://, or gs://") + } + name := strings.TrimSpace(parsed.Host) + if name == "" && scheme == "file" { + name = strings.Trim(strings.TrimSpace(parsed.Path), "/") + parsed.Path = "" + } + if name == "" { + return Target{}, errors.New("repository target must include a repository or bucket name") + } + prefix := strings.Trim(strings.TrimSpace(parsed.Path), "/") + if prefix == "" { + logical, err := normalizeTargetLogical(name) + if err != nil { + return Target{}, err + } + return Target{Kind: TargetStorageShorthand, Scheme: scheme, Logical: logical, Original: original}, nil + } + if scheme == "file" { + return Target{}, errors.New("file repository shorthand must not include a path") + } + parts := strings.Split(prefix, "/") + logical, err := normalizeTargetLogical(parts[len(parts)-1]) + if err != nil { + return Target{}, err + } + return Target{Kind: TargetStorageExplicit, Scheme: scheme, Logical: logical, Bucket: name, Prefix: prefix, Original: original}, nil +} + +func normalizeTargetLogical(name string) (string, error) { + name = strings.TrimSuffix(strings.TrimSpace(name), ".git") + if name == "" || name == "." || name == ".." || strings.ContainsAny(name, `/\\`) { + return "", errors.New("repository target must include a flat repository name") + } + return name + ".git", nil +} + +func (t Target) Provider() protocol.Provider { + switch t.Scheme { + case "file": + return protocol.ProviderFile + case "s3": + return protocol.ProviderS3 + case "gs": + return protocol.ProviderGCS + default: + return "" + } +} diff --git a/broker/local/target_test.go b/broker/local/target_test.go new file mode 100644 index 0000000..8319b81 --- /dev/null +++ b/broker/local/target_test.go @@ -0,0 +1,40 @@ +package local + +import "testing" + +func TestParseTarget(t *testing.T) { + tests := []struct { + name string + raw string + kind TargetKind + scheme string + bucket string + prefix string + }{ + {name: "logical", raw: "demo", kind: TargetLogicalAlias}, + {name: "gcs shorthand", raw: "bgit::gs://demo", kind: TargetStorageShorthand, scheme: "gs"}, + {name: "s3 shorthand", raw: "s3://demo.git", kind: TargetStorageShorthand, scheme: "s3"}, + {name: "file shorthand", raw: "file://demo", kind: TargetStorageShorthand, scheme: "file"}, + {name: "gcs explicit", raw: "gs://physical/repos/demo.git", kind: TargetStorageExplicit, scheme: "gs", bucket: "physical", prefix: "repos/demo.git"}, + {name: "s3 explicit", raw: "s3://physical/demo.git", kind: TargetStorageExplicit, scheme: "s3", bucket: "physical", prefix: "demo.git"}, + } + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + got, err := ParseTarget(test.raw) + if err != nil { + t.Fatal(err) + } + if got.Kind != test.kind || got.Scheme != test.scheme || got.Bucket != test.bucket || got.Prefix != test.prefix || got.Logical != "demo.git" { + t.Fatalf("target = %#v", got) + } + }) + } +} + +func TestParseTargetRejectsInvalidAddresses(t *testing.T) { + for _, raw := range []string{"", "https://example.com/demo.git", "gs://", "file://demo/path"} { + if _, err := ParseTarget(raw); err == nil { + t.Fatalf("ParseTarget(%q) succeeded", raw) + } + } +} diff --git a/internal/app/broker_commands.go b/internal/app/broker_commands.go index 835d835..07dcfb1 100644 --- a/internal/app/broker_commands.go +++ b/internal/app/broker_commands.go @@ -19,6 +19,7 @@ import ( "strings" "time" + localbroker "github.com/bucketgit/bgit/broker/local" internalconfig "github.com/bucketgit/bgit/internal/config" "github.com/bucketgit/bgit/protocol" "golang.org/x/crypto/ssh" @@ -2211,38 +2212,14 @@ func logicalRepoFromStorageTarget(target string) (string, error) { } func storageTargetParts(target string) (scheme, profile, region, bucket string, ok bool) { - raw := strings.TrimSpace(target) - switch { - case strings.HasPrefix(raw, "s3://"): - scheme = "s3" - raw = strings.TrimPrefix(raw, "s3://") - case strings.HasPrefix(raw, "gs://"): - scheme = "gs" - raw = strings.TrimPrefix(raw, "gs://") - case strings.HasPrefix(raw, "file://"): - scheme = "file" - raw = strings.TrimPrefix(raw, "file://") - default: + parsed, err := localbroker.ParseTarget(target) + if err != nil || (parsed.Kind != localbroker.TargetStorageShorthand && parsed.Kind != localbroker.TargetStorageExplicit) { return "", "", "", "", false } - if slash := strings.Index(raw, "/"); slash >= 0 { - if scheme == "s3" || scheme == "gs" { - return scheme, "", "", "", true - } - raw = raw[:slash] - } - raw = strings.TrimSuffix(raw, ".git") - parts := strings.Split(raw, ".") - labels := parts[:0] - for _, part := range parts { - if strings.TrimSpace(part) != "" { - labels = append(labels, strings.TrimSpace(part)) - } - } - if len(labels) == 0 { - return scheme, "", "", "", true + if parsed.Kind == localbroker.TargetStorageExplicit { + return parsed.Scheme, "", "", "", true } - return scheme, "", "", strings.Join(labels, "."), true + return parsed.Scheme, "", "", strings.TrimSuffix(parsed.Logical, ".git"), true } func storageProfileRegionFromOptions(target, selectedProfile, selectedRegion string) (string, string) { diff --git a/internal/app/remote_helper.go b/internal/app/remote_helper.go index 4c7d33e..bb444e5 100644 --- a/internal/app/remote_helper.go +++ b/internal/app/remote_helper.go @@ -6,18 +6,34 @@ import ( "fmt" "io" "net/url" + "os" "strings" + localbroker "github.com/bucketgit/bgit/broker/local" + internalconfig "github.com/bucketgit/bgit/internal/config" + "github.com/bucketgit/bgit/protocol" transportpkg "github.com/bucketgit/bgit/transport" ) +type remoteHelperSession struct { + config config + close func() +} + +func (s *remoteHelperSession) Close() { + if s != nil && s.close != nil { + s.close() + } +} + func remoteHelperCommand(args []string, stdin io.Reader, stdout, stderr io.Writer) error { resolver := transportpkg.ResolverFunc(func(ctx context.Context, address, service string, input io.Reader, output io.Writer) error { - cfg, err := configForRemoteHelperAddress(address) + session, err := resolveRemoteHelperSession(ctx, address) if err != nil { return err } - return serveGitServiceWithConfig(ctx, service, cfg, input, output) + defer session.Close() + return serveGitServiceWithConfig(ctx, service, session.config, input, output) }) return transportpkg.ServeRemoteHelper(context.Background(), resolver, args, stdin, stdout, stderr) } @@ -27,33 +43,112 @@ func remoteHelperAddress(args []string) string { } func configForRemoteHelperAddress(address string) (config, error) { + session, err := resolveRemoteHelperSession(context.Background(), address) + if err != nil { + return config{}, err + } + defer session.Close() + return session.config, nil +} + +func resolveRemoteHelperSession(ctx context.Context, address string) (*remoteHelperSession, error) { address = strings.TrimSpace(address) address = strings.TrimPrefix(address, "bgit::") if address == "" { - return config{}, errors.New("missing bgit remote helper URL") + return nil, errors.New("missing bgit remote helper URL") } if strings.HasPrefix(address, "bgit://") { parsed, err := url.Parse(address) if err != nil { - return config{}, err + return nil, err } repo := strings.Trim(strings.Trim(parsed.Host+"/"+strings.Trim(parsed.Path, "/"), "/"), "/") if repo == "" { - return config{}, errors.New("bgit remote helper URL must include a repository name") + return nil, errors.New("bgit remote helper URL must include a repository name") } - return configForRemoteHelperLogicalRepo(repo) + return resolveRemoteHelperLogicalSession(ctx, repo) } if strings.HasPrefix(address, "http://") || strings.HasPrefix(address, "https://") { - return configForRemoteHelperBrokerURL(address) + cfg, err := configForRemoteHelperBrokerURL(address) + return &remoteHelperSession{config: cfg}, err } - if strings.HasPrefix(address, "s3://") || strings.HasPrefix(address, "gs://") || strings.HasPrefix(address, "gcs://") { + target, err := localbroker.ParseTarget(address) + if err != nil { + return nil, err + } + switch target.Kind { + case localbroker.TargetStorageExplicit: cfg, _, err := parseRepoURI(address) if err != nil { - return config{}, err + return nil, err } - return mergeSSHRepoAuth(cfg), nil + return &remoteHelperSession{config: mergeSSHRepoAuth(cfg)}, nil + case localbroker.TargetStorageShorthand: + return resolveRemoteHelperStorageSession(ctx, target) + default: + return resolveRemoteHelperLogicalSession(ctx, target.Logical) + } +} + +func resolveRemoteHelperStorageSession(ctx context.Context, target localbroker.Target) (*remoteHelperSession, error) { + global, _, err := loadGlobalConfigForInit("") + if err != nil { + return nil, err + } + profile, region, managed, err := startDefaultLocalBroker(global) + if err != nil { + return nil, err + } + storageProfile, storageRegion := storageProfileRegionFromOptions(target.Original, "", "") + repo, err := localBrokerRepoForTarget(config{ + provider: "local", + brokerURL: managed.URL, + gcloudConfiguration: storageProfile, + region: storageRegion, + }, target.Original, coreTeamID) + if err != nil { + managed.Close() + return nil, err + } + server, err := localBrokerServerForURL(managed.URL) + if err != nil { + managed.Close() + return nil, err + } + state, err := server.loadRepoForRequest(repo) + if err != nil || strings.TrimSpace(state.Repo.Logical) == "" { + managed.Close() + return nil, fmt.Errorf("BucketGit repository %s was not found in existing %s storage; initialize it before using the remote helper: %w", target.Logical, target.Scheme, firstNonNil(err, os.ErrNotExist)) } - return configForRemoteHelperLogicalRepo(address) + cfg := configForRemoteHelperLocalRepository(state.Repo, profile.Name, region.Name, managed.URL) + return &remoteHelperSession{config: cfg, close: managed.Close}, nil +} + +func configForRemoteHelperLocalRepository(repo protocol.Repository, profile, region, brokerURL string) config { + logical := firstNonEmpty(strings.TrimSpace(repo.Logical), strings.TrimSpace(repo.Prefix)) + return mergeSSHRepoAuth(config{ + provider: "local", + bucket: strings.TrimSpace(repo.Bucket), + prefix: strings.Trim(strings.TrimSpace(repo.Prefix), "/"), + branch: defaultBranch, + origin: fmt.Sprintf("git@%s:%s", defaultSSHHost, logical), + brokerURL: brokerURL, + logicalRepo: logical, + teamID: firstNonEmpty(strings.TrimSpace(repo.TeamID), coreTeamID), + storageProvider: strings.TrimSpace(repo.Provider), + storageProfile: strings.TrimSpace(repo.Profile), + storageRegion: strings.TrimSpace(repo.Region), + gcloudConfiguration: "local:" + firstNonEmpty(profile, "default") + "/" + firstNonEmpty(region, "default"), + }) +} + +func firstNonNil(values ...error) error { + for _, value := range values { + if value != nil { + return value + } + } + return errors.New("repository state is missing") } func configForRemoteHelperBrokerURL(raw string) (config, error) { @@ -86,21 +181,89 @@ func configForRemoteHelperBrokerURL(raw string) (config, error) { } func configForRemoteHelperLogicalRepo(repo string) (config, error) { - logical, err := normalizeLogicalRepoName(repo) + session, err := resolveRemoteHelperLogicalSession(context.Background(), repo) if err != nil { return config{}, err } + defer session.Close() + return session.config, nil +} + +func resolveRemoteHelperLogicalSession(ctx context.Context, repo string) (*remoteHelperSession, error) { + logical, err := normalizeLogicalRepoName(repo) + if err != nil { + return nil, err + } if localCfg, err := readLocalConfig("."); err == nil && strings.TrimSpace(localCfg.brokerURL) != "" { localCfg.logicalRepo = logical localCfg.prefix = logical localCfg.origin = fmt.Sprintf("git@%s:%s", defaultSSHHost, logical) - return mergeSSHRepoAuth(localCfg), nil + managed, err := ensureLocalBrokerForCommand(ctx, &localCfg) + if err != nil { + return nil, err + } + return &remoteHelperSession{config: mergeSSHRepoAuth(localCfg), close: func() { managed.Close() }}, nil } - return mergeSSHRepoAuth(config{ - provider: "gcs", - logicalRepo: logical, - prefix: logical, - branch: defaultBranch, - origin: fmt.Sprintf("git@%s:%s", defaultSSHHost, logical), - }), nil + global, _, err := loadGlobalConfigForInit("") + if err != nil { + return nil, err + } + var candidates []*remoteHelperSession + for _, localProfile := range global.LocalProfiles { + regions := localProfile.Regions + if len(regions) == 0 { + regions = []internalconfig.ProfileRegion{{Name: firstNonEmpty(localProfile.Region, "default")}} + } + for _, localRegion := range regions { + managed, manageErr := ensureManagedLocalBroker(ctx, localProfile, localRegion) + if manageErr != nil { + return nil, manageErr + } + server, serverErr := localBrokerServerForURL(managed.URL) + if serverErr != nil { + managed.Close() + return nil, serverErr + } + indexed, ok := server.indexedRepo(logical) + if !ok { + managed.Close() + continue + } + state, stateErr := server.loadRepoForRequest(indexed) + if stateErr != nil { + managed.Close() + continue + } + candidates = append(candidates, &remoteHelperSession{ + config: configForRemoteHelperLocalRepository(state.Repo, localProfile.Name, localRegion.Name, managed.URL), + close: managed.Close, + }) + } + } + for _, known := range global.Repos { + knownLogical, normalizeErr := normalizeLogicalRepoName(known.Name) + if normalizeErr != nil || !strings.EqualFold(knownLogical, logical) || strings.TrimSpace(known.BrokerURL) == "" { + continue + } + cfg := mergeSSHRepoAuth(config{ + provider: "gcs", + brokerURL: strings.TrimRight(strings.TrimSpace(known.BrokerURL), "/"), + logicalRepo: logical, + prefix: logical, + branch: defaultBranch, + origin: fmt.Sprintf("git@%s:%s", defaultSSHHost, logical), + gcloudConfiguration: strings.TrimSpace(known.Profile), + }) + candidates = append(candidates, &remoteHelperSession{config: cfg}) + } + if len(candidates) == 1 { + return candidates[0], nil + } + for _, candidate := range candidates { + candidate.Close() + } + if len(candidates) > 1 { + return nil, fmt.Errorf("BucketGit repository %s is ambiguous in BGIT_HOME; use an explicit bgit::gs://, bgit::s3://, bgit::file://, or broker URL", logical) + } + return nil, fmt.Errorf("BucketGit repository %s has no checkout or BGIT_HOME mapping; use an explicit bgit::gs://, bgit::s3://, bgit::file://, or broker URL", logical) } diff --git a/internal/app/remote_helper_test.go b/internal/app/remote_helper_test.go index 27c88af..3172848 100644 --- a/internal/app/remote_helper_test.go +++ b/internal/app/remote_helper_test.go @@ -4,6 +4,9 @@ import ( "bytes" "strings" "testing" + + localbroker "github.com/bucketgit/bgit/broker/local" + "github.com/bucketgit/bgit/protocol" ) func TestRemoteHelperCapabilities(t *testing.T) { @@ -37,11 +40,44 @@ func TestRemoteHelperBrokerURLConfig(t *testing.T) { } func TestRemoteHelperLogicalURLConfig(t *testing.T) { - cfg, err := configForRemoteHelperAddress("bgit://demo.git") + t.Setenv("BGIT_HOME", t.TempDir()) + _, err := configForRemoteHelperAddress("bgit://demo.git") + if err == nil || !strings.Contains(err.Error(), "explicit bgit::gs://") { + t.Fatalf("error = %v", err) + } +} + +func TestRemoteHelperFileShorthandRehydratesExistingRepository(t *testing.T) { + t.Setenv("BGIT_HOME", t.TempDir()) + server, err := localBrokerServerForURL("local://default/default") if err != nil { t.Fatal(err) } - if cfg.logicalRepo != "demo.git" || cfg.prefix != "demo.git" { - t.Fatalf("cfg = %#v", cfg) + repo := protocol.Repository{Provider: "file", Bucket: "file://demo", Logical: "demo.git", TeamID: coreTeamID} + if err := server.saveRepo(localbroker.RepositoryState{Repo: repo}); err != nil { + t.Fatal(err) + } + session, err := resolveRemoteHelperSession(t.Context(), "bgit::file://demo") + if err != nil { + t.Fatal(err) + } + defer session.Close() + if session.config.provider != "local" || session.config.logicalRepo != "demo.git" || session.config.storageProvider != "file" { + t.Fatalf("config = %#v", session.config) + } +} + +func TestRemoteHelperFileShorthandDoesNotCreateMissingRepository(t *testing.T) { + t.Setenv("BGIT_HOME", t.TempDir()) + _, err := resolveRemoteHelperSession(t.Context(), "bgit::file://missing") + if err == nil || !strings.Contains(err.Error(), "was not found") { + t.Fatalf("error = %v", err) + } + server, serverErr := localBrokerServerForURL("local://default/default") + if serverErr != nil { + t.Fatal(serverErr) + } + if _, ok := server.indexedRepo("missing.git"); ok { + t.Fatal("missing read-only helper target was added to the repository index") } }