From 8f067dc61065d773136741b855060d3fb02f00d9 Mon Sep 17 00:00:00 2001 From: Adam Jasinski Date: Wed, 29 Mar 2023 17:31:49 +0300 Subject: [PATCH 01/16] initial changes, simple token creation function Signed-off-by: Adam Jasinski --- govcd/api.go | 17 +++++++++-------- govcd/api_token.go | 39 +++++++++++++++++++++++++++++++++++++++ types/v56/types.go | 8 ++++---- 3 files changed, 52 insertions(+), 12 deletions(-) diff --git a/govcd/api.go b/govcd/api.go index fd347ab8f..8447ecf9d 100644 --- a/govcd/api.go +++ b/govcd/api.go @@ -25,14 +25,15 @@ import ( // Client provides a client to VMware Cloud Director, values can be populated automatically using the Authenticate method. type Client struct { - APIVersion string // The API version required - VCDToken string // Access Token (authorization header) - VCDAuthHeader string // Authorization header - VCDHREF url.URL // VCD API ENDPOINT - Http http.Client // HttpClient is the client to use. Default will be used if not provided. - IsSysAdmin bool // flag if client is connected as system administrator - UsingBearerToken bool // flag if client is using a bearer token - UsingAccessToken bool // flag if client is using an API token + APIVersion string // The API version required + VCDToken string // Access Token (authorization header) + VCDAuthHeader string // Authorization header + VCDHREF url.URL // VCD API ENDPOINT + Http http.Client // HttpClient is the client to use. Default will be used if not provided. + IsSysAdmin bool // flag if client is connected as system administrator + UsingBearerToken bool // flag if client is using a bearer token + UsingAccessToken bool // flag if client is using an API token + UsingServiceAccount bool // flag if client is using a service account // MaxRetryTimeout specifies a time limit (in seconds) for retrying requests made by the SDK // where VMware Cloud Director may take time to respond and retry mechanism is needed. diff --git a/govcd/api_token.go b/govcd/api_token.go index 41040ff0b..5b1cf06e0 100644 --- a/govcd/api_token.go +++ b/govcd/api_token.go @@ -11,6 +11,7 @@ import ( "io" "net/http" "net/url" + "os" "strings" "github.com/vmware/go-vcloud-director/v2/types/v56" @@ -31,6 +32,44 @@ func (vcdClient *VCDClient) SetApiToken(org, apiToken string) (*types.ApiTokenRe return tokenRefresh, nil } +func (vcdClient *VCDClient) SetServiceAccountApiToken(org, apiTokenFile string) error { + saApiToken := &types.ApiTokenRefresh{} + data, err := os.ReadFile(apiTokenFile) + if err != nil { + return err + } + + err = json.Unmarshal(data, &saApiToken) + if err != nil { + return err + } + if saApiToken.RefreshToken != "" { + saApiToken, err = vcdClient.SetApiToken(org, saApiToken.RefreshToken) + if err != nil { + return err + } + } + + err = vcdClient.SetToken(org, BearerTokenHeader, saApiToken.AccessToken) + if err != nil { + return err + } + + data, err = json.Marshal(&types.ApiTokenRefresh{ + RefreshToken: saApiToken.RefreshToken, + }) + if err != nil { + return err + } + + err = os.WriteFile(apiTokenFile, data, 0644) + if err != nil { + return err + } + + return nil +} + // GetBearerTokenFromApiToken uses an API token to retrieve a bearer token // using the refresh token operation. func (vcdClient *VCDClient) GetBearerTokenFromApiToken(org, token string) (*types.ApiTokenRefresh, error) { diff --git a/types/v56/types.go b/types/v56/types.go index e814ba36d..0854ec48f 100644 --- a/types/v56/types.go +++ b/types/v56/types.go @@ -3257,10 +3257,10 @@ type UpdateVdcStorageProfiles struct { // ApiTokenRefresh contains the access token resulting from a refresh_token operation type ApiTokenRefresh struct { - AccessToken string `json:"access_token"` - TokenType string `json:"token_type"` - ExpiresIn int `json:"expires_in"` - RefreshToken interface{} `json:"refresh_token"` + AccessToken string `json:"access_token"` + TokenType string `json:"token_type"` + ExpiresIn int `json:"expires_in"` + RefreshToken string `json:"refresh_token"` } /**/ From dd0acae410410e194f9c451b5c431870b7aad959 Mon Sep 17 00:00:00 2001 From: Adam Jasinski Date: Mon, 3 Apr 2023 10:14:58 +0300 Subject: [PATCH 02/16] more improvements Signed-off-by: Adam Jasinski --- govcd/api_token.go | 7 +------ types/v56/types.go | 8 ++++---- 2 files changed, 5 insertions(+), 10 deletions(-) diff --git a/govcd/api_token.go b/govcd/api_token.go index 5b1cf06e0..eb768c0ce 100644 --- a/govcd/api_token.go +++ b/govcd/api_token.go @@ -33,12 +33,12 @@ func (vcdClient *VCDClient) SetApiToken(org, apiToken string) (*types.ApiTokenRe } func (vcdClient *VCDClient) SetServiceAccountApiToken(org, apiTokenFile string) error { - saApiToken := &types.ApiTokenRefresh{} data, err := os.ReadFile(apiTokenFile) if err != nil { return err } + saApiToken := &types.ApiTokenRefresh{} err = json.Unmarshal(data, &saApiToken) if err != nil { return err @@ -50,11 +50,6 @@ func (vcdClient *VCDClient) SetServiceAccountApiToken(org, apiTokenFile string) } } - err = vcdClient.SetToken(org, BearerTokenHeader, saApiToken.AccessToken) - if err != nil { - return err - } - data, err = json.Marshal(&types.ApiTokenRefresh{ RefreshToken: saApiToken.RefreshToken, }) diff --git a/types/v56/types.go b/types/v56/types.go index 0854ec48f..197f91172 100644 --- a/types/v56/types.go +++ b/types/v56/types.go @@ -3257,10 +3257,10 @@ type UpdateVdcStorageProfiles struct { // ApiTokenRefresh contains the access token resulting from a refresh_token operation type ApiTokenRefresh struct { - AccessToken string `json:"access_token"` - TokenType string `json:"token_type"` - ExpiresIn int `json:"expires_in"` - RefreshToken string `json:"refresh_token"` + AccessToken string `json:"access_token,omitempty"` + TokenType string `json:"token_type,omitempty"` + ExpiresIn int `json:"expires_in,omitempty"` + RefreshToken string `json:"refresh_token,omitempty"` } /**/ From 40f904a35ed46fc347aa2abd796a24756c5fd300 Mon Sep 17 00:00:00 2001 From: Adam Jasinski Date: Mon, 3 Apr 2023 10:45:59 +0300 Subject: [PATCH 03/16] update documentation Signed-off-by: Adam Jasinski --- README.md | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/README.md b/README.md index 33a6a0bd2..8536d4618 100644 --- a/README.md +++ b/README.md @@ -114,7 +114,7 @@ func main() { ## Authentication -You can authenticate to the vCD in four ways: +You can authenticate to the vCD in five ways: * With a System Administration user and password (`administrator@system`) * With an Organization user and password (`tenant-admin@org-name`) @@ -133,6 +133,11 @@ For the above two methods, you use: The file `scripts/get_token.sh` provides a handy method of extracting the token (`x-vcloud-authorization` value) for future use. +* With a service account token +```go + err := vcdClient.SetServiceAccountApiToken(Org, "tokenfile.json") +``` + * SAML user and password (works with ADFS as IdP using WS-TRUST endpoint "/adfs/services/trust/13/usernamemixed"). One must pass `govcd.WithSamlAdfs(true,customAdfsRptId)` and username must be formatted so that ADFS understands it ('user@contoso.com' or From fd3aa5898f9e19da8cc6d516e2836b053f84b502 Mon Sep 17 00:00:00 2001 From: Adam Jasinski Date: Mon, 3 Apr 2023 10:47:39 +0300 Subject: [PATCH 04/16] add error handling Signed-off-by: Adam Jasinski --- govcd/api_token.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/govcd/api_token.go b/govcd/api_token.go index eb768c0ce..6931ddcd5 100644 --- a/govcd/api_token.go +++ b/govcd/api_token.go @@ -48,6 +48,8 @@ func (vcdClient *VCDClient) SetServiceAccountApiToken(org, apiTokenFile string) if err != nil { return err } + } else { + return fmt.Errorf("failed to read token file") } data, err = json.Marshal(&types.ApiTokenRefresh{ From e491403965e28385fd23355f0c1de7fc4714fe69 Mon Sep 17 00:00:00 2001 From: Adam Jasinski Date: Mon, 3 Apr 2023 11:10:00 +0300 Subject: [PATCH 05/16] remove unnecessary things Signed-off-by: Adam Jasinski --- govcd/api.go | 17 ++++++++--------- govcd/api_token.go | 11 ++++------- 2 files changed, 12 insertions(+), 16 deletions(-) diff --git a/govcd/api.go b/govcd/api.go index 8447ecf9d..fd347ab8f 100644 --- a/govcd/api.go +++ b/govcd/api.go @@ -25,15 +25,14 @@ import ( // Client provides a client to VMware Cloud Director, values can be populated automatically using the Authenticate method. type Client struct { - APIVersion string // The API version required - VCDToken string // Access Token (authorization header) - VCDAuthHeader string // Authorization header - VCDHREF url.URL // VCD API ENDPOINT - Http http.Client // HttpClient is the client to use. Default will be used if not provided. - IsSysAdmin bool // flag if client is connected as system administrator - UsingBearerToken bool // flag if client is using a bearer token - UsingAccessToken bool // flag if client is using an API token - UsingServiceAccount bool // flag if client is using a service account + APIVersion string // The API version required + VCDToken string // Access Token (authorization header) + VCDAuthHeader string // Authorization header + VCDHREF url.URL // VCD API ENDPOINT + Http http.Client // HttpClient is the client to use. Default will be used if not provided. + IsSysAdmin bool // flag if client is connected as system administrator + UsingBearerToken bool // flag if client is using a bearer token + UsingAccessToken bool // flag if client is using an API token // MaxRetryTimeout specifies a time limit (in seconds) for retrying requests made by the SDK // where VMware Cloud Director may take time to respond and retry mechanism is needed. diff --git a/govcd/api_token.go b/govcd/api_token.go index 6931ddcd5..00df3426f 100644 --- a/govcd/api_token.go +++ b/govcd/api_token.go @@ -43,13 +43,10 @@ func (vcdClient *VCDClient) SetServiceAccountApiToken(org, apiTokenFile string) if err != nil { return err } - if saApiToken.RefreshToken != "" { - saApiToken, err = vcdClient.SetApiToken(org, saApiToken.RefreshToken) - if err != nil { - return err - } - } else { - return fmt.Errorf("failed to read token file") + + saApiToken, err = vcdClient.SetApiToken(org, saApiToken.RefreshToken) + if err != nil { + return err } data, err = json.Marshal(&types.ApiTokenRefresh{ From cd54a7f3e119a87360cad99a7b9e389764fa9913 Mon Sep 17 00:00:00 2001 From: Adam Jasinski Date: Mon, 3 Apr 2023 12:54:22 +0300 Subject: [PATCH 06/16] address gosec issues Signed-off-by: Adam Jasinski --- govcd/api_token.go | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/govcd/api_token.go b/govcd/api_token.go index 00df3426f..184d7434a 100644 --- a/govcd/api_token.go +++ b/govcd/api_token.go @@ -12,6 +12,7 @@ import ( "net/http" "net/url" "os" + "path/filepath" "strings" "github.com/vmware/go-vcloud-director/v2/types/v56" @@ -33,6 +34,7 @@ func (vcdClient *VCDClient) SetApiToken(org, apiToken string) (*types.ApiTokenRe } func (vcdClient *VCDClient) SetServiceAccountApiToken(org, apiTokenFile string) error { + apiTokenFile = filepath.Clean(apiTokenFile) data, err := os.ReadFile(apiTokenFile) if err != nil { return err @@ -56,7 +58,7 @@ func (vcdClient *VCDClient) SetServiceAccountApiToken(org, apiTokenFile string) return err } - err = os.WriteFile(apiTokenFile, data, 0644) + err = os.WriteFile(apiTokenFile, data, 0600) if err != nil { return err } From 75450c678c91b9a58b6d342f4ca455b271161716 Mon Sep 17 00:00:00 2001 From: Adam Jasinski Date: Mon, 3 Apr 2023 13:36:42 +0300 Subject: [PATCH 07/16] add version constraints Signed-off-by: Adam Jasinski --- govcd/api_token.go | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/govcd/api_token.go b/govcd/api_token.go index 184d7434a..c55107020 100644 --- a/govcd/api_token.go +++ b/govcd/api_token.go @@ -34,6 +34,15 @@ func (vcdClient *VCDClient) SetApiToken(org, apiToken string) (*types.ApiTokenRe } func (vcdClient *VCDClient) SetServiceAccountApiToken(org, apiTokenFile string) error { + if vcdClient.Client.APIVCDMaxVersionIs("< 37.0") { + version, err := vcdClient.Client.GetVcdFullVersion() + if err == nil { + return fmt.Errorf("minimum version for Service Account authentication is 10.4 - Version detected: %s", version.Version) + } + // If we can't get the VCD version, we return API version info + return fmt.Errorf("minimum API version for Service Account authentication is 37.0 - Version detected: %s", vcdClient.Client.APIVersion) + } + apiTokenFile = filepath.Clean(apiTokenFile) data, err := os.ReadFile(apiTokenFile) if err != nil { From 2b7d2352b8c1f5d4abd8c3fd9e0801e8d2516ce9 Mon Sep 17 00:00:00 2001 From: Adam Jasinski Date: Tue, 4 Apr 2023 10:38:58 +0300 Subject: [PATCH 08/16] address comments Signed-off-by: Adam Jasinski --- README.md | 2 +- govcd/api_token.go | 7 +++++-- 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/README.md b/README.md index 8536d4618..a3e52ae24 100644 --- a/README.md +++ b/README.md @@ -133,7 +133,7 @@ For the above two methods, you use: The file `scripts/get_token.sh` provides a handy method of extracting the token (`x-vcloud-authorization` value) for future use. -* With a service account token +* With a service account token (the file needs to have `r+w` rights) ```go err := vcdClient.SetServiceAccountApiToken(Org, "tokenfile.json") ``` diff --git a/govcd/api_token.go b/govcd/api_token.go index c55107020..366df39e0 100644 --- a/govcd/api_token.go +++ b/govcd/api_token.go @@ -33,6 +33,9 @@ func (vcdClient *VCDClient) SetApiToken(org, apiToken string) (*types.ApiTokenRe return tokenRefresh, nil } +// SetServiceAccountApiToken reads the current Service Account API token, +// sets the client's bearer token and fetches a new API token for next +// authentication request using SetApiToken and overwrites the old file. func (vcdClient *VCDClient) SetServiceAccountApiToken(org, apiTokenFile string) error { if vcdClient.Client.APIVCDMaxVersionIs("< 37.0") { version, err := vcdClient.Client.GetVcdFullVersion() @@ -46,13 +49,13 @@ func (vcdClient *VCDClient) SetServiceAccountApiToken(org, apiTokenFile string) apiTokenFile = filepath.Clean(apiTokenFile) data, err := os.ReadFile(apiTokenFile) if err != nil { - return err + return fmt.Errorf("failed to read tokenfile: %s", err) } saApiToken := &types.ApiTokenRefresh{} err = json.Unmarshal(data, &saApiToken) if err != nil { - return err + return fmt.Errorf("failed to unmarshal tokenfile, check if your JSON file is valid: %s", err) } saApiToken, err = vcdClient.SetApiToken(org, saApiToken.RefreshToken) From 60e183d8fa7a757c5ea6c16f036056e83a5f1a36 Mon Sep 17 00:00:00 2001 From: Adam Jasinski Date: Wed, 12 Apr 2023 15:06:50 +0300 Subject: [PATCH 09/16] add changelog entry Signed-off-by: Adam Jasinski --- .changes/v2.20.0/562-improvements.md | 2 ++ 1 file changed, 2 insertions(+) create mode 100644 .changes/v2.20.0/562-improvements.md diff --git a/.changes/v2.20.0/562-improvements.md b/.changes/v2.20.0/562-improvements.md new file mode 100644 index 000000000..25d068960 --- /dev/null +++ b/.changes/v2.20.0/562-improvements.md @@ -0,0 +1,2 @@ +* Add `SetServiceAccountApiToken` method of `*VCDClient` that allows + authenticating using a service account token file and handles the refresh token rotation [GH-562] From 9ae09314e606584f6262012dc83bd10c3e291eb7 Mon Sep 17 00:00:00 2001 From: Adam Jasinski Date: Fri, 14 Apr 2023 00:52:42 +0300 Subject: [PATCH 10/16] refactor service account token function Signed-off-by: Adam Jasinski --- govcd/api_token.go | 23 +++------ govcd/generic_functions.go | 40 ++++++++++++++- govcd/generic_functions_unit_test.go | 74 ++++++++++++++++++++++++++-- govcd/testdata/test.json | 1 + govcd/testdata/test_empty.json | 0 govcd/testdata/test_emptyJSON.json | 1 + 6 files changed, 117 insertions(+), 22 deletions(-) create mode 100644 govcd/testdata/test.json create mode 100644 govcd/testdata/test_empty.json create mode 100644 govcd/testdata/test_emptyJSON.json diff --git a/govcd/api_token.go b/govcd/api_token.go index 366df39e0..c49375ac1 100644 --- a/govcd/api_token.go +++ b/govcd/api_token.go @@ -11,8 +11,6 @@ import ( "io" "net/http" "net/url" - "os" - "path/filepath" "strings" "github.com/vmware/go-vcloud-director/v2/types/v56" @@ -46,31 +44,24 @@ func (vcdClient *VCDClient) SetServiceAccountApiToken(org, apiTokenFile string) return fmt.Errorf("minimum API version for Service Account authentication is 37.0 - Version detected: %s", vcdClient.Client.APIVersion) } - apiTokenFile = filepath.Clean(apiTokenFile) - data, err := os.ReadFile(apiTokenFile) - if err != nil { - return fmt.Errorf("failed to read tokenfile: %s", err) - } - saApiToken := &types.ApiTokenRefresh{} - err = json.Unmarshal(data, &saApiToken) + // Read file contents and unmarshal them to saApiToken + err := readFileAndUnmarshalJSON(apiTokenFile, saApiToken) if err != nil { - return fmt.Errorf("failed to unmarshal tokenfile, check if your JSON file is valid: %s", err) + return err } + // Get bearer token and update the refresh token for the next authentication request saApiToken, err = vcdClient.SetApiToken(org, saApiToken.RefreshToken) if err != nil { return err } - data, err = json.Marshal(&types.ApiTokenRefresh{ + // leave only the refresh token to not leave any sensitive information + saApiToken = &types.ApiTokenRefresh{ RefreshToken: saApiToken.RefreshToken, - }) - if err != nil { - return err } - - err = os.WriteFile(apiTokenFile, data, 0600) + err = marshalJSONAndWriteToFile(apiTokenFile, saApiToken, 0600) if err != nil { return err } diff --git a/govcd/generic_functions.go b/govcd/generic_functions.go index 16f2b8c79..efaf2ef41 100644 --- a/govcd/generic_functions.go +++ b/govcd/generic_functions.go @@ -1,6 +1,12 @@ package govcd -import "fmt" +import ( + "encoding/json" + "fmt" + "io/fs" + "os" + "path" +) // oneOrError is used to cover up a common pattern in this codebase which is usually used in // GetXByName functions. @@ -36,3 +42,35 @@ func oneOrError[T any](key, name string, entitySlice []*T) (*T, error) { return entitySlice[0], nil } + +// readFileAndUnmarshalJSON reads a file and unmarshals it to the given variable +func readFileAndUnmarshalJSON[T any](filename string, object T) error { + filename = path.Clean(filename) + data, err := os.ReadFile(filename) + if err != nil { + return fmt.Errorf("failed to read from file: %s", err) + } + + err = json.Unmarshal(data, object) + if err != nil { + return fmt.Errorf("failed to unmarshal file contents to the object: %s", err) + } + + return nil +} + +// marshalJSONAndWriteToFile marshalls the given object into JSON and writes +// to a file with the given permissions in octal format (e.g 0600) +func marshalJSONAndWriteToFile[T any](filename string, object *T, permissions int) error { + data, err := json.Marshal(object) + if err != nil { + return fmt.Errorf("error marshalling object to JSON: %s", err) + } + + err = os.WriteFile(filename, data, fs.FileMode(permissions)) + if err != nil { + return fmt.Errorf("error writing to the file: %s", err) + } + + return nil +} diff --git a/govcd/generic_functions_unit_test.go b/govcd/generic_functions_unit_test.go index fc27dc7fb..13eb8c249 100644 --- a/govcd/generic_functions_unit_test.go +++ b/govcd/generic_functions_unit_test.go @@ -29,9 +29,9 @@ func Test_oneOrError(t *testing.T) { args: args{ key: "name", name: "test", - entitySlice: []*testEntity{{name: "test"}}, + entitySlice: []*testEntity{{Name: "test"}}, }, - want: &testEntity{name: "test"}, + want: &testEntity{Name: "test"}, wantErr: false, }, { @@ -50,7 +50,7 @@ func Test_oneOrError(t *testing.T) { args: args{ key: "name", name: "test", - entitySlice: []*testEntity{{name: "test"}, {name: "best"}}, + entitySlice: []*testEntity{{Name: "test"}, {Name: "best"}}, }, want: nil, wantErr: true, @@ -60,7 +60,7 @@ func Test_oneOrError(t *testing.T) { args: args{ key: "name", name: "test", - entitySlice: []*testEntity{{name: "test"}, {name: "best"}, {name: "rest"}}, + entitySlice: []*testEntity{{Name: "test"}, {Name: "best"}, {Name: "rest"}}, }, want: nil, wantErr: true, @@ -98,5 +98,69 @@ func Test_oneOrError(t *testing.T) { } type testEntity struct { - name string + Name string `json:"name"` +} + +// TODO Write tests for marshalJSONAndWriteFile +func Test_readFileAndUnmarshalJSON(t *testing.T) { + type args struct { + filename string + object *testEntity + } + tests := []struct { + name string + args args + want *testEntity + wantErr bool + }{ + { + name: "simpleCase", + args: args{ + filename: "testdata/test.json", + object: &testEntity{}, + }, + want: &testEntity{Name: "test"}, + wantErr: false, + }, + { + name: "emptyFile", + args: args{ + filename: "testdata/test_empty.json", + object: &testEntity{}, + }, + want: &testEntity{}, + wantErr: true, + }, + { + name: "emptyJSON", + args: args{ + filename: "testdata/test_emptyjson.json", + object: &testEntity{}, + }, + want: &testEntity{}, + wantErr: false, + }, + { + name: "nonexistentFile", + args: args{ + filename: "thisfiledoesntexist.json", + object: &testEntity{}, + }, + want: &testEntity{}, + wantErr: true, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + err := readFileAndUnmarshalJSON(tt.args.filename, tt.args.object) + if (err != nil) != tt.wantErr { + t.Errorf("readFileAndUnmarshalJSON() error = %v, wantErr %v", err, tt.wantErr) + return + } + + if !reflect.DeepEqual(tt.args.object, tt.want) { + t.Errorf("readFileAndUnmarshalJSON() = %v, want %v", tt.args.object, tt.want) + } + }) + } } diff --git a/govcd/testdata/test.json b/govcd/testdata/test.json new file mode 100644 index 000000000..10f2877cb --- /dev/null +++ b/govcd/testdata/test.json @@ -0,0 +1 @@ +{ "Name": "test" } diff --git a/govcd/testdata/test_empty.json b/govcd/testdata/test_empty.json new file mode 100644 index 000000000..e69de29bb diff --git a/govcd/testdata/test_emptyJSON.json b/govcd/testdata/test_emptyJSON.json new file mode 100644 index 000000000..0967ef424 --- /dev/null +++ b/govcd/testdata/test_emptyJSON.json @@ -0,0 +1 @@ +{} From 8dab8bdd137bf83b4bee9b7c55010f6ccd1c65bb Mon Sep 17 00:00:00 2001 From: Adam Jasinski Date: Fri, 14 Apr 2023 01:08:03 +0300 Subject: [PATCH 11/16] try to fix gosec vulnerability Signed-off-by: Adam Jasinski --- govcd/generic_functions.go | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/govcd/generic_functions.go b/govcd/generic_functions.go index efaf2ef41..68a07f412 100644 --- a/govcd/generic_functions.go +++ b/govcd/generic_functions.go @@ -45,8 +45,7 @@ func oneOrError[T any](key, name string, entitySlice []*T) (*T, error) { // readFileAndUnmarshalJSON reads a file and unmarshals it to the given variable func readFileAndUnmarshalJSON[T any](filename string, object T) error { - filename = path.Clean(filename) - data, err := os.ReadFile(filename) + data, err := os.ReadFile(path.Clean(filename)) if err != nil { return fmt.Errorf("failed to read from file: %s", err) } From 9ac2a4a985380cc0c864d1aa2d5e4ad1e6ac97a1 Mon Sep 17 00:00:00 2001 From: Adam Jasinski Date: Fri, 14 Apr 2023 01:47:37 +0300 Subject: [PATCH 12/16] move test files to another directory Signed-off-by: Adam Jasinski --- govcd/generic_functions_unit_test.go | 6 +++--- govcd/{testdata => test-resources}/test.json | 0 govcd/{testdata => test-resources}/test_empty.json | 0 govcd/{testdata => test-resources}/test_emptyJSON.json | 0 4 files changed, 3 insertions(+), 3 deletions(-) rename govcd/{testdata => test-resources}/test.json (100%) rename govcd/{testdata => test-resources}/test_empty.json (100%) rename govcd/{testdata => test-resources}/test_emptyJSON.json (100%) diff --git a/govcd/generic_functions_unit_test.go b/govcd/generic_functions_unit_test.go index 13eb8c249..ae4d6c286 100644 --- a/govcd/generic_functions_unit_test.go +++ b/govcd/generic_functions_unit_test.go @@ -116,7 +116,7 @@ func Test_readFileAndUnmarshalJSON(t *testing.T) { { name: "simpleCase", args: args{ - filename: "testdata/test.json", + filename: "test-resources/test.json", object: &testEntity{}, }, want: &testEntity{Name: "test"}, @@ -125,7 +125,7 @@ func Test_readFileAndUnmarshalJSON(t *testing.T) { { name: "emptyFile", args: args{ - filename: "testdata/test_empty.json", + filename: "test-resources/test_empty.json", object: &testEntity{}, }, want: &testEntity{}, @@ -134,7 +134,7 @@ func Test_readFileAndUnmarshalJSON(t *testing.T) { { name: "emptyJSON", args: args{ - filename: "testdata/test_emptyjson.json", + filename: "test-resources/test_emptyjson.json", object: &testEntity{}, }, want: &testEntity{}, diff --git a/govcd/testdata/test.json b/govcd/test-resources/test.json similarity index 100% rename from govcd/testdata/test.json rename to govcd/test-resources/test.json diff --git a/govcd/testdata/test_empty.json b/govcd/test-resources/test_empty.json similarity index 100% rename from govcd/testdata/test_empty.json rename to govcd/test-resources/test_empty.json diff --git a/govcd/testdata/test_emptyJSON.json b/govcd/test-resources/test_emptyJSON.json similarity index 100% rename from govcd/testdata/test_emptyJSON.json rename to govcd/test-resources/test_emptyJSON.json From e0ea54018503594b30645e64805843dceb90eaa5 Mon Sep 17 00:00:00 2001 From: Adam Jasinski Date: Fri, 14 Apr 2023 16:10:31 +0300 Subject: [PATCH 13/16] remove todo comment Signed-off-by: Adam Jasinski --- govcd/generic_functions_unit_test.go | 1 - 1 file changed, 1 deletion(-) diff --git a/govcd/generic_functions_unit_test.go b/govcd/generic_functions_unit_test.go index ae4d6c286..1a81ff02a 100644 --- a/govcd/generic_functions_unit_test.go +++ b/govcd/generic_functions_unit_test.go @@ -101,7 +101,6 @@ type testEntity struct { Name string `json:"name"` } -// TODO Write tests for marshalJSONAndWriteFile func Test_readFileAndUnmarshalJSON(t *testing.T) { type args struct { filename string From cd3215b4c69998ad54f8096a18457909310cecd6 Mon Sep 17 00:00:00 2001 From: Adam Jasinski Date: Tue, 18 Apr 2023 15:48:15 +0300 Subject: [PATCH 14/16] update token structure Signed-off-by: Adam Jasinski --- govcd/api_token.go | 4 ++++ govcd/generic_functions.go | 2 +- types/v56/types.go | 2 ++ 3 files changed, 7 insertions(+), 1 deletion(-) diff --git a/govcd/api_token.go b/govcd/api_token.go index c49375ac1..b6032c34c 100644 --- a/govcd/api_token.go +++ b/govcd/api_token.go @@ -12,6 +12,7 @@ import ( "net/http" "net/url" "strings" + "time" "github.com/vmware/go-vcloud-director/v2/types/v56" "github.com/vmware/go-vcloud-director/v2/util" @@ -60,6 +61,9 @@ func (vcdClient *VCDClient) SetServiceAccountApiToken(org, apiTokenFile string) // leave only the refresh token to not leave any sensitive information saApiToken = &types.ApiTokenRefresh{ RefreshToken: saApiToken.RefreshToken, + TokenType: "Service Account", + UpdatedBy: vcdClient.Client.UserAgent, + UpdatedOn: time.Now().Format(time.RFC3339), } err = marshalJSONAndWriteToFile(apiTokenFile, saApiToken, 0600) if err != nil { diff --git a/govcd/generic_functions.go b/govcd/generic_functions.go index 68a07f412..18da44366 100644 --- a/govcd/generic_functions.go +++ b/govcd/generic_functions.go @@ -61,7 +61,7 @@ func readFileAndUnmarshalJSON[T any](filename string, object T) error { // marshalJSONAndWriteToFile marshalls the given object into JSON and writes // to a file with the given permissions in octal format (e.g 0600) func marshalJSONAndWriteToFile[T any](filename string, object *T, permissions int) error { - data, err := json.Marshal(object) + data, err := json.MarshalIndent(object, " ", " ") if err != nil { return fmt.Errorf("error marshalling object to JSON: %s", err) } diff --git a/types/v56/types.go b/types/v56/types.go index 197f91172..7a63f781b 100644 --- a/types/v56/types.go +++ b/types/v56/types.go @@ -3261,6 +3261,8 @@ type ApiTokenRefresh struct { TokenType string `json:"token_type,omitempty"` ExpiresIn int `json:"expires_in,omitempty"` RefreshToken string `json:"refresh_token,omitempty"` + UpdatedBy string `json:"updated_by,omitempty"` + UpdatedOn string `json:"updated_on,omitempty"` } /**/ From 5ad857a4d9ed9b7d92dceb022f636ab93f86db8f Mon Sep 17 00:00:00 2001 From: Adam Jasinski Date: Tue, 18 Apr 2023 20:56:58 +0300 Subject: [PATCH 15/16] make the i/o functions non-generic Signed-off-by: Adam Jasinski --- govcd/api_token.go | 34 ++++++++++++++++++++++++++++++++++ govcd/generic_functions.go | 35 ----------------------------------- 2 files changed, 34 insertions(+), 35 deletions(-) diff --git a/govcd/api_token.go b/govcd/api_token.go index b6032c34c..497cf776c 100644 --- a/govcd/api_token.go +++ b/govcd/api_token.go @@ -9,8 +9,11 @@ import ( "encoding/json" "fmt" "io" + "io/fs" "net/http" "net/url" + "os" + "path" "strings" "time" @@ -154,3 +157,34 @@ func (vcdClient *VCDClient) GetBearerTokenFromApiToken(org, token string) (*type } return &tokenDef, nil } + +// readFileAndUnmarshalJSON reads a file and unmarshals it to the given variable +func readFileAndUnmarshalJSON(filename string, object any) error { + data, err := os.ReadFile(path.Clean(filename)) + if err != nil { + return fmt.Errorf("failed to read from file: %s", err) + } + + err = json.Unmarshal(data, object) + if err != nil { + return fmt.Errorf("failed to unmarshal file contents to the object: %s", err) + } + + return nil +} + +// marshalJSONAndWriteToFile marshalls the given object into JSON and writes +// to a file with the given permissions in octal format (e.g 0600) +func marshalJSONAndWriteToFile(filename string, object any, permissions int) error { + data, err := json.MarshalIndent(object, " ", " ") + if err != nil { + return fmt.Errorf("error marshalling object to JSON: %s", err) + } + + err = os.WriteFile(filename, data, fs.FileMode(permissions)) + if err != nil { + return fmt.Errorf("error writing to the file: %s", err) + } + + return nil +} diff --git a/govcd/generic_functions.go b/govcd/generic_functions.go index 18da44366..80b9e39ac 100644 --- a/govcd/generic_functions.go +++ b/govcd/generic_functions.go @@ -1,11 +1,7 @@ package govcd import ( - "encoding/json" "fmt" - "io/fs" - "os" - "path" ) // oneOrError is used to cover up a common pattern in this codebase which is usually used in @@ -42,34 +38,3 @@ func oneOrError[T any](key, name string, entitySlice []*T) (*T, error) { return entitySlice[0], nil } - -// readFileAndUnmarshalJSON reads a file and unmarshals it to the given variable -func readFileAndUnmarshalJSON[T any](filename string, object T) error { - data, err := os.ReadFile(path.Clean(filename)) - if err != nil { - return fmt.Errorf("failed to read from file: %s", err) - } - - err = json.Unmarshal(data, object) - if err != nil { - return fmt.Errorf("failed to unmarshal file contents to the object: %s", err) - } - - return nil -} - -// marshalJSONAndWriteToFile marshalls the given object into JSON and writes -// to a file with the given permissions in octal format (e.g 0600) -func marshalJSONAndWriteToFile[T any](filename string, object *T, permissions int) error { - data, err := json.MarshalIndent(object, " ", " ") - if err != nil { - return fmt.Errorf("error marshalling object to JSON: %s", err) - } - - err = os.WriteFile(filename, data, fs.FileMode(permissions)) - if err != nil { - return fmt.Errorf("error writing to the file: %s", err) - } - - return nil -} From 42ed1cecbb12417ad59261f499f938c3d2bb080b Mon Sep 17 00:00:00 2001 From: Adam Jasinski Date: Wed, 19 Apr 2023 09:23:02 +0300 Subject: [PATCH 16/16] move i/o functions to other file Signed-off-by: Adam Jasinski --- govcd/api_token_unit_test.go | 75 ++++++++++++++++++++++++++++ govcd/generic_functions_unit_test.go | 64 ------------------------ 2 files changed, 75 insertions(+), 64 deletions(-) create mode 100644 govcd/api_token_unit_test.go diff --git a/govcd/api_token_unit_test.go b/govcd/api_token_unit_test.go new file mode 100644 index 000000000..556408c13 --- /dev/null +++ b/govcd/api_token_unit_test.go @@ -0,0 +1,75 @@ +//go:build unit || ALL + +/* +* Copyright 2023 VMware, Inc. All rights reserved. Licensed under the Apache v2 License. + */ + +package govcd + +import ( + "reflect" + "testing" +) + +func Test_readFileAndUnmarshalJSON(t *testing.T) { + type args struct { + filename string + object *testEntity + } + tests := []struct { + name string + args args + want *testEntity + wantErr bool + }{ + { + name: "simpleCase", + args: args{ + filename: "test-resources/test.json", + object: &testEntity{}, + }, + want: &testEntity{Name: "test"}, + wantErr: false, + }, + { + name: "emptyFile", + args: args{ + filename: "test-resources/test_empty.json", + object: &testEntity{}, + }, + want: &testEntity{}, + wantErr: true, + }, + { + name: "emptyJSON", + args: args{ + filename: "test-resources/test_emptyjson.json", + object: &testEntity{}, + }, + want: &testEntity{}, + wantErr: false, + }, + { + name: "nonexistentFile", + args: args{ + filename: "thisfiledoesntexist.json", + object: &testEntity{}, + }, + want: &testEntity{}, + wantErr: true, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + err := readFileAndUnmarshalJSON(tt.args.filename, tt.args.object) + if (err != nil) != tt.wantErr { + t.Errorf("readFileAndUnmarshalJSON() error = %v, wantErr %v", err, tt.wantErr) + return + } + + if !reflect.DeepEqual(tt.args.object, tt.want) { + t.Errorf("readFileAndUnmarshalJSON() = %v, want %v", tt.args.object, tt.want) + } + }) + } +} diff --git a/govcd/generic_functions_unit_test.go b/govcd/generic_functions_unit_test.go index 1a81ff02a..0dc39ba5f 100644 --- a/govcd/generic_functions_unit_test.go +++ b/govcd/generic_functions_unit_test.go @@ -3,7 +3,6 @@ /* * Copyright 2023 VMware, Inc. All rights reserved. Licensed under the Apache v2 License. */ - package govcd import ( @@ -100,66 +99,3 @@ func Test_oneOrError(t *testing.T) { type testEntity struct { Name string `json:"name"` } - -func Test_readFileAndUnmarshalJSON(t *testing.T) { - type args struct { - filename string - object *testEntity - } - tests := []struct { - name string - args args - want *testEntity - wantErr bool - }{ - { - name: "simpleCase", - args: args{ - filename: "test-resources/test.json", - object: &testEntity{}, - }, - want: &testEntity{Name: "test"}, - wantErr: false, - }, - { - name: "emptyFile", - args: args{ - filename: "test-resources/test_empty.json", - object: &testEntity{}, - }, - want: &testEntity{}, - wantErr: true, - }, - { - name: "emptyJSON", - args: args{ - filename: "test-resources/test_emptyjson.json", - object: &testEntity{}, - }, - want: &testEntity{}, - wantErr: false, - }, - { - name: "nonexistentFile", - args: args{ - filename: "thisfiledoesntexist.json", - object: &testEntity{}, - }, - want: &testEntity{}, - wantErr: true, - }, - } - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - err := readFileAndUnmarshalJSON(tt.args.filename, tt.args.object) - if (err != nil) != tt.wantErr { - t.Errorf("readFileAndUnmarshalJSON() error = %v, wantErr %v", err, tt.wantErr) - return - } - - if !reflect.DeepEqual(tt.args.object, tt.want) { - t.Errorf("readFileAndUnmarshalJSON() = %v, want %v", tt.args.object, tt.want) - } - }) - } -}