fix review comments.

Author: dushaniw

gateway/gateway-controller/pkg/api/handlers/handlers.go

@@ -279,8 +279,15 @@ func (s *APIServer) CreateAPI(c *gin.Context) {
 			// API was updated and no longer has policies, remove the existing policy configuration
 			policyID := result.StoredConfig.ID + "-policies"
 			if err := s.policyManager.RemovePolicy(policyID); err != nil {
-				// Log at debug level since policy may not exist if API never had policies
-				log.Debug("No policy configuration to remove", slog.String("policy_id", policyID))
+				// Only treat "policy not found" as non-error (API may never have had policies)
+				// Other errors (storage failures, snapshot update failures) should be logged as errors
+				if errors.Is(err, storage.ErrPolicyNotFound) {
+					log.Debug("No policy configuration to remove", slog.String("policy_id", policyID))
+				} else {
+					log.Error("Failed to remove policy configuration",
+						slog.Any("error", err),
+						slog.String("policy_id", policyID))
+				}
 			} else {
 				log.Info("Derived policy configuration removed (API no longer has policies)",
 					slog.String("policy_id", policyID))

gateway/gateway-controller/pkg/controlplane/client.go

@@ -22,6 +22,7 @@ import (
 	"context"
 	"crypto/tls"
 	"encoding/json"
+	"errors"
 	"fmt"
 	"log/slog"
 	"net/http"
@@ -657,21 +658,28 @@ func (c *Client) handleAPIDeployedEvent(event map[string]interface{}) {
 					slog.String("correlation_id", deployedEvent.CorrelationID))
 			}
 		} else if result.IsUpdate {
-			// No policies but this is an update, so remove any existing policies
+			// API was updated and no longer has policies, remove the existing policy configuration
 			policyID := result.StoredConfig.ID + "-policies"
-			if _, err := c.policyManager.GetPolicy(policyID); err == nil {
-				if err := c.policyManager.RemovePolicy(policyID); err != nil {
-					c.logger.Error("Failed to remove policy from policy engine",
-						slog.Any("error", err),
+			if err := c.policyManager.RemovePolicy(policyID); err != nil {
+				// Only treat "not found" as non-error (API may never have had policies)
+				// Other errors (storage failures, snapshot update failures) should be logged as errors
+				if errors.Is(err, storage.ErrPolicyNotFound) {
+					c.logger.Debug("No policy configuration to remove",
 						slog.String("api_id", apiID),
 						slog.String("policy_id", policyID),
 						slog.String("correlation_id", deployedEvent.CorrelationID))
 				} else {
-					c.logger.Info("Successfully removed policy from policy engine",
+					c.logger.Error("Failed to remove policy configuration",
+						slog.Any("error", err),
 						slog.String("api_id", apiID),
 						slog.String("policy_id", policyID),
 						slog.String("correlation_id", deployedEvent.CorrelationID))
 				}
+			} else {
+				c.logger.Info("Derived policy configuration removed",
+					slog.String("api_id", apiID),
+					slog.String("policy_id", policyID),
+					slog.String("correlation_id", deployedEvent.CorrelationID))
 			}
 		}
 	} else if c.policyManager == nil {

gateway/gateway-controller/pkg/storage/errors.go

@@ -25,6 +25,9 @@ var (
 	// ErrNotFound is returned when a configuration is not found
 	ErrNotFound = errors.New("configuration not found")
 
+	// ErrPolicyNotFound is returned when a policy configuration is not found
+	ErrPolicyNotFound = errors.New("policy configuration not found")
+
 	// ErrConflict is returned when a configuration with the same name/version already exists
 	ErrConflict = errors.New("configuration already exists")
 

gateway/gateway-controller/pkg/storage/policy_store.go

@@ -108,7 +108,7 @@ func (s *PolicyStore) Delete(id string) error {
 
 	policy, exists := s.policies[id]
 	if !exists {
-		return fmt.Errorf("policy configuration with ID %s not found", id)
+		return fmt.Errorf("policy configuration with ID %s: %w", id, ErrPolicyNotFound)
 	}
 
 	// Remove from both maps