From 3bacdbe302c436675605f3256025829e63c45330 Mon Sep 17 00:00:00 2001
From: maryliag <marylia@cockroachlabs.com>
Date: Tue, 18 Jul 2023 15:07:30 -0400
Subject: [PATCH] cli: fix debug zip with new columns for cluster settings

The addition of new columns on cluster_settings view done
on #104449 were causing debug zip fails to add the
information from that table, since the query used to
gather the information was doing a join and not considering
the new columns.

This commit updates the query to use the explicit columns,
so even if new columns are added it won't be a problem in the future.
It also adds tests for all custom querys used to generate the
debug zip, so this type of issue would have been caught.

The file `crdb_internal.cluster_settings.txt` in debug zips was
empty due to this bug on v23.1.5 (only version affected by this bug).

Fixes #107103

Release note (bug fix): Debug zip now are properly showing the
information from cluster_settings. The file
`crdb_internal.cluster_settings.txt` in debug zips was empty
due to this bug on v23.1.5 (only version affected by this bug).
---
 pkg/cli/zip_table_registry.go      | 18 +++++++-------
 pkg/cli/zip_table_registry_test.go | 39 ++++++++++++++++++++++++++++++
 2 files changed, 48 insertions(+), 9 deletions(-)

diff --git a/pkg/cli/zip_table_registry.go b/pkg/cli/zip_table_registry.go
index 2a9a069d90c7..c78970f42e62 100644
--- a/pkg/cli/zip_table_registry.go
+++ b/pkg/cli/zip_table_registry.go
@@ -218,15 +218,15 @@ var zipInternalTablesPerCluster = DebugZipTableRegistry{
 		},
 	},
 	"crdb_internal.cluster_settings": {
-		customQueryRedacted: `SELECT * FROM (
-    		SELECT * 
-    		FROM crdb_internal.cluster_settings
-    		WHERE type <> 's'
-		) UNION (
-		    SELECT variable, '<redacted>' as value, type, public, description
-			FROM crdb_internal.cluster_settings g
-			WHERE type = 's'
-		)`,
+		customQueryRedacted: `SELECT
+			variable,
+			CASE WHEN type = 's' AND value != default_value THEN '<redacted>' ELSE value END value,
+			type,
+			public,
+			description,
+			default_value,
+			origin
+		FROM crdb_internal.cluster_settings`,
 	},
 	"crdb_internal.cluster_transactions": {
 		// `last_auto_retry_reason` contains error text that may contain
diff --git a/pkg/cli/zip_table_registry_test.go b/pkg/cli/zip_table_registry_test.go
index 2596b7001ffd..3718fca41f7f 100644
--- a/pkg/cli/zip_table_registry_test.go
+++ b/pkg/cli/zip_table_registry_test.go
@@ -11,11 +11,17 @@
 package cli
 
 import (
+	"context"
 	"fmt"
 	"testing"
 
+	"github.com/cockroachdb/cockroach/pkg/base"
+	"github.com/cockroachdb/cockroach/pkg/sql/tests"
+	"github.com/cockroachdb/cockroach/pkg/testutils/serverutils"
+	"github.com/cockroachdb/cockroach/pkg/testutils/sqlutils"
 	"github.com/cockroachdb/cockroach/pkg/util/leaktest"
 	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
 )
 
 func TestQueryForTable(t *testing.T) {
@@ -134,3 +140,36 @@ you must redact them at the SQL level.`
 		}
 	}
 }
+
+func executeAllCustomQuerys(
+	t *testing.T, sqlDB *sqlutils.SQLRunner, tableRegistry DebugZipTableRegistry,
+) {
+	for table, regConfig := range tableRegistry {
+		if regConfig.customQueryRedacted != "" {
+			rows := sqlDB.Query(t, regConfig.customQueryRedacted)
+			require.NoError(t, rows.Err(), "failed to select for table %s redacted", table)
+		}
+
+		if regConfig.customQueryUnredacted != "" {
+			rows := sqlDB.Query(t, regConfig.customQueryUnredacted)
+			require.NoError(t, rows.Err(), "failed to select for table %s unredacted", table)
+		}
+	}
+}
+
+func TestCustomQuery(t *testing.T) {
+	defer leaktest.AfterTest(t)()
+	ctx := context.Background()
+
+	params, _ := tests.CreateTestServerParams()
+	cluster := serverutils.StartNewTestCluster(t, 3 /* numNodes */, base.TestClusterArgs{
+		ServerArgs: params,
+	})
+	testConn := cluster.ServerConn(0 /* idx */)
+	sqlDB := sqlutils.MakeSQLRunner(testConn)
+	defer cluster.Stopper().Stop(ctx)
+
+	executeAllCustomQuerys(t, sqlDB, zipInternalTablesPerCluster)
+	executeAllCustomQuerys(t, sqlDB, zipInternalTablesPerNode)
+	executeAllCustomQuerys(t, sqlDB, zipSystemTables)
+}