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

Reply via email to