fix(retention): prevent failures in tag retention rule deletion flow

cmd/harbor/root/tag/retention/delete.go

@@ -22,6 +22,11 @@ import (
 	"github.com/spf13/cobra"
 )
 
+var getProjectNameFromUser = prompt.GetProjectNameFromUser
+var getRetentionId = api.GetRetentionId
+var getRetentionTagRule = prompt.GetRetentionTagRule
+var deleteRetention = api.DeleteRetention
+
 func DeleteRetentionRuleCommand() *cobra.Command {
 	var projectName string
 	var projectID int
@@ -49,7 +54,7 @@ Examples:
 			}
 
 			if projectID == -1 && projectName == "" {
-				name, err := prompt.GetProjectNameFromUser()
+				name, err := getProjectNameFromUser()
 				if err != nil {
 					return err
 				}
@@ -66,7 +71,7 @@ Examples:
 				projectIDStr = projectName
 			}
 
-			retentionID, err := api.GetRetentionId(projectIDStr, isName)
+			retentionID, err := getRetentionId(projectIDStr, isName)
 			if err != nil {
 				if err.Error() == "No retention policy exists for this project" {
 					fmt.Println("No retention policy exists for this project")
@@ -75,8 +80,15 @@ Examples:
 					return fmt.Errorf("error retrieving retention policy ID: %w", err)
 				}
 			}
-			retentionIndex := prompt.GetRetentionTagRule(retentionID)
-			err = api.DeleteRetention(projectName, int(retentionIndex))
+			retentionIndex, err := getRetentionTagRule(retentionID)
+			if err != nil {
+				return err
+			}
+			if retentionIndex < 0 {
+				fmt.Println("No retention rules found")
+				return nil
+			}
+			err = deleteRetention(retentionID, int(retentionIndex))
 			if err != nil {
 				return fmt.Errorf("%w", err)
 			}

cmd/harbor/root/tag/retention/delete_test.go

@@ -0,0 +1,162 @@
+// Copyright Project Harbor Authors
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+package retention
+
+import (
+	"bytes"
+	"errors"
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+)
+
+func TestDeleteRetentionRuleCommand(t *testing.T) {
+	cmd := DeleteRetentionRuleCommand()
+
+	assert.Equal(t, "delete", cmd.Use)
+	assert.Equal(t, "Delete a tag retention rule for a project", cmd.Short)
+	assert.NotNil(t, cmd.RunE)
+}
+
+func TestDeleteRetentionRuleCommand_Errors(t *testing.T) {
+	// Backup and restore package-level variables
+	orgGetProjectName := getProjectNameFromUser
+	orgGetRetentionId := getRetentionId
+	orgGetRetentionTagRule := getRetentionTagRule
+	orgDeleteRetention := deleteRetention
+
+	t.Cleanup(func() {
+		getProjectNameFromUser = orgGetProjectName
+		getRetentionId = orgGetRetentionId
+		getRetentionTagRule = orgGetRetentionTagRule
+		deleteRetention = orgDeleteRetention
+	})
+
+	t.Run("No retention policy exists - success no-op", func(t *testing.T) {
+		getRetentionId = func(projectNameorID string, isName bool) (string, error) {
+			return "", errors.New("No retention policy exists for this project")
+		}
+
+		cmd := DeleteRetentionRuleCommand()
+		var buf bytes.Buffer
+		cmd.SetOut(&buf)
+		cmd.SetErr(&buf)
+		cmd.SetArgs([]string{"--project-name", "test-project"})
+
+		err := cmd.Execute()
+		assert.NoError(t, err)
+	})
+
+	t.Run("GetRetentionId error - returns error", func(t *testing.T) {
+		getRetentionId = func(projectNameorID string, isName bool) (string, error) {
+			return "", errors.New("some network error")
+		}
+
+		cmd := DeleteRetentionRuleCommand()
+		var buf bytes.Buffer
+		cmd.SetOut(&buf)
+		cmd.SetErr(&buf)
+		cmd.SetArgs([]string{"--project-name", "test-project"})
+
+		err := cmd.Execute()
+		assert.Error(t, err)
+		assert.Contains(t, err.Error(), "error retrieving retention policy ID: some network error")
+	})
+
+	t.Run("GetRetentionTagRule error - returns error", func(t *testing.T) {
+		getRetentionId = func(projectNameorID string, isName bool) (string, error) {
+			return "42", nil
+		}
+		getRetentionTagRule = func(retentionID string) (int64, error) {
+			return 0, errors.New("selection aborted")
+		}
+
+		cmd := DeleteRetentionRuleCommand()
+		var buf bytes.Buffer
+		cmd.SetOut(&buf)
+		cmd.SetErr(&buf)
+		cmd.SetArgs([]string{"--project-name", "test-project"})
+
+		err := cmd.Execute()
+		assert.Error(t, err)
+		assert.Equal(t, "selection aborted", err.Error())
+	})
+
+	t.Run("No retention rules found - success no-op", func(t *testing.T) {
+		getRetentionId = func(projectNameorID string, isName bool) (string, error) {
+			return "42", nil
+		}
+		getRetentionTagRule = func(retentionID string) (int64, error) {
+			return -1, nil
+		}
+
+		cmd := DeleteRetentionRuleCommand()
+		var buf bytes.Buffer
+		cmd.SetOut(&buf)
+		cmd.SetErr(&buf)
+		cmd.SetArgs([]string{"--project-name", "test-project"})
+
+		err := cmd.Execute()
+		assert.NoError(t, err)
+	})
+
+	t.Run("DeleteRetention error - returns error", func(t *testing.T) {
+		getRetentionId = func(projectNameorID string, isName bool) (string, error) {
+			return "42", nil
+		}
+		getRetentionTagRule = func(retentionID string) (int64, error) {
+			return 0, nil
+		}
+		deleteRetention = func(retentionID string, ruleIndex int) error {
+			return errors.New("delete failed")
+		}
+
+		cmd := DeleteRetentionRuleCommand()
+		var buf bytes.Buffer
+		cmd.SetOut(&buf)
+		cmd.SetErr(&buf)
+		cmd.SetArgs([]string{"--project-name", "test-project"})
+
+		err := cmd.Execute()
+		assert.Error(t, err)
+		assert.Contains(t, err.Error(), "delete failed")
+	})
+
+	t.Run("Delete retention rule - success", func(t *testing.T) {
+		getRetentionId = func(projectNameorID string, isName bool) (string, error) {
+			return "42", nil
+		}
+		getRetentionTagRule = func(retentionID string) (int64, error) {
+			return 0, nil
+		}
+		var calledRetentionID string
+		var calledIndex int
+		deleteRetention = func(retentionID string, ruleIndex int) error {
+			calledRetentionID = retentionID
+			calledIndex = ruleIndex
+			return nil
+		}
+
+		cmd := DeleteRetentionRuleCommand()
+		var buf bytes.Buffer
+		cmd.SetOut(&buf)
+		cmd.SetErr(&buf)
+		cmd.SetArgs([]string{"--project-name", "test-project"})
+
+		err := cmd.Execute()
+		assert.NoError(t, err)
+		assert.Equal(t, "42", calledRetentionID)
+		assert.Equal(t, 0, calledIndex)
+	})
+}

pkg/api/retention_handler.go

@@ -121,28 +121,29 @@ func GetRetentionId(projectNameorID string, isName bool) (string, error) {
 	return retentionid, nil
 }
 
-func DeleteRetention(projectName string, ruleIndex int) error {
+func DeleteRetention(retentionID string, ruleIndex int) error {
 	ctx, client, err := utils.ContextWithClient()
 	if err != nil {
 		return err
 	}
 
-	retentionIDStr, err := GetRetentionId(projectName, true)
+	retentionResp, err := ListRetention(retentionID)
 	if err != nil {
 		return err
 	}
 
-	retentionResp, err := ListRetention(retentionIDStr)
-	if err != nil {
-		return err
+	existingPolicy := retentionResp.Payload
+	if existingPolicy == nil {
+		return fmt.Errorf("retention policy payload is empty for retention ID %s", retentionID)
 	}
 
-	existingPolicy := retentionResp.Payload
 	if ruleIndex < 0 || ruleIndex >= len(existingPolicy.Rules) {
 		return fmt.Errorf("invalid rule index")
 	}
 
-	existingPolicy.Rules = append(existingPolicy.Rules[:ruleIndex], existingPolicy.Rules[ruleIndex+1:]...)
+	copy(existingPolicy.Rules[ruleIndex:], existingPolicy.Rules[ruleIndex+1:])
+	existingPolicy.Rules[len(existingPolicy.Rules)-1] = nil
+	existingPolicy.Rules = existingPolicy.Rules[:len(existingPolicy.Rules)-1]
 
 	_, err = client.Retention.UpdateRetention(ctx, &retention.UpdateRetentionParams{
 		ID:     int64(retentionResp.Payload.ID),

pkg/prompt/prompt.go

@@ -309,7 +309,7 @@ func GetQuotaIDFromUser() int64 {
 	QuotaID := make(chan int64)
 
 	go func() {
-		response, err := api.ListQuota(*&api.ListQuotaFlags{})
+		response, err := api.ListQuota(api.ListQuotaFlags{})
 		if err != nil {
 			log.Errorf("failed to list quota: %v", err)
 		}
@@ -462,13 +462,15 @@ func GetRoleIDFromUser() int64 {
 	return <-roleID
 }
 
-func GetRetentionTagRule(retentionID string) int64 {
-	retentionIndex := make(chan int64)
-	go func() {
-		response, _ := api.ListRetention(retentionID)
-		retview.RetentionList(response.Payload.Rules, retentionIndex)
-	}()
-	return <-retentionIndex
+func GetRetentionTagRule(retentionID string) (int64, error) {
+	response, err := api.ListRetention(retentionID)
+	if err != nil {
+		return 0, fmt.Errorf("failed to list retention rules for retention policy %s: %w", retentionID, err)
+	}
+	if response.Payload == nil || len(response.Payload.Rules) == 0 {
+		return -1, nil
+	}
+	return retview.RetentionList(response.Payload.Rules)
 }
 
 func GetPreheatPolicyNameFromUser(projectName string) (string, error) {

pkg/views/retention/select/view.go

@@ -14,8 +14,8 @@
 package retention
 
 import (
+	"errors"
 	"fmt"
-	"os"
 	"strings"
 
 	"github.com/charmbracelet/bubbles/list"
@@ -24,7 +24,9 @@ import (
 	"github.com/goharbor/harbor-cli/pkg/views/base/selection"
 )
 
-func RetentionList(rules []*models.RetentionRule, choice chan<- int64) {
+var ErrUserAborted = errors.New("user aborted selection")
+
+func RetentionList(rules []*models.RetentionRule) (int64, error) {
 	itemsList := make([]list.Item, len(rules))
 	items := map[string]int64{}
 
@@ -55,21 +57,25 @@ func RetentionList(rules []*models.RetentionRule, choice chan<- int64) {
 			rule.Template,
 		)
 
-		items[display] = rule.ID
+		items[display] = int64(i)
 		itemsList[i] = selection.Item(display)
 	}
 
-	m := selection.NewModel(itemsList, "Select a Retention Rule")
+	m := selection.NewModel(itemsList, "Retention Rule")
 
 	p, err := tea.NewProgram(m, tea.WithAltScreen()).Run()
 	if err != nil {
-		fmt.Println("Error running selection UI:", err)
-		os.Exit(1)
+		return 0, fmt.Errorf("error running selection UI: %w", err)
 	}
 
-	if p, ok := p.(selection.Model); ok {
-		choice <- items[p.Choice]
+	if model, ok := p.(selection.Model); ok {
+		if model.Choice == "" {
+			return 0, ErrUserAborted
+		}
+		return items[model.Choice], nil
 	}
+
+	return 0, errors.New("unexpected program result")
 }
 
 func formatParams(params map[string]interface{}) string {