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

Use: Standard interface #8

Merged
merged 2 commits into from
Dec 6, 2023
Merged
Show file tree
Hide file tree
Changes from all 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
7 changes: 7 additions & 0 deletions docs/style-guide/sdk.md
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ When this document refers to the SDK, it references the code in the [/proxmox](.
- [Safe function](#safe-function)
- [Unsafe function](#unsafe-function)
- [Public and Private](#public-and-private)
- [Standardized Interfaces](#standardized-interfaces)
- [Pure and Impure Functions](#pure-and-impure-functions)
- [Pure Functions](#pure-functions)
- [Impure Functions](#impure-functions)
Expand Down Expand Up @@ -329,6 +330,12 @@ Public functions should follow this guide as much as possible, to realize a homo
It is important to note that every public function becomes a critical point of reliance for some software.
As such, it is imperative to uphold backward compatibility when making changes. See [Versioning](#versioning) for more information.

## Standardized Interfaces

Standardized interfaces are key in software development for consistent functionality exposure. They enhance code understanding, usage, and system interoperability.

If the Go standard library has an interface for existing functionality, use it. It's designed with best practices, well-documented, and tested. Ignoring it can lead to complexity and compatibility issues.

## Pure and Impure Functions

In software development, functions are categorized into two main types: [pure functions](https://en.wikipedia.org/wiki/Pure_function#Pure_functions) and [impure functions](https://en.wikipedia.org/wiki/Pure_function#Impure_functions).
Expand Down
6 changes: 3 additions & 3 deletions main.go
Original file line number Diff line number Diff line change
Expand Up @@ -415,7 +415,7 @@
//Users
case "getUser":
var config interface{}
userId, err := proxmox.NewUserID(flag.Args()[1])

Check failure on line 418 in main.go

View workflow job for this annotation

GitHub Actions / audit

var userId should be userID
failError(err)
config, err = proxmox.NewConfigUserFromApi(userId, c)
failError(err)
Expand All @@ -438,37 +438,37 @@
log.Printf("Error: Userid and Password required")
os.Exit(1)
}
userId, err := proxmox.NewUserID(flag.Args()[1])

Check failure on line 441 in main.go

View workflow job for this annotation

GitHub Actions / audit

var userId should be userID
failError(err)
err = proxmox.ConfigUser{
Password: proxmox.UserPassword(flag.Args()[2]),
User: userId,
}.UpdateUserPassword(c)
failError(err)
fmt.Printf("Password of User %s updated\n", userId.ToString())
fmt.Printf("Password of User %s updated\n", userId.String())

case "setUser":
var password proxmox.UserPassword
config, err := proxmox.NewConfigUserFromJson(GetConfig(*fConfigFile))
failError(err)
userId, err := proxmox.NewUserID(flag.Args()[1])

Check failure on line 454 in main.go

View workflow job for this annotation

GitHub Actions / audit

var userId should be userID
failError(err)
if len(flag.Args()) > 2 {
password = proxmox.UserPassword(flag.Args()[2])
}
failError(config.SetUser(userId, password, c))
log.Printf("User %s has been configured\n", userId.ToString())
log.Printf("User %s has been configured\n", userId.String())

case "deleteUser":
if len(flag.Args()) < 2 {
log.Printf("Error: userId required")
os.Exit(1)
}
userId, err := proxmox.NewUserID(flag.Args()[1])

Check failure on line 467 in main.go

View workflow job for this annotation

GitHub Actions / audit

var userId should be userID
failError(err)
err = proxmox.ConfigUser{User: userId}.DeleteUser(c)
failError(err)
fmt.Printf("User %s removed\n", userId.ToString())
fmt.Printf("User %s removed\n", userId.String())

//ACME Account
case "getAcmeAccountList":
Expand Down
2 changes: 1 addition & 1 deletion proxmox/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -1510,7 +1510,7 @@ func (c *Client) GetUserPermissions(id UserID, path string) (permissions []strin
if !existence {
return nil, fmt.Errorf("cannot get user (%s) permissions, the user does not exist", id)
}
permlist, err := c.getItemList("/access/permissions?userid=" + id.ToString() + "&path=" + path)
permlist, err := c.getItemList("/access/permissions?userid=" + id.String() + "&path=" + path)
failError(err)
data := permlist["data"].(map[string]interface{})
for pth, prm := range data {
Expand Down
6 changes: 3 additions & 3 deletions proxmox/config_group.go
Original file line number Diff line number Diff line change
Expand Up @@ -261,7 +261,7 @@ func (group GroupName) removeAllUsersFromGroupExcept(allUsers []interface{}, mem
}
var userInMembers bool
for _, ee := range *members {
if params["userid"] == ee.ToString() {
if params["userid"] == ee.String() {
userInMembers = true
break
}
Expand Down Expand Up @@ -324,7 +324,7 @@ func (group GroupName) usersToAddToGroup(allUsers []interface{}, members *[]User
continue
}
for ii, ee := range *members {
if params["userid"] == ee.ToString() {
if params["userid"] == ee.String() {
var groups []GroupName
if _, isSet := params["groups"]; isSet {
groups = GroupName("").csvToArray(params["groups"].(string))
Expand All @@ -350,7 +350,7 @@ func (group GroupName) usersToRemoveFromGroup(allUsers []interface{}, members *[
continue
}
for ii, ee := range *members {
if params["userid"] == ee.ToString() {
if params["userid"] == ee.String() {
var groups []GroupName
if _, isSet := params["groups"]; isSet {
groups = GroupName("").csvToArray(params["groups"].(string))
Expand Down
28 changes: 14 additions & 14 deletions proxmox/config_user.go
Original file line number Diff line number Diff line change
Expand Up @@ -45,10 +45,10 @@ func (config ConfigUser) DeleteUser(client *Client) (err error) {
return
}
if !existence {
return fmt.Errorf("user (%s) could not be deleted, the user does not exist", config.User.ToString())
return fmt.Errorf("user (%s) could not be deleted, the user does not exist", config.User.String())
}
// Proxmox silently fails a user delete if the users does not exist
return client.delete("/access/users/" + config.User.ToString())
return client.delete("/access/users/" + config.User.String())
}

// Maps the struct to the API values proxmox understands
Expand All @@ -65,7 +65,7 @@ func (config ConfigUser) mapToApiValues(create bool) (params map[string]interfac
}
if create {
params["password"] = config.Password
params["userid"] = config.User.ToString()
params["userid"] = config.User.String()
}
return
}
Expand Down Expand Up @@ -150,7 +150,7 @@ func (config *ConfigUser) SetUser(userId UserID, password UserPassword, client *

func (config *ConfigUser) UpdateUser(client *Client) (err error) {
params := config.mapToApiValues(false)
err = client.put(params, "/access/users/"+config.User.ToString())
err = client.put(params, "/access/users/"+config.User.String())
if err != nil {
params, _ := json.Marshal(&params)
return fmt.Errorf("error updating User: %v, (params: %v)", err, string(params))
Expand All @@ -167,7 +167,7 @@ func (config ConfigUser) UpdateUserPassword(client *Client) (err error) {
return err
}
return client.put(map[string]interface{}{
"userid": config.User.ToString(),
"userid": config.User.String(),
"password": config.Password,
}, "/access/password")
}
Expand Down Expand Up @@ -219,7 +219,7 @@ func (config ConfigUser) CreateApiToken(client *Client, token ApiToken) (value s
"comment": token.Comment,
"expire": token.Expire,
"privsep": token.Privsep,
}, "/access/users/"+config.User.ToString()+"/token/"+token.TokenId)
}, "/access/users/"+config.User.String()+"/token/"+token.TokenId)
if err != nil {
return
}
Expand All @@ -234,12 +234,12 @@ func (config ConfigUser) UpdateApiToken(client *Client, token ApiToken) (err err
"comment": token.Comment,
"expire": token.Expire,
"privsep": token.Privsep,
}, "/access/users/"+config.User.ToString()+"/token/"+token.TokenId)
}, "/access/users/"+config.User.String()+"/token/"+token.TokenId)
return
}

func (config ConfigUser) ListApiTokens(client *Client) (tokens *[]ApiToken, err error) {
status, err := client.getItemListInterfaceArray("/access/users/" + config.User.ToString() + "/token")
status, err := client.getItemListInterfaceArray("/access/users/" + config.User.String() + "/token")
if err != nil {
return
}
Expand All @@ -248,7 +248,7 @@ func (config ConfigUser) ListApiTokens(client *Client) (tokens *[]ApiToken, err
}

func (config ConfigUser) DeleteApiToken(client *Client, token ApiToken) (err error) {
err = client.delete("/access/users/" + config.User.ToString() + "/token/" + token.TokenId)
err = client.delete("/access/users/" + config.User.String() + "/token/" + token.TokenId)
return
}

Expand Down Expand Up @@ -333,7 +333,7 @@ func (UserID) mapToStruct(userId string) UserID {

// Converts the userID to "username@realm"
// Returns an empty string when either the Name or Realm is empty
func (id UserID) ToString() string {
func (id UserID) String() string {
if id.Name == "" || id.Realm == "" {
return ""
}
Expand Down Expand Up @@ -369,9 +369,9 @@ func CheckUserExistence(userId UserID, client *Client) (existence bool, err erro
}
// This should be the case where you have an API Token with privilege separation but no permissions attached
if len(list) == 0 {
return false, fmt.Errorf("user %s has valid credentials but cannot retrieve user list, check privilege separation of api token", userId.ToString())
return false, fmt.Errorf("user %s has valid credentials but cannot retrieve user list, check privilege separation of api token", userId.String())
}
existence = ItemInKeyOfArray(list, "userid", userId.ToString())
existence = ItemInKeyOfArray(list, "userid", userId.String())
return
}

Expand Down Expand Up @@ -403,7 +403,7 @@ func listUsersFull(client *Client) ([]interface{}, error) {
}

func NewConfigUserFromApi(userId UserID, client *Client) (*ConfigUser, error) {
userConfig, err := client.getItemConfigMapStringInterface("/access/users/"+userId.ToString(), "user", "CONFIG")
userConfig, err := client.getItemConfigMapStringInterface("/access/users/"+userId.String(), "user", "CONFIG")
if err != nil {
return nil, err
}
Expand Down Expand Up @@ -451,5 +451,5 @@ func NewUserIDs(userIds string) (*[]UserID, error) {

// URL for updating users
func updateUser(user UserID, params map[string]interface{}, client *Client) error {
return client.put(params, "/access/users/"+user.ToString())
return client.put(params, "/access/users/"+user.String())
}
4 changes: 2 additions & 2 deletions proxmox/config_user_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -482,7 +482,7 @@ func Test_UserID_mapToStruct(t *testing.T) {
}

// TODO improve test when a validation function for the UserID exists
func Test_UserID_ToString(t *testing.T) {
func Test_UserID_String(t *testing.T) {
testData := []struct {
input UserID
Output string
Expand All @@ -499,7 +499,7 @@ func Test_UserID_ToString(t *testing.T) {
{input: UserID{}},
}
for _, e := range testData {
require.Equal(t, e.Output, e.input.ToString())
require.Equal(t, e.Output, e.input.String())
}
}

Expand Down
10 changes: 5 additions & 5 deletions test/cli/Users/user_sub_tests/user_sub_tests.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,24 +11,24 @@ import (
func Cleanup(t *testing.T, user proxmox.UserID) {
Test := cliTest.Test{
ReqErr: true,
ErrContains: user.ToString(),
Args: []string{"-i", "delete", "user", user.ToString()},
ErrContains: user.String(),
Args: []string{"-i", "delete", "user", user.String()},
}
Test.StandardTest(t)
}

// Default DELETE test for User
func Delete(t *testing.T, user proxmox.UserID) {
Test := cliTest.Test{
Contains: []string{user.ToString()},
Args: []string{"-i", "delete", "user", user.ToString()},
Contains: []string{user.String()},
Args: []string{"-i", "delete", "user", user.String()},
}
Test.StandardTest(t)
}

// Default SET test for User
func Set(t *testing.T, user proxmox.ConfigUser) {
userID := user.User.ToString()
userID := user.User.String()
user.User = proxmox.UserID{}
Test := cliTest.Test{
InputJson: user,
Expand Down
Loading