Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

chore(sdk): introduce User type separate from UserID type #152

Merged
merged 6 commits into from
Feb 20, 2025
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion internal/controller/accesskey/accesskey.go
Original file line number Diff line number Diff line change
Expand Up @@ -187,7 +187,7 @@ func (c *external) Create(ctx context.Context, mg resource.Managed) (managed.Ext

cr.SetConditions(xpv1.Creating())

creds, err := c.cloudianService.CreateUserCredentials(ctx, cloudian.User{
creds, err := c.cloudianService.CreateUserCredentials(ctx, cloudian.UserID{
GroupID: cr.Spec.ForProvider.GroupID,
UserID: cr.Spec.ForProvider.UserID,
})
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -151,7 +151,7 @@ func (c *external) Observe(ctx context.Context, mg resource.Managed) (managed.Ex
return managed.ExternalObservation{}, nil
}

user := cloudian.User{
user := cloudian.UserID{
GroupID: groupID,
UserID: "*",
}
Expand Down Expand Up @@ -200,7 +200,7 @@ func (c *external) Create(ctx context.Context, mg resource.Managed) (managed.Ext
return managed.ExternalCreation{}, err
}

user := cloudian.User{
user := cloudian.UserID{
GroupID: cr.Spec.ForProvider.GroupID,
UserID: "*",
}
Expand All @@ -226,7 +226,7 @@ func (c *external) Update(ctx context.Context, mg resource.Managed) (managed.Ext
return managed.ExternalUpdate{}, err
}

user := cloudian.User{
user := cloudian.UserID{
GroupID: cr.Spec.ForProvider.GroupID,
UserID: "*",
}
Expand All @@ -249,7 +249,7 @@ func (c *external) Delete(ctx context.Context, mg resource.Managed) (managed.Ext

cr.SetConditions(xpv1.Deleting())

user := cloudian.User{
user := cloudian.UserID{
GroupID: cr.Spec.ForProvider.GroupID,
UserID: "*",
}
Expand Down
13 changes: 8 additions & 5 deletions internal/controller/user/user.go
Original file line number Diff line number Diff line change
Expand Up @@ -157,7 +157,7 @@ func (c *external) Observe(ctx context.Context, mg resource.Managed) (managed.Ex
return managed.ExternalObservation{}, nil
}

_, err := c.cloudianService.GetUser(ctx, cloudian.User{
_, err := c.cloudianService.GetUser(ctx, cloudian.UserID{
GroupID: group,
UserID: externalName})
if errors.Is(err, cloudian.ErrNotFound) {
Expand Down Expand Up @@ -193,16 +193,19 @@ func (c *external) Create(ctx context.Context, mg resource.Managed) (managed.Ext
}

user := cloudian.User{
GroupID: cr.Spec.ForProvider.GroupID,
UserID: meta.GetExternalName(mg),
UserID: cloudian.UserID{
GroupID: cr.Spec.ForProvider.GroupID,
UserID: meta.GetExternalName(mg),
},
UserType: cloudian.UserTypeUser,
}
if err := c.cloudianService.CreateUser(ctx, user); err != nil {
return managed.ExternalCreation{}, errors.Wrap(err, errCreateUser)
}

// When Cloudian creates a user, a single access key is created inside it.
// Delete the access key, so that the user does not have any non-managed access keys.
creds, err := c.cloudianService.ListUserCredentials(ctx, user)
creds, err := c.cloudianService.ListUserCredentials(ctx, user.UserID)
if err != nil {
return managed.ExternalCreation{}, errors.Wrap(err, "failed to list access keys of user")
}
Expand Down Expand Up @@ -240,7 +243,7 @@ func (c *external) Delete(ctx context.Context, mg resource.Managed) (managed.Ext
return managed.ExternalDelete{}, errors.New(errNotUser)
}

user := cloudian.User{
user := cloudian.UserID{
GroupID: cr.Spec.ForProvider.GroupID,
UserID: meta.GetExternalName(mg),
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -155,7 +155,7 @@ func (c *external) Observe(ctx context.Context, mg resource.Managed) (managed.Ex
return managed.ExternalObservation{}, nil
}

user := cloudian.User{
user := cloudian.UserID{
GroupID: groupID,
UserID: userID,
}
Expand Down Expand Up @@ -204,7 +204,7 @@ func (c *external) Create(ctx context.Context, mg resource.Managed) (managed.Ext
return managed.ExternalCreation{}, err
}

user := cloudian.User{
user := cloudian.UserID{
GroupID: cr.Spec.ForProvider.GroupID,
UserID: cr.Spec.ForProvider.UserID,
}
Expand All @@ -230,7 +230,7 @@ func (c *external) Update(ctx context.Context, mg resource.Managed) (managed.Ext
return managed.ExternalUpdate{}, err
}

user := cloudian.User{
user := cloudian.UserID{
GroupID: cr.Spec.ForProvider.GroupID,
UserID: cr.Spec.ForProvider.UserID,
}
Expand All @@ -253,7 +253,7 @@ func (c *external) Delete(ctx context.Context, mg resource.Managed) (managed.Ext

cr.SetConditions(xpv1.Deleting())

user := cloudian.User{
user := cloudian.UserID{
GroupID: cr.Spec.ForProvider.GroupID,
UserID: cr.Spec.ForProvider.UserID,
}
Expand Down
6 changes: 3 additions & 3 deletions internal/sdk/cloudian/qos.go
Original file line number Diff line number Diff line change
Expand Up @@ -139,7 +139,7 @@ func (qos *QualityOfService) queryParams(params map[string]string) error {
// Default user-level QoS for the whole region (GroupID="*", UserID="ALL")
// Group-level QoS for a specific group (GroupID="<groupId>", UserID="*")
// Default group-level QoS for the whole region (GroupID="ALL", UserID="*")
func (client Client) SetQOS(ctx context.Context, user User, region string, qos QualityOfService) error {
func (client Client) SetQOS(ctx context.Context, user UserID, region string, qos QualityOfService) error {
for _, val := range qos.rawQueryParams() {
if val != nil && *val < -1 {
return fmt.Errorf("QoS limit values must be >= -1")
Expand Down Expand Up @@ -179,7 +179,7 @@ func (client Client) SetQOS(ctx context.Context, user User, region string, qos Q

// SetQOS gets QualityOfService limits for a Group or User, depending on the value of GroupID and UserID.
// See SetQOS for details.
func (client Client) GetQOS(ctx context.Context, user User, region string) (*QualityOfService, error) {
func (client Client) GetQOS(ctx context.Context, user UserID, region string) (*QualityOfService, error) {
params := make(map[string]string)
if region != DefaultRegion {
params["region"] = region
Expand Down Expand Up @@ -218,7 +218,7 @@ func (client Client) GetQOS(ctx context.Context, user User, region string) (*Qua

// DeleteQOS deletes QualityOfService limits for a Group or User, depending on the value of GroupID and UserID.
// See SetQOS for details.
func (client Client) DeleteQOS(ctx context.Context, user User, region string) error {
func (client Client) DeleteQOS(ctx context.Context, user UserID, region string) error {
params := make(map[string]string)
if region != DefaultRegion {
params["region"] = region
Expand Down
49 changes: 25 additions & 24 deletions internal/sdk/cloudian/sdk.go
Original file line number Diff line number Diff line change
Expand Up @@ -87,23 +87,22 @@ func fromInternal(g groupInternal) Group {
}
}

type User struct {
UserID string `json:"userId"`
GroupID string `json:"groupId"`
}
type UserType string

type userInternal struct {
UserID string `json:"userId"`
GroupID string `json:"groupId"`
UserType string `json:"userType"`
const (
UserTypeSystemAdmin UserType = "SystemAdmin"
UserTypeGroupAdmin UserType = "GroupAdmin"
UserTypeUser UserType = "User"
)

type UserID struct {
GroupID string `json:"groupId"`
UserID string `json:"userId"`
}

func toInternalUser(u User) userInternal {
return userInternal{
UserID: u.UserID,
GroupID: u.GroupID,
UserType: "User",
}
type User struct {
UserID `json:",inline"`
UserType UserType `json:"userType"`
}

// SecurityInfo is the Cloudian API's term for secure credentials
Expand Down Expand Up @@ -157,7 +156,7 @@ func (client Client) ListUsers(ctx context.Context, groupID string, userID *stri
// Paginated API endpoint where limit+1 elements indicates more pages
if len(users) > ListLimit {
// Fetch remaining users starting from the user after the limit
moreUsers, err := client.ListUsers(ctx, groupID, &users[ListLimit].UserID)
moreUsers, err := client.ListUsers(ctx, groupID, &users[ListLimit].UserID.UserID)
if err != nil {
return nil, err
}
Expand All @@ -169,7 +168,7 @@ func (client Client) ListUsers(ctx context.Context, groupID string, userID *stri
}

// Delete a single user. Errors if the user does not exist.
func (client Client) DeleteUser(ctx context.Context, user User) error {
func (client Client) DeleteUser(ctx context.Context, user UserID) error {
resp, err := client.newRequest(ctx).
SetQueryParams(map[string]string{
"groupId": user.GroupID,
Expand All @@ -192,7 +191,7 @@ func (client Client) DeleteUser(ctx context.Context, user User) error {
// Create a single user of type `User` into a groupId
func (client Client) CreateUser(ctx context.Context, user User) error {
resp, err := client.newRequest(ctx).
SetBody(toInternalUser(user)).
SetBody(user).
Put("/user")
if err != nil {
return err
Expand All @@ -208,13 +207,15 @@ func (client Client) CreateUser(ctx context.Context, user User) error {

// GetUser gets a user. Returns an error even in the case of a user not found.
// This error can then be checked against ErrNotFound: errors.Is(err, ErrNotFound)
func (client Client) GetUser(ctx context.Context, user User) (*User, error) {
// FIXME: Introduce UserId struct and enrich User
func (client Client) GetUser(ctx context.Context, userID UserID) (*User, error) {
var user User

resp, err := client.newRequest(ctx).
SetQueryParams(map[string]string{
"groupId": user.GroupID,
"userId": user.UserID,
"groupId": userID.GroupID,
"userId": userID.UserID,
}).
SetResult(&user).
Get("/user")
if err != nil {
return nil, err
Expand All @@ -232,7 +233,7 @@ func (client Client) GetUser(ctx context.Context, user User) (*User, error) {
}

// CreateUserCredentials creates a new set of credentials for a user.
func (client Client) CreateUserCredentials(ctx context.Context, user User) (*SecurityInfo, error) {
func (client Client) CreateUserCredentials(ctx context.Context, user UserID) (*SecurityInfo, error) {
var securityInfo SecurityInfo

resp, err := client.newRequest(ctx).
Expand Down Expand Up @@ -275,7 +276,7 @@ func (client Client) GetUserCredentials(ctx context.Context, accessKey string) (
}

// ListUserCredentials fetches all the credentials of a user.
func (client Client) ListUserCredentials(ctx context.Context, user User) ([]SecurityInfo, error) {
func (client Client) ListUserCredentials(ctx context.Context, user UserID) ([]SecurityInfo, error) {
var securityInfo []SecurityInfo

resp, err := client.newRequest(ctx).
Expand Down Expand Up @@ -322,7 +323,7 @@ func (client Client) DeleteGroupRecursive(ctx context.Context, groupID string) e
}

for _, user := range users {
if err := client.DeleteUser(ctx, user); err != nil {
if err := client.DeleteUser(ctx, user.UserID); err != nil {
return fmt.Errorf("error deleting user: %w", err)
}
}
Expand Down
33 changes: 27 additions & 6 deletions internal/sdk/cloudian/sdk_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import (
"fmt"
"net/http"
"net/http/httptest"
"reflect"
"strconv"
"testing"

Expand Down Expand Up @@ -68,7 +69,7 @@ func TestCreateCredentials(t *testing.T) {
})
defer testServer.Close()

credentials, err := cloudianClient.CreateUserCredentials(context.TODO(), User{GroupID: "QA", UserID: "user1"})
credentials, err := cloudianClient.CreateUserCredentials(context.TODO(), UserID{GroupID: "QA", UserID: "user1"})
if err != nil {
t.Errorf("Error creating credentials: %v", err)
}
Expand Down Expand Up @@ -104,7 +105,7 @@ func TestListUserCredentials(t *testing.T) {
defer testServer.Close()

credentials, err := cloudianClient.ListUserCredentials(
context.TODO(), User{UserID: "", GroupID: ""},
context.TODO(), UserID{UserID: "", GroupID: ""},
)
if err != nil {
t.Errorf("Error listing credentials: %v", err)
Expand All @@ -117,7 +118,7 @@ func TestListUserCredentials(t *testing.T) {
func TestListUsers(t *testing.T) {
var expected []User
for i := 0; i < 500; i++ {
expected = append(expected, User{GroupID: "QA", UserID: strconv.Itoa(i)})
expected = append(expected, User{UserID: UserID{GroupID: "QA", UserID: strconv.Itoa(i)}})
}

cloudianClient, testServer := mockBy(func(w http.ResponseWriter, r *http.Request) {
Expand Down Expand Up @@ -162,23 +163,43 @@ func TestClient_GetUser(t *testing.T) {
status int
wantErr error
}{
{name: "Exists", user: User{UserID: strconv.Itoa(http.StatusOK)}},
{name: "Not found", user: User{UserID: strconv.Itoa(http.StatusNoContent)}, wantErr: ErrNotFound},
{name: "Exists", user: User{UserID: UserID{UserID: strconv.Itoa(http.StatusOK)}}},
{name: "Not found", user: User{UserID: UserID{UserID: strconv.Itoa(http.StatusNoContent)}}, wantErr: ErrNotFound},
}

client, testServer := mockBy(func(w http.ResponseWriter, r *http.Request) {
userId := r.URL.Query().Get("userId")
statusCode, _ := strconv.Atoi(userId)
if statusCode == http.StatusOK {
for _, tt := range tests {
if tt.user.UserID.UserID == userId {
json.NewEncoder(w).Encode(tt.user)
break
}
}
}
w.WriteHeader(statusCode)
})
defer testServer.Close()

for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
_, err := client.GetUser(context.Background(), tt.user)
user, err := client.GetUser(context.Background(), tt.user.UserID)

if !errors.Is(err, tt.wantErr) {
t.Errorf("GetUser() error = %v, wantErr %v", err, tt.wantErr)
}

switch tt.wantErr {
case nil:
if !reflect.DeepEqual(user, &tt.user) {
t.Errorf("GetUser() got = %v, expected %v", user, tt.user)
}
default:
if user != nil {
t.Errorf("GetUser() got = %v, expected nil", user)
}
}
})
}
}
Loading