fix(grant_access): correctly pass arguments to new commands

Also refactor some business code for simplicity/separation of concerns.

Co-authored-by: @sindrerh2 <sindre.rodseth.hansen@nav.no>
Co-authored-by: @ybelMekk <youssef.bel.mekki@nav.no>

Author: 3 people

flake.nix

@@ -6,12 +6,12 @@
   outputs = { nixpkgs, ... }:
     let
       goOverlay = final: prev: let
-        version = "1.25.5";
+        version = "1.25.6";
         newerGoVersion = prev.go.overrideAttrs (old: {
           inherit version;
           src = prev.fetchurl {
             url = "https://go.dev/dl/go${version}.src.tar.gz";
-            hash = ""; # TODO: if `version` is changed in the future
+            hash = "sha256-WMv3ceRNdt5vVtGeM7d9dFoeSJNAkih15GWFuXXCsFk=";
           };
         });
         nixpkgsVersion = prev.go.version;

internal/aiven/command/flag/flag.go

@@ -24,10 +24,15 @@ type CreateOpenSearch struct {
 
 type GrantAccess struct {
 	*Aiven
-	Access string `name:"access" short:"x" usage:"Access |LEVEL| (readwrite, read and write)."`
 }
 
-type GrantAccessCommon struct {
+type GrantAccessStream struct {
 	*GrantAccess
-	Namespace string `name:"namespace" short:"n" usage:"|NAMESPACE| of the Stream.kafka.nais.io's |NAMESPACE|."`
+	Namespace string `name:"namespace" short:"n" usage:"|NAMESPACE| of the stream.kafka.nais.io resource."`
+}
+
+type GrantAccessTopic struct {
+	*GrantAccess
+	Access    string `name:"access" short:"x" usage:"Access |LEVEL| (readwrite, read and write)."`
+	Namespace string `name:"namespace" short:"n" usage:"|NAMESPACE| of the topic.kafka.nais.io resource."`
 }

internal/aiven/command/grant_access_stream.go

@@ -2,14 +2,15 @@ package command
 
 import (
 	"context"
+	"fmt"
 
 	"github.com/nais/cli/internal/aiven"
 	"github.com/nais/cli/internal/aiven/command/flag"
 	"github.com/nais/naistrix"
 )
 
 func grantAccessStream(parentFlags *flag.GrantAccess) *naistrix.Command {
-	grantAccessStreamFlags := &flag.GrantAccessCommon{GrantAccess: parentFlags}
+	grantAccessStreamFlags := &flag.GrantAccessStream{GrantAccess: parentFlags}
 
 	return &naistrix.Command{
 		Name:  "stream",
@@ -19,8 +20,15 @@ func grantAccessStream(parentFlags *flag.GrantAccess) *naistrix.Command {
 			{Name: "username"},
 			{Name: "stream"},
 		},
+		ValidateFunc: func(context.Context, *naistrix.Arguments) error {
+			if grantAccessStreamFlags.Namespace == "" {
+				return fmt.Errorf("--namespace is required\n\tPS: Check `nais config set`")
+			}
+
+			return nil
+		},
 		RunFunc: func(ctx context.Context, args *naistrix.Arguments, out *naistrix.OutputWriter) error {
-			namespace := args.Get("namespace")
+			namespace := grantAccessStreamFlags.Namespace
 			userName := args.Get("username")
 			stream := args.Get("stream")
 
@@ -29,7 +37,7 @@ func grantAccessStream(parentFlags *flag.GrantAccess) *naistrix.Command {
 				return err
 			}
 
-			if !accessResult.Added {
+			if accessResult.AlreadyAdded {
 				out.Printf("Username '%s' already listed in Stream '%s/%s' ACLs.", userName, namespace, stream)
 				return nil
 			}

internal/aiven/command/grant_access_topic.go

@@ -2,6 +2,7 @@ package command
 
 import (
 	"context"
+	"fmt"
 
 	"github.com/nais/cli/internal/aiven"
 	"github.com/nais/cli/internal/aiven/command/flag"
@@ -10,7 +11,7 @@ import (
 )
 
 func grantAccessTopic(parentFlags *flag.GrantAccess) *naistrix.Command {
-	grantAccessTopicFlags := &flag.GrantAccessCommon{GrantAccess: parentFlags}
+	grantAccessTopicFlags := &flag.GrantAccessTopic{GrantAccess: parentFlags, Access: "read"}
 
 	return &naistrix.Command{
 		Name:  "topic",
@@ -20,30 +21,38 @@ func grantAccessTopic(parentFlags *flag.GrantAccess) *naistrix.Command {
 			{Name: "username"},
 			{Name: "topic"},
 		},
+		ValidateFunc: func(context.Context, *naistrix.Arguments) error {
+			if grantAccessTopicFlags.Namespace == "" {
+				return fmt.Errorf("--namespace is required\n\tPS: Check `nais config set`")
+			}
+
+			return nil
+		},
 		RunFunc: func(ctx context.Context, args *naistrix.Arguments, out *naistrix.OutputWriter) error {
-			namespace := args.Get("namespace")
+			access := grantAccessTopicFlags.Access
+			namespace := grantAccessTopicFlags.Namespace
 			topicName := args.Get("topic")
+			username := args.Get("username")
 
-			acl := nais_kafka.TopicACL{
+			newAcl := nais_kafka.TopicACL{
 				Team:        namespace,
-				Application: args.Get("username"),
-				Access:      args.Get("access"),
+				Application: username,
+				Access:      access,
 			}
-
-			accessResult, err := aiven.GrantAccessToTopic(ctx, namespace, topicName, acl)
+			accessResult, err := aiven.GrantAccessToTopic(ctx, namespace, topicName, newAcl)
 			if err != nil {
 				return err
 			}
 
-			if !accessResult.Added {
-				out.Printf("ACL already exists for team '%s', application '%s', access '%s' on topic '%s/%s'.",
-					acl.Team, acl.Application, acl.Access, namespace, topicName,
+			if accessResult.AlreadyAdded {
+				out.Printf("An ACL already exists for user/access '%s' on topic '%s/%s'.",
+					newAcl.Application, newAcl.Access, namespace, topicName,
 				)
 				return nil
 			}
 
 			out.Printf("ACL added for team '%s', application '%s', access '%s' on topic '%s/%s'.",
-				acl.Team, acl.Application, acl.Access, namespace, topicName,
+				newAcl.Team, newAcl.Application, newAcl.Access, namespace, topicName,
 			)
 			return nil
 		},

internal/aiven/grant_access.go

@@ -10,14 +10,12 @@ import (
 )
 
 type GrantAccessResult struct {
-	Added     bool
-	Namespace string
-	Name      string
-	Kind      string
-	Detail    string
+	AlreadyAdded bool
+	Namespace    string
+	Name         string
 }
 
-func GrantAccessToTopic(ctx context.Context, namespace, topicName string, acl nais_kafka.TopicACL) (*GrantAccessResult, error) {
+func GrantAccessToTopic(ctx context.Context, namespace, topicName string, newAcl nais_kafka.TopicACL) (*GrantAccessResult, error) {
 	client := k8s.SetupControllerRuntimeClient()
 
 	if err := validateNamespace(ctx, client, namespace); err != nil {
@@ -26,37 +24,26 @@ func GrantAccessToTopic(ctx context.Context, namespace, topicName string, acl na
 
 	var topic nais_kafka.Topic
 	if err := client.Get(ctx, ctrl.ObjectKey{Name: topicName, Namespace: namespace}, &topic); err != nil {
-		return nil, fmt.Errorf("validate topic: %w", err)
+		return nil, fmt.Errorf("get topic: %w", err)
 	}
 
-	// Default to read access if not specified
-	if acl.Access == "" {
-		acl.Access = "read"
-	}
-
-	newACLs, added := ensureTopicACL(topic.Spec.ACL, acl)
-	if !added {
+	if checkIfAclInList(topic.Spec.ACL, newAcl) {
 		return &GrantAccessResult{
-			Added:     false,
-			Namespace: namespace,
-			Name:      topic.Name,
-			Kind:      topic.Kind,
-			Detail:    acl.Access,
+			AlreadyAdded: true,
+			Namespace:    namespace,
+			Name:         topicName,
 		}, nil
 	}
-
-	topic.Spec.ACL = newACLs
+	topic.Spec.ACL = append(topic.Spec.ACL, newAcl)
 
 	if err := client.Update(ctx, &topic); err != nil {
 		return nil, fmt.Errorf("update topic: %w", err)
 	}
 
 	return &GrantAccessResult{
-		Added:     true,
-		Namespace: namespace,
-		Name:      topic.Name,
-		Kind:      topic.Kind,
-		Detail:    acl.Access,
+		AlreadyAdded: false,
+		Namespace:    namespace,
+		Name:         topicName,
 	}, nil
 }
 
@@ -69,49 +56,43 @@ func GrantAccessToStream(ctx context.Context, namespace, streamName, userName st
 
 	var stream nais_kafka.Stream
 	if err := client.Get(ctx, ctrl.ObjectKey{Name: streamName, Namespace: namespace}, &stream); err != nil {
-		return nil, fmt.Errorf("validate stream: %w", err)
+		return nil, fmt.Errorf("get stream: %w", err)
 	}
 
-	newUsers, added := ensureAdditionalStreamUser(stream.Spec.AdditionalUsers, userName)
-	if !added {
+	if checkIfUserInList(stream.Spec.AdditionalUsers, userName) {
 		return &GrantAccessResult{
-			Added:     false,
-			Namespace: namespace,
-			Name:      stream.Name,
-			Kind:      stream.Kind,
-			Detail:    userName,
+			AlreadyAdded: true,
+			Namespace:    namespace,
+			Name:         streamName,
 		}, nil
 	}
-
-	stream.Spec.AdditionalUsers = newUsers
+	stream.Spec.AdditionalUsers = append(stream.Spec.AdditionalUsers, nais_kafka.AdditionalStreamUser{Username: userName})
 
 	if err := client.Update(ctx, &stream); err != nil {
 		return nil, fmt.Errorf("update stream: %w", err)
 	}
 
 	return &GrantAccessResult{
-		Added:     true,
-		Namespace: namespace,
-		Name:      stream.Name,
-		Kind:      stream.Kind,
-		Detail:    userName,
+		AlreadyAdded: false,
+		Namespace:    namespace,
+		Name:         streamName,
 	}, nil
 }
 
-func ensureTopicACL(existing []nais_kafka.TopicACL, wanted nais_kafka.TopicACL) ([]nais_kafka.TopicACL, bool) {
+func checkIfAclInList(existing []nais_kafka.TopicACL, wanted nais_kafka.TopicACL) bool {
 	for _, e := range existing {
 		if e.Team == wanted.Team && e.Application == wanted.Application && e.Access == wanted.Access {
-			return existing, false
+			return true
 		}
 	}
-	return append(existing, wanted), true
+	return false
 }
 
-func ensureAdditionalStreamUser(existing []nais_kafka.AdditionalStreamUser, userName string) ([]nais_kafka.AdditionalStreamUser, bool) {
+func checkIfUserInList(existing []nais_kafka.AdditionalStreamUser, userName string) bool {
 	for _, u := range existing {
 		if u.Username == userName {
-			return existing, false
+			return true
 		}
 	}
-	return append(existing, nais_kafka.AdditionalStreamUser{Username: userName}), true
+	return false
 }