From 91a690b02f239b286d939087fc07437027c0db2b Mon Sep 17 00:00:00 2001 From: pkulik0 Date: Tue, 28 May 2024 10:51:51 +0000 Subject: [PATCH 01/13] Remove outdated docker compose check, rename target to match how it's referred to in other files --- Makefile | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/Makefile b/Makefile index f2d2d7004..c16acd75e 100644 --- a/Makefile +++ b/Makefile @@ -111,13 +111,12 @@ endef # Install packages required to develop JIMM and run tests. APT_BASED := $(shell command -v apt-get >/dev/null; echo $$?) -sysdeps: +sys-deps: ifeq ($(APT_BASED),0) @$(call check_dep,go,Missing Go - install from https://go.dev/doc/install or 'sudo snap install go') @$(call check_dep,git,Missing Git - install with 'sudo apt install git') @$(call check_dep,gcc,Missing gcc - install with 'sudo apt install build-essentials') @$(call check_dep,docker,Missing Docker - install from https://docs.docker.com/engine/install/') - @$(call check_dep,docker-compose,Missing Docker Compose - install from https://docs.docker.com/engine/install/') @$(call check_dep,juju-db.mongo,Missing juju-db - install with 'sudo snap install juju-db --channel=4.4/stable') else @echo sysdeps runs only on systems with apt-get From 8240162ceec51d1581002c4d8afd8047bec63603 Mon Sep 17 00:00:00 2001 From: pkulik0 Date: Tue, 28 May 2024 10:52:40 +0000 Subject: [PATCH 02/13] Fix incorrect paging in `fetchRelations`, adjust `TestListRelations` to include a multipage result --- cmd/jimmctl/cmd/export_test.go | 1 + cmd/jimmctl/cmd/relation.go | 9 ++--- cmd/jimmctl/cmd/relation_test.go | 62 ++++++++++++++++---------------- 3 files changed, 37 insertions(+), 35 deletions(-) diff --git a/cmd/jimmctl/cmd/export_test.go b/cmd/jimmctl/cmd/export_test.go index 7b232d042..66d590051 100644 --- a/cmd/jimmctl/cmd/export_test.go +++ b/cmd/jimmctl/cmd/export_test.go @@ -14,6 +14,7 @@ var ( AccessMessage = accessMessageFormat AccessResultAllowed = accessResultAllowed AccessResultDenied = accessResultDenied + DefaultPageSize = defaultPageSize ) type AccessResult = accessResult diff --git a/cmd/jimmctl/cmd/relation.go b/cmd/jimmctl/cmd/relation.go index 2bb087b99..458ab53a9 100644 --- a/cmd/jimmctl/cmd/relation.go +++ b/cmd/jimmctl/cmd/relation.go @@ -137,7 +137,7 @@ Examples: returns the list of relations, where target object matches the specified one - jimmctl auth relation list --target --relation --relation returns the list of relations, where target object and relation match the specified ones ` @@ -544,7 +544,7 @@ func (c *listRelationsCommand) Run(ctxt *cmd.Context) error { Tuple: c.tuple, PageSize: defaultPageSize, } - result, err := fetchRelations(client, params, "") + result, err := fetchRelations(client, params) if err != nil { return errors.E(err) } @@ -557,13 +557,14 @@ func (c *listRelationsCommand) Run(ctxt *cmd.Context) error { return nil } -func fetchRelations(client *api.Client, params apiparams.ListRelationshipTuplesRequest, continuationToken string) (*apiparams.ListRelationshipTuplesResponse, error) { +func fetchRelations(client *api.Client, params apiparams.ListRelationshipTuplesRequest) (*apiparams.ListRelationshipTuplesResponse, error) { response, err := client.ListRelationshipTuples(¶ms) if err != nil { return nil, errors.E(err) } if response.ContinuationToken != "" { - response1, err := fetchRelations(client, params, response.ContinuationToken) + params.ContinuationToken = response.ContinuationToken + response1, err := fetchRelations(client, params) if err != nil { return nil, errors.E(err) } diff --git a/cmd/jimmctl/cmd/relation_test.go b/cmd/jimmctl/cmd/relation_test.go index c240b2a23..02ad395d5 100644 --- a/cmd/jimmctl/cmd/relation_test.go +++ b/cmd/jimmctl/cmd/relation_test.go @@ -347,20 +347,10 @@ func (s *relationSuite) TestListRelations(c *gc.C) { // alice is superuser bClient := jimmtest.NewUserSessionLogin(c, "alice") - groups := []string{"group-1", "group-2", "group-3"} - for _, group := range groups { - _, err := cmdtesting.RunCommand(c, cmd.NewAddGroupCommandForTesting(s.ClientStore(), bClient), group) - c.Assert(err, gc.IsNil) - } - relations := []apiparams.RelationshipTuple{{ Object: "user-" + env.users[0].Name, Relation: "member", TargetObject: "group-group-1", - }, { - Object: "user-" + env.users[1].Name, - Relation: "member", - TargetObject: "group-group-2", }, { Object: "group-group-2#member", Relation: "member", @@ -378,11 +368,38 @@ func (s *relationSuite) TestListRelations(c *gc.C) { Relation: "administrator", TargetObject: "applicationoffer-" + env.controllers[0].Name + ":" + env.applicationOffers[0].Model.OwnerIdentityName + "/" + env.applicationOffers[0].Model.Name + "." + env.applicationOffers[0].Name, }} + + tabularOutput := `Object Relation Target Object +user-admin administrator controller-jimm +user-alice@canonical.com administrator controller-jimm +user-alice@canonical.com member group-group-1 +group-group-2#member member group-group-3 +group-group-3#member administrator controller-test-controller-1 +group-group-1#member administrator model-test-controller-1:alice@canonical.com/test-model-1 +user-eve@canonical.com administrator applicationoffer-test-controller-1:alice@canonical.com/test-model-1.testoffer1` + + for i := 0; i < cmd.DefaultPageSize+1; i++ { + groupName := fmt.Sprintf("group-%d", i) + _, err := cmdtesting.RunCommand(c, cmd.NewAddGroupCommandForTesting(s.ClientStore(), bClient), groupName) + c.Assert(err, gc.IsNil) + + tabularOutput += fmt.Sprintf("\nuser-eve@canonical.com member group-%-72s", groupName) + relations = append(relations, apiparams.RelationshipTuple{ + Object: "user-" + env.users[1].Name, + Relation: "member", + TargetObject: "group-" + groupName, + }) + } + for _, relation := range relations { _, err := cmdtesting.RunCommand(c, cmd.NewAddRelationCommandForTesting(s.ClientStore(), bClient), relation.Object, relation.Relation, relation.TargetObject) c.Assert(err, gc.IsNil) } + context, err := cmdtesting.RunCommand(c, cmd.NewListRelationsCommandForTesting(s.ClientStore(), bClient), "--format", "tabular") + c.Assert(err, gc.IsNil) + c.Assert(cmdtesting.Stdout(context), gc.Equals, tabularOutput) + expectedJSONData, err := json.Marshal(append( []apiparams.RelationshipTuple{{ Object: "user-admin", @@ -396,6 +413,10 @@ func (s *relationSuite) TestListRelations(c *gc.C) { relations..., )) c.Assert(err, gc.IsNil) + context, err = cmdtesting.RunCommand(c, cmd.NewListRelationsCommandForTesting(s.ClientStore(), bClient), "--format", "json") + c.Assert(err, gc.IsNil) + c.Assert(strings.TrimRight(cmdtesting.Stdout(context), "\n"), gc.Equals, string(expectedJSONData)) + expectedYAMLData, err := yaml.Marshal(append( []apiparams.RelationshipTuple{{ Object: "user-admin", @@ -409,30 +430,9 @@ func (s *relationSuite) TestListRelations(c *gc.C) { relations..., )) c.Assert(err, gc.IsNil) - - context, err := cmdtesting.RunCommand(c, cmd.NewListRelationsCommandForTesting(s.ClientStore(), bClient), "--format", "json") - c.Assert(err, gc.IsNil) - c.Assert(strings.TrimRight(cmdtesting.Stdout(context), "\n"), gc.Equals, string(expectedJSONData)) - context, err = cmdtesting.RunCommand(c, cmd.NewListRelationsCommandForTesting(s.ClientStore(), bClient)) c.Assert(err, gc.IsNil) c.Assert(cmdtesting.Stdout(context), gc.Equals, string(expectedYAMLData)) - - context, err = cmdtesting.RunCommand(c, cmd.NewListRelationsCommandForTesting(s.ClientStore(), bClient), "--format", "tabular") - c.Assert(err, gc.IsNil) - c.Assert( - cmdtesting.Stdout(context), - gc.Equals, - `Object Relation Target Object -user-admin administrator controller-jimm -user-alice@canonical.com administrator controller-jimm -user-alice@canonical.com member group-group-1 -user-eve@canonical.com member group-group-2 -group-group-2#member member group-group-3 -group-group-3#member administrator controller-test-controller-1 -group-group-1#member administrator model-test-controller-1:alice@canonical.com/test-model-1 -user-eve@canonical.com administrator applicationoffer-test-controller-1:alice@canonical.com/test-model-1.testoffer1`, - ) } // TODO: remove boilerplate of env setup and use initialiseEnvironment From 5a903321b2ac17549b78208c8891665a70c77be8 Mon Sep 17 00:00:00 2001 From: pkulik0 Date: Tue, 28 May 2024 16:29:13 +0000 Subject: [PATCH 03/13] Add comment about tabular output padding --- cmd/jimmctl/cmd/relation_test.go | 1 + 1 file changed, 1 insertion(+) diff --git a/cmd/jimmctl/cmd/relation_test.go b/cmd/jimmctl/cmd/relation_test.go index 02ad395d5..2a8587fc9 100644 --- a/cmd/jimmctl/cmd/relation_test.go +++ b/cmd/jimmctl/cmd/relation_test.go @@ -383,6 +383,7 @@ user-eve@canonical.com administrator applicationoffer-test-controller-1:alice@ _, err := cmdtesting.RunCommand(c, cmd.NewAddGroupCommandForTesting(s.ClientStore(), bClient), groupName) c.Assert(err, gc.IsNil) + // -72 is used to fill the rest of the line with whitespace as the output is padded to the length of the longest row tabularOutput += fmt.Sprintf("\nuser-eve@canonical.com member group-%-72s", groupName) relations = append(relations, apiparams.RelationshipTuple{ Object: "user-" + env.users[1].Name, From d9e2b53f428201a1e3089808e3b48fe26b4d67b0 Mon Sep 17 00:00:00 2001 From: pkulik0 Date: Wed, 29 May 2024 08:29:18 +0000 Subject: [PATCH 04/13] Add serviceaccount tag to parsing logic, add relations to table function for testing --- cmd/jimmctl/cmd/relation_test.go | 65 +++++++++++++++++--------------- internal/jimm/access.go | 9 +++++ 2 files changed, 43 insertions(+), 31 deletions(-) diff --git a/cmd/jimmctl/cmd/relation_test.go b/cmd/jimmctl/cmd/relation_test.go index 2a8587fc9..57d9bc9a1 100644 --- a/cmd/jimmctl/cmd/relation_test.go +++ b/cmd/jimmctl/cmd/relation_test.go @@ -342,10 +342,29 @@ func initializeEnvironment(c *gc.C, ctx context.Context, db *db.Database, u dbmo return &env } +func relationsToTabularStr(relations []apiparams.RelationshipTuple) string { + objHeader, relHeader, targObjHeader := "Object", "Relation", "Target Object" + objColSize, relColSize, targObjColSize := len(objHeader), len(relHeader), len(targObjHeader) + for _, r := range relations { + objColSize = max(objColSize, len(r.Object)) + relColSize = max(relColSize, len(r.Relation)) + targObjColSize = max(targObjColSize, len(r.TargetObject)) + } + + formatRow := func(object, relation, targetObject string) string { + return fmt.Sprintf("%-*s\t%-*s\t%-*s", objColSize, object, relColSize, relation, targObjColSize, targetObject) + } + tabularStr := formatRow(objHeader, relHeader, targObjHeader) + for _, r := range relations { + tabularStr += "\n" + formatRow(r.Object, r.Relation, r.TargetObject) + } + + return tabularStr +} + func (s *relationSuite) TestListRelations(c *gc.C) { env := initializeEnvironment(c, context.Background(), &s.JIMM.Database, *s.AdminUser) - // alice is superuser - bClient := jimmtest.NewUserSessionLogin(c, "alice") + bClient := jimmtest.NewUserSessionLogin(c, "alice") // alice is superuser relations := []apiparams.RelationshipTuple{{ Object: "user-" + env.users[0].Name, @@ -367,24 +386,17 @@ func (s *relationSuite) TestListRelations(c *gc.C) { Object: "user-" + env.users[1].Name, Relation: "administrator", TargetObject: "applicationoffer-" + env.controllers[0].Name + ":" + env.applicationOffers[0].Model.OwnerIdentityName + "/" + env.applicationOffers[0].Model.Name + "." + env.applicationOffers[0].Name, + }, { + Object: "user-" + env.users[0].Name, + Relation: "administrator", + TargetObject: "serviceaccount-test@serviceaccount", }} - tabularOutput := `Object Relation Target Object -user-admin administrator controller-jimm -user-alice@canonical.com administrator controller-jimm -user-alice@canonical.com member group-group-1 -group-group-2#member member group-group-3 -group-group-3#member administrator controller-test-controller-1 -group-group-1#member administrator model-test-controller-1:alice@canonical.com/test-model-1 -user-eve@canonical.com administrator applicationoffer-test-controller-1:alice@canonical.com/test-model-1.testoffer1` - for i := 0; i < cmd.DefaultPageSize+1; i++ { groupName := fmt.Sprintf("group-%d", i) _, err := cmdtesting.RunCommand(c, cmd.NewAddGroupCommandForTesting(s.ClientStore(), bClient), groupName) c.Assert(err, gc.IsNil) - // -72 is used to fill the rest of the line with whitespace as the output is padded to the length of the longest row - tabularOutput += fmt.Sprintf("\nuser-eve@canonical.com member group-%-72s", groupName) relations = append(relations, apiparams.RelationshipTuple{ Object: "user-" + env.users[1].Name, Relation: "member", @@ -397,11 +409,7 @@ user-eve@canonical.com administrator applicationoffer-test-controller-1:alice@ c.Assert(err, gc.IsNil) } - context, err := cmdtesting.RunCommand(c, cmd.NewListRelationsCommandForTesting(s.ClientStore(), bClient), "--format", "tabular") - c.Assert(err, gc.IsNil) - c.Assert(cmdtesting.Stdout(context), gc.Equals, tabularOutput) - - expectedJSONData, err := json.Marshal(append( + expectedRelations := append( []apiparams.RelationshipTuple{{ Object: "user-admin", Relation: "administrator", @@ -412,24 +420,19 @@ user-eve@canonical.com administrator applicationoffer-test-controller-1:alice@ TargetObject: "controller-jimm", }}, relations..., - )) + ) + + context, err := cmdtesting.RunCommand(c, cmd.NewListRelationsCommandForTesting(s.ClientStore(), bClient), "--format", "tabular") + c.Assert(err, gc.IsNil) + c.Assert(cmdtesting.Stdout(context), gc.Equals, relationsToTabularStr(expectedRelations)) + + expectedJSONData, err := json.Marshal(expectedRelations) c.Assert(err, gc.IsNil) context, err = cmdtesting.RunCommand(c, cmd.NewListRelationsCommandForTesting(s.ClientStore(), bClient), "--format", "json") c.Assert(err, gc.IsNil) c.Assert(strings.TrimRight(cmdtesting.Stdout(context), "\n"), gc.Equals, string(expectedJSONData)) - expectedYAMLData, err := yaml.Marshal(append( - []apiparams.RelationshipTuple{{ - Object: "user-admin", - Relation: "administrator", - TargetObject: "controller-jimm", - }, { - Object: "user-alice@canonical.com", - Relation: "administrator", - TargetObject: "controller-jimm", - }}, - relations..., - )) + expectedYAMLData, err := yaml.Marshal(expectedRelations) c.Assert(err, gc.IsNil) context, err = cmdtesting.RunCommand(c, cmd.NewListRelationsCommandForTesting(s.ClientStore(), bClient)) c.Assert(err, gc.IsNil) diff --git a/internal/jimm/access.go b/internal/jimm/access.go index b02c2b2a2..05454d800 100644 --- a/internal/jimm/access.go +++ b/internal/jimm/access.go @@ -394,6 +394,8 @@ func (j *JIMM) ToJAASTag(ctx context.Context, tag *ofganames.Tag) (string, error switch tag.Kind { case names.UserTagKind: return names.UserTagKind + "-" + tag.ID, nil + case jimmnames.ServiceAccountTagKind: + return jimmnames.ServiceAccountTagKind + "-" + tag.ID, nil case names.ControllerTagKind: if tag.ID == j.ResourceTag().Id() { return "controller-jimm", nil @@ -603,6 +605,13 @@ func resolveTag(jimmUUID string, db *db.Database, tag string) (*ofganames.Tag, e } return ofganames.ConvertTagWithRelation(names.NewApplicationOfferTag(offer.UUID), relation), nil + case jimmnames.ServiceAccountTagKind: + zapctx.Debug( + ctx, + "Resolving JIMM tags to Juju tags for tag kind: serviceaccount", + zap.String("serviceaccount-name", trailer), + ) + return ofganames.ConvertTagWithRelation(jimmnames.NewServiceAccountTag(trailer), relation), nil } return nil, errors.E("failed to map tag " + matches[1]) } From 3f0c0b801d781370fb504a665498ca645d46a034 Mon Sep 17 00:00:00 2001 From: pkulik0 Date: Wed, 29 May 2024 10:02:53 +0000 Subject: [PATCH 05/13] Update Makefile help message --- Makefile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Makefile b/Makefile index c16acd75e..fdc4648c7 100644 --- a/Makefile +++ b/Makefile @@ -130,7 +130,7 @@ help: @echo 'make install - Install the package.' @echo 'make server - Start the JIMM server.' @echo 'make clean - Remove object files from package source directories.' - @echo 'make sysdeps - Install the development environment system packages.' + @echo 'make sys-deps - Install the development environment system packages.' @echo 'make format - Format the source files.' @echo 'make simplify - Format and simplify the source files.' @echo 'make get-local-auth - Get local auth to the API WSS endpoint locally.' From 09ba675406d7badf8665c8132ccf0f2209a99d24 Mon Sep 17 00:00:00 2001 From: pkulik0 Date: Tue, 11 Jun 2024 13:19:30 +0000 Subject: [PATCH 06/13] Remove helper fn, export formatRelationsTabular for tests --- cmd/jimmctl/cmd/export_test.go | 6 ++++++ cmd/jimmctl/cmd/relation_test.go | 25 ++++--------------------- 2 files changed, 10 insertions(+), 21 deletions(-) diff --git a/cmd/jimmctl/cmd/export_test.go b/cmd/jimmctl/cmd/export_test.go index 66d590051..834a29ddd 100644 --- a/cmd/jimmctl/cmd/export_test.go +++ b/cmd/jimmctl/cmd/export_test.go @@ -3,6 +3,8 @@ package cmd import ( + "io" + "github.com/juju/cmd/v3" jujuapi "github.com/juju/juju/api" "github.com/juju/juju/cloud" @@ -321,3 +323,7 @@ func NewMigrateModelCommandForTesting(store jujuclient.ClientStore, lp jujuapi.L return modelcmd.WrapBase(cmd) } + +func FormatRelationsTabularForTesting(writer io.Writer, value interface{}) error { + return formatRelationsTabular(writer, value) +} diff --git a/cmd/jimmctl/cmd/relation_test.go b/cmd/jimmctl/cmd/relation_test.go index 57d9bc9a1..85f73de77 100644 --- a/cmd/jimmctl/cmd/relation_test.go +++ b/cmd/jimmctl/cmd/relation_test.go @@ -342,26 +342,6 @@ func initializeEnvironment(c *gc.C, ctx context.Context, db *db.Database, u dbmo return &env } -func relationsToTabularStr(relations []apiparams.RelationshipTuple) string { - objHeader, relHeader, targObjHeader := "Object", "Relation", "Target Object" - objColSize, relColSize, targObjColSize := len(objHeader), len(relHeader), len(targObjHeader) - for _, r := range relations { - objColSize = max(objColSize, len(r.Object)) - relColSize = max(relColSize, len(r.Relation)) - targObjColSize = max(targObjColSize, len(r.TargetObject)) - } - - formatRow := func(object, relation, targetObject string) string { - return fmt.Sprintf("%-*s\t%-*s\t%-*s", objColSize, object, relColSize, relation, targObjColSize, targetObject) - } - tabularStr := formatRow(objHeader, relHeader, targObjHeader) - for _, r := range relations { - tabularStr += "\n" + formatRow(r.Object, r.Relation, r.TargetObject) - } - - return tabularStr -} - func (s *relationSuite) TestListRelations(c *gc.C) { env := initializeEnvironment(c, context.Background(), &s.JIMM.Database, *s.AdminUser) bClient := jimmtest.NewUserSessionLogin(c, "alice") // alice is superuser @@ -424,7 +404,10 @@ func (s *relationSuite) TestListRelations(c *gc.C) { context, err := cmdtesting.RunCommand(c, cmd.NewListRelationsCommandForTesting(s.ClientStore(), bClient), "--format", "tabular") c.Assert(err, gc.IsNil) - c.Assert(cmdtesting.Stdout(context), gc.Equals, relationsToTabularStr(expectedRelations)) + var builder strings.Builder + err = cmd.FormatRelationsTabularForTesting(&builder, expectedRelations) + c.Assert(err, gc.IsNil) + c.Assert(cmdtesting.Stdout(context), gc.Equals, builder.String()) expectedJSONData, err := json.Marshal(expectedRelations) c.Assert(err, gc.IsNil) From 9d3e4edf0db8cbc24a235749088bbcf3892e8f6b Mon Sep 17 00:00:00 2001 From: pkulik0 Date: Tue, 11 Jun 2024 20:07:32 +0000 Subject: [PATCH 07/13] Merge v3 into CSS 8387 --- cmd/jimmctl/cmd/relation_test.go | 67 +++++++++++++++++++++++++++++++- 1 file changed, 65 insertions(+), 2 deletions(-) diff --git a/cmd/jimmctl/cmd/relation_test.go b/cmd/jimmctl/cmd/relation_test.go index 4c0a4791d..faae4f1cb 100644 --- a/cmd/jimmctl/cmd/relation_test.go +++ b/cmd/jimmctl/cmd/relation_test.go @@ -18,7 +18,6 @@ import ( "github.com/juju/names/v5" gc "gopkg.in/check.v1" yamlv2 "gopkg.in/yaml.v2" - "sigs.k8s.io/yaml" apiparams "github.com/canonical/jimm/api/params" "github.com/canonical/jimm/cmd/jimmctl/cmd" @@ -415,13 +414,77 @@ func (s *relationSuite) TestListRelations(c *gc.C) { c.Assert(err, gc.IsNil) c.Assert(strings.TrimRight(cmdtesting.Stdout(context), "\n"), gc.Equals, string(expectedJSONData)) - expectedYAMLData, err := yaml.Marshal(expectedRelations) + // Necessary to use yamlv2 to match what Juju does. + expectedYAMLData, err := yamlv2.Marshal(expectedRelations) c.Assert(err, gc.IsNil) context, err = cmdtesting.RunCommand(c, cmd.NewListRelationsCommandForTesting(s.ClientStore(), bClient)) c.Assert(err, gc.IsNil) c.Assert(cmdtesting.Stdout(context), gc.Equals, string(expectedYAMLData)) } +func (s *relationSuite) TestListRelationsWithError(c *gc.C) { + env := initializeEnvironment(c, context.Background(), &s.JIMM.Database, *s.AdminUser) + // alice is superuser + bClient := jimmtest.NewUserSessionLogin(c, "alice") + + _, err := cmdtesting.RunCommand(c, cmd.NewAddGroupCommandForTesting(s.ClientStore(), bClient), "group-1") + c.Assert(err, gc.IsNil) + + relation := apiparams.RelationshipTuple{ + Object: "user-" + env.users[0].Name, + Relation: "member", + TargetObject: "group-group-1", + } + _, err = cmdtesting.RunCommand(c, cmd.NewAddRelationCommandForTesting(s.ClientStore(), bClient), relation.Object, relation.Relation, relation.TargetObject) + c.Assert(err, gc.IsNil) + + ctx := context.Background() + group := &dbmodel.GroupEntry{Name: "group-1"} + err = s.JIMM.DB().GetGroup(ctx, group) + c.Assert(err, gc.IsNil) + err = s.JIMM.DB().RemoveGroup(ctx, group) + c.Assert(err, gc.IsNil) + + expectedData := apiparams.ListRelationshipTuplesResponse{ + Tuples: []apiparams.RelationshipTuple{{ + Object: "user-admin", + Relation: "administrator", + TargetObject: "controller-jimm", + }, { + Object: "user-alice@canonical.com", + Relation: "administrator", + TargetObject: "controller-jimm", + }, { + Object: "user-" + env.users[0].Name, + Relation: "member", + TargetObject: "group:" + group.UUID, + }}, + Errors: []string{ + "failed to parse target: failed to fetch group information: " + group.UUID, + }, + } + expectedJSONData, err := json.Marshal(expectedData) + c.Assert(err, gc.IsNil) + // Necessary to use yamlv2 to match what Juju does. + expectedYAMLData, err := yamlv2.Marshal(expectedData) + c.Assert(err, gc.IsNil) + + context, err := cmdtesting.RunCommand(c, cmd.NewListRelationsCommandForTesting(s.ClientStore(), bClient), "--format", "json") + c.Assert(err, gc.IsNil) + c.Assert(strings.TrimRight(cmdtesting.Stdout(context), "\n"), gc.Equals, string(expectedJSONData)) + + context, err = cmdtesting.RunCommand(c, cmd.NewListRelationsCommandForTesting(s.ClientStore(), bClient)) + c.Assert(err, gc.IsNil) + c.Assert(cmdtesting.Stdout(context), gc.Equals, string(expectedYAMLData)) + + context, err = cmdtesting.RunCommand(c, cmd.NewListRelationsCommandForTesting(s.ClientStore(), bClient), "--format", "tabular") + c.Assert(err, gc.IsNil) + var builder strings.Builder + err = cmd.FormatRelationsTabularForTesting(&builder, expectedData) + c.Assert(err, gc.IsNil) + c.Assert(cmdtesting.Stdout(context), gc.Equals, builder.String()) +} + // TODO: remove boilerplate of env setup and use initialiseEnvironment func (s *relationSuite) TestCheckRelationViaSuperuser(c *gc.C) { ctx := context.TODO() From bae9a7f03edc0cd4e93ab5dcc1f8e8a0f67fc931 Mon Sep 17 00:00:00 2001 From: pkulik0 Date: Tue, 11 Jun 2024 20:56:05 +0000 Subject: [PATCH 08/13] Change fetchRelations to a loop based approach --- cmd/jimmctl/cmd/relation.go | 20 +++++++++++--------- 1 file changed, 11 insertions(+), 9 deletions(-) diff --git a/cmd/jimmctl/cmd/relation.go b/cmd/jimmctl/cmd/relation.go index 915238011..3cec09c0b 100644 --- a/cmd/jimmctl/cmd/relation.go +++ b/cmd/jimmctl/cmd/relation.go @@ -560,19 +560,21 @@ func (c *listRelationsCommand) Run(ctxt *cmd.Context) error { } func fetchRelations(client *api.Client, params apiparams.ListRelationshipTuplesRequest) (*apiparams.ListRelationshipTuplesResponse, error) { - response, err := client.ListRelationshipTuples(¶ms) - if err != nil { - return nil, errors.E(err) - } - if response.ContinuationToken != "" { - params.ContinuationToken = response.ContinuationToken - response1, err := fetchRelations(client, params) + tuples := make([]apiparams.RelationshipTuple, 0) + for { + response, err := client.ListRelationshipTuples(¶ms) if err != nil { return nil, errors.E(err) } - response.Tuples = append(response.Tuples, response1.Tuples...) + + tuples = append(response.Tuples, tuples...) + + if response.ContinuationToken == "" { + response.Tuples = tuples + return response, nil + } + params.ContinuationToken = response.ContinuationToken } - return response, nil } func formatRelationsTabular(writer io.Writer, value interface{}) error { From 45602ea1b66dbd1b8aabc457b4ee48a4cab2587f Mon Sep 17 00:00:00 2001 From: pkulik0 Date: Thu, 13 Jun 2024 05:21:44 +0000 Subject: [PATCH 09/13] Fix type of expectedData & order of paginated relations --- cmd/jimmctl/cmd/relation.go | 3 +-- cmd/jimmctl/cmd/relation_test.go | 12 ++++++------ 2 files changed, 7 insertions(+), 8 deletions(-) diff --git a/cmd/jimmctl/cmd/relation.go b/cmd/jimmctl/cmd/relation.go index 3cec09c0b..9435c06d9 100644 --- a/cmd/jimmctl/cmd/relation.go +++ b/cmd/jimmctl/cmd/relation.go @@ -566,8 +566,7 @@ func fetchRelations(client *api.Client, params apiparams.ListRelationshipTuplesR if err != nil { return nil, errors.E(err) } - - tuples = append(response.Tuples, tuples...) + tuples = append(tuples, response.Tuples...) if response.ContinuationToken == "" { response.Tuples = tuples diff --git a/cmd/jimmctl/cmd/relation_test.go b/cmd/jimmctl/cmd/relation_test.go index faae4f1cb..dbba0ebba 100644 --- a/cmd/jimmctl/cmd/relation_test.go +++ b/cmd/jimmctl/cmd/relation_test.go @@ -388,7 +388,7 @@ func (s *relationSuite) TestListRelations(c *gc.C) { c.Assert(err, gc.IsNil) } - expectedRelations := append( + expectedData := apiparams.ListRelationshipTuplesResponse{Tuples: append( []apiparams.RelationshipTuple{{ Object: "user-admin", Relation: "administrator", @@ -399,23 +399,23 @@ func (s *relationSuite) TestListRelations(c *gc.C) { TargetObject: "controller-jimm", }}, relations..., - ) + )} context, err := cmdtesting.RunCommand(c, cmd.NewListRelationsCommandForTesting(s.ClientStore(), bClient), "--format", "tabular") c.Assert(err, gc.IsNil) var builder strings.Builder - err = cmd.FormatRelationsTabularForTesting(&builder, expectedRelations) + err = cmd.FormatRelationsTabularForTesting(&builder, &expectedData) c.Assert(err, gc.IsNil) c.Assert(cmdtesting.Stdout(context), gc.Equals, builder.String()) - expectedJSONData, err := json.Marshal(expectedRelations) + expectedJSONData, err := json.Marshal(expectedData) c.Assert(err, gc.IsNil) context, err = cmdtesting.RunCommand(c, cmd.NewListRelationsCommandForTesting(s.ClientStore(), bClient), "--format", "json") c.Assert(err, gc.IsNil) c.Assert(strings.TrimRight(cmdtesting.Stdout(context), "\n"), gc.Equals, string(expectedJSONData)) // Necessary to use yamlv2 to match what Juju does. - expectedYAMLData, err := yamlv2.Marshal(expectedRelations) + expectedYAMLData, err := yamlv2.Marshal(expectedData) c.Assert(err, gc.IsNil) context, err = cmdtesting.RunCommand(c, cmd.NewListRelationsCommandForTesting(s.ClientStore(), bClient)) c.Assert(err, gc.IsNil) @@ -480,7 +480,7 @@ func (s *relationSuite) TestListRelationsWithError(c *gc.C) { context, err = cmdtesting.RunCommand(c, cmd.NewListRelationsCommandForTesting(s.ClientStore(), bClient), "--format", "tabular") c.Assert(err, gc.IsNil) var builder strings.Builder - err = cmd.FormatRelationsTabularForTesting(&builder, expectedData) + err = cmd.FormatRelationsTabularForTesting(&builder, &expectedData) c.Assert(err, gc.IsNil) c.Assert(cmdtesting.Stdout(context), gc.Equals, builder.String()) } From deb676afb3e1570ed4e2a7d536597898555ffff6 Mon Sep 17 00:00:00 2001 From: pkulik0 Date: Thu, 13 Jun 2024 13:12:22 +0000 Subject: [PATCH 10/13] Add test cases for ToJAASTag, make the way fetchRelations returns tuples clearer --- cmd/jimmctl/cmd/relation.go | 3 +-- internal/jujuapi/access_control_test.go | 10 +++++++++- 2 files changed, 10 insertions(+), 3 deletions(-) diff --git a/cmd/jimmctl/cmd/relation.go b/cmd/jimmctl/cmd/relation.go index 9435c06d9..554a4a670 100644 --- a/cmd/jimmctl/cmd/relation.go +++ b/cmd/jimmctl/cmd/relation.go @@ -569,8 +569,7 @@ func fetchRelations(client *api.Client, params apiparams.ListRelationshipTuplesR tuples = append(tuples, response.Tuples...) if response.ContinuationToken == "" { - response.Tuples = tuples - return response, nil + return &apiparams.ListRelationshipTuplesResponse{Tuples: tuples, Errors: response.Errors}, nil } params.ContinuationToken = response.ContinuationToken } diff --git a/internal/jujuapi/access_control_test.go b/internal/jujuapi/access_control_test.go index b5327f9af..074951ddc 100644 --- a/internal/jujuapi/access_control_test.go +++ b/internal/jujuapi/access_control_test.go @@ -22,6 +22,7 @@ import ( "github.com/canonical/jimm/internal/jujuapi" "github.com/canonical/jimm/internal/openfga" ofganames "github.com/canonical/jimm/internal/openfga/names" + "github.com/canonical/jimm/pkg/names" ) type accessControlSuite struct { @@ -792,7 +793,8 @@ func (s *accessControlSuite) TestJAASTag(c *gc.C) { ctx := context.Background() db := s.JIMM.Database - user, _, controller, model, applicationOffer, cloud, _, _, closeClient := createTestControllerEnvironment(ctx, c, s) + user, group, controller, model, applicationOffer, cloud, _, _, closeClient := createTestControllerEnvironment(ctx, c, s) + serviceAccountId := petname.Generate(2, "-") + "@serviceaccount" closeClient() tests := []struct { @@ -802,6 +804,12 @@ func (s *accessControlSuite) TestJAASTag(c *gc.C) { }{{ tag: ofganames.ConvertTag(user.ResourceTag()), expectedJAASTag: "user-" + user.Name, + }, { + tag: ofganames.ConvertTag(names.NewServiceAccountTag(serviceAccountId)), + expectedJAASTag: "serviceaccount-" + serviceAccountId, + }, { + tag: ofganames.ConvertTag(group.ResourceTag()), + expectedJAASTag: "group-" + group.Name, }, { tag: ofganames.ConvertTag(controller.ResourceTag()), expectedJAASTag: "controller-" + controller.Name, From 97b0cfb406358a08dc3b9f4a60d3fd8517346271 Mon Sep 17 00:00:00 2001 From: pkulik0 Date: Thu, 13 Jun 2024 13:15:38 +0000 Subject: [PATCH 11/13] Rename remaining instances of `sysdeps` to `sys-deps` --- Makefile | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/Makefile b/Makefile index 0119bbc40..069968987 100644 --- a/Makefile +++ b/Makefile @@ -32,14 +32,14 @@ clean: certs: @cd local/traefik/certs; ./certs.sh; cd - -test-env: sysdeps certs +test-env: sys-deps certs @touch ./local/vault/approle.json && touch ./local/vault/roleid.txt && touch ./local/vault/vault.env @docker compose up --force-recreate -d --wait test-env-cleanup: @docker compose down -v --remove-orphans -dev-env-setup: sysdeps certs +dev-env-setup: sys-deps certs @touch ./local/vault/approle.json && touch ./local/vault/roleid.txt && touch ./local/vault/vault.env @make version/commit.txt && make version/version.txt @@ -121,7 +121,7 @@ ifeq ($(APT_BASED),0) @$(call check_dep,docker,Missing Docker - install from https://docs.docker.com/engine/install/') @$(call check_dep,juju-db.mongo,Missing juju-db - install with 'sudo snap install juju-db --channel=4.4/stable') else - @echo sysdeps runs only on systems with apt-get + @echo sys-deps runs only on systems with apt-get @echo on OS X with homebrew try: brew install bazaar mongodb endif @@ -137,6 +137,6 @@ help: @echo 'make simplify - Format and simplify the source files.' @echo 'make get-local-auth - Get local auth to the API WSS endpoint locally.' -.PHONY: build check install release clean format server simplify sysdeps help FORCE +.PHONY: build check install release clean format server simplify sys-deps help FORCE FORCE: From bf4fa4331f9bf7f00ba7e250267ef0a9b05e37f8 Mon Sep 17 00:00:00 2001 From: pkulik0 Date: Thu, 13 Jun 2024 13:18:39 +0000 Subject: [PATCH 12/13] Add error message after a failed fetch of relationship tuples --- cmd/jimmctl/cmd/relation.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cmd/jimmctl/cmd/relation.go b/cmd/jimmctl/cmd/relation.go index 554a4a670..2eb6032cb 100644 --- a/cmd/jimmctl/cmd/relation.go +++ b/cmd/jimmctl/cmd/relation.go @@ -564,7 +564,7 @@ func fetchRelations(client *api.Client, params apiparams.ListRelationshipTuplesR for { response, err := client.ListRelationshipTuples(¶ms) if err != nil { - return nil, errors.E(err) + return nil, errors.E(fmt.Sprintf("failed to fetch list of relationship tuples: %s", err.Error())) } tuples = append(tuples, response.Tuples...) From fd4638de49487cf0962318500184517541f8e098 Mon Sep 17 00:00:00 2001 From: pkulik0 Date: Thu, 13 Jun 2024 13:20:44 +0000 Subject: [PATCH 13/13] Simplify export of `FormatRelationsTabular` for tests --- cmd/jimmctl/cmd/export_test.go | 15 +++++---------- cmd/jimmctl/cmd/relation_test.go | 4 ++-- 2 files changed, 7 insertions(+), 12 deletions(-) diff --git a/cmd/jimmctl/cmd/export_test.go b/cmd/jimmctl/cmd/export_test.go index 834a29ddd..e32b9ff94 100644 --- a/cmd/jimmctl/cmd/export_test.go +++ b/cmd/jimmctl/cmd/export_test.go @@ -3,8 +3,6 @@ package cmd import ( - "io" - "github.com/juju/cmd/v3" jujuapi "github.com/juju/juju/api" "github.com/juju/juju/cloud" @@ -13,10 +11,11 @@ import ( ) var ( - AccessMessage = accessMessageFormat - AccessResultAllowed = accessResultAllowed - AccessResultDenied = accessResultDenied - DefaultPageSize = defaultPageSize + AccessMessage = accessMessageFormat + AccessResultAllowed = accessResultAllowed + AccessResultDenied = accessResultDenied + DefaultPageSize = defaultPageSize + FormatRelationsTabular = formatRelationsTabular ) type AccessResult = accessResult @@ -323,7 +322,3 @@ func NewMigrateModelCommandForTesting(store jujuclient.ClientStore, lp jujuapi.L return modelcmd.WrapBase(cmd) } - -func FormatRelationsTabularForTesting(writer io.Writer, value interface{}) error { - return formatRelationsTabular(writer, value) -} diff --git a/cmd/jimmctl/cmd/relation_test.go b/cmd/jimmctl/cmd/relation_test.go index dbba0ebba..1988d7e06 100644 --- a/cmd/jimmctl/cmd/relation_test.go +++ b/cmd/jimmctl/cmd/relation_test.go @@ -404,7 +404,7 @@ func (s *relationSuite) TestListRelations(c *gc.C) { context, err := cmdtesting.RunCommand(c, cmd.NewListRelationsCommandForTesting(s.ClientStore(), bClient), "--format", "tabular") c.Assert(err, gc.IsNil) var builder strings.Builder - err = cmd.FormatRelationsTabularForTesting(&builder, &expectedData) + err = cmd.FormatRelationsTabular(&builder, &expectedData) c.Assert(err, gc.IsNil) c.Assert(cmdtesting.Stdout(context), gc.Equals, builder.String()) @@ -480,7 +480,7 @@ func (s *relationSuite) TestListRelationsWithError(c *gc.C) { context, err = cmdtesting.RunCommand(c, cmd.NewListRelationsCommandForTesting(s.ClientStore(), bClient), "--format", "tabular") c.Assert(err, gc.IsNil) var builder strings.Builder - err = cmd.FormatRelationsTabularForTesting(&builder, &expectedData) + err = cmd.FormatRelationsTabular(&builder, &expectedData) c.Assert(err, gc.IsNil) c.Assert(cmdtesting.Stdout(context), gc.Equals, builder.String()) }