event: mark FormatMajorVersion as safe from redaction
#2992
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
This change is |
Previously nearly all events logged by the `EventListener` used types that had been marked as safe by implementing `SafeFormatter`, defining them as non-sensitive and not requiring redaction in logs, however `FormatMajorVersion` had been excluded. This change makes `FormatMajorVersion` a `redact.SafeValue` as well.
AlexTalks
force-pushed
the
unredact_fmv
branch
from
13f4196 to
11bc1d6
Compare
AlexTalks
added a commit
to AlexTalks/cockroach
that referenced
this pull request
Oct 11, 2023
This change updates a few previously redacted constants and simple types often printed to logs to be marked as safe, not needing redaction. These do not represent sensitive data, such as such as the "s" pluralizer, the type of Admission Control work queue, or a node's liveness record, but they had not been marked safe or had `SafeFormatter` implemented previously. The types/constants marked as safe are listed as follows with examples: - `livenesspb.Liveness`: e.g. `liveness(nid:124 epo:12 exp:12345.6789,0)` - `livenesspb.MembershipStatus`: e.g. `active`, `decommissioning` - `admission.WorkKind`: e.g. `kv`, `sql-kv-response`, etc - `util.Pluralize()`: e.g. `s` on end of strings - `upgrade.Name()`: e.g. `Upgrade to 0.0-2: "add users and roles"` This includes a fix for the format string used in printing `enginepb.EngineType` at startup, printing `pebble` instead of `‹2›`. Lastly, this also adds `pebble.FormatMajorVersion` to the list of allowed `redact.SafeValue`s in anticipation of cockroachdb/pebble#2992, in the same vein as this PR. Epic: none Release note: None
jbowens
approved these changes
Oct 11, 2023
There was a problem hiding this comment.
Reviewed 1 of 1 files at r1, all commit messages.
Reviewable status:complete! all files reviewed, all discussions resolved (waiting on @itsbilal)
Closed
jbowens
added a commit
to jbowens/cockroach
that referenced
this pull request
Oct 12, 2023
``` ede31f1a event: mark `FormatMajorVersion` as safe from redaction 36b3f57b db: return structured errors from checkConsistency e3cbe650 db: use DiskFileName for manifest files and memtables d57a0be4 db: tolerate errors during async ingest sstable validation f79ae812 db: avoid use of physical filesystem in TestIngestValidation 5ddf67cc db: deflake TestIngestLoadRand ``` Resolves cockroachdb/pebble#2992. Epic: none Release note: none
|
Should we backport to crl-release-23.2? |
craig bot
pushed a commit
to cockroachdb/cockroach
that referenced
this pull request
Oct 12, 2023
112231: go.mod: bump Pebble to ede31f1a8e4b r=RaduBerinde a=jbowens Includes `@AlexTalks's` changes from cockroachdb/pebble#2992 to add `FormatMajorVersion` to the list of allowlisted `SafeValue` types. ``` ede31f1a event: mark `FormatMajorVersion` as safe from redaction 36b3f57b db: return structured errors from checkConsistency e3cbe650 db: use DiskFileName for manifest files and memtables d57a0be4 db: tolerate errors during async ingest sstable validation f79ae812 db: avoid use of physical filesystem in TestIngestValidation 5ddf67cc db: deflake TestIngestLoadRand ``` Resolves cockroachdb/pebble#2992. Epic: none Release note: none Co-authored-by: Jackson Owens <[email protected]>
andrewbaptist
pushed a commit
to AlexTalks/cockroach
that referenced
this pull request
Nov 14, 2023
This change updates a few previously redacted constants and simple types often printed to logs to be marked as safe, not needing redaction. These do not represent sensitive data, such as such as the "s" pluralizer, the type of Admission Control work queue, or a node's liveness record, but they had not been marked safe or had `SafeFormatter` implemented previously. The types/constants marked as safe are listed as follows with examples: - `livenesspb.Liveness`: e.g. `liveness(nid:124 epo:12 exp:12345.6789,0)` - `livenesspb.MembershipStatus`: e.g. `active`, `decommissioning` - `admission.WorkKind`: e.g. `kv`, `sql-kv-response`, etc - `util.Pluralize()`: e.g. `s` on end of strings - `upgrade.Name()`: e.g. `Upgrade to 0.0-2: "add users and roles"` This includes a fix for the format string used in printing `enginepb.EngineType` at startup, printing `pebble` instead of `‹2›`. Lastly, this also adds `pebble.FormatMajorVersion` to the list of allowed `redact.SafeValue`s in anticipation of cockroachdb/pebble#2992, in the same vein as this PR. Epic: none Release note: None
craig bot
pushed a commit
to cockroachdb/cockroach
that referenced
this pull request
Nov 17, 2023
112080: *: mark various non-sensitive strings as safe r=miraradeva a=AlexTalks This change updates a few previously redacted constants and simple types often printed to logs to be marked as safe, not needing redaction. These do not represent sensitive data, such as such as the "s" pluralizer, the type of Admission Control work queue, or a node's liveness record, but they had not been marked safe or had `SafeFormatter` implemented previously. The types/constants marked as safe are listed as follows with examples: - `livenesspb.Liveness`: e.g. `liveness(nid:124 epo:12 exp:12345.6789,0)` - `livenesspb.MembershipStatus`: e.g. `active`, `decommissioning` - `admission.WorkKind`: e.g. `kv`, `sql-kv-response`, etc - `util.Pluralize()`: e.g. `s` on end of strings - `upgrade.Name()`: e.g. `Upgrade to 0.0-2: "add users and roles"` This includes a fix for the format string used in printing `enginepb.EngineType` at startup, printing `pebble` instead of `‹2›`. Lastly, this also adds `pebble.FormatMajorVersion` to the list of allowed `redact.SafeValue`s in anticipation of cockroachdb/pebble#2992, in the same vein as this PR. Epic: none Release note: None Co-authored-by: Alex Sarkesian <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Previously nearly all events logged by the
EventListenerused types that had been marked as safe by implementingSafeFormatter, defining them as non-sensitive and not requiring redaction in logs, howeverFormatMajorVersionhad been excluded. This change makesFormatMajorVersionaredact.SafeValueas well.Requires the following in the cockroachdb/cockroach
redactcheck.go: