Tweak Identity APIs (#174)

If we have an empty list of say group IDs for a user, then the UI need
to fill this in so it can bind to it, and then yout still get a bunch of
if statements that clutter up the code.  What would be better is to
return an empty array so it's already setup.  Cleaner code at the
expense of a few extra bytes on the wire.  This should be backwards
compatible with existing code as it's just making things that are
already handled required rather than optional.

charts/identity/Chart.yaml

@@ -4,8 +4,8 @@ description: A Helm chart for deploying Unikorn's IdP
 
 type: application
 
-version: v0.2.53-rc6
-appVersion: v0.2.53-rc6
+version: v0.2.53-rc7
+appVersion: v0.2.53-rc7
 
 icon: https://raw.githubusercontent.com/unikorn-cloud/assets/main/images/logos/dark-on-light/icon.png
 

pkg/handler/groups/client.go

@@ -53,16 +53,22 @@ func convert(in *unikornv1.Group) *openapi.GroupRead {
 	out := &openapi.GroupRead{
 		Metadata: conversion.OrganizationScopedResourceReadMetadata(in, in.Spec.Tags, coreopenapi.ResourceProvisioningStatusProvisioned),
 		Spec: openapi.GroupSpec{
-			RoleIDs: in.Spec.RoleIDs,
+			RoleIDs:           openapi.StringList{},
+			UserIDs:           openapi.StringList{},
+			ServiceAccountIDs: openapi.StringList{},
 		},
 	}
 
-	if len(in.Spec.UserIDs) > 0 {
-		out.Spec.UserIDs = &in.Spec.UserIDs
+	if in.Spec.RoleIDs != nil {
+		out.Spec.RoleIDs = in.Spec.RoleIDs
 	}
 
-	if len(in.Spec.ServiceAccountIDs) > 0 {
-		out.Spec.ServiceAccountIDs = &in.Spec.ServiceAccountIDs
+	if in.Spec.UserIDs != nil {
+		out.Spec.UserIDs = in.Spec.UserIDs
+	}
+
+	if in.Spec.ServiceAccountIDs != nil {
+		out.Spec.ServiceAccountIDs = in.Spec.ServiceAccountIDs
 	}
 
 	return out
@@ -156,19 +162,13 @@ func (c *Client) generate(ctx context.Context, organization *organizations.Meta,
 	out := &unikornv1.Group{
 		ObjectMeta: conversion.NewObjectMetadata(&in.Metadata, organization.Namespace, info.Userinfo.Sub).WithOrganization(organization.ID).Get(),
 		Spec: unikornv1.GroupSpec{
-			Tags:    conversion.GenerateTagList(in.Metadata.Tags),
-			RoleIDs: in.Spec.RoleIDs,
+			Tags:              conversion.GenerateTagList(in.Metadata.Tags),
+			RoleIDs:           in.Spec.RoleIDs,
+			UserIDs:           in.Spec.UserIDs,
+			ServiceAccountIDs: in.Spec.ServiceAccountIDs,
 		},
 	}
 
-	if in.Spec.UserIDs != nil {
-		out.Spec.UserIDs = *in.Spec.UserIDs
-	}
-
-	if in.Spec.ServiceAccountIDs != nil {
-		out.Spec.ServiceAccountIDs = *in.Spec.ServiceAccountIDs
-	}
-
 	return out, nil
 }
 

pkg/handler/projects/client.go

@@ -62,10 +62,13 @@ func convert(in *unikornv1.Project) *openapi.ProjectRead {
 
 	out := &openapi.ProjectRead{
 		Metadata: conversion.OrganizationScopedResourceReadMetadata(in, in.Spec.Tags, provisioningStatus),
+		Spec: openapi.ProjectSpec{
+			GroupIDs: openapi.GroupIDs{},
+		},
 	}
 
 	if in.Spec.GroupIDs != nil {
-		out.Spec.GroupIDs = &in.Spec.GroupIDs
+		out.Spec.GroupIDs = in.Spec.GroupIDs
 	}
 
 	return out
@@ -136,24 +139,22 @@ func (c *Client) generate(ctx context.Context, organization *organizations.Meta,
 
 	out := &unikornv1.Project{
 		ObjectMeta: conversion.NewObjectMetadata(&in.Metadata, organization.Namespace, info.Userinfo.Sub).WithOrganization(organization.ID).Get(),
+		Spec: unikornv1.ProjectSpec{
+			Tags:     conversion.GenerateTagList(in.Metadata.Tags),
+			GroupIDs: in.Spec.GroupIDs,
+		},
 	}
 
-	out.Spec.Tags = conversion.GenerateTagList(in.Metadata.Tags)
-
-	if in.Spec.GroupIDs != nil {
-		for _, groupID := range *in.Spec.GroupIDs {
-			var resource unikornv1.Group
+	for _, groupID := range in.Spec.GroupIDs {
+		var resource unikornv1.Group
 
-			if err := c.client.Get(ctx, client.ObjectKey{Namespace: organization.Namespace, Name: groupID}, &resource); err != nil {
-				if kerrors.IsNotFound(err) {
-					return nil, errors.OAuth2InvalidRequest(fmt.Sprintf("group ID %s does not exist", groupID)).WithError(err)
-				}
-
-				return nil, errors.OAuth2ServerError("failed to validate group ID").WithError(err)
+		if err := c.client.Get(ctx, client.ObjectKey{Namespace: organization.Namespace, Name: groupID}, &resource); err != nil {
+			if kerrors.IsNotFound(err) {
+				return nil, errors.OAuth2InvalidRequest(fmt.Sprintf("group ID %s does not exist", groupID)).WithError(err)
 			}
-		}
 
-		out.Spec.GroupIDs = *in.Spec.GroupIDs
+			return nil, errors.OAuth2ServerError("failed to validate group ID").WithError(err)
+		}
 	}
 
 	return out, nil

pkg/handler/serviceaccounts/client.go

@@ -78,31 +78,18 @@ func New(client client.Client, namespace, host string, oauth2 *oauth2.Authentica
 func convert(in *unikornv1.ServiceAccount, groups *unikornv1.GroupList) *openapi.ServiceAccountRead {
 	out := &openapi.ServiceAccountRead{
 		Metadata: conversion.OrganizationScopedResourceReadMetadata(in, in.Spec.Tags, coreopenapi.ResourceProvisioningStatusProvisioned),
+		Spec: openapi.ServiceAccountSpec{
+			GroupIDs: make(openapi.GroupIDs, 0, len(groups.Items)),
+		},
 		Status: openapi.ServiceAccountStatus{
 			Expiry: in.Spec.Expiry.Time,
 		},
 	}
 
-	// NOTE: Deep copy as this may be reused and DeleteFunc will modify the underlying
-	// slice's array.
-	memberGroups := groups.DeepCopy()
-
-	memberGroups.Items = slices.DeleteFunc(memberGroups.Items, func(group unikornv1.Group) bool {
-		return !slices.Contains(group.Spec.ServiceAccountIDs, in.Name)
-	})
-
-	var memberGroupIDs openapi.GroupIDs
-
-	for _, group := range memberGroups.Items {
-		memberGroupIDs = append(memberGroupIDs, group.Name)
-	}
-
-	if len(memberGroupIDs) > 0 {
-		if out.Spec == nil {
-			out.Spec = &openapi.ServiceAccountSpec{}
+	for _, group := range groups.Items {
+		if slices.Contains(group.Spec.ServiceAccountIDs, in.Name) {
+			out.Spec.GroupIDs = append(out.Spec.GroupIDs, group.Name)
 		}
-
-		out.Spec.GroupIDs = &memberGroupIDs
 	}
 
 	return out
@@ -227,13 +214,13 @@ func (c *Client) listGroups(ctx context.Context, organization *organizations.Met
 
 // updateGroups takes a user name and a requested list of groups and adds to
 // the groups it should be a member of and removes itself from groups it shouldn't.
-func (c *Client) updateGroups(ctx context.Context, serviceAccountID string, groupIDs *openapi.GroupIDs, groups *unikornv1.GroupList) error {
+func (c *Client) updateGroups(ctx context.Context, serviceAccountID string, groupIDs openapi.GroupIDs, groups *unikornv1.GroupList) error {
 	for i := range groups.Items {
 		current := &groups.Items[i]
 
 		updated := current.DeepCopy()
 
-		if groupIDs != nil && slices.Contains(*groupIDs, current.Name) {
+		if slices.Contains(groupIDs, current.Name) {
 			// Add to a group where it should be a member but isn't.
 			if slices.Contains(current.Spec.ServiceAccountIDs, serviceAccountID) {
 				continue

pkg/handler/users/client.go

@@ -219,8 +219,9 @@ func convert(in *unikornv1.User, groups *unikornv1.GroupList) *openapi.UserRead
 	out := &openapi.UserRead{
 		Metadata: conversion.OrganizationScopedResourceReadMetadata(in, in.Spec.Tags, coreopenapi.ResourceProvisioningStatusProvisioned),
 		Spec: openapi.UserSpec{
-			Subject: in.Spec.Subject,
-			State:   convertUserState(in.Spec.State),
+			Subject:  in.Spec.Subject,
+			State:    convertUserState(in.Spec.State),
+			GroupIDs: make(openapi.GroupIDs, 0, len(groups.Items)),
 		},
 	}