Skip to content

Commit 8de131b

Browse files
quark-zjumeta-codesync[bot]
authored andcommitted
monad: make redaction create-key-list support multiple commits
Summary: Currently, `redaction create-key-list` takes a single commit with multiple paths. This is inconvenient. For example, in S675789, there are 5 commits, 2 paths, and 9 different blobs to redact. `create-key-list-from-ids` can be used to specify the blob ids from multiple commits. However, it's not as intuitive to use. This diff makes `redaction create-key-list --input-file` support specifying `commit:path`, so one redaction can handle multiple commits. The `commit:path` format is inspired by git. ___ Differential Revision: D108946138 fbshipit-source-id: 6b4aba3fbce051d73e47caff7018c8933e602ebc
1 parent 5de90ca commit 8de131b

2 files changed

Lines changed: 112 additions & 38 deletions

File tree

eden/mononoke/tests/integration/test-redaction-config.t

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -73,6 +73,23 @@ setup repo-pull and repo-push
7373
$ COMMIT_B=$(hg log -r 'desc("add b")' -T '{node}')
7474
$ hg up -q $COMMIT_B
7575

76+
Redact files from multiple commits using commit:path input
77+
$ cat > "$TESTTMP/redaction_pairs" <<EOF
78+
> $C:c
79+
> $COMMIT_B:b
80+
> EOF
81+
$ mononoke_admin redaction create-key-list -R repo --input-file "$TESTTMP/redaction_pairs" --main-bookmark master_bookmark --force --output-file rs_pairs --skip-aws-sync
82+
Checking redacted content doesn't exist in 'master_bookmark' bookmark
83+
Redacted content in main bookmark: b content.blake2.21c519fe0eb401bc97888f270902935f858d0c5361211f892fd26ed9ce127ff9
84+
Creating key list despite 1 files being redacted in the main bookmark (master_bookmark) (--force)
85+
Redaction saved as: * (glob)
86+
To finish the redaction process, you need to commit this id to scm/mononoke/redaction/redaction_sets.cconf in configerator
87+
88+
$ mononoke_admin redaction fetch-key-list -R repo $(cat rs_pairs) | sort
89+
content.blake2.000a1a9b74aa3da71fcceb653a62cb6987ae440c2b5c3d7e5d08d7c526b1dca8
90+
content.blake2.21c519fe0eb401bc97888f270902935f858d0c5361211f892fd26ed9ce127ff9
91+
$ rm rs_pairs
92+
7693
Redact file 'c' in commit '$C'
7794
$ mononoke_admin redaction create-key-list -R repo -i $C c --main-bookmark master_bookmark --output-file rs_0 --skip-aws-sync
7895
Checking redacted content doesn't exist in 'master_bookmark' bookmark

eden/mononoke/tools/admin/src/commands/redaction/create_key_list.rs

Lines changed: 95 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,8 @@ use content_manifest_derivation::RootContentManifestId;
2727
use context::CoreContext;
2828
use derivation_queue_thrift::DerivationPriority;
2929
use fsnodes::RootFsnodeId;
30+
use futures::stream;
31+
use futures::stream::StreamExt;
3032
use futures::stream::TryStreamExt;
3133
use manifest::Entry;
3234
use manifest::ManifestOps;
@@ -44,14 +46,18 @@ use repo_derived_data::RepoDerivedDataRef;
4446
use super::Repo;
4547
use super::list::paths_for_content_keys;
4648

49+
const COMMIT_LOOKUP_CONCURRENCY: usize = 10;
50+
4751
#[derive(Args)]
4852
#[clap(group(ArgGroup::new("files-input=file").args(&["files", "input_file"]).required(true)))]
4953
pub struct RedactionCreateKeyListArgs {
5054
#[clap(flatten)]
5155
repo_args: RepoArgs,
5256

57+
/// Commit containing all files to redact. If omitted, --input-file must
58+
/// contain one commit:path pair per line.
5359
#[clap(long, short = 'i')]
54-
commit_id: String,
60+
commit_id: Option<String>,
5561

5662
/// Fail if any of the content to be redacted is reachable from this main
5763
/// bookmark unless --force is set.
@@ -63,7 +69,8 @@ pub struct RedactionCreateKeyListArgs {
6369
#[clap(long)]
6470
force: bool,
6571

66-
/// Name of a file with a list of filenames to redact.
72+
/// Name of a file with a list of filenames to redact. Without --commit-id,
73+
/// each line must be a commit:path pair.
6774
#[clap(long)]
6875
input_file: Option<PathBuf>,
6976

@@ -178,6 +185,7 @@ async fn content_keys_for_paths(
178185
ctx: &CoreContext,
179186
repo: &Repo,
180187
cs_id: ChangesetId,
188+
commit_label: &str,
181189
paths: Vec<NonRootMPath>,
182190
) -> Result<HashSet<String>> {
183191
let use_content_manifests = justknobs::eval(
@@ -217,71 +225,121 @@ async fn content_keys_for_paths(
217225
let mut missing_paths = 0;
218226
for path in paths.iter() {
219227
if !path_content_keys.contains_key(path) {
220-
eprintln!("Missing file: {path}");
228+
eprintln!("Missing file in commit {commit_label}: {path}");
221229
missing_paths += 1;
222230
}
223231
}
224232
if missing_paths > 0 {
225-
bail!("Failed to find {missing_paths} files in this commit");
233+
bail!("Failed to find {missing_paths} files in commit {commit_label}");
226234
}
227235

228236
Ok(path_content_keys.into_values().collect())
229237
}
230238

239+
fn parse_commit_path_pair(line: &str, line_number: usize) -> Result<(String, NonRootMPath)> {
240+
let (commit_id, path) = line
241+
.split_once(':')
242+
.ok_or_else(|| anyhow!("Invalid input line {line_number}: expected a commit:path pair"))?;
243+
if commit_id.is_empty() {
244+
bail!("Invalid input line {line_number}: commit is empty");
245+
}
246+
if path.is_empty() {
247+
bail!("Invalid input line {line_number}: path is empty");
248+
}
249+
let path = NonRootMPath::new(path)
250+
.with_context(|| format!("Invalid path on input line {line_number}"))?;
251+
Ok((commit_id.to_string(), path))
252+
}
253+
254+
async fn content_keys_for_commit_paths(
255+
ctx: &CoreContext,
256+
repo: &Repo,
257+
paths_by_commit: HashMap<String, Vec<NonRootMPath>>,
258+
) -> Result<HashSet<String>> {
259+
stream::iter(paths_by_commit)
260+
.map(|(commit_id, paths)| async move {
261+
let cs_id = parse_commit_id(ctx, repo, &commit_id)
262+
.await
263+
.with_context(|| format!("Failed to parse commit id '{commit_id}'"))?;
264+
content_keys_for_paths(ctx, repo, cs_id, &commit_id, paths)
265+
.await
266+
.with_context(|| format!("Failed to find content keys in commit '{commit_id}'"))
267+
})
268+
.buffer_unordered(COMMIT_LOOKUP_CONCURRENCY)
269+
.try_fold(HashSet::new(), |mut keys, commit_keys| async move {
270+
keys.extend(commit_keys);
271+
Ok(keys)
272+
})
273+
.await
274+
}
275+
231276
pub async fn create_key_list_from_commit_files(
232277
ctx: &CoreContext,
233278
app: &MononokeApp,
234279
create_args: RedactionCreateKeyListArgs,
235280
) -> Result<()> {
236-
let mut files = create_args
237-
.files
238-
.iter()
239-
.map(NonRootMPath::new)
240-
.collect::<Result<Vec<_>>>()?;
241-
if let Some(input_file) = create_args.input_file {
281+
let RedactionCreateKeyListArgs {
282+
repo_args,
283+
commit_id,
284+
main_bookmark,
285+
force,
286+
input_file,
287+
output_file,
288+
skip_aws_sync,
289+
files,
290+
} = create_args;
291+
292+
let mut paths_by_commit: HashMap<String, Vec<NonRootMPath>> = HashMap::new();
293+
if let Some(commit_id) = commit_id {
294+
let mut paths = files
295+
.into_iter()
296+
.map(NonRootMPath::new)
297+
.collect::<Result<Vec<_>>>()?;
298+
if let Some(input_file) = input_file {
299+
let input_file =
300+
BufReader::new(File::open(input_file).context("Failed to open input file")?);
301+
for line in input_file.lines() {
302+
paths.push(NonRootMPath::new(line?)?);
303+
}
304+
}
305+
paths_by_commit.insert(commit_id, paths);
306+
} else {
307+
if !files.is_empty() {
308+
bail!("--commit-id is required when passing FILE arguments");
309+
}
310+
let input_file = input_file
311+
.ok_or_else(|| anyhow!("--input-file is required when --commit-id is omitted"))?;
242312
let input_file =
243313
BufReader::new(File::open(input_file).context("Failed to open input file")?);
244-
for line in input_file.lines() {
245-
files.push(NonRootMPath::new(line?)?);
314+
for (line_number, line) in input_file.lines().enumerate() {
315+
let (commit_id, path) = parse_commit_path_pair(&line?, line_number + 1)?;
316+
paths_by_commit.entry(commit_id).or_default().push(path);
246317
}
247318
}
248-
if files.is_empty() {
319+
if paths_by_commit.values().all(Vec::is_empty) {
249320
bail!("No files to redact");
250321
}
251322
let repo: Repo = app
252-
.open_repo(&create_args.repo_args)
323+
.open_repo(&repo_args)
253324
.await
254325
.context("Failed to open repo")?;
255326

256-
let cs_id = parse_commit_id(ctx, &repo, &create_args.commit_id).await?;
257-
258-
let keys = content_keys_for_paths(ctx, &repo, cs_id, files).await?;
327+
let keys = content_keys_for_commit_paths(ctx, &repo, paths_by_commit).await?;
259328

260-
println!(
261-
"Checking redacted content doesn't exist in '{}' bookmark",
262-
create_args.main_bookmark
263-
);
329+
println!("Checking redacted content doesn't exist in '{main_bookmark}' bookmark");
264330
let main_cs_id = repo
265331
.bookmarks()
266332
.get(
267333
ctx.clone(),
268-
&create_args.main_bookmark,
334+
&main_bookmark,
269335
bookmarks::Freshness::MostRecent,
270336
)
271337
.await?
272-
.ok_or_else(|| {
273-
anyhow!(
274-
"Main bookmark '{}' does not exist",
275-
create_args.main_bookmark
276-
)
277-
})?;
338+
.ok_or_else(|| anyhow!("Main bookmark '{main_bookmark}' does not exist"))?;
278339
let main_redacted = paths_for_content_keys(ctx, &repo, main_cs_id, &keys).await?;
279340

280341
if main_redacted.is_empty() {
281-
println!(
282-
"No files would be redacted in the main bookmark ({})",
283-
create_args.main_bookmark
284-
);
342+
println!("No files would be redacted in the main bookmark ({main_bookmark})");
285343
} else {
286344
for (path, content_id) in main_redacted.iter() {
287345
println!(
@@ -290,28 +348,27 @@ pub async fn create_key_list_from_commit_files(
290348
content_id.blobstore_key(),
291349
);
292350
}
293-
if create_args.force {
351+
if force {
294352
println!(
295353
"Creating key list despite {} files being redacted in the main bookmark ({}) (--force)",
296354
main_redacted.len(),
297-
create_args.main_bookmark
355+
main_bookmark
298356
);
299357
} else {
300358
bail!(
301359
"Refusing to create key list because {} files would be redacted in the main bookmark ({})",
302360
main_redacted.len(),
303-
create_args.main_bookmark
361+
main_bookmark
304362
);
305363
}
306364
}
307365

308366
let keys_vec: Vec<String> = keys.into_iter().collect();
309367
let keys_for_sync = keys_vec.clone();
310368

311-
let key_list_id =
312-
create_key_list(ctx, app, keys_vec, create_args.output_file.as_deref()).await?;
369+
let key_list_id = create_key_list(ctx, app, keys_vec, output_file.as_deref()).await?;
313370

314-
if !create_args.skip_aws_sync {
371+
if !skip_aws_sync {
315372
super::aws_sync::sync_to_aws(&keys_for_sync, key_list_id, repo.repo_identity.name()).await;
316373
}
317374

0 commit comments

Comments
 (0)