invalidate CDN in more build error cases, & invalidate after yank / create-delete

Author: syphar

src/bin/cratesfyi.rs

@@ -606,6 +606,7 @@ impl Context for BinContext {
             self.pool()?,
             self.metrics()?,
             self.config()?,
+            self.cdn()?,
             self.storage()?,
         );
         fn storage(self) -> Storage = Storage::new(

src/build_queue.rs

@@ -1,3 +1,4 @@
+use crate::cdn::{self, CdnBackend};
 use crate::db::{delete_crate, Pool};
 use crate::docbuilder::PackageKind;
 use crate::error::Result;
@@ -27,6 +28,7 @@ pub(crate) struct QueuedCrate {
 #[derive(Debug)]
 pub struct BuildQueue {
     config: Arc<Config>,
+    cdn: Arc<CdnBackend>,
     storage: Arc<Storage>,
     pub(crate) db: Pool,
     metrics: Arc<Metrics>,
@@ -38,11 +40,13 @@ impl BuildQueue {
         db: Pool,
         metrics: Arc<Metrics>,
         config: Arc<Config>,
+        cdn: Arc<CdnBackend>,
         storage: Arc<Storage>,
     ) -> Self {
         BuildQueue {
             max_attempts: config.build_attempts.into(),
             config,
+            cdn,
             db,
             metrics,
             storage,
@@ -173,6 +177,10 @@ impl BuildQueue {
             )
         });
         self.metrics.total_builds.inc();
+        if let Err(err) = cdn::invalidate_crate(&self.config, &self.cdn, &to_process.name) {
+            report_error(&err);
+        }
+
         match res {
             Ok(()) => {
                 transaction.execute("DELETE FROM queue WHERE id = $1;", &[&to_process.id])?;
@@ -284,6 +292,10 @@ impl BuildQueue {
                         Ok(_) => debug!("{}-{} yanked", release.name, release.version),
                         Err(err) => report_error(&err),
                     }
+                    if let Err(err) = cdn::invalidate_crate(&self.config, &self.cdn, &release.name)
+                    {
+                        report_error(&err);
+                    }
                 }
 
                 Change::Added(release) => {
@@ -319,6 +331,9 @@ impl BuildQueue {
                             Ok(_) => info!("crate {} was deleted from the index and will be deleted from the database", krate), 
                             Err(err) => report_error(&err),
                         }
+                    if let Err(err) = cdn::invalidate_crate(&self.config, &self.cdn, krate) {
+                        report_error(&err);
+                    }
                 }
             }
         }
@@ -520,6 +535,67 @@ mod tests {
             assert_eq!(metrics.total_builds.get(), 9);
             assert_eq!(metrics.failed_builds.get(), 1);
 
+            // no invalidations were run since we don't have a distribution id configured
+            assert!(matches!(*env.cdn(), CdnBackend::Dummy(_)));
+            if let CdnBackend::Dummy(ref invalidation_requests) = *env.cdn() {
+                let ir = invalidation_requests.lock().unwrap();
+                assert!(ir.is_empty());
+            }
+
+            Ok(())
+        })
+    }
+
+    #[test]
+    fn test_invalidate_cdn_after_build_and_error() {
+        crate::test::wrapper(|env| {
+            env.override_config(|config| {
+                config.cloudfront_distribution_id_web = Some("distribution_id".into());
+            });
+
+            let queue = env.build_queue();
+
+            queue.add_crate("will_succeed", "1.0.0", -1, None)?;
+            queue.add_crate("will_fail", "1.0.0", 0, None)?;
+
+            assert!(matches!(*env.cdn(), CdnBackend::Dummy(_)));
+            if let CdnBackend::Dummy(ref invalidation_requests) = *env.cdn() {
+                let ir = invalidation_requests.lock().unwrap();
+                assert!(ir.is_empty());
+            }
+
+            queue.process_next_crate(|krate| {
+                assert_eq!("will_succeed", krate.name);
+                Ok(())
+            })?;
+            if let CdnBackend::Dummy(ref invalidation_requests) = *env.cdn() {
+                let ir = invalidation_requests.lock().unwrap();
+                assert_eq!(
+                    *ir,
+                    [
+                        ("distribution_id".into(), "/will_succeed*".into()),
+                        ("distribution_id".into(), "/crate/will_succeed*".into()),
+                    ]
+                );
+            }
+
+            queue.process_next_crate(|krate| {
+                assert_eq!("will_fail", krate.name);
+                anyhow::bail!("simulate a failure");
+            })?;
+            if let CdnBackend::Dummy(ref invalidation_requests) = *env.cdn() {
+                let ir = invalidation_requests.lock().unwrap();
+                assert_eq!(
+                    *ir,
+                    [
+                        ("distribution_id".into(), "/will_succeed*".into()),
+                        ("distribution_id".into(), "/crate/will_succeed*".into()),
+                        ("distribution_id".into(), "/will_fail*".into()),
+                        ("distribution_id".into(), "/crate/will_fail*".into()),
+                    ]
+                );
+            }
+
             Ok(())
         })
     }

src/cdn.rs

@@ -1,5 +1,5 @@
 use crate::Config;
-use anyhow::{Error, Result};
+use anyhow::{Context, Error, Result};
 use aws_sdk_cloudfront::{
     model::{InvalidationBatch, Paths},
     Client, RetryConfig,
@@ -18,6 +18,7 @@ pub(crate) enum CdnKind {
     CloudFront,
 }
 
+#[derive(Debug)]
 pub enum CdnBackend {
     Dummy(Arc<Mutex<Vec<(String, String)>>>),
     CloudFront {
@@ -116,6 +117,18 @@ impl CdnBackend {
     }
 }
 
+pub(crate) fn invalidate_crate(config: &Config, cdn: &CdnBackend, name: &str) -> Result<()> {
+    if let Some(distribution_id) = config.cloudfront_distribution_id_web.as_ref() {
+        cdn.create_invalidation(
+            distribution_id,
+            &[&format!("/{}*", name), &format!("/crate/{}*", name)],
+        )
+        .context("error creating CDN invalidation")?;
+    }
+
+    Ok(())
+}
+
 #[cfg(test)]
 mod tests {
     use super::*;

src/docbuilder/rustwide_builder.rs

@@ -1,4 +1,3 @@
-use crate::cdn::CdnBackend;
 use crate::db::file::add_path_into_database;
 use crate::db::{
     add_build_into_database, add_doc_coverage, add_package_into_database,
@@ -10,12 +9,11 @@ use crate::index::api::ReleaseData;
 use crate::repositories::RepositoryStatsUpdater;
 use crate::storage::{rustdoc_archive_path, source_archive_path};
 use crate::utils::{
-    copy_dir_all, parse_rustc_version, queue_builder, report_error, set_config, CargoMetadata,
-    ConfigName,
+    copy_dir_all, parse_rustc_version, queue_builder, set_config, CargoMetadata, ConfigName,
 };
 use crate::{db::blacklist::is_blacklisted, utils::MetadataPackage};
 use crate::{Config, Context, Index, Metrics, Storage};
-use anyhow::{anyhow, bail, Context as _, Error};
+use anyhow::{anyhow, bail, Error};
 use docsrs_metadata::{Metadata, DEFAULT_TARGETS, HOST_TARGET};
 use failure::Error as FailureError;
 use log::{debug, info, warn, LevelFilter};
@@ -44,7 +42,6 @@ pub struct RustwideBuilder {
     config: Arc<Config>,
     db: Pool,
     storage: Arc<Storage>,
-    cdn: Arc<CdnBackend>,
     metrics: Arc<Metrics>,
     index: Arc<Index>,
     rustc_version: String,
@@ -83,7 +80,6 @@ impl RustwideBuilder {
             config,
             db: context.pool()?,
             storage: context.storage()?,
-            cdn: context.cdn()?,
             metrics: context.metrics()?,
             index: context.index()?,
             rustc_version: String::new(),
@@ -504,18 +500,6 @@ impl RustwideBuilder {
             .purge_from_cache(&self.workspace)
             .map_err(FailureError::compat)?;
         local_storage.close()?;
-        if let Some(distribution_id) = self.config.cloudfront_distribution_id_web.as_ref() {
-            if let Err(err) = self
-                .cdn
-                .create_invalidation(
-                    distribution_id,
-                    &[&format!("/{}*", name), &format!("/crate/{}*", name)],
-                )
-                .context("error creating CDN invalidation")
-            {
-                report_error(&err);
-            }
-        }
         Ok(successful)
     }
 
@@ -822,10 +806,7 @@ pub(crate) struct BuildResult {
 #[cfg(test)]
 mod tests {
     use super::*;
-    use crate::{
-        cdn::CdnKind,
-        test::{assert_redirect, assert_success, wrapper},
-    };
+    use crate::test::{assert_redirect, assert_success, wrapper};
     use serde_json::Value;
 
     #[test]
@@ -942,12 +923,6 @@ mod tests {
                 }
             }
 
-            assert!(matches!(*env.cdn(), CdnBackend::Dummy(_)));
-            if let CdnBackend::Dummy(ref invalidation_requests) = *env.cdn() {
-                let ir = invalidation_requests.lock().unwrap();
-                assert!(ir.is_empty());
-            }
-
             Ok(())
         })
     }
@@ -1069,37 +1044,6 @@ mod tests {
         });
     }
 
-    #[test]
-    #[ignore]
-    fn test_cdn_invalidation() {
-        wrapper(|env| {
-            env.override_config(|cfg| {
-                cfg.cdn_backend = CdnKind::Dummy;
-                cfg.cloudfront_distribution_id_web = Some("distribution_id".into());
-            });
-
-            let mut builder = RustwideBuilder::init(env).unwrap();
-            assert!(builder.build_package(
-                DUMMY_CRATE_NAME,
-                DUMMY_CRATE_VERSION,
-                PackageKind::CratesIo
-            )?);
-
-            assert!(matches!(*env.cdn(), CdnBackend::Dummy(_)));
-            if let CdnBackend::Dummy(ref invalidation_requests) = *env.cdn() {
-                let ir = invalidation_requests.lock().unwrap();
-                assert_eq!(ir.len(), 2);
-                assert_eq!(ir[0], ("distribution_id".into(), "/empty-library*".into()));
-                assert_eq!(
-                    ir[1],
-                    ("distribution_id".into(), "/crate/empty-library*".into())
-                );
-            }
-
-            Ok(())
-        });
-    }
-
     #[test]
     #[ignore]
     fn test_locked_fails_unlocked_needs_new_deps() {

src/test/mod.rs

@@ -292,6 +292,7 @@ impl TestEnvironment {
                     self.db().pool(),
                     self.metrics(),
                     self.config(),
+                    self.cdn(),
                     self.storage(),
                 ))
             })