Skip to content

Commit 795736f

Browse files
x10an14-navsindrerh2ybelMekk
committed
fix(grant_access): correctly pass arguments to new commands
Also: - refactor some business code for simplicity/separation of concerns. - fix mise Co-authored-by: @sindrerh2 <sindre.rodseth.hansen@nav.no> Co-authored-by: @ybelMekk <youssef.bel.mekki@nav.no>
1 parent 97dd205 commit 795736f

6 files changed

Lines changed: 70 additions & 67 deletions

File tree

flake.nix

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6,12 +6,12 @@
66
outputs = { nixpkgs, ... }:
77
let
88
goOverlay = final: prev: let
9-
version = "1.25.5";
9+
version = "1.25.6";
1010
newerGoVersion = prev.go.overrideAttrs (old: {
1111
inherit version;
1212
src = prev.fetchurl {
1313
url = "https://go.dev/dl/go${version}.src.tar.gz";
14-
hash = ""; # TODO: if `version` is changed in the future
14+
hash = "sha256-WMv3ceRNdt5vVtGeM7d9dFoeSJNAkih15GWFuXXCsFk=";
1515
};
1616
});
1717
nixpkgsVersion = prev.go.version;

internal/aiven/command/flag/flag.go

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -24,10 +24,15 @@ type CreateOpenSearch struct {
2424

2525
type GrantAccess struct {
2626
*Aiven
27-
Access string `name:"access" short:"x" usage:"Access |LEVEL| (readwrite, read and write)."`
2827
}
2928

30-
type GrantAccessCommon struct {
29+
type GrantAccessStream struct {
3130
*GrantAccess
32-
Namespace string `name:"namespace" short:"n" usage:"|NAMESPACE| of the Stream.kafka.nais.io's |NAMESPACE|."`
31+
Namespace string `name:"namespace" short:"n" usage:"|NAMESPACE| of the stream.kafka.nais.io resource."`
32+
}
33+
34+
type GrantAccessTopic struct {
35+
*GrantAccess
36+
Access string `name:"access" short:"x" usage:"Access |LEVEL| (readwrite, read and write)."`
37+
Namespace string `name:"namespace" short:"n" usage:"|NAMESPACE| of the topic.kafka.nais.io resource."`
3338
}

internal/aiven/command/grant_access_stream.go

Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2,14 +2,15 @@ package command
22

33
import (
44
"context"
5+
"fmt"
56

67
"github.com/nais/cli/internal/aiven"
78
"github.com/nais/cli/internal/aiven/command/flag"
89
"github.com/nais/naistrix"
910
)
1011

1112
func grantAccessStream(parentFlags *flag.GrantAccess) *naistrix.Command {
12-
grantAccessStreamFlags := &flag.GrantAccessCommon{GrantAccess: parentFlags}
13+
grantAccessStreamFlags := &flag.GrantAccessStream{GrantAccess: parentFlags}
1314

1415
return &naistrix.Command{
1516
Name: "stream",
@@ -19,8 +20,15 @@ func grantAccessStream(parentFlags *flag.GrantAccess) *naistrix.Command {
1920
{Name: "username"},
2021
{Name: "stream"},
2122
},
23+
ValidateFunc: func(context.Context, *naistrix.Arguments) error {
24+
if grantAccessStreamFlags.Namespace == "" {
25+
return fmt.Errorf("--namespace is required\n\tPS: Check `nais config set`")
26+
}
27+
28+
return nil
29+
},
2230
RunFunc: func(ctx context.Context, args *naistrix.Arguments, out *naistrix.OutputWriter) error {
23-
namespace := args.Get("namespace")
31+
namespace := grantAccessStreamFlags.Namespace
2432
userName := args.Get("username")
2533
stream := args.Get("stream")
2634

@@ -29,7 +37,7 @@ func grantAccessStream(parentFlags *flag.GrantAccess) *naistrix.Command {
2937
return err
3038
}
3139

32-
if !accessResult.Added {
40+
if accessResult.AlreadyAdded {
3341
out.Printf("Username '%s' already listed in Stream '%s/%s' ACLs.", userName, namespace, stream)
3442
return nil
3543
}

internal/aiven/command/grant_access_topic.go

Lines changed: 20 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ package command
22

33
import (
44
"context"
5+
"fmt"
56

67
"github.com/nais/cli/internal/aiven"
78
"github.com/nais/cli/internal/aiven/command/flag"
@@ -10,7 +11,7 @@ import (
1011
)
1112

1213
func grantAccessTopic(parentFlags *flag.GrantAccess) *naistrix.Command {
13-
grantAccessTopicFlags := &flag.GrantAccessCommon{GrantAccess: parentFlags}
14+
grantAccessTopicFlags := &flag.GrantAccessTopic{GrantAccess: parentFlags, Access: "read"}
1415

1516
return &naistrix.Command{
1617
Name: "topic",
@@ -20,30 +21,38 @@ func grantAccessTopic(parentFlags *flag.GrantAccess) *naistrix.Command {
2021
{Name: "username"},
2122
{Name: "topic"},
2223
},
24+
ValidateFunc: func(context.Context, *naistrix.Arguments) error {
25+
if grantAccessTopicFlags.Namespace == "" {
26+
return fmt.Errorf("--namespace is required\n\tPS: Check `nais config set`")
27+
}
28+
29+
return nil
30+
},
2331
RunFunc: func(ctx context.Context, args *naistrix.Arguments, out *naistrix.OutputWriter) error {
24-
namespace := args.Get("namespace")
32+
access := grantAccessTopicFlags.Access
33+
namespace := grantAccessTopicFlags.Namespace
2534
topicName := args.Get("topic")
35+
username := args.Get("username")
2636

27-
acl := nais_kafka.TopicACL{
37+
newAcl := nais_kafka.TopicACL{
2838
Team: namespace,
29-
Application: args.Get("username"),
30-
Access: args.Get("access"),
39+
Application: username,
40+
Access: access,
3141
}
32-
33-
accessResult, err := aiven.GrantAccessToTopic(ctx, namespace, topicName, acl)
42+
accessResult, err := aiven.GrantAccessToTopic(ctx, namespace, topicName, newAcl)
3443
if err != nil {
3544
return err
3645
}
3746

38-
if !accessResult.Added {
39-
out.Printf("ACL already exists for team '%s', application '%s', access '%s' on topic '%s/%s'.",
40-
acl.Team, acl.Application, acl.Access, namespace, topicName,
47+
if accessResult.AlreadyAdded {
48+
out.Printf("An ACL already exists for user/access '%s' on topic '%s/%s'.",
49+
newAcl.Application, newAcl.Access, namespace, topicName,
4150
)
4251
return nil
4352
}
4453

4554
out.Printf("ACL added for team '%s', application '%s', access '%s' on topic '%s/%s'.",
46-
acl.Team, acl.Application, acl.Access, namespace, topicName,
55+
newAcl.Team, newAcl.Application, newAcl.Access, namespace, topicName,
4756
)
4857
return nil
4958
},

internal/aiven/grant_access.go

Lines changed: 28 additions & 47 deletions
Original file line numberDiff line numberDiff line change
@@ -10,14 +10,12 @@ import (
1010
)
1111

1212
type GrantAccessResult struct {
13-
Added bool
14-
Namespace string
15-
Name string
16-
Kind string
17-
Detail string
13+
AlreadyAdded bool
14+
Namespace string
15+
Name string
1816
}
1917

20-
func GrantAccessToTopic(ctx context.Context, namespace, topicName string, acl nais_kafka.TopicACL) (*GrantAccessResult, error) {
18+
func GrantAccessToTopic(ctx context.Context, namespace, topicName string, newAcl nais_kafka.TopicACL) (*GrantAccessResult, error) {
2119
client := k8s.SetupControllerRuntimeClient()
2220

2321
if err := validateNamespace(ctx, client, namespace); err != nil {
@@ -26,37 +24,26 @@ func GrantAccessToTopic(ctx context.Context, namespace, topicName string, acl na
2624

2725
var topic nais_kafka.Topic
2826
if err := client.Get(ctx, ctrl.ObjectKey{Name: topicName, Namespace: namespace}, &topic); err != nil {
29-
return nil, fmt.Errorf("validate topic: %w", err)
27+
return nil, fmt.Errorf("get topic: %w", err)
3028
}
3129

32-
// Default to read access if not specified
33-
if acl.Access == "" {
34-
acl.Access = "read"
35-
}
36-
37-
newACLs, added := ensureTopicACL(topic.Spec.ACL, acl)
38-
if !added {
30+
if checkIfAclInList(topic.Spec.ACL, newAcl) {
3931
return &GrantAccessResult{
40-
Added: false,
41-
Namespace: namespace,
42-
Name: topic.Name,
43-
Kind: topic.Kind,
44-
Detail: acl.Access,
32+
AlreadyAdded: true,
33+
Namespace: namespace,
34+
Name: topicName,
4535
}, nil
4636
}
47-
48-
topic.Spec.ACL = newACLs
37+
topic.Spec.ACL = append(topic.Spec.ACL, newAcl)
4938

5039
if err := client.Update(ctx, &topic); err != nil {
5140
return nil, fmt.Errorf("update topic: %w", err)
5241
}
5342

5443
return &GrantAccessResult{
55-
Added: true,
56-
Namespace: namespace,
57-
Name: topic.Name,
58-
Kind: topic.Kind,
59-
Detail: acl.Access,
44+
AlreadyAdded: false,
45+
Namespace: namespace,
46+
Name: topicName,
6047
}, nil
6148
}
6249

@@ -69,49 +56,43 @@ func GrantAccessToStream(ctx context.Context, namespace, streamName, userName st
6956

7057
var stream nais_kafka.Stream
7158
if err := client.Get(ctx, ctrl.ObjectKey{Name: streamName, Namespace: namespace}, &stream); err != nil {
72-
return nil, fmt.Errorf("validate stream: %w", err)
59+
return nil, fmt.Errorf("get stream: %w", err)
7360
}
7461

75-
newUsers, added := ensureAdditionalStreamUser(stream.Spec.AdditionalUsers, userName)
76-
if !added {
62+
if checkIfUserInList(stream.Spec.AdditionalUsers, userName) {
7763
return &GrantAccessResult{
78-
Added: false,
79-
Namespace: namespace,
80-
Name: stream.Name,
81-
Kind: stream.Kind,
82-
Detail: userName,
64+
AlreadyAdded: true,
65+
Namespace: namespace,
66+
Name: streamName,
8367
}, nil
8468
}
85-
86-
stream.Spec.AdditionalUsers = newUsers
69+
stream.Spec.AdditionalUsers = append(stream.Spec.AdditionalUsers, nais_kafka.AdditionalStreamUser{Username: userName})
8770

8871
if err := client.Update(ctx, &stream); err != nil {
8972
return nil, fmt.Errorf("update stream: %w", err)
9073
}
9174

9275
return &GrantAccessResult{
93-
Added: true,
94-
Namespace: namespace,
95-
Name: stream.Name,
96-
Kind: stream.Kind,
97-
Detail: userName,
76+
AlreadyAdded: false,
77+
Namespace: namespace,
78+
Name: streamName,
9879
}, nil
9980
}
10081

101-
func ensureTopicACL(existing []nais_kafka.TopicACL, wanted nais_kafka.TopicACL) ([]nais_kafka.TopicACL, bool) {
82+
func checkIfAclInList(existing []nais_kafka.TopicACL, wanted nais_kafka.TopicACL) bool {
10283
for _, e := range existing {
10384
if e.Team == wanted.Team && e.Application == wanted.Application && e.Access == wanted.Access {
104-
return existing, false
85+
return true
10586
}
10687
}
107-
return append(existing, wanted), true
88+
return false
10889
}
10990

110-
func ensureAdditionalStreamUser(existing []nais_kafka.AdditionalStreamUser, userName string) ([]nais_kafka.AdditionalStreamUser, bool) {
91+
func checkIfUserInList(existing []nais_kafka.AdditionalStreamUser, userName string) bool {
11192
for _, u := range existing {
11293
if u.Username == userName {
113-
return existing, false
94+
return true
11495
}
11596
}
116-
return append(existing, nais_kafka.AdditionalStreamUser{Username: userName}), true
97+
return false
11798
}

mise/config.toml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
[tools]
22
git-cliff = "2.10.1"
3-
go = "1.25.5"
3+
go = "1.25.6"
44
yamlfmt = "0.17.2"
55

66
[settings]

0 commit comments

Comments
 (0)