Merge #150364

150364: sql, settings, cli: Add sensitive, reportable columns to cluster_settings and update debug zip setting redaction logic r=kyle-a-wong a=kyle-a-wong

commit 1/2:

sql,settings: add sensitive, reportable to cluster_settings VTable
updates the crdb_internal.cluster_settings VTable to include 2 new
columns: sensitive and reportable. These two new columns expose the
isSensitive and isReportable attributes from settings objects.

Epic: None
Release note: None

---

commit 2/2:

cli: updates redaction policy for cluster settings in debug zip
Previously, debug zip always exposed all settings values for
unredacted debug zips, and redacted all string setting types
for redacted debug zips.

Now, "sensitive" cluster settings will always be redacted
in debug zip, and "sensitive" and "non-reportable" cluster
settings will be redacted for redacted debug zips.

Release note (cli change): debug zips will now always redact
"sensitive" cluster settings, whether or not a redacted debug
zip is requested. For redacted debug zips, both "sensitive"
and "non-reportable" cluster settings are redacted. This
replaces the old logic where string-type cluster settings
were always redacted for redacted debug-zips.

Co-authored-by: Kyle Wong <[email protected]>

Author: craig[bot]

pkg/ccl/logictestccl/testdata/logic_test/crdb_internal_tenant

@@ -127,10 +127,10 @@ SELECT * FROM crdb_internal.session_trace WHERE span_idx < 0
 ----
 span_idx  message_idx  timestamp  duration  operation  loc  tag  message age
 
-query TTTBTTTT colnames
+query TTTBBBTTTT colnames
 SELECT * FROM crdb_internal.cluster_settings WHERE variable = ''
 ----
-variable  value  type  public  description default_value origin key
+variable  value  type  public  sensitive  reportable  description  default_value  origin  key
 
 query TI colnames
 SELECT * FROM crdb_internal.feature_usage WHERE feature_name = ''

pkg/cli/zip_table_registry.go

@@ -222,11 +222,31 @@ var zipInternalTablesPerCluster = DebugZipTableRegistry{
 		},
 	},
 	"crdb_internal.cluster_settings": {
+		customQueryUnredacted: `SELECT
+			variable,
+			CASE 
+			  WHEN sensitive THEN '<redacted>'
+			  ELSE value 
+			END value,
+			type,
+			public,
+			sensitive,
+			reportable,
+			description,
+			default_value,
+			origin
+		FROM crdb_internal.cluster_settings`,
 		customQueryRedacted: `SELECT
 			variable,
-			CASE WHEN type = 's' AND value != default_value THEN '<redacted>' ELSE value END value,
+			CASE 
+			  WHEN NOT reportable AND value != default_value THEN '<redacted>'
+			  WHEN sensitive THEN '<redacted>'
+			  ELSE value 
+			END value,
 			type,
 			public,
+			sensitive,
+			reportable,
 			description,
 			default_value,
 			origin
@@ -1384,18 +1404,29 @@ var zipSystemTables = DebugZipTableRegistry{
 		},
 	},
 	"system.settings": {
-		customQueryUnredacted: `SELECT * FROM system.settings`,
-		customQueryRedacted: `SELECT * FROM (
-				SELECT *
-				FROM system.settings
-				WHERE "valueType" <> 's'
-    	) UNION (
-				SELECT name, '<redacted>' as value,
-				"lastUpdated",
-				"valueType"
-				FROM system.settings
-				WHERE "valueType"  = 's'
-    	)`,
+		customQueryUnredacted: `
+SELECT 
+     name,
+     CASE
+          WHEN cs.sensitive THEN '<redacted>'
+          ELSE s.value
+     END value,
+	s."lastUpdated",
+	s."valueType"
+FROM system.settings s
+JOIN crdb_internal.cluster_settings cs ON cs.variable = s.name`,
+		customQueryRedacted: `
+SELECT 
+     name,
+     CASE
+          WHEN cs.sensitive THEN '<redacted>'
+          WHEN NOT cs.reportable THEN '<redacted>'
+          ELSE s.value
+     END value,
+	s."lastUpdated",
+	s."valueType"
+FROM system.settings s
+JOIN crdb_internal.cluster_settings cs ON cs.variable = s.name`,
 	},
 	"system.span_configurations": {
 		nonSensitiveCols: NonSensitiveColumns{

pkg/settings/common.go

@@ -67,15 +67,15 @@ func (c common) Visibility() Visibility {
 	return c.visibility
 }
 
-func (c common) isReportable() bool {
+func (c common) IsReportable() bool {
 	return !c.nonReportable
 }
 
 func (c *common) isRetired() bool {
 	return c.retired
 }
 
-func (c *common) isSensitive() bool {
+func (c *common) IsSensitive() bool {
 	return c.sensitive
 }
 
@@ -150,17 +150,17 @@ type internalSetting interface {
 
 	init(class Class, key InternalKey, description string, slot slotIdx)
 	isRetired() bool
-	isSensitive() bool
+	IsSensitive() bool
 	getSlot() slotIdx
 
-	// isReportable indicates whether the value of the setting can be
+	// IsReportable indicates whether the value of the setting can be
 	// included in user-facing reports such as that produced by SHOW ALL
 	// CLUSTER SETTINGS.
 	// This only affects reports though; direct access is unconstrained.
 	// For example, `enterprise.license` is non-reportable:
 	// it cannot be listed, but can be accessed with `SHOW CLUSTER
 	// SETTING enterprise.license` or SET CLUSTER SETTING.
-	isReportable() bool
+	IsReportable() bool
 
 	setToDefault(ctx context.Context, sv *Values)
 
@@ -179,15 +179,15 @@ func TestingIsReportable(s Setting) bool {
 		return false
 	}
 	if e, ok := s.(internalSetting); ok {
-		return e.isReportable()
+		return e.IsReportable()
 	}
 	return true
 }
 
 // TestingIsSensitive is used in testing for sensitivity.
 func TestingIsSensitive(s Setting) bool {
 	if e, ok := s.(internalSetting); ok {
-		return e.isSensitive()
+		return e.IsSensitive()
 	}
 	return false
 }

pkg/settings/internal_test.go

@@ -45,7 +45,7 @@ func TestSensitiveSetting(t *testing.T) {
 	sv := &Values{}
 	sv.Init(ctx, TestOpaque)
 
-	require.True(t, sensitiveSetting.isSensitive())
+	require.True(t, sensitiveSetting.IsSensitive())
 	u := NewUpdater(sv)
 	require.NoError(t, u.Set(ctx, sensitiveSetting.InternalKey(), EncodedValue{Value: "foo", Type: "s"}))
 

pkg/settings/masked.go

@@ -35,7 +35,7 @@ func (s *MaskedSetting) String(sv *Values) string {
 	}
 	isSensitive := false
 	if st, ok := s.setting.(internalSetting); ok {
-		isSensitive = st.isSensitive()
+		isSensitive = st.IsSensitive()
 	}
 	sensitiveRedactionEnabled := redactSensitiveSettingsEnabled.Get(sv)
 	// Non-reportable settings are always redacted. Sensitive settings are
@@ -56,6 +56,16 @@ func (s *MaskedSetting) Visibility() Visibility {
 	return s.setting.Visibility()
 }
 
+// IsSensitive returns whether the underlying setting is sensitive.
+func (s *MaskedSetting) IsSensitive() bool {
+	return s.setting.IsSensitive()
+}
+
+// IsReportable returns whether the underlying setting is reportable.
+func (s *MaskedSetting) IsReportable() bool {
+	return s.setting.IsReportable()
+}
+
 // InternalKey returns the key string for the underlying setting.
 func (s *MaskedSetting) InternalKey() InternalKey {
 	return s.setting.InternalKey()

pkg/settings/protobuf.go

@@ -109,6 +109,10 @@ func (s *ProtobufSetting) Validate(sv *Values, p protoutil.Message) error {
 	return s.validateFn(sv, p)
 }
 
+func (s *ProtobufSetting) IsSensitive() bool {
+	return s.sensitive
+}
+
 // Override sets the setting to the given value, assuming it passes validation.
 func (s *ProtobufSetting) Override(ctx context.Context, sv *Values, p protoutil.Message) {
 	sv.setValueOrigin(ctx, s.slot, OriginOverride)

pkg/settings/registry.go

@@ -494,7 +494,7 @@ func LookupForReportingByKey(key InternalKey, forSystemTenant bool) (Setting, bo
 	if !forSystemTenant && s.Class() == SystemOnly {
 		return nil, false
 	}
-	if !s.isReportable() {
+	if !s.IsReportable() {
 		return &MaskedSetting{setting: s}, true
 	}
 	return s, true
@@ -531,7 +531,7 @@ func LookupForDisplayByKey(
 	if !forSystemTenant && s.Class() == SystemOnly {
 		return nil, false
 	}
-	if s.isSensitive() && !canViewSensitive {
+	if s.IsSensitive() && !canViewSensitive {
 		return &MaskedSetting{setting: s}, true
 	}
 	return s, true
@@ -565,7 +565,7 @@ var ReadableTypes = map[string]string{
 //   - "<unknown>" if there is no setting with this name.
 func RedactedValue(key InternalKey, values *Values, forSystemTenant bool) string {
 	if k, ok := registry[key]; ok {
-		if k.Typ() == "s" || k.isSensitive() || !k.isReportable() {
+		if k.Typ() == "s" || k.IsSensitive() || !k.IsReportable() {
 			return "<redacted>"
 		}
 	}

pkg/settings/setting.go

@@ -77,6 +77,10 @@ type Setting interface {
 	// retrieving all settings.
 	Visibility() Visibility
 
+	IsSensitive() bool
+
+	IsReportable() bool
+
 	// IsUnsafe returns whether the setting is unsafe, and thus requires
 	// a special interlock to set.
 	IsUnsafe() bool

pkg/settings/settings_test.go

@@ -718,12 +718,12 @@ func TestIsReportable(t *testing.T) {
 	if v, ok, _ := settings.LookupForLocalAccess(
 		"bool.t", settings.ForSystemTenant,
 	); !ok || !settings.TestingIsReportable(v) {
-		t.Errorf("expected 'bool.t' to be marked as isReportable() = true")
+		t.Errorf("expected 'bool.t' to be marked as IsReportable() = true")
 	}
 	if v, ok, _ := settings.LookupForLocalAccess(
 		"sekretz", settings.ForSystemTenant,
 	); !ok || settings.TestingIsReportable(v) {
-		t.Errorf("expected 'sekretz' to be marked as isReportable() = false")
+		t.Errorf("expected 'sekretz' to be marked as IsReportable() = false")
 	}
 }
 

pkg/sql/crdb_internal.go

@@ -2025,6 +2025,8 @@ CREATE TABLE crdb_internal.cluster_settings (
   value         STRING NOT NULL,
   type          STRING NOT NULL,
   public        BOOL NOT NULL, -- whether the setting is documented, which implies the user can expect support.
+  sensitive     BOOL NOT NULL, -- whether the setting is sensitive and should not be exposed to users without the appropriate privileges
+  reportable    BOOL NOT NULL, -- whether the setting is reportable
   description   STRING NOT NULL,
   default_value STRING NOT NULL,
   origin        STRING NOT NULL, -- the origin of the value: 'default' , 'override' or 'external-override'
@@ -2070,6 +2072,8 @@ CREATE TABLE crdb_internal.cluster_settings (
 				tree.NewDString(strVal),
 				tree.NewDString(setting.Typ()),
 				tree.MakeDBool(tree.DBool(isPublic)),
+				tree.MakeDBool(tree.DBool(setting.IsSensitive())),
+				tree.MakeDBool(tree.DBool(setting.IsReportable())),
 				tree.NewDString(desc),
 				tree.NewDString(defaultVal),
 				tree.NewDString(origin),