Like for rsync repos files in the RRDP repos should be delayed until after the validation finished. As with anything RPKI related there is little trust in the repositories and their abilities to not botch an update.
One thing I'm not sure is what should happen if a file is supposed to be removed but is still referenced by some other file. For now this fact is logged and the file is kept in the repo. I'm unsure about keeping the file, it feels like the right move but may result in unreferenced files piling up in the rrdp repo dirs. There is no way to detect stale files in RRDP repos (apart from removing all files and fetching a snapshot). So until RRDP grows up to a real sync protocol the only thing one can do is to provide a large enough partition and to remove the cache from time to time. -- :wq Claudio Index: repo.c =================================================================== RCS file: /cvs/src/usr.sbin/rpki-client/repo.c,v retrieving revision 1.6 diff -u -p -r1.6 repo.c --- repo.c 19 Apr 2021 17:04:35 -0000 1.6 +++ repo.c 29 Apr 2021 13:12:34 -0000 @@ -553,16 +553,19 @@ rrdp_free(void) } } -static int +static struct rrdprepo * rrdp_basedir(const char *dir) { struct rrdprepo *rr; SLIST_FOREACH(rr, &rrdprepos, entry) - if (strcmp(dir, rr->basedir) == 0) - return 1; + if (strcmp(dir, rr->basedir) == 0) { + if (rr->state == REPO_FAILED) + return NULL; + return rr; + } - return 0; + return NULL; } /* @@ -840,27 +843,6 @@ rrdp_merge_repo(struct rrdprepo *rr) struct filepath *fp, *nfp; char *fn, *rfn; - /* XXX should delay deletes */ - RB_FOREACH_SAFE(fp, filepath_tree, &rr->deleted, nfp) { - fn = rrdp_filename(rr, fp->file, 1); - rfn = rrdp_filename(rr, fp->file, 0); - - if (fn == NULL || rfn == NULL) - errx(1, "bad filepath"); /* should not happen */ - - if (unlink(rfn) == -1) { - if (errno == ENOENT) { - if (unlink(fn) == -1) - warn("%s: unlink", fn); - } else - warn("%s: unlink", rfn); - } - - free(rfn); - free(fn); - filepath_put(&rr->deleted, fp); - } - RB_FOREACH_SAFE(fp, filepath_tree, &rr->added, nfp) { fn = rrdp_filename(rr, fp->file, 1); rfn = rrdp_filename(rr, fp->file, 0); @@ -1146,12 +1128,40 @@ add_to_del(char **del, size_t *dsz, char *dsz = i + 1; return del; } + +static char ** +repo_rrdp_cleanup(struct filepath_tree *tree, struct rrdprepo *rr, + char **del, size_t *delsz) +{ + struct filepath *fp, *nfp; + char *fn; + + RB_FOREACH_SAFE(fp, filepath_tree, &rr->deleted, nfp) { + fn = rrdp_filename(rr, fp->file, 0); + /* temp dir will be cleaned up by repo_cleanup() */ + + if (fn == NULL) + errx(1, "bad filepath"); /* should not happen */ + + if (!filepath_exists(tree, fn)) + del = add_to_del(del, delsz, fn); + else + warnx("%s: referenced file supposed to be deleted", fn); + + free(fn); + filepath_put(&rr->deleted, fp); + } + + return del; +} + void repo_cleanup(struct filepath_tree *tree) { - size_t i, delsz = 0, dirsz = 0; + size_t i, cnt, delsz = 0, dirsz = 0; char **del = NULL, **dir = NULL; char *argv[4] = { "ta", "rsync", "rrdp", NULL }; + struct rrdprepo *rr; FTS *fts; FTSENT *e; @@ -1166,10 +1176,12 @@ repo_cleanup(struct filepath_tree *tree) e->fts_path); break; case FTS_D: - /* skip rrdp base directories during cleanup */ - if (rrdp_basedir(e->fts_path)) + /* special cleanup for rrdp directories */ + if ((rr = rrdp_basedir(e->fts_path)) != NULL) { + del = repo_rrdp_cleanup(tree, rr, del, &delsz); if (fts_set(fts, e, FTS_SKIP) == -1) err(1, "fts_set"); + } break; case FTS_DP: if (!filepath_dir_exists(tree, e->fts_path)) @@ -1203,25 +1215,31 @@ repo_cleanup(struct filepath_tree *tree) if (fts_close(fts) == -1) err(1, "fts_close"); + cnt = 0; for (i = 0; i < delsz; i++) { - if (unlink(del[i]) == -1) - warn("unlink %s", del[i]); - if (verbose > 1) - logx("deleted %s", del[i]); + if (unlink(del[i]) == -1) { + if (errno != ENOENT) + warn("unlink %s", del[i]); + } else { + if (verbose > 1) + logx("deleted %s", del[i]); + cnt++; + } free(del[i]); } free(del); - stats.del_files = delsz; + stats.del_files = cnt; + cnt = 0; for (i = 0; i < dirsz; i++) { if (rmdir(dir[i]) == -1) warn("rmdir %s", dir[i]); - if (verbose > 1) - logx("deleted dir %s", dir[i]); + else + cnt++; free(dir[i]); } free(dir); - stats.del_dirs = dirsz; + stats.del_dirs = cnt; } void