From b972463951bcd53e9729a443183c4681bbabbbc8 Mon Sep 17 00:00:00 2001 From: Quentin Brosse Date: Tue, 5 Nov 2019 18:47:27 +0100 Subject: [PATCH 1/5] feat: add ClientCredentialError --- internal/e2e/errors_test.go | 2 +- internal/e2e/human_test.go | 16 ++++---- scw/client_option.go | 11 ++++- scw/client_option_test.go | 80 ++++++++++++++++++++++--------------- scw/errors.go | 19 +++++++++ 5 files changed, 86 insertions(+), 42 deletions(-) diff --git a/internal/e2e/errors_test.go b/internal/e2e/errors_test.go index c87b7884e..c2aa273ed 100644 --- a/internal/e2e/errors_test.go +++ b/internal/e2e/errors_test.go @@ -9,7 +9,7 @@ import ( ) func TestStandardErrors(t *testing.T) { - client, _, err := newE2EClient(true) + client, _, _, err := newE2EClient(true) testhelpers.AssertNoError(t, err) t.Run("not found", func(t *testing.T) { diff --git a/internal/e2e/human_test.go b/internal/e2e/human_test.go index 379d37d74..3a8600ddc 100644 --- a/internal/e2e/human_test.go +++ b/internal/e2e/human_test.go @@ -8,13 +8,13 @@ import ( "github.com/scaleway/scaleway-sdk-go/scw" ) -func newE2EClient(withAuthInClient bool) (*test.API, string, error) { +func newE2EClient(withAuthInClient bool) (*test.API, string, string, error) { client, err := scw.NewClient( scw.WithDefaultRegion(scw.RegionFrPar), scw.WithUserAgent("sdk-e2e-test"), ) if err != nil { - return nil, "", err + return nil, "", "", err } testClient := test.NewAPI(client) @@ -22,31 +22,31 @@ func newE2EClient(withAuthInClient bool) (*test.API, string, error) { Username: "sidi", }) if err != nil { - return nil, "", err + return nil, "", "", err } if withAuthInClient { client, err = scw.NewClient( scw.WithDefaultRegion(scw.RegionFrPar), - scw.WithAuth("", registerResponse.SecretKey), + scw.WithAuth(registerResponse.AccessKey, registerResponse.SecretKey), scw.WithUserAgent("sdk-e2e-test"), ) testClient = test.NewAPI(client) } - return testClient, registerResponse.SecretKey, err + return testClient, registerResponse.AccessKey, registerResponse.SecretKey, err } func TestAuthInRequest(t *testing.T) { - client, secretKey, err := newE2EClient(false) + client, accessKey, secretKey, err := newE2EClient(false) testhelpers.AssertNoError(t, err) - requestWithAuth := scw.WithAuthRequest("", secretKey) + requestWithAuth := scw.WithAuthRequest(accessKey, secretKey) _, err = client.CreateHuman(&test.CreateHumanRequest{}, requestWithAuth) testhelpers.AssertNoError(t, err) } func TestHuman(t *testing.T) { - client, _, err := newE2EClient(true) + client, _, _, err := newE2EClient(true) testhelpers.AssertNoError(t, err) // create diff --git a/scw/client_option.go b/scw/client_option.go index e10ec59c8..396625715 100644 --- a/scw/client_option.go +++ b/scw/client_option.go @@ -172,7 +172,16 @@ func (s *settings) apply(opts []ClientOption) { func (s *settings) validate() error { var err error if s.token == nil { - return errors.New("no credential option provided") + return &ClientCredentialError{Type: clientCredentialError_NoOption} + } + + if token, isToken := s.token.(*auth.Token); isToken { + if token.AccessKey == "" { + return &ClientCredentialError{Type: clientCredentialError_EmptyAccessKey} + } + if token.SecretKey == "" { + return &ClientCredentialError{Type: clientCredentialError_EmptySecreyKey} + } } _, err = url.Parse(s.apiURL) diff --git a/scw/client_option_test.go b/scw/client_option_test.go index bdf87a87a..5ee8953dd 100644 --- a/scw/client_option_test.go +++ b/scw/client_option_test.go @@ -15,8 +15,8 @@ const ( var ( defaultOrganizationID = "6170692e-7363-616c-6577-61792e636f6d" // hint: | xxd -ps -r - defaultRegion = RegionNlAms - defaultZone = ZoneNlAms1 + defaultRegion = RegionNlAms + defaultZone = ZoneNlAms1 ) func TestClientOptions(t *testing.T) { @@ -37,12 +37,28 @@ func TestClientOptions(t *testing.T) { }, }, { - name: "Should throw an credential error", + name: "Should throw a no credential option provided", clientOption: func(s *settings) { s.apiURL = apiURL }, errStr: "scaleway-sdk-go: no credential option provided", }, + { + name: "Should throw a access key error", + clientOption: func(s *settings) { + s.apiURL = apiURL + s.token = auth.NewToken("", testSecretKey) + }, + errStr: "scaleway-sdk-go: access key cannot be empty", + }, + { + name: "Should throw a secret key error", + clientOption: func(s *settings) { + s.apiURL = apiURL + s.token = auth.NewToken(testSecretKey, "") + }, + errStr: "scaleway-sdk-go: secret key cannot be empty", + }, { name: "Should throw an url error", clientOption: func(s *settings) { @@ -107,56 +123,56 @@ func TestCombinedClientOptions(t *testing.T) { env map[string]string files map[string]string - expectedError string - expectedAccessKey string - expectedSecretKey string - expectedAPIURL string + expectedError string + expectedAccessKey string + expectedSecretKey string + expectedAPIURL string expectedDefaultOrganizationID *string - expectedDefaultRegion *Region - expectedDefaultZone *Zone + expectedDefaultRegion *Region + expectedDefaultZone *Zone }{ { name: "Complete config file with env variables", env: map[string]string{ - "HOME": "{HOME}", - scwAccessKeyEnv: v2ValidAccessKey2, - scwSecretKeyEnv: v2ValidSecretKey2, - scwAPIURLEnv: v2ValidAPIURL2, + "HOME": "{HOME}", + scwAccessKeyEnv: v2ValidAccessKey2, + scwSecretKeyEnv: v2ValidSecretKey2, + scwAPIURLEnv: v2ValidAPIURL2, scwDefaultOrganizationIDEnv: v2ValidDefaultOrganizationID2, - scwDefaultRegionEnv: v2ValidDefaultRegion2, - scwDefaultZoneEnv: v2ValidDefaultZone2, + scwDefaultRegionEnv: v2ValidDefaultRegion2, + scwDefaultZoneEnv: v2ValidDefaultZone2, }, files: map[string]string{ ".config/scw/config.yaml": v2CompleteValidConfigFile, }, - expectedAccessKey: v2ValidAccessKey2, - expectedSecretKey: v2ValidSecretKey2, - expectedAPIURL: v2ValidAPIURL2, + expectedAccessKey: v2ValidAccessKey2, + expectedSecretKey: v2ValidSecretKey2, + expectedAPIURL: v2ValidAPIURL2, expectedDefaultOrganizationID: s(v2ValidDefaultOrganizationID2), - expectedDefaultRegion: r(Region(v2ValidDefaultRegion2)), - expectedDefaultZone: z(Zone(v2ValidDefaultZone2)), + expectedDefaultRegion: r(Region(v2ValidDefaultRegion2)), + expectedDefaultZone: z(Zone(v2ValidDefaultZone2)), }, { name: "Complete config with active profile env variable and all env variables", env: map[string]string{ - "HOME": "{HOME}", - scwActiveProfileEnv: v2ValidProfile, - scwAccessKeyEnv: v2ValidAccessKey, - scwSecretKeyEnv: v2ValidSecretKey, - scwAPIURLEnv: v2ValidAPIURL, + "HOME": "{HOME}", + scwActiveProfileEnv: v2ValidProfile, + scwAccessKeyEnv: v2ValidAccessKey, + scwSecretKeyEnv: v2ValidSecretKey, + scwAPIURLEnv: v2ValidAPIURL, scwDefaultOrganizationIDEnv: v2ValidDefaultOrganizationID, - scwDefaultRegionEnv: v2ValidDefaultRegion, - scwDefaultZoneEnv: v2ValidDefaultZone, + scwDefaultRegionEnv: v2ValidDefaultRegion, + scwDefaultZoneEnv: v2ValidDefaultZone, }, files: map[string]string{ ".config/scw/config.yaml": v2CompleteValidConfigFile, }, - expectedAccessKey: v2ValidAccessKey, - expectedSecretKey: v2ValidSecretKey, - expectedAPIURL: v2ValidAPIURL, + expectedAccessKey: v2ValidAccessKey, + expectedSecretKey: v2ValidSecretKey, + expectedAPIURL: v2ValidAPIURL, expectedDefaultOrganizationID: s(v2ValidDefaultOrganizationID), - expectedDefaultRegion: r(Region(v2ValidDefaultRegion)), - expectedDefaultZone: z(Zone(v2ValidDefaultZone)), + expectedDefaultRegion: r(Region(v2ValidDefaultRegion)), + expectedDefaultZone: z(Zone(v2ValidDefaultZone)), }, } diff --git a/scw/errors.go b/scw/errors.go index 86672113d..c84523a58 100644 --- a/scw/errors.go +++ b/scw/errors.go @@ -255,3 +255,22 @@ func (e *OutOfStockError) Error() string { func (e *OutOfStockError) GetRawBody() json.RawMessage { return e.RawBody } + +type clientCredentialErrorType string + +const ( + clientCredentialError_NoOption = clientCredentialErrorType("no credential option provided") + clientCredentialError_EmptyAccessKey = clientCredentialErrorType("access key cannot be empty") + clientCredentialError_EmptySecreyKey = clientCredentialErrorType("secret key cannot be empty") +) + +// clientCredentialError indicates that credentials have been badly provided for the client creation. +type ClientCredentialError struct { + Type clientCredentialErrorType +} + +// IsScwSdkError implements the SdkError interface +func (e ClientCredentialError) IsScwSdkError() {} +func (e ClientCredentialError) Error() string { + return fmt.Sprintf("scaleway-sdk-go: %s", e.Type) +} From 3f3f429939f3f745cd1949b0ec216fa2993fb541 Mon Sep 17 00:00:00 2001 From: Quentin Brosse Date: Tue, 12 Nov 2019 18:21:38 +0100 Subject: [PATCH 2/5] panic --- scw/client_option.go | 3 ++- scw/errors.go | 1 - 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/scw/client_option.go b/scw/client_option.go index 396625715..843976749 100644 --- a/scw/client_option.go +++ b/scw/client_option.go @@ -172,7 +172,8 @@ func (s *settings) apply(opts []ClientOption) { func (s *settings) validate() error { var err error if s.token == nil { - return &ClientCredentialError{Type: clientCredentialError_NoOption} + // It should not happen, WithoutAuth option is used by default. + panic(errors.New("no credential option provided")) } if token, isToken := s.token.(*auth.Token); isToken { diff --git a/scw/errors.go b/scw/errors.go index c84523a58..3c7739575 100644 --- a/scw/errors.go +++ b/scw/errors.go @@ -259,7 +259,6 @@ func (e *OutOfStockError) GetRawBody() json.RawMessage { type clientCredentialErrorType string const ( - clientCredentialError_NoOption = clientCredentialErrorType("no credential option provided") clientCredentialError_EmptyAccessKey = clientCredentialErrorType("access key cannot be empty") clientCredentialError_EmptySecreyKey = clientCredentialErrorType("secret key cannot be empty") ) From aa5c3d46a77d73b8840263af8d841df4749905e3 Mon Sep 17 00:00:00 2001 From: Quentin Brosse Date: Tue, 12 Nov 2019 18:31:05 +0100 Subject: [PATCH 3/5] fix-tests --- scw/client_option_test.go | 7 ------- 1 file changed, 7 deletions(-) diff --git a/scw/client_option_test.go b/scw/client_option_test.go index 5ee8953dd..05596221b 100644 --- a/scw/client_option_test.go +++ b/scw/client_option_test.go @@ -36,13 +36,6 @@ func TestClientOptions(t *testing.T) { s.defaultZone = &defaultZone }, }, - { - name: "Should throw a no credential option provided", - clientOption: func(s *settings) { - s.apiURL = apiURL - }, - errStr: "scaleway-sdk-go: no credential option provided", - }, { name: "Should throw a access key error", clientOption: func(s *settings) { From 868de4d278c5e335753a6c91280d88e79623845a Mon Sep 17 00:00:00 2001 From: Quentin Brosse Date: Wed, 13 Nov 2019 10:29:28 +0100 Subject: [PATCH 4/5] address comment --- scw/client_option_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/scw/client_option_test.go b/scw/client_option_test.go index 05596221b..1299a5357 100644 --- a/scw/client_option_test.go +++ b/scw/client_option_test.go @@ -37,7 +37,7 @@ func TestClientOptions(t *testing.T) { }, }, { - name: "Should throw a access key error", + name: "Should throw an access key error", clientOption: func(s *settings) { s.apiURL = apiURL s.token = auth.NewToken("", testSecretKey) From baa2b082844e620a0b09a720130ee16d48f78a77 Mon Sep 17 00:00:00 2001 From: Quentin Brosse Date: Wed, 13 Nov 2019 10:39:40 +0100 Subject: [PATCH 5/5] make ClientCredentialError.Type private --- scw/client_option.go | 4 ++-- scw/errors.go | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/scw/client_option.go b/scw/client_option.go index 843976749..782238be1 100644 --- a/scw/client_option.go +++ b/scw/client_option.go @@ -178,10 +178,10 @@ func (s *settings) validate() error { if token, isToken := s.token.(*auth.Token); isToken { if token.AccessKey == "" { - return &ClientCredentialError{Type: clientCredentialError_EmptyAccessKey} + return &ClientCredentialError{errorType: clientCredentialError_EmptyAccessKey} } if token.SecretKey == "" { - return &ClientCredentialError{Type: clientCredentialError_EmptySecreyKey} + return &ClientCredentialError{errorType: clientCredentialError_EmptySecreyKey} } } diff --git a/scw/errors.go b/scw/errors.go index 3c7739575..cd1e201e8 100644 --- a/scw/errors.go +++ b/scw/errors.go @@ -265,11 +265,11 @@ const ( // clientCredentialError indicates that credentials have been badly provided for the client creation. type ClientCredentialError struct { - Type clientCredentialErrorType + errorType clientCredentialErrorType } // IsScwSdkError implements the SdkError interface func (e ClientCredentialError) IsScwSdkError() {} func (e ClientCredentialError) Error() string { - return fmt.Sprintf("scaleway-sdk-go: %s", e.Type) + return fmt.Sprintf("scaleway-sdk-go: %s", e.errorType) }