Remove extra API call from kafka acl list (#2040)

internal/cmd/kafka/command_acl.go

@@ -6,8 +6,6 @@ import (
 	"github.com/hashicorp/go-multierror"
 	"github.com/spf13/cobra"
 
-	ccloudv1 "github.com/confluentinc/ccloud-sdk-go-v1-public"
-
 	"github.com/confluentinc/cli/internal/pkg/ccloudv2"
 	"github.com/confluentinc/cli/internal/pkg/ccstructs"
 	pcmd "github.com/confluentinc/cli/internal/pkg/cmd"
@@ -102,28 +100,6 @@ func convertToFilter(binding *ccstructs.ACLBinding) *ccstructs.ACLFilter {
 	}
 }
 
-func (c *aclCommand) getAllUsers() ([]*ccloudv1.User, error) {
-	serviceAccounts, err := c.Client.User.GetServiceAccounts()
-	if err != nil {
-		return nil, err
-	}
-
-	users, err := c.Client.User.List()
-	if err != nil {
-		return nil, err
-	}
-
-	return append(serviceAccounts, users...), nil
-}
-
-func mapNumericIdToResourceId(users []*ccloudv1.User) map[int32]string {
-	numericIdToResourceId := make(map[int32]string)
-	for _, user := range users {
-		numericIdToResourceId[user.Id] = user.ResourceId
-	}
-	return numericIdToResourceId
-}
-
 func (c *aclCommand) provisioningClusterCheck(lkc string) error {
 	environmentId, err := c.Context.EnvironmentId()
 	if err != nil {

internal/cmd/kafka/command_acl_create.go

@@ -78,22 +78,15 @@ func (c *aclCommand) create(cmd *cobra.Command, _ []string) error {
 		return err
 	}
 
-	users, err := c.getAllUsers()
-	if err != nil {
-		return err
-	}
-
-	numericIdToResourceId := mapNumericIdToResourceId(users)
-
 	for i, binding := range bindings {
 		data := pacl.GetCreateAclRequestData(binding)
 		if httpResp, err := kafkaREST.CloudClient.CreateKafkaAcls(kafkaClusterConfig.ID, data); err != nil {
 			if i > 0 {
-				_ = pacl.PrintACLsWithResourceIdMap(cmd, bindings[:i], numericIdToResourceId)
+				_ = pacl.PrintACLs(cmd, bindings[:i])
 			}
 			return kafkarest.NewError(kafkaREST.CloudClient.GetUrl(), err, httpResp)
 		}
 	}
 
-	return pacl.PrintACLsWithResourceIdMap(cmd, bindings, numericIdToResourceId)
+	return pacl.PrintACLs(cmd, bindings)
 }

internal/cmd/kafka/command_acl_create_onprem.go

@@ -77,5 +77,5 @@ func (c *aclCommand) createOnPrem(cmd *cobra.Command, _ []string) error {
 	}
 
 	aclData := aclutil.CreateAclRequestDataToAclData(acl)
-	return aclutil.PrintACLsFromKafkaRestResponse(cmd, []kafkarestv3.AclData{aclData})
+	return aclutil.PrintACLsFromKafkaRestResponseOnPrem(cmd, []kafkarestv3.AclData{aclData})
 }

internal/cmd/kafka/command_acl_delete_onprem.go

@@ -83,5 +83,5 @@ func (c *aclCommand) deleteOnPrem(cmd *cobra.Command, _ []string) error {
 		return kafkarest.NewError(restClient.GetConfig().BasePath, err, httpResp)
 	}
 
-	return aclutil.PrintACLsFromKafkaRestResponse(cmd, aclDeleteResp.Data)
+	return aclutil.PrintACLsFromKafkaRestResponseOnPrem(cmd, aclDeleteResp.Data)
 }

internal/cmd/kafka/command_acl_list.go

@@ -37,11 +37,6 @@ func (c *aclCommand) list(cmd *cobra.Command, _ []string) error {
 		return err
 	}
 
-	users, err := c.getAllUsers()
-	if err != nil {
-		return err
-	}
-
 	if acl[0].errors != nil {
 		return acl[0].errors
 	}
@@ -65,5 +60,5 @@ func (c *aclCommand) list(cmd *cobra.Command, _ []string) error {
 		return kafkarest.NewError(kafkaREST.CloudClient.GetUrl(), err, httpResp)
 	}
 
-	return aclutil.PrintACLsFromKafkaRestResponseWithResourceIdMap(cmd, aclDataList.Data, mapNumericIdToResourceId(users))
+	return aclutil.PrintACLsFromKafkaRestResponse(cmd, aclDataList.Data)
 }

internal/cmd/kafka/command_acl_list_onprem.go

@@ -61,5 +61,5 @@ func (c *aclCommand) listOnPrem(cmd *cobra.Command, _ []string) error {
 		return kafkarest.NewError(restClient.GetConfig().BasePath, err, httpResp)
 	}
 
-	return aclutil.PrintACLsFromKafkaRestResponse(cmd, aclGetResp.Data)
+	return aclutil.PrintACLsFromKafkaRestResponseOnPrem(cmd, aclGetResp.Data)
 }

internal/cmd/kafka/kafka_api.go

@@ -33,6 +33,13 @@ func parse(cmd *cobra.Command) ([]*ACLConfiguration, error) {
 	if cmd.Name() == "list" {
 		aclConfig := NewACLConfig()
 		cmd.Flags().Visit(fromArgs(aclConfig))
+
+		if aclConfig.Entry.Principal == "" {
+			aclConfig.Entry.Principal = "UserV2:*"
+		} else {
+			aclConfig.Entry.Principal = strings.Replace(aclConfig.Entry.Principal, "User", "UserV2", 1)
+		}
+
 		return []*ACLConfiguration{aclConfig}, nil
 	}
 

internal/pkg/acl/acl.go

@@ -3,7 +3,6 @@ package acl
 import (
 	"fmt"
 	"sort"
-	"strconv"
 	"strings"
 
 	"github.com/antihax/optional"
@@ -18,8 +17,6 @@ import (
 	"github.com/confluentinc/cli/internal/pkg/ccstructs"
 	"github.com/confluentinc/cli/internal/pkg/errors"
 	"github.com/confluentinc/cli/internal/pkg/output"
-	"github.com/confluentinc/cli/internal/pkg/resource"
-	"github.com/confluentinc/cli/internal/pkg/types"
 	"github.com/confluentinc/cli/internal/pkg/utils"
 )
 
@@ -60,7 +57,7 @@ type AclRequestDataWithError struct {
 	Errors       error
 }
 
-func PrintACLsFromKafkaRestResponse(cmd *cobra.Command, acls []cpkafkarestv3.AclData) error {
+func PrintACLsFromKafkaRestResponseOnPrem(cmd *cobra.Command, acls []cpkafkarestv3.AclData) error {
 	list := output.NewList(cmd)
 	for _, acl := range acls {
 		list.Add(&out{
@@ -317,88 +314,22 @@ func CreateAclRequestDataToAclData(data *AclRequestDataWithError) cpkafkarestv3.
 	}
 }
 
-func PrintACLsFromKafkaRestResponseWithResourceIdMap(cmd *cobra.Command, acls []cckafkarestv3.AclData, numericIdToResourceId map[int32]string) error {
+func PrintACLsFromKafkaRestResponse(cmd *cobra.Command, acls []cckafkarestv3.AclData) error {
 	list := output.NewList(cmd)
 	for _, acl := range acls {
-		prefix, resourceId, err := getPrefixAndResourceIdFromPrincipal(acl.Principal, numericIdToResourceId)
-		if err != nil {
-			if err.Error() == errors.UserIdNotValidErrorMsg {
-				continue // skip the entry if not a valid user id
-			}
-			return err
-		}
-		list.Add(&out{
-			Principal:    prefix + ":" + resourceId,
-			Permission:   acl.Permission,
-			Operation:    acl.Operation,
-			ResourceType: string(acl.ResourceType),
-			ResourceName: acl.ResourceName,
-			PatternType:  acl.PatternType,
-		})
-	}
-	list.Filter(listFields)
-	return list.Print()
-}
-
-func PrintACLsWithResourceIdMap(cmd *cobra.Command, acls []*ccstructs.ACLBinding, numericIdToResourceId map[int32]string) error {
-	list := output.NewList(cmd)
-	for _, acl := range acls {
-		prefix, resourceId, err := getPrefixAndResourceIdFromPrincipal(acl.Entry.Principal, numericIdToResourceId)
-		if err != nil {
-			if err.Error() == errors.UserIdNotValidErrorMsg {
-				continue // skip the entry if not a valid user id
-			}
-			return err
-		}
 		list.Add(&out{
-			Principal:    prefix + ":" + resourceId,
-			Permission:   acl.Entry.PermissionType.String(),
-			Operation:    acl.Entry.Operation.String(),
-			ResourceType: acl.Pattern.ResourceType.String(),
-			ResourceName: acl.Pattern.Name,
-			PatternType:  acl.Pattern.PatternType.String(),
+			Principal:    acl.GetPrincipal(),
+			Permission:   acl.GetPermission(),
+			Operation:    acl.GetOperation(),
+			ResourceType: string(acl.GetResourceType()),
+			ResourceName: acl.GetResourceName(),
+			PatternType:  acl.GetPatternType(),
 		})
 	}
 	list.Filter(listFields)
 	return list.Print()
 }
 
-func getPrefixAndResourceIdFromPrincipal(principal string, numericIdToResourceId map[int32]string) (string, string, error) {
-	if principal == "" {
-		return "", "", nil
-	}
-
-	x := strings.Split(principal, ":")
-	if len(x) < 2 {
-		return "", "", errors.Errorf("unrecognized principal format %s", principal)
-	}
-	prefix := x[0]
-	suffix := x[1]
-
-	// The principal has a resource ID
-	resources := []string{
-		resource.IdentityPool,
-		resource.ServiceAccount,
-		resource.User,
-	}
-	if types.Contains(resources, resource.LookupType(suffix)) {
-		return prefix, suffix, nil
-	}
-
-	// The principal has a numeric ID
-	id, err := strconv.ParseInt(suffix, 10, 32)
-	if err != nil {
-		return "", "", errors.New(errors.UserIdNotValidErrorMsg)
-	}
-
-	resourceId, ok := numericIdToResourceId[int32(id)]
-	if !ok {
-		return "", "", errors.New(errors.UserIdNotValidErrorMsg)
-	}
-
-	return prefix, resourceId, nil
-}
-
 func GetCreateAclRequestData(binding *ccstructs.ACLBinding) cckafkarestv3.CreateAclRequestData {
 	data := cckafkarestv3.CreateAclRequestData{
 		Host:         binding.GetEntry().GetHost(),

internal/pkg/acl/acl_test.go

@@ -113,39 +113,6 @@ func TestValidateCreateDeleteAclRequestData(t *testing.T) {
 	}
 }
 
-func TestGetPrefixAndResourceIdFromPrincipal_Empty(t *testing.T) {
-	prefix, resourceId, err := getPrefixAndResourceIdFromPrincipal("", nil)
-	require.NoError(t, err)
-	require.Equal(t, "", prefix)
-	require.Equal(t, "", resourceId)
-}
-
-func TestGetPrefixAndResourceIdFromPrincipal_UnrecognizedFormat(t *testing.T) {
-	_, _, err := getPrefixAndResourceIdFromPrincipal("string with no colon", nil)
-	require.Error(t, err)
-}
-
-func TestGetPrefixAndResourceIdFromPrincipal_ResourceId(t *testing.T) {
-	prefix, resourceId, err := getPrefixAndResourceIdFromPrincipal("User:sa-123456", nil)
-	require.NoError(t, err)
-	require.Equal(t, "User", prefix)
-	require.Equal(t, "sa-123456", resourceId)
-}
-
-func TestGetPrefixAndResourceIdFromPrincipal_NumericId(t *testing.T) {
-	prefix, resourceId, err := getPrefixAndResourceIdFromPrincipal("User:123456", map[int32]string{123456: "sa-123456"})
-	require.NoError(t, err)
-	require.Equal(t, "User", prefix)
-	require.Equal(t, "sa-123456", resourceId)
-}
-
-func TestGetPrefixAndResourceIdFromPrincipal_UserIdNotValid(t *testing.T) {
-	for _, principal := range []string{"User:123456", "User:abcdef"} {
-		_, _, err := getPrefixAndResourceIdFromPrincipal(principal, nil)
-		require.Error(t, err)
-	}
-}
-
 func TestAclBindingToClustersClusterIdAclsPostOpts(t *testing.T) {
 	req := require.New(t)
 

internal/pkg/errors/error_message.go

@@ -77,7 +77,6 @@ const (
 	MustSetResourceTypeErrorMsg   = "exactly one resource type (%v) must be set"
 	InvalidOperationValueErrorMsg = "invalid operation value: %s"
 	ExactlyOneSetErrorMsg         = "exactly one of %v must be set"
-	UserIdNotValidErrorMsg        = "can't map user id to a valid service account"
 
 	// iam rbac role commands
 	UnknownRoleErrorMsg    = `unknown role "%s"`

test/fixtures/output/kafka/acl/delete-json.golden

@@ -1,6 +1,6 @@
 [
   {
-    "principal": "User:12345",
+    "principal": "User:sa-12345",
     "permission": "ALLOW",
     "operation": "READ",
     "host": "*",

test/fixtures/output/kafka/acl/delete-prompt.golden

@@ -1,3 +1,3 @@
-Are you sure you want to delete the ACL corresponding to these parameters? (y/n):   Principal  | Permission | Operation | Host | Resource Type | Resource Name | Pattern Type  
--------------+------------+-----------+------+---------------+---------------+---------------
-  User:12345 | ALLOW      | READ      | *    | TOPIC         | test-topic    | LITERAL       
+Are you sure you want to delete the ACL corresponding to these parameters? (y/n):     Principal   | Permission | Operation | Host | Resource Type | Resource Name | Pattern Type  
+----------------+------------+-----------+------+---------------+---------------+---------------
+  User:sa-12345 | ALLOW      | READ      | *    | TOPIC         | test-topic    | LITERAL       

test/fixtures/output/kafka/acl/delete-yaml.golden

@@ -1,4 +1,4 @@
-- principal: User:12345
+- principal: User:sa-12345
   permission: ALLOW
   operation: READ
   host: '*'

test/fixtures/output/kafka/acl/delete.golden

@@ -1,3 +1,3 @@
-  Principal  | Permission | Operation | Host | Resource Type | Resource Name | Pattern Type  
--------------+------------+-----------+------+---------------+---------------+---------------
-  User:12345 | ALLOW      | READ      | *    | TOPIC         | test-topic    | LITERAL       
+    Principal   | Permission | Operation | Host | Resource Type | Resource Name | Pattern Type  
+----------------+------------+-----------+------+---------------+---------------+---------------
+  User:sa-12345 | ALLOW      | READ      | *    | TOPIC         | test-topic    | LITERAL       

test/fixtures/output/kafka/acl/list-json.golden

@@ -1,6 +1,6 @@
 [
   {
-    "principal": "User:12345",
+    "principal": "User:sa-12345",
     "permission": "ALLOW",
     "operation": "READ",
     "host": "*",

test/fixtures/output/kafka/acl/list-yaml.golden

@@ -1,4 +1,4 @@
-- principal: User:12345
+- principal: User:sa-12345
   permission: ALLOW
   operation: READ
   host: '*'

test/fixtures/output/kafka/acl/list.golden

@@ -1,3 +1,3 @@
-  Principal  | Permission | Operation | Host | Resource Type | Resource Name | Pattern Type  
--------------+------------+-----------+------+---------------+---------------+---------------
-  User:12345 | ALLOW      | READ      | *    | TOPIC         | test-topic    | LITERAL       
+    Principal   | Permission | Operation | Host | Resource Type | Resource Name | Pattern Type  
+----------------+------------+-----------+------+---------------+---------------+---------------
+  User:sa-12345 | ALLOW      | READ      | *    | TOPIC         | test-topic    | LITERAL       

test/test-server/kafka_rest_router.go

@@ -106,17 +106,15 @@ func handleKafkaRPClusters(t *testing.T) http.HandlerFunc {
 // Handler for: "/kafka/v3/clusters/{cluster}/acls"
 func handleKafkaRPACLs(t *testing.T) http.HandlerFunc {
 	return func(w http.ResponseWriter, r *http.Request) {
-		data := []cckafkarestv3.AclData{
-			{
-				ResourceType: cckafkarestv3.TOPIC,
-				ResourceName: "test-topic",
-				Operation:    "READ",
-				Permission:   "ALLOW",
-				Host:         "*",
-				Principal:    "User:12345",
-				PatternType:  "LITERAL",
-			},
-		}
+		data := []cckafkarestv3.AclData{{
+			ResourceType: cckafkarestv3.TOPIC,
+			ResourceName: "test-topic",
+			Operation:    "READ",
+			Permission:   "ALLOW",
+			Host:         "*",
+			Principal:    "User:sa-12345",
+			PatternType:  "LITERAL",
+		}}
 
 		var res any