From 62fb28e0dfdc1c6e950e5970972584046b285a63 Mon Sep 17 00:00:00 2001 From: Brian Walsh Date: Fri, 13 Feb 2026 08:08:50 -0800 Subject: [PATCH 1/3] feature/drs-remote-remove #210 --- README.md | 2 + cmd/remote/remote_test.go | 52 +++++++++++++++++++ cmd/remote/remove.go | 47 +++++++++++++++++ cmd/remote/root.go | 1 + config/config.go | 70 +++++++++++++++++++++++++ config/config_test.go | 92 +++++++++++++++++++++++++++++++++ docs/commands.md | 23 +++++++++ docs/remote-remove-use-cases.md | 92 +++++++++++++++++++++++++++++++++ 8 files changed, 379 insertions(+) create mode 100644 cmd/remote/remove.go create mode 100644 docs/remote-remove-use-cases.md diff --git a/README.md b/README.md index 71457644..d1d5bfa8 100644 --- a/README.md +++ b/README.md @@ -75,6 +75,7 @@ For detailed setup and usage information: - **[Getting Started](docs/getting-started.md)** - Repository setup and basic workflows - **[Commands Reference](docs/commands.md)** - Complete command documentation +- **[Remote Remove Use Cases](docs/remote-remove-use-cases.md)** - Practical scenarios for remote cleanup - **[Installation Guide](docs/installation.md)** - Platform-specific installation - **[Troubleshooting](docs/troubleshooting.md)** - Common issues and solutions - **[S3 Integration](docs/adding-s3-files.md)** - Adding files via S3 URLs @@ -98,6 +99,7 @@ For detailed setup and usage information: | `git drs remote add` | Add a DRS remote server | | `git drs remote list` | List configured remotes | | `git drs remote set` | Set default remote | +| `git drs remote remove`| Remove a configured remote | | `git drs add-url` | Add files via S3 URLs | | `git lfs track` | Track file patterns with LFS | | `git lfs ls-files` | List tracked files | diff --git a/cmd/remote/remote_test.go b/cmd/remote/remote_test.go index b03b3121..0ab16ba1 100644 --- a/cmd/remote/remote_test.go +++ b/cmd/remote/remote_test.go @@ -3,8 +3,11 @@ package remote import ( "testing" + "github.com/calypr/git-drs/client/indexd" + "github.com/calypr/git-drs/config" "github.com/calypr/git-drs/internal/testutils" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" ) func TestRemoteListArgs(t *testing.T) { @@ -44,3 +47,52 @@ func TestRemoteSetArgs(t *testing.T) { err = SetCmd.Args(SetCmd, []string{"origin", "extra"}) assert.Error(t, err) } + +func TestRemoteRemoveArgs(t *testing.T) { + err := RemoveCmd.Args(RemoveCmd, []string{"origin"}) + assert.NoError(t, err) + + err = RemoveCmd.Args(RemoveCmd, []string{}) + assert.Error(t, err) + + err = RemoveCmd.Args(RemoveCmd, []string{"origin", "extra"}) + assert.Error(t, err) +} + +func TestRemoteRemoveAliases(t *testing.T) { + assert.Contains(t, RemoveCmd.Aliases, "rm") +} + +func TestRemoteRemoveRun(t *testing.T) { + tmpDir := testutils.SetupTestGitRepo(t) + testutils.CreateTestConfig(t, tmpDir, &config.Config{ + DefaultRemote: config.Remote("origin"), + Remotes: map[config.Remote]config.RemoteSelect{ + "origin": { + Gen3: &indexd.Gen3Remote{Endpoint: "https://one.example", ProjectID: "proj-a", Bucket: "bucket-a"}, + }, + "staging": { + Gen3: &indexd.Gen3Remote{Endpoint: "https://two.example", ProjectID: "proj-b", Bucket: "bucket-b"}, + }, + }, + }) + + err := RemoveCmd.RunE(RemoveCmd, []string{"origin"}) + require.NoError(t, err) + + cfg, err := config.LoadConfig() + require.NoError(t, err) + + _, hasOrigin := cfg.Remotes[config.Remote("origin")] + assert.False(t, hasOrigin) + assert.Equal(t, config.Remote("staging"), cfg.DefaultRemote) +} + +func TestRemoteRemoveRunNotFound(t *testing.T) { + tmpDir := testutils.SetupTestGitRepo(t) + testutils.CreateDefaultTestConfig(t, tmpDir) + + err := RemoveCmd.RunE(RemoveCmd, []string{"missing"}) + require.Error(t, err) + assert.Contains(t, err.Error(), "remote 'missing' not found") +} diff --git a/cmd/remote/remove.go b/cmd/remote/remove.go new file mode 100644 index 00000000..462d9cd0 --- /dev/null +++ b/cmd/remote/remove.go @@ -0,0 +1,47 @@ +package remote + +import ( + "fmt" + "sort" + + "github.com/calypr/git-drs/config" + "github.com/spf13/cobra" +) + +var RemoveCmd = &cobra.Command{ + Use: "remove ", + Aliases: []string{"rm"}, + Short: "Remove a configured DRS remote", + Long: "Remove a configured DRS remote and update the default remote if needed", + Args: func(cmd *cobra.Command, args []string) error { + if len(args) != 1 { + cmd.SilenceUsage = false + return fmt.Errorf("error: requires exactly 1 argument (remote name), received %d\n\nUsage: %s\n\nRun 'git drs remote list' to see available remotes or 'git drs remote rm --help' for more details", len(args), cmd.UseLine()) + } + return nil + }, + RunE: func(cmd *cobra.Command, args []string) error { + remoteName := args[0] + + cfg, err := config.LoadConfig() + if err != nil { + return fmt.Errorf("failed to load config: %w", err) + } + + remote := config.Remote(remoteName) + if _, ok := cfg.Remotes[remote]; !ok { + availableRemotes := make([]string, 0, len(cfg.Remotes)) + for name := range cfg.Remotes { + availableRemotes = append(availableRemotes, string(name)) + } + sort.Strings(availableRemotes) + return fmt.Errorf("remote '%s' not found.\nAvailable remotes: %v", remoteName, availableRemotes) + } + + if err := config.RemoveRemote(remote); err != nil { + return fmt.Errorf("failed to remove remote: %w", err) + } + + return nil + }, +} diff --git a/cmd/remote/root.go b/cmd/remote/root.go index 7d865720..e5ed4dc9 100644 --- a/cmd/remote/root.go +++ b/cmd/remote/root.go @@ -15,4 +15,5 @@ func init() { Cmd.AddCommand(add.Cmd) Cmd.AddCommand(ListCmd) Cmd.AddCommand(SetCmd) + Cmd.AddCommand(RemoveCmd) } diff --git a/config/config.go b/config/config.go index 3dfb354c..643b1868 100644 --- a/config/config.go +++ b/config/config.go @@ -5,6 +5,7 @@ import ( "log" "log/slog" "path/filepath" + "sort" "strings" "github.com/calypr/data-client/g3client" @@ -301,6 +302,75 @@ func GetProjectId(remote Remote) (string, error) { return rmt.GetProjectId(), nil } +// RemoveRemote removes a configured remote and updates default-remote when required +func RemoveRemote(name Remote) error { + repo, err := getRepo() + if err != nil { + return err + } + + conf, err := repo.Config() + if err != nil { + return err + } + + section := conf.Raw.Section(newConfigSection) + legacySection := conf.Raw.Section(legacyConfigSection) + root := section.Subsection(newConfigSubsectionRoot) + + remoteSubsectionName := fmt.Sprintf("%s.%s%s", newConfigSubsectionRoot, remoteSubsectionPrefix, name) + legacyRemoteSubsectionName := fmt.Sprintf("%s%s", remoteSubsectionPrefix, name) + + hasNamespaced := section.HasSubsection(remoteSubsectionName) + hasLegacy := legacySection.HasSubsection(legacyRemoteSubsectionName) + if !hasNamespaced && !hasLegacy { + return fmt.Errorf("remote '%s' not found", name) + } + + if hasNamespaced { + section.RemoveSubsection(remoteSubsectionName) + } + if hasLegacy { + legacySection.RemoveSubsection(legacyRemoteSubsectionName) + } + + defaultRemote := root.Option("default-remote") + if defaultRemote == "" { + defaultRemote = legacySection.Option("default-remote") + } + + if defaultRemote == string(name) { + remainingSet := make(map[string]struct{}) + for _, subsection := range section.Subsections { + if !strings.HasPrefix(subsection.Name, newConfigSubsectionRoot+"."+remoteSubsectionPrefix) { + continue + } + rest := strings.TrimPrefix(subsection.Name, newConfigSubsectionRoot+".") + remainingSet[strings.TrimPrefix(rest, remoteSubsectionPrefix)] = struct{}{} + } + for _, subsection := range legacySection.Subsections { + if !strings.HasPrefix(subsection.Name, remoteSubsectionPrefix) { + continue + } + remainingSet[strings.TrimPrefix(subsection.Name, remoteSubsectionPrefix)] = struct{}{} + } + + remaining := make([]string, 0, len(remainingSet)) + for remoteName := range remainingSet { + remaining = append(remaining, remoteName) + } + sort.Strings(remaining) + + root.RemoveOption("default-remote") + legacySection.RemoveOption("default-remote") + if len(remaining) > 0 { + root.SetOption("default-remote", remaining[0]) + } + } + + return repo.Storer.SetConfig(conf) +} + // SaveConfig writes the configuration using go-git func SaveConfig(cfg *Config) error { repo, err := getRepo() diff --git a/config/config_test.go b/config/config_test.go index 4e8d7c2d..76aa9071 100644 --- a/config/config_test.go +++ b/config/config_test.go @@ -266,3 +266,95 @@ func TestLoadConfig_NamespacedKeysTakePrecedence(t *testing.T) { t.Fatalf("expected namespaced gen3 remote loaded, got %#v", newRemote) } } + +func TestRemoveRemote(t *testing.T) { + setupTestRepo(t) + + _, err := UpdateRemote(Remote("origin"), RemoteSelect{Gen3: &indexd.Gen3Remote{Endpoint: "https://origin.example", ProjectID: "origin-proj", Bucket: "origin-bucket"}}) + if err != nil { + t.Fatalf("UpdateRemote origin error: %v", err) + } + _, err = UpdateRemote(Remote("staging"), RemoteSelect{Gen3: &indexd.Gen3Remote{Endpoint: "https://staging.example", ProjectID: "staging-proj", Bucket: "staging-bucket"}}) + if err != nil { + t.Fatalf("UpdateRemote staging error: %v", err) + } + + if err := RemoveRemote(Remote("origin")); err != nil { + t.Fatalf("RemoveRemote error: %v", err) + } + + cfg, err := LoadConfig() + if err != nil { + t.Fatalf("LoadConfig error: %v", err) + } + + if _, ok := cfg.Remotes[Remote("origin")]; ok { + t.Fatalf("expected origin to be removed") + } + if cfg.DefaultRemote != Remote("staging") { + t.Fatalf("expected default remote to switch to staging, got %s", cfg.DefaultRemote) + } +} + +func TestRemoveRemote_LastRemoteClearsDefault(t *testing.T) { + setupTestRepo(t) + + _, err := UpdateRemote(Remote("origin"), RemoteSelect{Gen3: &indexd.Gen3Remote{Endpoint: "https://origin.example", ProjectID: "origin-proj", Bucket: "origin-bucket"}}) + if err != nil { + t.Fatalf("UpdateRemote origin error: %v", err) + } + + if err := RemoveRemote(Remote("origin")); err != nil { + t.Fatalf("RemoveRemote error: %v", err) + } + + cfg, err := LoadConfig() + if err != nil { + t.Fatalf("LoadConfig error: %v", err) + } + if cfg.DefaultRemote != "" { + t.Fatalf("expected default remote to be cleared, got %s", cfg.DefaultRemote) + } + if len(cfg.Remotes) != 0 { + t.Fatalf("expected no remotes, got %d", len(cfg.Remotes)) + } +} + +func TestRemoveRemote_LegacyRemote(t *testing.T) { + tmpDir := setupTestRepo(t) + + commands := [][]string{ + {"config", "drs.default-remote", "legacy"}, + {"config", "drs.remote.legacy.type", "gen3"}, + {"config", "drs.remote.legacy.endpoint", "https://legacy.example"}, + {"config", "drs.remote.legacy.project", "legacy-proj"}, + {"config", "drs.remote.legacy.bucket", "legacy-bucket"}, + {"config", "lfs.customtransfer.drs.remote.new.type", "gen3"}, + {"config", "lfs.customtransfer.drs.remote.new.endpoint", "https://new.example"}, + {"config", "lfs.customtransfer.drs.remote.new.project", "new-proj"}, + {"config", "lfs.customtransfer.drs.remote.new.bucket", "new-bucket"}, + } + for _, args := range commands { + cmd := exec.Command("git", args...) + cmd.Dir = tmpDir + if out, err := cmd.CombinedOutput(); err != nil { + t.Fatalf("git %v failed: %v: %s", args, err, string(out)) + } + } + + if err := RemoveRemote(Remote("legacy")); err != nil { + t.Fatalf("RemoveRemote legacy error: %v", err) + } + + cfg, err := LoadConfig() + if err != nil { + t.Fatalf("LoadConfig error: %v", err) + } + + if _, ok := cfg.Remotes[Remote("legacy")]; ok { + t.Fatalf("expected legacy remote to be removed") + } + if cfg.DefaultRemote != Remote("new") { + t.Fatalf("expected default remote to be reassigned to new, got %s", cfg.DefaultRemote) + } +} diff --git a/docs/commands.md b/docs/commands.md index 8912c3ef..3613b082 100644 --- a/docs/commands.md +++ b/docs/commands.md @@ -157,6 +157,29 @@ git drs remote set production git drs remote list ``` + +#### `git drs remote remove ` / `git drs remote rm ` + +Remove a configured DRS remote. If you remove the current default remote, Git DRS automatically selects another configured remote as the new default. + +**Usage:** + +```bash +git drs remote remove +# alias +git drs remote rm +``` + +**Examples:** + +```bash +# Remove an old staging remote +git drs remote remove staging + +# Confirm remaining remotes and default +git drs remote list +``` + ### `git drs fetch [remote-name]` Fetch DRS object metadata from remote server. Downloads metadata only, not actual files. diff --git a/docs/remote-remove-use-cases.md b/docs/remote-remove-use-cases.md new file mode 100644 index 00000000..54ed7a45 --- /dev/null +++ b/docs/remote-remove-use-cases.md @@ -0,0 +1,92 @@ +# `git drs remote remove` Use Cases + +This guide provides practical scenarios for removing Git DRS remotes safely. + +## Adds missing `remove` verb for Git DRS remotes + + +To remove **Git DRS** remotes under `lfs.customtransfer.drs.remote.*` in `.git/config`. + +Use `git drs remove`. For example, to remove a remote named `foobar`: + +```bash +# view a remote named 'foobar' that is no longer needed +git drs remote list +* primary gen3 https://example.com + foobar gen3 https://example.com +git drs remote rm foobar +# or: git drs remote remove foobar +git drs remote list +* primary gen3 https://example.com +``` + +Expected outcome: +- `foobar` is removed from `git drs remote list`. +- Related `lfs.customtransfer.drs.remote.foobar.*` entries are removed from config. +- If `foobar` was the default, another remote becomes default automatically (or default is cleared if none remain). + +## When to use `git drs remote remove` + +Use this command when a remote configuration is no longer valid or no longer needed. + +```bash +git drs remote remove +``` + +## Use Case 1: Retire a staging environment + +A temporary staging DRS server is decommissioned after release. + +```bash +git drs remote list +git drs remote remove staging +git drs remote list +``` + +Expected outcome: +- `staging` no longer appears in `git drs remote list`. +- Remaining remotes continue to work. + +## Use Case 2: Remove and replace a misconfigured remote + +A remote was created with the wrong endpoint, bucket, or project. + +```bash +git drs remote remove production +git drs remote add gen3 production \ + --url https://correct.example.org \ + --cred /path/to/credentials.json \ + --project correct-project \ + --bucket correct-bucket +``` + +Expected outcome: +- Old configuration is removed. +- New remote can be added under the same name. + +## Use Case 3: Remove current default remote + +If you remove the default remote, Git DRS automatically assigns another existing remote as default. + +```bash +git drs remote list +git drs remote remove origin +git drs remote list +``` + +Expected outcome: +- A remaining remote is marked with `*` as the new default. +- If no remotes remain, there is no default remote. + +## Use Case 4: Clean up local test remotes + +During development, test remotes can accumulate and should be deleted after validation. + +```bash +git drs remote remove test-local +git drs remote remove test-ci +``` + +Expected outcome: +- Configuration is simplified. +- Team members avoid accidentally targeting stale remotes. From d79b9799ef59c8c21d7fee7d23648420893cb6e1 Mon Sep 17 00:00:00 2001 From: Brian Walsh Date: Fri, 13 Feb 2026 08:14:07 -0800 Subject: [PATCH 2/3] feature/drs-remote-remove #210 --- coverage/combined.html | 366 +- coverage/combined.out | 253 +- coverage/integration/coverage.out | 216 +- coverage/unit/coverage.out | 7712 ++++++++++++++++------------- 4 files changed, 4758 insertions(+), 3789 deletions(-) diff --git a/coverage/combined.html b/coverage/combined.html index 7c5d2e7a..d510b541 100644 --- a/coverage/combined.html +++ b/coverage/combined.html @@ -115,59 +115,61 @@ - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + + + @@ -4476,6 +4478,55 @@ + + -