From fade096103c2bfb653927e41f50a5be1d99343e8 Mon Sep 17 00:00:00 2001 From: MarlenKoch Date: Wed, 18 Dec 2024 18:03:34 +0100 Subject: [PATCH] fix: provider secret special chars (#53) * fix(servicemanager): improve Secret-Data error handling * fix(cisclient): improve error message when secret contains special chars * test: add tests for NewBTPClient function * fix(tfclient): add more declarative error message when parsing secret data * fix: add error message to TerraformSetupBuilder function --------- Co-authored-by: sdischer-sap --- btp/cisclient.go | 8 +++-- btp/client_test.go | 44 +++++++++++++++++++++++++++ internal/clients/tfclient/tfclient.go | 19 ++++++------ 3 files changed, 59 insertions(+), 12 deletions(-) create mode 100644 btp/client_test.go diff --git a/btp/cisclient.go b/btp/cisclient.go index 0036d2b..3cea1ef 100644 --- a/btp/cisclient.go +++ b/btp/cisclient.go @@ -19,8 +19,9 @@ import ( ) const ( - errInstanceDoesNotExist = "cannot delete instance does not exist" - errCouldNotParseCISSecret = "CIS Secret seems malformed" + errInstanceDoesNotExist = "cannot delete instance does not exist" + errCouldNotParseCISSecret = "CIS Secret seems malformed" + errCouldNotParseUserCredential = "error while parsing sa-provider-secret JSON" ) type InstanceParameters = map[string]interface{} @@ -242,7 +243,8 @@ func ServiceClientFromSecret(cisSecret []byte, userSecret []byte) (Client, error var userCredential UserCredential if err := json.Unmarshal(userSecret, &userCredential); err != nil { - return Client{}, err + return Client{}, errors.Wrap(err, errCouldNotParseUserCredential) + } credential := &Credentials{ diff --git a/btp/client_test.go b/btp/client_test.go new file mode 100644 index 0000000..93f2898 --- /dev/null +++ b/btp/client_test.go @@ -0,0 +1,44 @@ +package btp + +import ( + "strings" + "testing" + + "github.com/sap/crossplane-provider-btp/internal" +) + +func TestNewBTPClient(t *testing.T) { + + tests := []struct { + name string + cisSecretData []byte + serviceAccountSecretData []byte + wantErr *string + }{ + { + name: "sucessfully create new btp client", + cisSecretData: []byte("{\"endpoints\": {\"accounts_service_url\": \"xxx\", \"cloud_automation_url\": \"xxx\", \"entitlements_service_url\": \"xxx\", \"events_service_url\": \"xxx\", \"external_provider_registry_url\": \"xxx\", \"metadata_service_url\": \"xxx\", \"order_processing_url\": \"xxx\", \"provisioning_service_url\": \"xxx\", \"saas_registry_service_url\": \"xxx\" }, \"grant_type\": \"client_credentials\", \"sap.cloud.service\": \"xxx\", \"uaa\": { \"apiurl\": \"xxx\", \"clientid\": \"xxx\", \"clientsecret\": \"xxx\", \"credential-type\": \"binding-secret\", \"identityzone\": \"xxx\", \"identityzoneid\": \"xxx\", \"sburl\": \"xxx\", \"subaccountid\": \"xxx\", \"tenantid\": \"xxx\", \"tenantmode\": \"shared\", \"uaadomain\": \"xxx\", \"url\": \"xxx\", \"verificationkey\": \"xxx\", \"xsappname\": \"xxx\", \"xsmasterappname\": \"xxx\", \"zoneid\": \"xxx\"}}"), + serviceAccountSecretData: []byte("{\"email\": \"1@sap.com\",\"username\": \"xxx\",\"password\": \"xxx\"}"), + wantErr: nil, + }, + { + name: "fail on invalid json", + cisSecretData: []byte("{\"endpoints\": {\"accounts_service_url\": \"xxx\", \"cloud_automation_url\": \"xxx\", \"entitlements_service_url\": \"xxx\", \"events_service_url\": \"xxx\", \"external_provider_registry_url\": \"xxx\", \"metadata_service_url\": \"xxx\", \"order_processing_url\": \"xxx\", \"provisioning_service_url\": \"xxx\", \"saas_registry_service_url\": \"xxx\" }, \"grant_type\": \"client_credentials\", \"sap.cloud.service\": \"xxx\", \"uaa\": { \"apiurl\": \"xxx\", \"clientid\": \"xxx\", \"clientsecret\": \"xxx\", \"credential-type\": \"binding-secret\", \"identityzone\": \"xxx\", \"identityzoneid\": \"xxx\", \"sburl\": \"xxx\", \"subaccountid\": \"xxx\", \"tenantid\": \"xxx\", \"tenantmode\": \"shared\", \"uaadomain\": \"xxx\", \"url\": \"xxx\", \"verificationkey\": \"xxx\", \"xsappname\": \"xxx\", \"xsmasterappname\": \"xxx\", \"zoneid\": \"xxx\"}}"), + serviceAccountSecretData: []byte("{\"email\": \"1@sap.com\",\"username\": \"xxx\",\"password\": \"xx\"x\"}"), + wantErr: internal.Ptr(errCouldNotParseUserCredential), + }, + } + for _, tt := range tests { + t.Run( + tt.name, func(t *testing.T) { + _, err := NewBTPClient(tt.cisSecretData, tt.serviceAccountSecretData) + if err != nil && tt.wantErr == nil { + t.Errorf("unexpected error output: %s", err) + } + if err != nil && !strings.Contains(err.Error(), internal.Val(tt.wantErr)) { + t.Errorf("error does not contain wanted error message: %s", err) + } + }, + ) + } +} diff --git a/internal/clients/tfclient/tfclient.go b/internal/clients/tfclient/tfclient.go index f025d74..7d42ea8 100644 --- a/internal/clients/tfclient/tfclient.go +++ b/internal/clients/tfclient/tfclient.go @@ -21,13 +21,14 @@ import ( ) const ( - errNoProviderConfig = "no providerConfigRef provided" - errGetProviderConfig = "cannot get referenced ProviderConfig" - errTrackUsage = "cannot track ProviderConfig usage" - errExtractCredentials = "cannot extract credentials" - errUnmarshalCredentials = "cannot unmarshal btp-account-tf credentials as JSON" - errTrackRUsage = "cannot track ResourceUsage" - errGetServiceAccountCreds = "cannot get Service Account credentials" + errNoProviderConfig = "no providerConfigRef provided" + errGetProviderConfig = "cannot get referenced ProviderConfig" + errTrackUsage = "cannot track ProviderConfig usage" + errExtractCredentials = "cannot extract credentials" + errUnmarshalCredentials = "cannot unmarshal btp-account-tf credentials as JSON" + errTrackRUsage = "cannot track ResourceUsage" + errGetServiceAccountCreds = "cannot get Service Account credentials" + errCouldNotParseUserCredential = "error while parsing sa-provider-secret JSON" ) var ( @@ -91,7 +92,7 @@ func TerraformSetupBuilder(version, providerSource, providerVersion string) terr var userCredential btp.UserCredential if err := json.Unmarshal(ServiceAccountSecretData, &userCredential); err != nil { - return ps, err + return ps, errors.Wrap(err, errCouldNotParseUserCredential) } ps.Configuration = map[string]any{ @@ -135,7 +136,7 @@ func TerraformSetupBuilderNoTracking(version, providerSource, providerVersion st var userCredential btp.UserCredential if err := json.Unmarshal(ServiceAccountSecretData, &userCredential); err != nil { - return ps, err + return ps, errors.Wrap(err, errCouldNotParseUserCredential) } ps.Configuration = map[string]any{