Merge pull request #24 from Conjur-Enterprise/ss-bugfixes-1

CNJR-9026: Improve Error Logging

Author: imheresamir

CHANGELOG.md

@@ -4,6 +4,15 @@ All notable changes to this project will be documented in this file.
 The format is based on [Keep a Changelog](http://keepachangelog.com/en/1.0.0/)
 and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0.html).
 
+## [0.3.1] - 2025-04-23
+
+### Fixed
+- Provide error messages from the API back to the user
+- Make `retention` and `purge` fields for safes `computed` so the API can provide default values if they are not user-provided
+- Safe resource update should look at safe members in the plan, not the existing state
+- Handle removing or adding the optional `address` property when updating an account
+- Fix crash when debug logging is enabled
+
 ## [0.3.0] - 2025-04-11
 
 ### Added

internal/cyberark/client.go

@@ -54,12 +54,18 @@ func (c *Client) DoRequest(ctx context.Context, method string, path string, body
 
 	if c.logResponse {
 		responseBody, err := io.ReadAll(response.Body)
+
+		var url url.URL
+		if req.URL != nil {
+			url = *req.URL
+		}
+
 		if err == nil && len(responseBody) > 0 {
 			tflog.Debug(
 				ctx,
 				"Response from CyberArk API",
 				map[string]interface{}{
-					"request_url":     req.URL,
+					"request_url":     url.String(),
 					"method":          method,
 					"response_status": response.Status,
 					"response_body":   string(responseBody),

internal/cyberark/error.go

@@ -0,0 +1,32 @@
+package cyberark
+
+import (
+	"encoding/json"
+	"errors"
+	"fmt"
+	"io"
+)
+
+func APIErrorFromResponse(code int, body io.ReadCloser) error {
+	errorStr := fmt.Sprintf("HTTP status code %d", code)
+
+	var jsonError interface{}
+	err := json.NewDecoder(body).Decode(&jsonError)
+	if err != nil {
+		return errors.New(errorStr)
+	}
+
+	if jsonError != nil {
+		if jsonErrorStr, ok := jsonError.(string); ok {
+			// If the error is a string, just append it to the error message
+			errorStr = fmt.Sprintf("%s\n\n%s", errorStr, jsonErrorStr)
+		} else {
+			prettyJSON, err := json.MarshalIndent(jsonError, "", "  ")
+			if err == nil {
+				errorStr = fmt.Sprintf("%s\n%s", errorStr, string(prettyJSON))
+			}
+		}
+	}
+
+	return errors.New(errorStr)
+}

internal/cyberark/pam.go

@@ -70,11 +70,8 @@ func (a *pamAPI) AddAccount(ctx context.Context, credential Credential) (*Creden
 		return nil, err
 	}
 
-	if response.StatusCode == 409 {
-		tflog.Info(ctx, fmt.Sprintf("Account [%s] already exists.", *credential.Name))
-		return nil, nil
-	} else if response.StatusCode != 201 {
-		return nil, fmt.Errorf("failed to add account, expected status code 201, got %d", response.StatusCode)
+	if response.StatusCode != 201 {
+		return nil, APIErrorFromResponse(response.StatusCode, response.Body)
 	}
 
 	createdAccount := CredentialResponse{}
@@ -83,9 +80,6 @@ func (a *pamAPI) AddAccount(ctx context.Context, credential Credential) (*Creden
 		return nil, err
 	}
 
-	tflog.Info(ctx, fmt.Sprintf("Successfully added new account [%s]: Name [%s] - ID [%v]",
-		*createdAccount.UserName, *createdAccount.Name, *createdAccount.CredID))
-
 	return &createdAccount, nil
 }
 
@@ -104,7 +98,7 @@ func (a *pamAPI) GetAccount(ctx context.Context, accountID string) (*CredentialR
 	}
 
 	if response.StatusCode != 200 {
-		return nil, fmt.Errorf("failed to get account. Expected status code 200, got %d", response.StatusCode)
+		return nil, APIErrorFromResponse(response.StatusCode, response.Body)
 	}
 
 	account := CredentialResponse{}
@@ -133,7 +127,7 @@ func (a *pamAPI) FilterAccounts(ctx context.Context, search string, filter []str
 	}
 
 	if response.StatusCode != 200 {
-		return nil, fmt.Errorf("failed to filter accounts. Expected status code 200, got %d", response.StatusCode)
+		return nil, APIErrorFromResponse(response.StatusCode, response.Body)
 	}
 
 	searchAccounts := CredentialSearchResponse{}
@@ -198,8 +192,7 @@ func (a *pamAPI) UpdateAccount(ctx context.Context, accountID string, credential
 	}
 
 	if response.StatusCode != 200 {
-		tflog.Error(ctx, fmt.Sprintf("failed to update account, got response: %s", response.Body))
-		return nil, fmt.Errorf("failed to update account. Expected status code 200, got %d", response.StatusCode)
+		return nil, APIErrorFromResponse(response.StatusCode, response.Body)
 	}
 
 	updatedAccount := CredentialResponse{}
@@ -208,9 +201,6 @@ func (a *pamAPI) UpdateAccount(ctx context.Context, accountID string, credential
 		return nil, err
 	}
 
-	tflog.Info(ctx, fmt.Sprintf("Successfully updated account [%s]: Name [%s] - ID [%v]",
-		*updatedAccount.UserName, *updatedAccount.Name, *updatedAccount.CredID))
-
 	return &updatedAccount, nil
 }
 
@@ -237,6 +227,19 @@ func generateAccountPatch(existing *CredentialResponse, desired *Credential) ([]
 			"path":  "/address",
 			"value": *desired.Address,
 		})
+	} else if desired.Address != nil && existing.Address == nil {
+		// If existing has no address but desired does, add it
+		patch = append(patch, map[string]interface{}{
+			"op":    "add",
+			"path":  "/address",
+			"value": *desired.Address,
+		})
+	} else if desired.Address == nil && existing.Address != nil {
+		// If existing has an address but desired does not, remove it
+		patch = append(patch, map[string]interface{}{
+			"op":   "remove",
+			"path": "/address",
+		})
 	}
 
 	if desired.UserName != nil && existing.UserName != nil && *existing.UserName != *desired.UserName {
@@ -332,11 +335,9 @@ func (a *pamAPI) DeleteAccount(ctx context.Context, accountID string) error {
 	}
 
 	if response.StatusCode != 204 {
-		tflog.Error(ctx, fmt.Sprintf("failed to delete account, got response: %s", response.Body))
-		return fmt.Errorf("failed to delete account. Expected status code 204, got %d", response.StatusCode)
+		return APIErrorFromResponse(response.StatusCode, response.Body)
 	}
 
-	tflog.Info(ctx, fmt.Sprintf("Successfully deleted account with ID [%s]", accountID))
 	return nil
 }
 
@@ -359,11 +360,8 @@ func (a *pamAPI) AddSafe(ctx context.Context, safe SafeData) (*SafeData, error)
 		return nil, err
 	}
 
-	if response.StatusCode == 409 {
-		tflog.Info(ctx, fmt.Sprintf("Safe [%s] already exists.", *safe.Name))
-		return nil, nil
-	} else if response.StatusCode != 201 {
-		return nil, fmt.Errorf("failed to add safe, expected status code 201, got %d", response.StatusCode)
+	if response.StatusCode != 201 {
+		return nil, APIErrorFromResponse(response.StatusCode, response.Body)
 	}
 
 	savedSafe := SafeData{}
@@ -372,9 +370,6 @@ func (a *pamAPI) AddSafe(ctx context.Context, safe SafeData) (*SafeData, error)
 		return nil, err
 	}
 
-	tflog.Info(ctx, fmt.Sprintf("Successfully added new safe [%s]: Name [%s] - ID [%v]",
-		*savedSafe.Name, *savedSafe.URLID, *savedSafe.NUMBER))
-
 	return &savedSafe, nil
 }
 
@@ -393,7 +388,7 @@ func (a *pamAPI) GetSafe(ctx context.Context, safeID string) (*SafeData, error)
 	}
 
 	if response.StatusCode != 200 {
-		return nil, fmt.Errorf("failed to get safe. Expected status code 200, got %d", response.StatusCode)
+		return nil, APIErrorFromResponse(response.StatusCode, response.Body)
 	}
 
 	safe := SafeData{}
@@ -425,8 +420,7 @@ func (a *pamAPI) UpdateSafe(ctx context.Context, safeID string, safe SafeData) (
 	}
 
 	if response.StatusCode != 200 {
-		tflog.Error(ctx, fmt.Sprintf("failed to update safe, got response: %s", response.Body))
-		return nil, fmt.Errorf("failed to update safe. Expected status code 200, got %d", response.StatusCode)
+		return nil, APIErrorFromResponse(response.StatusCode, response.Body)
 	}
 
 	updatedSafe := SafeData{}
@@ -435,9 +429,6 @@ func (a *pamAPI) UpdateSafe(ctx context.Context, safeID string, safe SafeData) (
 		return nil, err
 	}
 
-	tflog.Info(ctx, fmt.Sprintf("Successfully updated safe [%s]: Name [%s] - ID [%v]",
-		*updatedSafe.Name, *updatedSafe.URLID, *updatedSafe.NUMBER))
-
 	return &updatedSafe, nil
 }
 
@@ -456,11 +447,9 @@ func (a *pamAPI) DeleteSafe(ctx context.Context, safeID string) error {
 	}
 
 	if response.StatusCode != 204 {
-		tflog.Error(ctx, fmt.Sprintf("failed to delete safe, got response: %s", response.Body))
-		return fmt.Errorf("failed to delete safe. Expected status code 204, got %d", response.StatusCode)
+		return APIErrorFromResponse(response.StatusCode, response.Body)
 	}
 
-	tflog.Info(ctx, fmt.Sprintf("Successfully deleted safe with ID [%s]", safeID))
 	return nil
 }
 
@@ -489,11 +478,8 @@ func (a *pamAPI) AddSafeMember(ctx context.Context, safe SafeData) (*Member, err
 		return nil, err
 	}
 
-	if response.StatusCode == 409 {
-		tflog.Info(ctx, fmt.Sprintf("Safe [%s] already has member [%s].", *safe.Name, *safe.Owner))
-		return nil, nil
-	} else if response.StatusCode != 201 {
-		return nil, fmt.Errorf("failed to add safe member, expected status code 201, got %d", response.StatusCode)
+	if response.StatusCode != 201 {
+		return nil, APIErrorFromResponse(response.StatusCode, response.Body)
 	}
 
 	safeMember := Member{}
@@ -520,7 +506,7 @@ func (a *pamAPI) GetSafeMember(ctx context.Context, safe SafeData) (*Member, err
 	}
 
 	if response.StatusCode != 200 {
-		return nil, fmt.Errorf("failed to get safe member. Expected status code 200, got %d", response.StatusCode)
+		return nil, APIErrorFromResponse(response.StatusCode, response.Body)
 	}
 
 	safeMember := Member{}
@@ -565,18 +551,14 @@ func (a *pamAPI) UpdateSafeMember(ctx context.Context, safe SafeData) (*Member,
 	}
 
 	if response.StatusCode != 200 {
-		tflog.Error(ctx, fmt.Sprintf("failed to update safe member, got response: %s", response.Body))
-		return nil, fmt.Errorf("failed to update safe member, expected status code 200, got %d", response.StatusCode)
+		return nil, APIErrorFromResponse(response.StatusCode, response.Body)
 	}
 
-	tflog.Info(ctx, fmt.Sprintf("Successfully updated member [%s] permissions in safe [%s]", *safe.Owner, *safe.Name))
 	return &updatedSafeMember, nil
 }
 
 // DeleteSafeMember deletes a safe member from the SecretsHub.
 func (a *pamAPI) DeleteSafeMember(ctx context.Context, safeName string, memberName string) error {
-	tflog.Debug(ctx, fmt.Sprintf("Attempting to delete member [%s] from safe [%s]", memberName, safeName))
-
 	response, err := a.client.DoRequest(
 		ctx,
 		"DELETE",
@@ -589,14 +571,10 @@ func (a *pamAPI) DeleteSafeMember(ctx context.Context, safeName string, memberNa
 		return err
 	}
 
-	if response.StatusCode == 404 {
-		tflog.Info(ctx, fmt.Sprintf("Member [%s] not found in safe [%s]", memberName, safeName))
-		return nil
-	} else if response.StatusCode != 204 {
-		return fmt.Errorf("failed to delete safe member, expected status code 204, got %d", response.StatusCode)
+	if response.StatusCode != 204 {
+		return APIErrorFromResponse(response.StatusCode, response.Body)
 	}
 
-	tflog.Info(ctx, fmt.Sprintf("Successfully removed member [%s] from safe [%s]", memberName, safeName))
 	return nil
 }