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 <stephan.discher@sap.com>
SAP · Dec 18, 2024 · fade096 · fade096
1 parent 5b8b863
commit fade096
Show file tree

Hide file tree

Showing 3 changed files with 59 additions and 12 deletions.
diff --git 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
@@ -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
@@ -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{