backup: add redaction markers to online restore URIs#166332
Merged
trunk-io[bot] merged 1 commit intocockroachdb:masterfrom Mar 25, 2026
Merged
backup: add redaction markers to online restore URIs#166332trunk-io[bot] merged 1 commit intocockroachdb:masterfrom
trunk-io[bot] merged 1 commit intocockroachdb:masterfrom
Conversation
Contributor
|
😎 Merged successfully - details. |
|
It looks like your PR touches production code but doesn't add or edit any test code. Did you consider adding tests to your PR? 🦉 Hoot! I am a Blathers, a bot for CockroachDB. My owner is dev-inf. |
Member
|
This change is |
andrew-r-thomas
force-pushed
the
or-redaction
branch
from
ae4d228 to
6f830ab
Compare
andrew-r-thomas
requested review from
annrpom,
dt and
msbutler
and removed request for
a team
andrew-r-thomas
force-pushed
the
or-redaction
branch
4 times, most recently
from
0802048 to
9c5bf9b
Compare
msbutler
reviewed
Mar 24, 2026
| // MakeRedactableLocatorURI builds a redact.RedactableString from a storage URI. | ||
| // The query parameter section is treated as unsafe and surrounded with redaction markers. | ||
| // Intended for use with pebble's remote.Locator. | ||
| func MakeRedactableLocatorURI(uri string) redact.RedactableString { |
Collaborator
There was a problem hiding this comment.
nice! could you add a little unit test for this func? and then an integration test in backup_test that shows an error message with the redacted locator(maybe a missing sst test would do this)?
andrew-r-thomas
force-pushed
the
or-redaction
branch
from
9c5bf9b to
2aac734
Compare
msbutler
approved these changes
Mar 24, 2026
| ) | ||
| require.Error(t, err) | ||
| for _, secret := range fakeSecrets { | ||
| require.NotContains(t, err.Error(), secret, |
Collaborator
There was a problem hiding this comment.
could you also validate that the error contains the uri with the redaction?
This patch adds redaction markers to URIs passed to pebble in online restore. Specifically, all query params in the URIs are marked as unsafe and as such will be redacted in logs and error messages. Informs: cockroachdb#161169 Release note: None
andrew-r-thomas
force-pushed
the
or-redaction
branch
from
2aac734 to
263b1ff
Compare
msbutler
approved these changes
Mar 24, 2026
andrew-r-thomas
added
backport-26.2.x
Contributor
Author
|
blathers backport 26.2 |
|
Successfully created backport PRs for: 26.2 |
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
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.
This patch adds redaction markers to URIs passed to pebble in online
restore. Specifically, all query params in the URIs are marked as unsafe
and as such will be redacted in logs and error messages.
Informs: #161169
Release note: None