fix: Webhook provider PushSecret not working (#5445)

* fix: Webhook provider PushSecret not working

Signed-off-by: Gergely Brautigam <182850+Skarlso@users.noreply.github.com>

On-behalf-of: Gergely Brautigam <gergely.brautigam@sap.com>

* removed the customizable delete operation

Signed-off-by: Gergely Brautigam <182850+Skarlso@users.noreply.github.com>

---------

Signed-off-by: Gergely Brautigam <182850+Skarlso@users.noreply.github.com>

apis/externalsecrets/v1/secretstore_webhook_types.go

@@ -48,7 +48,8 @@ type WebhookProvider struct {
 	Timeout *metav1.Duration `json:"timeout,omitempty"`
 
 	// Result formatting
-	Result WebhookResult `json:"result"`
+	// +optional
+	Result WebhookResult `json:"result,omitempty"`
 
 	// Secrets to fill in templates
 	// These secrets will be passed to the templating function as key value pairs under the given name

config/crds/bases/external-secrets.io_clustersecretstores.yaml

@@ -5379,7 +5379,6 @@ spec:
                         description: Webhook url to call
                         type: string
                     required:
-                    - result
                     - url
                     type: object
                   yandexcertificatemanager:

config/crds/bases/external-secrets.io_secretstores.yaml

@@ -5379,7 +5379,6 @@ spec:
                         description: Webhook url to call
                         type: string
                     required:
-                    - result
                     - url
                     type: object
                   yandexcertificatemanager:

deploy/crds/bundle.yaml

@@ -7074,7 +7074,6 @@ spec:
                           description: Webhook url to call
                           type: string
                       required:
-                        - result
                         - url
                       type: object
                     yandexcertificatemanager:
@@ -18280,7 +18279,6 @@ spec:
                           description: Webhook url to call
                           type: string
                       required:
-                        - result
                         - url
                       type: object
                     yandexcertificatemanager:

docs/api/spec.md

@@ -11579,6 +11579,7 @@ WebhookResult
 </em>
 </td>
 <td>
+<em>(Optional)</em>
 <p>Result formatting</p>
 </td>
 </tr>

pkg/provider/webhook/webhook.go

@@ -21,6 +21,7 @@ import (
 	"encoding/json"
 	"errors"
 	"fmt"
+	"net/http"
 	"strconv"
 	"time"
 
@@ -60,9 +61,9 @@ func init() {
 	}, esv1.MaintenanceStatusMaintained)
 }
 
-// Capabilities return the provider supported capabilities (ReadOnly, WriteOnly, ReadWrite).
+// Capabilities return the provider-supported capabilities (ReadOnly, WriteOnly, ReadWrite).
 func (p *Provider) Capabilities() esv1.SecretStoreCapabilities {
-	return esv1.SecretStoreReadOnly
+	return esv1.SecretStoreReadWrite
 }
 
 // NewClient creates a new WebHook client for accessing secrets.
@@ -114,13 +115,88 @@ func getProvider(store esv1.GenericStore) (*webhook.Spec, error) {
 }
 
 // DeleteSecret deletes a secret from a remote store.
-func (w *WebHook) DeleteSecret(_ context.Context, _ esv1.PushSecretRemoteRef) error {
-	return errors.New(errNotImplemented)
+func (w *WebHook) DeleteSecret(ctx context.Context, remoteRef esv1.PushSecretRemoteRef) error {
+	provider, err := getProvider(w.store)
+	if err != nil {
+		return fmt.Errorf(errFailedToGetStore, err)
+	}
+
+	escapedData := map[string]map[string]string{
+		"remoteRef": {
+			"remoteKey": remoteRef.GetRemoteKey(),
+		},
+	}
+
+	url, err := webhook.ExecuteTemplateString(provider.URL, escapedData)
+	if err != nil {
+		return fmt.Errorf("failed to parse url: %w", err)
+	}
+
+	method := http.MethodDelete
+	req, err := http.NewRequestWithContext(ctx, method, url, http.NoBody)
+	if err != nil {
+		return fmt.Errorf("failed to create delete request: %w", err)
+	}
+
+	rawData := map[string]map[string]string{
+		"remoteRef": {
+			"remoteKey": remoteRef.GetRemoteKey(),
+		},
+	}
+	if provider.Headers != nil {
+		req, err = w.wh.ReqAddHeaders(req, provider, rawData)
+		if err != nil {
+			return err
+		}
+	}
+
+	if provider.Auth != nil {
+		req, err = w.wh.ReqAddAuth(ctx, req, provider)
+		if err != nil {
+			return err
+		}
+	}
+
+	resp, err := w.wh.HTTP.Do(req)
+	if err != nil {
+		return fmt.Errorf("failed to delete secret: %w", err)
+	}
+	defer func() {
+		_ = resp.Body.Close()
+	}()
+
+	if resp.StatusCode == 404 {
+		// Secret doesn't exist, that's OK for delete
+		return nil
+	}
+
+	if resp.StatusCode < 200 || resp.StatusCode >= 300 {
+		return fmt.Errorf("delete endpoint gave error %s", resp.Status)
+	}
+
+	return nil
 }
 
 // SecretExists checks if a secret exists in the remote store.
-func (w *WebHook) SecretExists(_ context.Context, _ esv1.PushSecretRemoteRef) (bool, error) {
-	return false, errors.New(errNotImplemented)
+func (w *WebHook) SecretExists(ctx context.Context, remoteRef esv1.PushSecretRemoteRef) (bool, error) {
+	provider, err := getProvider(w.store)
+	if err != nil {
+		return false, fmt.Errorf(errFailedToGetStore, err)
+	}
+
+	_, err = w.wh.GetWebhookData(ctx, provider, &esv1.ExternalSecretDataRemoteRef{
+		Key: remoteRef.GetRemoteKey(),
+	})
+	if err != nil {
+		var noSecretErr esv1.NoSecretError
+		if errors.As(err, &noSecretErr) {
+			return false, nil
+		}
+
+		return false, err
+	}
+
+	return true, nil
 }
 
 // PushSecret pushes a secret to a remote store.

pkg/provider/webhook/webhook_test.go

@@ -704,3 +704,134 @@ func makeClusterSecretStore(url string, args args) *esv1.ClusterSecretStore {
 	}
 	return store
 }
+
+func TestSecretExists(t *testing.T) {
+	tests := []struct {
+		name         string
+		statusCode   int
+		responseBody string
+		wantExists   bool
+		wantErr      bool
+	}{
+		{
+			name:         "secret exists - returns 200",
+			statusCode:   http.StatusOK,
+			responseBody: `{"data":"secret-value"}`,
+			wantExists:   true,
+			wantErr:      false,
+		},
+		{
+			name:         "secret does not exist - returns 404",
+			statusCode:   http.StatusNotFound,
+			responseBody: "not found",
+			wantExists:   false,
+			wantErr:      false,
+		},
+		{
+			name:         "server error - returns 500",
+			statusCode:   http.StatusInternalServerError,
+			responseBody: "internal server error",
+			wantExists:   false,
+			wantErr:      true,
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+				w.WriteHeader(tt.statusCode)
+				w.Write([]byte(tt.responseBody))
+			}))
+			defer ts.Close()
+
+			store := makeClusterSecretStore(ts.URL, args{URL: "/api/secret"})
+			prov := &Provider{}
+			client, err := prov.NewClient(context.Background(), store, nil, "testnamespace")
+			if err != nil {
+				t.Fatalf("failed to create client: %v", err)
+			}
+
+			remoteRef := v1alpha1.PushSecretRemoteRef{
+				RemoteKey: "test-key",
+			}
+
+			exists, err := client.SecretExists(context.Background(), remoteRef)
+
+			if (err != nil) != tt.wantErr {
+				t.Errorf("SecretExists() error = %v, wantErr %v", err, tt.wantErr)
+				return
+			}
+
+			if exists != tt.wantExists {
+				t.Errorf("SecretExists() = %v, want %v", exists, tt.wantExists)
+			}
+		})
+	}
+}
+
+func TestDeleteSecret(t *testing.T) {
+	tests := []struct {
+		name         string
+		statusCode   int
+		wantErr      bool
+		expectMethod string
+	}{
+		{
+			name:         "successful delete - returns 200",
+			statusCode:   http.StatusOK,
+			wantErr:      false,
+			expectMethod: http.MethodDelete,
+		},
+		{
+			name:         "successful delete - returns 204",
+			statusCode:   http.StatusNoContent,
+			wantErr:      false,
+			expectMethod: http.MethodDelete,
+		},
+		{
+			name:         "secret not found - returns 404",
+			statusCode:   http.StatusNotFound,
+			wantErr:      false,
+			expectMethod: http.MethodDelete,
+		},
+		{
+			name:         "server error - returns 500",
+			statusCode:   http.StatusInternalServerError,
+			wantErr:      true,
+			expectMethod: http.MethodDelete,
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			var receivedMethod string
+			ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+				receivedMethod = r.Method
+				w.WriteHeader(tt.statusCode)
+			}))
+			defer ts.Close()
+
+			store := makeClusterSecretStore(ts.URL, args{URL: "/api/secret"})
+			prov := &Provider{}
+			client, err := prov.NewClient(context.Background(), store, nil, "testnamespace")
+			if err != nil {
+				t.Fatalf("failed to create client: %v", err)
+			}
+
+			remoteRef := v1alpha1.PushSecretRemoteRef{
+				RemoteKey: "test-key",
+			}
+
+			err = client.DeleteSecret(context.Background(), remoteRef)
+
+			if (err != nil) != tt.wantErr {
+				t.Errorf("DeleteSecret() error = %v, wantErr %v", err, tt.wantErr)
+				return
+			}
+
+			if receivedMethod != tt.expectMethod {
+				t.Errorf("DeleteSecret() used method %v, want %v", receivedMethod, tt.expectMethod)
+			}
+		})
+	}
+}