Skip to content

Commit

Permalink
Revert "Revert ACL changes to handle integer IDs (#2181)" (#2187)
Browse files Browse the repository at this point in the history
  • Loading branch information
sgagniere committed Aug 11, 2023
1 parent 4047726 commit 2829e46
Show file tree
Hide file tree
Showing 10 changed files with 21 additions and 153 deletions.
24 changes: 0 additions & 24 deletions internal/cmd/kafka/command_acl.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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.GetId()] = user.GetResourceId()
}
return numericIdToResourceId
}

func (c *aclCommand) provisioningClusterCheck(lkc string) error {
environmentId, err := c.Context.EnvironmentId()
if err != nil {
Expand Down
11 changes: 2 additions & 9 deletions internal/cmd/kafka/command_acl_create.go
Original file line number Diff line number Diff line change
Expand Up @@ -74,22 +74,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 err := kafkaREST.CloudClient.CreateKafkaAcls(data); err != nil {
if i > 0 {
_ = pacl.PrintACLsWithResourceIdMap(cmd, bindings[:i], numericIdToResourceId)
_ = pacl.PrintACLs(cmd, bindings[:i])
}
return err
}
}

return pacl.PrintACLsWithResourceIdMap(cmd, bindings, numericIdToResourceId)
return pacl.PrintACLs(cmd, bindings)
}
2 changes: 1 addition & 1 deletion internal/cmd/kafka/command_acl_create_onprem.go
Original file line number Diff line number Diff line change
Expand Up @@ -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})
}
2 changes: 1 addition & 1 deletion internal/cmd/kafka/command_acl_delete_onprem.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
7 changes: 1 addition & 6 deletions internal/cmd/kafka/command_acl_list.go
Original file line number Diff line number Diff line change
Expand Up @@ -38,11 +38,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
}
Expand All @@ -61,5 +56,5 @@ func (c *aclCommand) list(cmd *cobra.Command, _ []string) error {
return err
}

return aclutil.PrintACLsFromKafkaRestResponseWithResourceIdMap(cmd, aclDataList.Data, mapNumericIdToResourceId(users))
return aclutil.PrintACLsFromKafkaRestResponse(cmd, aclDataList.Data)
}
2 changes: 1 addition & 1 deletion internal/cmd/kafka/command_acl_list_onprem.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
7 changes: 7 additions & 0 deletions internal/cmd/kafka/kafka_api.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}

Expand Down
85 changes: 8 additions & 77 deletions internal/pkg/acl/acl.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@ package acl
import (
"fmt"
"sort"
"strconv"
"strings"

"github.com/antihax/optional"
Expand All @@ -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"
)

Expand Down Expand Up @@ -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{
Expand Down Expand Up @@ -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(),
Expand Down
33 changes: 0 additions & 33 deletions internal/pkg/acl/acl_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Expand Down
1 change: 0 additions & 1 deletion internal/pkg/errors/error_message.go
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,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"`
Expand Down

0 comments on commit 2829e46

Please sign in to comment.