fix(destinations): make deployment public and idempotent

cmd/deploy_destination.go

@@ -2,8 +2,6 @@ package cmd
 
 import (
 	"context"
-	"errors"
-	"reflect"
 
 	"github.com/amp-labs/cli/flags"
 	"github.com/amp-labs/cli/logger"
@@ -14,10 +12,9 @@ import (
 )
 
 var deployDestinationCmd = &cobra.Command{ //nolint:gochecknoglobals
-	Use:    "deploy:destination -i <input file path> [-o <output file path>] [-f <format>]",
-	Short:  "Deploy a destination",
-	Long:   "Deploy a destination",
-	Hidden: true,
+	Use:   "deploy:destination -i <input file path> [-o <output file path>] [-f <format>]",
+	Short: "Deploy a destination",
+	Long:  "Create or update a destination from a JSON or YAML file.",
 	Run: func(cmd *cobra.Command, args []string) {
 		projectId := flags.GetProjectOrFail()
 		apiKey := flags.GetAPIKey()
@@ -37,6 +34,11 @@ var deployDestinationCmd = &cobra.Command{ //nolint:gochecknoglobals
 		client := request.NewAPIClient(projectId, &apiKey)
 		oldDest := getOldDest(cmd.Context(), client, &dest)
 
+		format, err := cmd.Flags().GetString("format")
+		if err != nil {
+			logger.FatalErr("Unable to read output format", err)
+		}
+
 		var output *request.Destination
 
 		if oldDest == nil {
@@ -45,7 +47,7 @@ var deployDestinationCmd = &cobra.Command{ //nolint:gochecknoglobals
 			patch := generatePatch(oldDest, &dest)
 			if len(patch.UpdateMask) == 0 {
 				err := utils.WriteStructToFile(viper.GetString("output"),
-					flags.GetOutputFormat(), oldDest)
+					utils.Format(format), oldDest)
 				if err != nil {
 					logger.FatalErr("Unable to write destination file", err)
 				}
@@ -61,7 +63,7 @@ var deployDestinationCmd = &cobra.Command{ //nolint:gochecknoglobals
 		}
 
 		err = utils.WriteStructToFile(viper.GetString("output"),
-			flags.GetOutputFormat(), output)
+			utils.Format(format), output)
 		if err != nil {
 			logger.FatalErr("Unable to write destination file", err)
 		}
@@ -83,59 +85,31 @@ func generatePatch(oldDest *request.Destination, newDest *request.Destination) *
 		patch.UpdateMask = append(patch.UpdateMask, "type")
 	}
 
-	if oldDest.Metadata != nil { //nolint:nestif
-		if newDest.Metadata == nil {
-			patch.Destination["metadata"] = nil
-			patch.UpdateMask = append(patch.UpdateMask, "metadata")
-		} else if !reflect.DeepEqual(oldDest.Metadata, newDest.Metadata) {
-			patch.Destination["metadata"] = newDest.Metadata
-			patch.UpdateMask = append(patch.UpdateMask, "metadata")
-		}
-	} else {
-		if newDest.Metadata != nil {
-			patch.Destination["metadata"] = newDest.Metadata
-			patch.UpdateMask = append(patch.UpdateMask, "metadata")
-		}
+	if newDest.Metadata != nil && (oldDest.Metadata == nil || oldDest.Metadata.URL != newDest.Metadata.URL) {
+		patch.Destination["metadata"] = map[string]any{"url": newDest.Metadata.URL}
+		patch.UpdateMask = append(patch.UpdateMask, "metadata.url")
 	}
 
 	return patch
 }
 
 func getOldDest(ctx context.Context, client *request.APIClient, dest *request.Destination) *request.Destination {
-	id := findDestId(ctx, client, dest)
-	if id == "" {
-		return nil
-	}
-
-	dst, err := client.GetDestination(ctx, id)
-	if err != nil {
-		if errors.Is(err, request.ErrNotFound) {
-			return nil
-		} else {
-			logger.FatalErr("Unable to get destination", err)
-		}
-	}
-
-	return dst
-}
-
-func findDestId(ctx context.Context, client *request.APIClient, dest *request.Destination) string {
-	if dest.Id != "" {
-		return dest.Id
-	}
-
-	dests, err := client.ListDestinations(ctx)
+	destinations, err := client.ListDestinations(ctx)
 	if err != nil {
 		logger.FatalErr("Unable to list destinations", err)
 	}
 
-	for _, d := range dests {
-		if d.Name == dest.Name {
-			return d.Id
+	for _, existing := range destinations {
+		if dest.Id != "" && existing.Id == dest.Id {
+			return existing
+		}
+
+		if dest.Id == "" && existing.Name == dest.Name {
+			return existing
 		}
 	}
 
-	return ""
+	return nil
 }
 
 func init() {

cmd/deploy_destination_test.go

@@ -0,0 +1,69 @@
+package cmd
+
+import (
+	"net/http"
+	"net/http/httptest"
+	"reflect"
+	"testing"
+
+	"github.com/amp-labs/cli/request"
+)
+
+func TestGeneratePatchUpdatesOnlyDestinationURL(t *testing.T) {
+	t.Parallel()
+
+	oldDestination := &request.Destination{
+		Metadata: &request.WebhookMetadata{
+			URL:            "https://old.example.com/webhook",
+			Headers:        map[string]string{"Authorization": "secret"},
+			SvixAppId:      "svix-app-id",
+			SvixEndpointId: "svix-endpoint-id",
+		},
+	}
+	newDestination := &request.Destination{
+		Metadata: &request.WebhookMetadata{URL: "https://new.example.com/webhook"},
+	}
+
+	patch := generatePatch(oldDestination, newDestination)
+
+	wantDestination := map[string]any{
+		"metadata": map[string]any{"url": "https://new.example.com/webhook"},
+	}
+	if !reflect.DeepEqual(patch.Destination, wantDestination) {
+		t.Fatalf("patch destination = %#v, want %#v", patch.Destination, wantDestination)
+	}
+
+	wantUpdateMask := []string{"metadata.url"}
+	if !reflect.DeepEqual(patch.UpdateMask, wantUpdateMask) {
+		t.Fatalf("patch update mask = %#v, want %#v", patch.UpdateMask, wantUpdateMask)
+	}
+}
+
+func TestGetOldDestFindsExistingDestinationByName(t *testing.T) {
+	t.Parallel()
+
+	server := httptest.NewServer(http.HandlerFunc(func(writer http.ResponseWriter, req *http.Request) {
+		if req.Method != http.MethodGet || req.URL.Path != "/v1/projects/test-project/destinations" {
+			http.NotFound(writer, req)
+
+			return
+		}
+
+		writer.Header().Set("Content-Type", "application/json")
+		_, _ = writer.Write([]byte(`[{"id":"destination-id","name":"local-webhook","type":"webhook"}]`))
+	}))
+	t.Cleanup(server.Close)
+
+	apiKey := "test-key"
+	client := &request.APIClient{
+		Root:      server.URL + "/v1",
+		ProjectId: "test-project",
+		APIKey:    &apiKey,
+		Client:    request.NewRequestClient(),
+	}
+
+	destination := getOldDest(t.Context(), client, &request.Destination{Name: "local-webhook"})
+	if destination == nil || destination.Id != "destination-id" {
+		t.Fatalf("getOldDest() = %#v, want existing destination", destination)
+	}
+}