Merge pull request #1213 from alesstimec/group-uuid

Adds UUID to groups

api/params/params.go

@@ -294,6 +294,7 @@ type RemoveGroupRequest struct {
 
 // Group holds the details of a group currently residing in JIMM.
 type Group struct {
+	UUID      string `json:"uuid"`
 	Name      string `json:"name"`
 	CreatedAt string `json:"created_at"`
 	UpdatedAt string `json:"updated_at"`

cmd/jaas/cmd/grant_test.go

@@ -4,6 +4,7 @@ package cmd_test
 
 import (
 	"context"
+	"fmt"
 
 	"github.com/juju/cmd/v3/cmdtesting"
 	"github.com/juju/names/v5"
@@ -51,6 +52,12 @@ func (s *grantSuite) TestGrant(c *gc.C) {
 	err = s.JIMM.Database.AddGroup(ctx, "1")
 	c.Assert(err, gc.IsNil)
 
+	group := dbmodel.GroupEntry{
+		Name: "1",
+	}
+	err = s.JIMM.Database.GetGroup(ctx, &group)
+	c.Assert(err, gc.IsNil)
+
 	cmdContext, err := cmdtesting.RunCommand(c, cmd.NewGrantCommandForTesting(s.ClientStore(), bClient), clientID, "user-bob", "group-1")
 	c.Assert(err, gc.IsNil)
 	c.Assert(cmdtesting.Stdout(cmdContext), gc.Equals, "access granted\n")
@@ -64,7 +71,7 @@ func (s *grantSuite) TestGrant(c *gc.C) {
 	c.Assert(ok, gc.Equals, true)
 
 	ok, err = s.JIMM.OpenFGAClient.CheckRelation(ctx, openfga.Tuple{
-		Object:   ofganames.ConvertTag(jimmnames.NewGroupTag("1#member")),
+		Object:   ofganames.ConvertTag(jimmnames.NewGroupTag(fmt.Sprintf("%s#member", group.UUID))),
 		Relation: ofganames.AdministratorRelation,
 		Target:   ofganames.ConvertTag(jimmnames.NewServiceAccountTag(clientIdWithDomain)),
 	}, false)

cmd/jimmctl/cmd/group_test.go

@@ -33,6 +33,7 @@ func (s *groupSuite) TestAddGroupSuperuser(c *gc.C) {
 	c.Assert(err, gc.IsNil)
 	c.Assert(group.ID, gc.Equals, uint(1))
 	c.Assert(group.Name, gc.Equals, "test-group")
+	c.Assert(group.UUID, gc.Not(gc.Equals), "")
 }
 
 func (s *groupSuite) TestAddGroup(c *gc.C) {

internal/db/export_test.go

@@ -9,4 +9,5 @@ var (
 	JwksExpiryTag     = jwksExpiryTag
 	OAuthKind         = oauthKind
 	OAuthKeyTag       = oauthKeyTag
+	NewUUID           = &newUUID
 )

internal/db/group.go

@@ -5,10 +5,16 @@ package db
 import (
 	"context"
 
+	"github.com/google/uuid"
+
 	"github.com/canonical/jimm/internal/dbmodel"
 	"github.com/canonical/jimm/internal/errors"
 )
 
+var newUUID = func() string {
+	return uuid.NewString()
+}
+
 // AddGroup adds a new group.
 func (d *Database) AddGroup(ctx context.Context, name string) error {
 	const op = errors.Op("db.AddGroup")
@@ -17,6 +23,7 @@ func (d *Database) AddGroup(ctx context.Context, name string) error {
 	}
 	ge := dbmodel.GroupEntry{
 		Name: name,
+		UUID: newUUID(),
 	}
 
 	if err := d.DB.WithContext(ctx).Create(&ge).Error; err != nil {
@@ -36,6 +43,9 @@ func (d *Database) GetGroup(ctx context.Context, group *dbmodel.GroupEntry) erro
 	if group.ID != 0 {
 		db = db.Where("id = ?", group.ID)
 	}
+	if group.UUID != "" {
+		db = db.Where("uuid = ?", group.UUID)
+	}
 	if group.Name != "" {
 		db = db.Where("name = ?", group.Name)
 	}
@@ -83,6 +93,9 @@ func (d *Database) UpdateGroup(ctx context.Context, group *dbmodel.GroupEntry) e
 	if group.ID == 0 {
 		return errors.E(errors.CodeNotFound)
 	}
+	if group.UUID == "" {
+		return errors.E("group uuid not specified", errors.CodeNotFound)
+	}
 	if err := d.DB.WithContext(ctx).Save(group).Error; err != nil {
 		return errors.E(op, dbError(err))
 	}
@@ -98,6 +111,9 @@ func (d *Database) RemoveGroup(ctx context.Context, group *dbmodel.GroupEntry) e
 	if group.ID == 0 {
 		return errors.E(errors.CodeNotFound)
 	}
+	if group.UUID == "" {
+		return errors.E(errors.CodeNotFound)
+	}
 	if err := d.DB.WithContext(ctx).Delete(group).Error; err != nil {
 		return errors.E(op, dbError(err))
 	}

internal/db/group_test.go

@@ -7,6 +7,7 @@ import (
 	"testing"
 
 	qt "github.com/frankban/quicktest"
+	"github.com/google/uuid"
 
 	"github.com/canonical/jimm/internal/db"
 	"github.com/canonical/jimm/internal/dbmodel"
@@ -25,6 +26,11 @@ func TestAddGroupUnconfiguredDatabase(t *testing.T) {
 func (s *dbSuite) TestAddGroup(c *qt.C) {
 	ctx := context.Background()
 
+	uuid := uuid.NewString()
+	c.Patch(db.NewUUID, func() string {
+		return uuid
+	})
+
 	err := s.Database.AddGroup(ctx, "test-group")
 	c.Check(errors.ErrorCode(err), qt.Equals, errors.CodeUpgradeInProgress)
 
@@ -44,9 +50,15 @@ func (s *dbSuite) TestAddGroup(c *qt.C) {
 	c.Assert(tx.Error, qt.IsNil)
 	c.Assert(ge.ID, qt.Equals, uint(1))
 	c.Assert(ge.Name, qt.Equals, "test-group")
+	c.Assert(ge.UUID, qt.Equals, uuid)
 }
 
 func (s *dbSuite) TestGetGroup(c *qt.C) {
+	uuid1 := uuid.NewString()
+	c.Patch(db.NewUUID, func() string {
+		return uuid1
+	})
+
 	err := s.Database.GetGroup(context.Background(), &dbmodel.GroupEntry{
 		Name: "test-group",
 	})
@@ -68,6 +80,12 @@ func (s *dbSuite) TestGetGroup(c *qt.C) {
 	c.Check(err, qt.IsNil)
 	c.Assert(group.ID, qt.Equals, uint(1))
 	c.Assert(group.Name, qt.Equals, "test-group")
+	c.Assert(group.UUID, qt.Equals, uuid1)
+
+	uuid2 := uuid.NewString()
+	c.Patch(db.NewUUID, func() string {
+		return uuid2
+	})
 
 	err = s.Database.AddGroup(context.Background(), "test-group1")
 	c.Assert(err, qt.IsNil)
@@ -80,6 +98,7 @@ func (s *dbSuite) TestGetGroup(c *qt.C) {
 	c.Check(err, qt.IsNil)
 	c.Assert(group.ID, qt.Equals, uint(2))
 	c.Assert(group.Name, qt.Equals, "test-group1")
+	c.Assert(group.UUID, qt.Equals, uuid2)
 }
 
 func (s *dbSuite) TestUpdateGroup(c *qt.C) {

internal/dbmodel/group.go

@@ -3,7 +3,6 @@
 package dbmodel
 
 import (
-	"strconv"
 	"time"
 
 	"github.com/juju/names/v5"
@@ -19,12 +18,16 @@ type GroupEntry struct {
 
 	// Name holds the name of the group.
 	Name string `gorm:"index;column:name"`
+
+	// UUID holds the uuid of the group.
+	UUID string `gotm:"index;column:uuid"`
 }
 
 // ToAPIGroup converts a group entry to a JIMM API
 // Group.
 func (g GroupEntry) ToAPIGroupEntry() apiparams.Group {
 	var group apiparams.Group
+	group.UUID = g.UUID
 	group.Name = g.Name
 	group.CreatedAt = g.CreatedAt.Format(time.RFC3339)
 	group.UpdatedAt = g.UpdatedAt.Format(time.RFC3339)
@@ -47,5 +50,5 @@ func (g *GroupEntry) Tag() names.Tag {
 // a concrete type names.GroupTag instead of the
 // names.Tag interface.
 func (g *GroupEntry) ResourceTag() jimmnames.GroupTag {
-	return jimmnames.NewGroupTag(strconv.Itoa(int(g.ID)))
+	return jimmnames.NewGroupTag(g.UUID)
 }

internal/dbmodel/sql/postgres/1_7.sql

@@ -0,0 +1,4 @@
+-- 1_7.sql is a migration that adds a UUID column to the identity table.
+ALTER TABLE groups ADD COLUMN uuid TEXT NOT NULL UNIQUE;
+
+UPDATE versions SET major=1, minor=7 WHERE component='jimmdb';

internal/dbmodel/version.go

@@ -20,7 +20,7 @@ const (
 	// Minor is the minor version of the model described in the dbmodel
 	// package. It should be incremented for any change made to the
 	// database model from database model in a released JIMM.
-	Minor = 6
+	Minor = 7
 )
 
 type Version struct {

internal/jimm/access.go

@@ -7,7 +7,6 @@ import (
 	"database/sql"
 	"fmt"
 	"regexp"
-	"strconv"
 	"strings"
 	"sync"
 
@@ -18,7 +17,6 @@ import (
 	"github.com/juju/zaputil"
 	"github.com/juju/zaputil/zapctx"
 	"go.uber.org/zap"
-	"gorm.io/gorm"
 
 	"github.com/canonical/jimm/internal/db"
 	"github.com/canonical/jimm/internal/dbmodel"
@@ -442,16 +440,10 @@ func (j *JIMM) ToJAASTag(ctx context.Context, tag *ofganames.Tag) (string, error
 		}
 		return aoString, nil
 	case jimmnames.GroupTagKind:
-		id, err := strconv.ParseUint(tag.ID, 10, 32)
-		if err != nil {
-			return "", errors.E(err, fmt.Sprintf("failed to parse group id: %v", tag.ID))
-		}
 		group := dbmodel.GroupEntry{
-			Model: gorm.Model{
-				ID: uint(id),
-			},
+			UUID: tag.ID,
 		}
-		err = j.Database.GetGroup(ctx, &group)
+		err := j.Database.GetGroup(ctx, &group)
 		if err != nil {
 			return "", errors.E(err, "failed to fetch group information")
 		}
@@ -532,7 +524,7 @@ func resolveTag(jimmUUID string, db *db.Database, tag string) (*ofganames.Tag, e
 		if err != nil {
 			return nil, errors.E(fmt.Sprintf("group %s not found", trailer))
 		}
-		return ofganames.ConvertTagWithRelation(jimmnames.NewGroupTag(strconv.FormatUint(uint64(entry.ID), 10)), relation), nil
+		return ofganames.ConvertTagWithRelation(jimmnames.NewGroupTag(entry.UUID), relation), nil
 
 	case names.ControllerTagKind:
 		zapctx.Debug(

internal/jimm/access_test.go

@@ -638,7 +638,7 @@ func TestResolveTupleObjectMapsGroups(t *testing.T) {
 	c.Assert(err, qt.IsNil)
 	tag, err := jimm.ResolveTag(j.UUID, &j.Database, "group-"+group.Name+"#member")
 	c.Assert(err, qt.IsNil)
-	c.Assert(tag, qt.DeepEquals, ofganames.ConvertTagWithRelation(jimmnames.NewGroupTag("1"), ofganames.MemberRelation))
+	c.Assert(tag, qt.DeepEquals, ofganames.ConvertTagWithRelation(jimmnames.NewGroupTag(group.UUID), ofganames.MemberRelation))
 }
 
 func TestResolveTagObjectMapsUsers(t *testing.T) {