cli: fix debug zip with new columns for cluster settings

[maryliag]

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).

[cockroach-teamcity]

This change is 

[michae2]

Reviewable status: complete! 0 of 0 LGTMs obtained (waiting on @maryliag and @rharding6373)

---

pkg/cli/zip_table_registry.go line 221 at r1 (raw file):

	"crdb_internal.cluster_settings": {
		customQueryRedacted: `SELECT variable, value, type, public, description, default_value, origin FROM (
    		SELECT * 

I'm actually more scared of this * than the outer one. I think it would be better to replace both with explicit column names.

[mgartner]

pkg/cli/zip_table_registry.go line 221 at r1 (raw file):

Previously, michae2 (Michael Erickson) wrote…

I'm actually more scared of this * than the outer one. I think it would be better to replace both with explicit column names.

+1

Also, I think this reads more idiomatically without the unnecessary subquery and extra parens, but up to you:

SELECT variable, value, type, public, description, default_value, origin
FROM crdb_internal.cluster_settings
WHERE type <> 's'
UNION
SELECT variable, '<redacted>' as value, type, public, description, default_value, origin
FROM crdb_internal.cluster_settings g
WHERE type = 's'

[mgartner]

pkg/cli/zip_table_registry.go line 221 at r1 (raw file):

Previously, mgartner (Marcus Gartner) wrote…

+1

Also, I think this reads more idiomatically without the unnecessary subquery and extra parens, but up to you:

SELECT variable, value, type, public, description, default_value, origin
FROM crdb_internal.cluster_settings
WHERE type <> 's'
UNION
SELECT variable, '<redacted>' as value, type, public, description, default_value, origin
FROM crdb_internal.cluster_settings g
WHERE type = 's'

Another option:

SELECT
  variable,
  CASE WHEN type = 's' THEN '<redacted>' ELSE value END value,
  type,
  public,
  description,
  default_value,
  origin
FROM crdb_internal.cluster_settings

[maryliag]

Reviewable status: complete! 0 of 0 LGTMs obtained (waiting on @michae2 and @rharding6373)

---

pkg/cli/zip_table_registry.go line 221 at r1 (raw file):

Previously, mgartner (Marcus Gartner) wrote…

Another option:

SELECT
  variable,
  CASE WHEN type = 's' THEN '<redacted>' ELSE value END value,
  type,
  public,
  description,
  default_value,
  origin
FROM crdb_internal.cluster_settings

duh... I did the fix, then remove just so I could test and check if the test was caughting the error and forgot to move back to the right location

[maryliag]

Reviewable status: complete! 0 of 0 LGTMs obtained (waiting on @michae2 and @rharding6373)

---

pkg/cli/zip_table_registry.go line 221 at r1 (raw file):

Previously, maryliag (Marylia Gutierrez) wrote…

duh... I did the fix, then remove just so I could test and check if the test was caughting the error and forgot to move back to the right location

Fixed

[mgartner]

Reviewed 2 of 2 files at r1.
Reviewable status: complete! 1 of 0 LGTMs obtained (waiting on @maryliag, @michae2, and @rharding6373)

---

-- commits line 18 at r1:
Can you mention that the crdb_internal.cluster_settings.txt file in debug zips was empty due to this bug, and that it only affects v23.1.5?

[michae2]

Reviewed all commit messages.
Reviewable status: complete! 0 of 0 LGTMs obtained (and 1 stale) (waiting on @maryliag, @mgartner, and @rharding6373)

---

pkg/cli/zip_table_registry.go line 222 at r4 (raw file):

		customQueryRedacted: `SELECT
			variable,
			CASE WHEN type = 's' THEN '<redacted>' ELSE value END value,

Since we're here anyway, I have another request: could we rewrite this item as:

CASE WHEN type = 's' AND value != default_value THEN '<redacted>' ELSE value END value,

[maryliag]

Reviewable status: complete! 0 of 0 LGTMs obtained (and 1 stale) (waiting on @mgartner, @michae2, and @rharding6373)

---

pkg/cli/zip_table_registry.go line 222 at r4 (raw file):

Previously, michae2 (Michael Erickson) wrote…

Since we're here anyway, I have another request: could we rewrite this item as:

CASE WHEN type = 's' AND value != default_value THEN '<redacted>' ELSE value END value,

Done

[michae2]

Reviewed 2 of 2 files at r5, all commit messages.
Reviewable status: complete! 1 of 0 LGTMs obtained (and 1 stale) (waiting on @maryliag and @rharding6373)

[maryliag]

TFTRs!
bors r+

[craig]

Build succeeded:

• Bazel Essential CI (Cockroach)