From 9b2767652166ad01d41f1ae5ab079a916af91b5f Mon Sep 17 00:00:00 2001
From: Brian Picciano <me@mediocregopher.com>
Date: Tue, 10 Dec 2024 15:35:13 +0100
Subject: [PATCH] Don't error from 'vpn firewall show --staged' if nothing is
 staged, return the live config instead

---
 go/cmd/entrypoint/vpn_firewall.go      | 23 ++++++++++-------
 go/cmd/entrypoint/vpn_firewall_test.go | 34 ++++++++++++++------------
 2 files changed, 33 insertions(+), 24 deletions(-)

diff --git a/go/cmd/entrypoint/vpn_firewall.go b/go/cmd/entrypoint/vpn_firewall.go
index 328c8b8..c8be3ec 100644
--- a/go/cmd/entrypoint/vpn_firewall.go
+++ b/go/cmd/entrypoint/vpn_firewall.go
@@ -257,21 +257,26 @@ var subCmdVPNFirewallShow = subCmd{
 			return nil, fmt.Errorf("parsing flags: %w", err)
 		}
 
-		var firewallConfig daecommon.ConfigFirewall
-		if !*staged {
+		var (
+			firewallConfig daecommon.ConfigFirewall
+			foundStaged    bool
+		)
+		if *staged {
+			var err error
+			if foundStaged, err = ctx.opts.changeStager.get(
+				&firewallConfig, vpnFirewallConfigChangeStagerName,
+			); err != nil {
+				return nil, fmt.Errorf("checking for staged changes: %w", err)
+			}
+		}
+
+		if !foundStaged {
 			config, err := ctx.getDaemonRPC().GetConfig(ctx)
 			if err != nil {
 				return nil, fmt.Errorf("getting network config: %w", err)
 			}
 
 			firewallConfig = config.VPN.Firewall
-
-		} else if ok, err := ctx.opts.changeStager.get(
-			&firewallConfig, vpnFirewallConfigChangeStagerName,
-		); err != nil {
-			return nil, fmt.Errorf("checking for staged changes: %w", err)
-		} else if !ok {
-			return nil, errors.New("no firewall configuration changes have been staged")
 		}
 
 		return newFirewallView(firewallConfig), nil
diff --git a/go/cmd/entrypoint/vpn_firewall_test.go b/go/cmd/entrypoint/vpn_firewall_test.go
index 2f3427c..4d3480d 100644
--- a/go/cmd/entrypoint/vpn_firewall_test.go
+++ b/go/cmd/entrypoint/vpn_firewall_test.go
@@ -7,12 +7,12 @@ import (
 	"isle/daemon/daecommon"
 	"isle/toolkit"
 	"os"
+	"slices"
 	"strings"
 	"testing"
 
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
-	"golang.org/x/exp/slices"
 )
 
 func TestVPNFirewallAdd(t *testing.T) {
@@ -296,7 +296,6 @@ func TestVPNFirewallShow(t *testing.T) {
 		staged            string
 		flags             []string
 		want              map[string][]any
-		wantErr           string
 	}{
 		{
 			name: "empty",
@@ -357,9 +356,22 @@ func TestVPNFirewallShow(t *testing.T) {
 			},
 		},
 		{
-			name:    "staged/nothing staged",
-			flags:   []string{"--staged"},
-			wantErr: "no firewall configuration changes have been staged",
+			name: "staged/nothing staged",
+			outbound: []string{
+				`{"port":"any","proto":"icmp","host":"any"}`,
+			},
+			flags: []string{"--staged"},
+			want: map[string][]any{
+				"outbound": {
+					map[string]any{
+						"index": 0,
+						"port":  "any",
+						"proto": "icmp",
+						"host":  "any",
+					},
+				},
+				"inbound": {},
+			},
 		},
 		{
 			name: "staged/staged but no flag",
@@ -443,7 +455,7 @@ func TestVPNFirewallShow(t *testing.T) {
 				[]byte(inboundRawJSON), &config.VPN.Firewall.Inbound,
 			))
 
-			if !slices.Contains(test.flags, "--staged") {
+			if !slices.Contains(test.flags, "--staged") || test.staged == "" {
 				h.daemonRPC.
 					On("GetConfig", toolkit.MockArg[context.Context]()).
 					Return(config, nil).
@@ -451,15 +463,7 @@ func TestVPNFirewallShow(t *testing.T) {
 			}
 
 			args := append([]string{"vpn", "firewall", "show"}, test.flags...)
-
-			if test.wantErr == "" {
-				h.runAssertStdout(t, test.want, args...)
-			} else {
-				err := h.run(t, args...)
-				if assert.Error(t, err) {
-					assert.Contains(t, err.Error(), test.wantErr)
-				}
-			}
+			h.runAssertStdout(t, test.want, args...)
 		})
 	}
 }