From f17b09898af5d48e9e5f804668553bf6b99a0493 Mon Sep 17 00:00:00 2001
From: Eliza Weisman <eliza@buoyant.io>
Date: Wed, 6 Jun 2018 13:43:17 -0700
Subject: [PATCH]  proxy: Canonicalize TLS config paths before polling
 filesystem metadata (#1076)

This branch changes the polling-based implementation of TLS config file watches
to fully canonicalize the path to each config file prior to polling for its
metadata. Doing so fixes the issues detecting changes when the watched path is
a symbolic link to another symbolic link (see #1061), which is how Kubernetes
implements ConfigMaps mounted as volumes.

I've manually tested this with Conduit running in Docker for Mac Kubernetes,
by volume-mounting a ConfigMap containing the TLS config files, and
regenerating, deleting, and adding the certificates. Watching the Conduit logs
confirms that the changes are now successfully detected.

Note that we have to re-canonicalize the path every time we poll the filesystem
for metadata. Otherwise, if the file is a symlink and the link target changes,
we will continue polling the _old_ link target's path, and fail to detect any
changes to the _new_ link target.

Signed-off-by: Eliza Weisman <eliza@buoyant.io>
---
 proxy/src/transport/tls/config.rs | 32 ++++++++++++++++++++++---------
 1 file changed, 23 insertions(+), 9 deletions(-)

diff --git a/proxy/src/transport/tls/config.rs b/proxy/src/transport/tls/config.rs
index 6167df4bd..e59aac2e2 100644
--- a/proxy/src/transport/tls/config.rs
+++ b/proxy/src/transport/tls/config.rs
@@ -1,5 +1,5 @@
 use std::{
-    fs::{self, File},
+    fs::File,
     io::{self, Cursor, Read},
     path::PathBuf,
     sync::Arc,
@@ -73,19 +73,33 @@ impl CommonSettings {
             self.end_entity_cert.clone(),
             self.private_key.clone(),
         ];
+
+        fn last_modified(path: &PathBuf) -> Option<SystemTime> {
+            // We have to canonicalize the path _every_ time we poll the fs,
+            // rather than once when we start watching, because if it's a
+            // symlink, the target may change. If that happened, and we
+            // continued watching the original canonical path, we wouldn't see
+            // any subsequent changes to the new symlink target.
+            path.canonicalize()
+                .and_then(|canonical| {
+                    trace!("last_modified: {:?} -> {:?}", path, canonical);
+                    canonical.symlink_metadata()
+                        .and_then(|meta| meta.modified())
+                })
+                .map_err(|e| if e.kind() != io::ErrorKind::NotFound {
+                    // Don't log if the files don't exist, since this
+                    // makes the logs *quite* noisy.
+                    warn!("error reading metadata for {:?}: {}", path, e)
+                })
+                .ok()
+        }
+
         let mut max: Option<SystemTime> = None;
         Interval::new(Instant::now(), interval)
             .map_err(|e| error!("timer error: {:?}", e))
             .filter_map(move |_| {
                 for path in &paths  {
-                    let t = fs::metadata(path)
-                        .and_then(|meta| meta.modified())
-                        .map_err(|e| if e.kind() != io::ErrorKind::NotFound {
-                            // Don't log if the files don't exist, since this
-                            // makes the logs *quite* noisy.
-                            warn!("metadata for {:?}: {}", path, e)
-                        })
-                        .ok();
+                    let t = last_modified(path);
                     if t > max {
                         max = t;
                         trace!("{:?} changed at {:?}", path, t);