From 52db334ee3f492aa1aaaccd768cd06b4645a0d1e Mon Sep 17 00:00:00 2001 From: Radhika Rao Date: Wed, 18 Mar 2026 15:49:41 +0530 Subject: [PATCH] added changes for 412 error handling --- commands/registry.go | 177 ++++++++++++++++++++++-------------- commands/registry_test.go | 2 +- do/mocks/RegistryService.go | 15 +++ do/registry.go | 15 +++ 4 files changed, 138 insertions(+), 71 deletions(-) diff --git a/commands/registry.go b/commands/registry.go index 79db9a8d2..48a34a966 100644 --- a/commands/registry.go +++ b/commands/registry.go @@ -412,6 +412,18 @@ func RunRegistryCreate(c *CmdConfig) error { func RunRegistryGet(c *CmdConfig) error { reg, err := c.Registry().Get() if err != nil { + if is412Error(err) { + // List available registries to show in the error message + registries, listErr := c.Registry().List() + if listErr == nil && len(registries) > 0 { + names := make([]string, len(registries)) + for i, r := range registries { + names[i] = r.Name + } + return fmt.Errorf("multiple registries found. Please use 'doctl registries get ' instead.\nAvailable registries: %s", strings.Join(names, ", ")) + } + return fmt.Errorf("multiple registries found. Please use 'doctl registries get ' instead") + } return err } @@ -645,19 +657,11 @@ func RunRegistryLogout(c *CmdConfig) error { // RunListRepositories lists repositories for the registry func RunListRepositories(c *CmdConfig) error { - registryName, err := c.Doit.GetString(c.NS, doctl.ArgRegistry) + registryName, err := getRegistryNameWithFallback(c) if err != nil { return err } - if registryName == "" { - registry, err := c.Registry().Get() - if err != nil { - return fmt.Errorf("failed to get registry: %w", err) - } - registryName = registry.Name - } - repositories, err := c.Registry().ListRepositories(registryName) if err != nil { return err @@ -668,19 +672,11 @@ func RunListRepositories(c *CmdConfig) error { // RunListRepositoriesV2 lists repositories for the registry func RunListRepositoriesV2(c *CmdConfig) error { - registryName, err := c.Doit.GetString(c.NS, doctl.ArgRegistry) + registryName, err := getRegistryNameWithFallback(c) if err != nil { return err } - if registryName == "" { - registry, err := c.Registry().Get() - if err != nil { - return fmt.Errorf("failed to get registry: %w", err) - } - registryName = registry.Name - } - repositories, err := c.Registry().ListRepositoriesV2(registryName) if err != nil { return err @@ -696,19 +692,11 @@ func RunListRepositoryTags(c *CmdConfig) error { return err } - registryName, err := c.Doit.GetString(c.NS, doctl.ArgRegistry) + registryName, err := getRegistryNameWithFallback(c) if err != nil { return err } - if registryName == "" { - registry, err := c.Registry().Get() - if err != nil { - return fmt.Errorf("failed to get registry: %w", err) - } - registryName = registry.Name - } - tags, err := c.Registry().ListRepositoryTags(registryName, c.Args[0]) if err != nil { return err @@ -724,19 +712,11 @@ func RunListRepositoryManifests(c *CmdConfig) error { return err } - registryName, err := c.Doit.GetString(c.NS, doctl.ArgRegistry) + registryName, err := getRegistryNameWithFallback(c) if err != nil { return err } - if registryName == "" { - registry, err := c.Registry().Get() - if err != nil { - return fmt.Errorf("failed to get registry: %w", err) - } - registryName = registry.Name - } - manifests, err := c.Registry().ListRepositoryManifests(registryName, c.Args[0]) if err != nil { return err @@ -756,19 +736,11 @@ func RunRepositoryDeleteTag(c *CmdConfig) error { return doctl.NewMissingArgsErr(c.NS) } - registryName, err := c.Doit.GetString(c.NS, doctl.ArgRegistry) + registryName, err := getRegistryNameWithFallback(c) if err != nil { return err } - if registryName == "" { - registry, err := c.Registry().Get() - if err != nil { - return fmt.Errorf("failed to get registry: %w", err) - } - registryName = registry.Name - } - repository := c.Args[0] tags := c.Args[1:] @@ -801,19 +773,11 @@ func RunRepositoryDeleteManifest(c *CmdConfig) error { return doctl.NewMissingArgsErr(c.NS) } - registryName, err := c.Doit.GetString(c.NS, doctl.ArgRegistry) + registryName, err := getRegistryNameWithFallback(c) if err != nil { return err } - if registryName == "" { - registry, err := c.Registry().Get() - if err != nil { - return fmt.Errorf("failed to get registry: %w", err) - } - registryName = registry.Name - } - repository := c.Args[0] digests := c.Args[1:] @@ -835,6 +799,82 @@ func RunRepositoryDeleteManifest(c *CmdConfig) error { return nil } +// is412Error checks if an error is a 412 (Precondition Failed) response, which indicates +// multiple registries exist and a specific registry name is required +func is412Error(err error) bool { + if err == nil { + return false + } + // Check if it's a godo ErrorResponse with status 412 + if errResp, ok := err.(*godo.ErrorResponse); ok { + return errResp.Response != nil && errResp.Response.StatusCode == 412 + } + return false +} + +// getRegistryNameWithFallback attempts to get a registry name from the flag first. +// If not provided, it falls back to using c.Registry().Get(), but provides a helpful +// error message if that fails due to multiple registries (412 error). +func getRegistryNameWithFallback(c *CmdConfig) (string, error) { + registryName, err := c.Doit.GetString(c.NS, doctl.ArgRegistry) + if err != nil { + return "", err + } + + if registryName != "" { + return registryName, nil + } + + // No registry name provided, try to get the default registry + registry, err := c.Registry().Get() + if err != nil { + if is412Error(err) { + // List available registries to show in the error message + registries, listErr := c.Registry().List() + if listErr == nil && len(registries) > 0 { + names := make([]string, len(registries)) + for i, r := range registries { + names[i] = r.Name + } + return "", fmt.Errorf("multiple registries found. Please specify one with --registry .\nAvailable registries: %s", strings.Join(names, ", ")) + } + return "", fmt.Errorf("multiple registries found. Please specify one with --registry ") + } + return "", fmt.Errorf("failed to get registry: %w", err) + } + + return registry.Name, nil +} + +// getRegistryNameFromArgs gets a registry name from command line args if provided, +// otherwise falls back to getting the default registry. This is used by commands +// that accept an optional positional registry argument. +func getRegistryNameFromArgs(c *CmdConfig, args []string, argIndex int) (string, error) { + if len(args) > argIndex && args[argIndex] != "" { + return args[argIndex], nil + } + + // No registry name in args, try to get the default registry + registry, err := c.Registry().Get() + if err != nil { + if is412Error(err) { + // List available registries to show in the error message + registries, listErr := c.Registry().List() + if listErr == nil && len(registries) > 0 { + names := make([]string, len(registries)) + for i, r := range registries { + names[i] = r.Name + } + return "", fmt.Errorf("multiple registries found. Please specify registry as an argument.\nAvailable registries: %s", strings.Join(names, ", ")) + } + return "", fmt.Errorf("multiple registries found. Please specify registry as an argument") + } + return "", fmt.Errorf("failed to get registry: %w", err) + } + + return registry.Name, nil +} + func displayRegistries(c *CmdConfig, registries ...do.Registry) error { item := &displayers.Registry{ Registries: registries, @@ -876,16 +916,15 @@ func displayRepositoryManifests(c *CmdConfig, manifests ...do.RepositoryManifest // registry. func RunStartGarbageCollection(c *CmdConfig) error { var registryName string + var err error // we anticipate supporting multiple registries in the future by allowing the // user to specify a registry argument on the command line but defaulting to // the default registry returned by c.Registry().Get() if len(c.Args) == 0 { - var err error - registry, err := c.Registry().Get() + registryName, err = getRegistryNameFromArgs(c, c.Args, 0) if err != nil { - return fmt.Errorf("failed to get registry: %w", err) + return err } - registryName = registry.Name } else if len(c.Args) == 1 { registryName = c.Args[0] } else { @@ -936,16 +975,15 @@ func RunStartGarbageCollection(c *CmdConfig) error { // garbage collection. func RunGetGarbageCollection(c *CmdConfig) error { var registryName string + var err error // we anticipate supporting multiple registries in the future by allowing the // user to specify a registry argument on the command line but defaulting to // the default registry returned by c.Registry().Get() if len(c.Args) == 0 { - var err error - registry, err := c.Registry().Get() + registryName, err = getRegistryNameFromArgs(c, c.Args, 0) if err != nil { - return fmt.Errorf("failed to get registry: %w", err) + return err } - registryName = registry.Name } else if len(c.Args) == 1 { registryName = c.Args[0] } else { @@ -964,16 +1002,15 @@ func RunGetGarbageCollection(c *CmdConfig) error { // garbage collection. func RunListGarbageCollections(c *CmdConfig) error { var registryName string + var err error // we anticipate supporting multiple registries in the future by allowing the // user to specify a registry argument on the command line but defaulting to // the default registry returned by c.Registry().Get() if len(c.Args) == 0 { - var err error - registry, err := c.Registry().Get() + registryName, err = getRegistryNameFromArgs(c, c.Args, 0) if err != nil { - return fmt.Errorf("failed to get registry: %w", err) + return err } - registryName = registry.Name } else if len(c.Args) == 1 { registryName = c.Args[0] } else { @@ -994,6 +1031,7 @@ func RunCancelGarbageCollection(c *CmdConfig) error { var ( registryName string gcUUID string + err error ) if len(c.Args) == 0 { @@ -1011,14 +1049,13 @@ func RunCancelGarbageCollection(c *CmdConfig) error { // user to specify a registry argument on the command line but defaulting to // the default registry returned by c.Registry().Get() if registryName == "" { - registry, err := c.Registry().Get() + registryName, err = getRegistryNameFromArgs(c, []string{}, 0) if err != nil { - return fmt.Errorf("failed to get registry: %w", err) + return err } - registryName = registry.Name } - _, err := c.Registry().CancelGarbageCollection(registryName, gcUUID) + _, err = c.Registry().CancelGarbageCollection(registryName, gcUUID) if err != nil { return err } diff --git a/commands/registry_test.go b/commands/registry_test.go index 4d9d0c761..95d56431d 100644 --- a/commands/registry_test.go +++ b/commands/registry_test.go @@ -129,7 +129,7 @@ var ( invalidGCUUID = "invalid-gc-uuid" testTime = time.Date(2020, 4, 1, 0, 0, 0, 0, time.UTC) testGarbageCollection = &do.GarbageCollection{ - &godo.GarbageCollection{ + GarbageCollection: &godo.GarbageCollection{ UUID: testGCUUID, RegistryName: testRegistryName, Status: testGCStatus, diff --git a/do/mocks/RegistryService.go b/do/mocks/RegistryService.go index f214112c5..0b58c3ee3 100644 --- a/do/mocks/RegistryService.go +++ b/do/mocks/RegistryService.go @@ -157,6 +157,21 @@ func (mr *MockRegistryServiceMockRecorder) Get() *gomock.Call { return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Get", reflect.TypeOf((*MockRegistryService)(nil).Get)) } +// List mocks base method. +func (m *MockRegistryService) List() ([]do.Registry, error) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "List") + ret0, _ := ret[0].([]do.Registry) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// List indicates an expected call of List. +func (mr *MockRegistryServiceMockRecorder) List() *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "List", reflect.TypeOf((*MockRegistryService)(nil).List)) +} + // GetAvailableRegions mocks base method. func (m *MockRegistryService) GetAvailableRegions() ([]string, error) { m.ctrl.T.Helper() diff --git a/do/registry.go b/do/registry.go index e1e83fe65..13270448e 100644 --- a/do/registry.go +++ b/do/registry.go @@ -70,6 +70,7 @@ func (r *Registry) Endpoint() string { // RegistryService is the godo RegistryService interface. type RegistryService interface { Get() (*Registry, error) + List() ([]Registry, error) Create(*godo.RegistryCreateRequest) (*Registry, error) Delete() error DockerCredentials(*godo.RegistryDockerCredentialsRequest) (*godo.DockerCredentials, error) @@ -113,6 +114,20 @@ func (rs *registryService) Get() (*Registry, error) { return &Registry{Registry: r}, nil } +func (rs *registryService) List() ([]Registry, error) { + registries, _, err := rs.client.Registries.List(rs.ctx) + if err != nil { + return nil, err + } + + list := make([]Registry, 0, len(registries)) + for _, r := range registries { + list = append(list, Registry{Registry: r}) + } + + return list, nil +} + func (rs *registryService) Create(cr *godo.RegistryCreateRequest) (*Registry, error) { r, _, err := rs.client.Registry.Create(rs.ctx, cr) if err != nil {