Merge pull request #54 from rossabaker/issue-53

Interpolate environment variables in imports

.travis.yml

@@ -60,3 +60,4 @@ env:
     - secure: "emCycBlDj0nwdK/SI4q/38WGlZS1tvttZbgzb38KegPUIOv6hr78L73t/2pb8FT/AHPdhYF4q6q8AXxAJi3QoiznMSAT1xyxZZ+wWrWqPuphhkqMSEdzcG8fbi3YGkB0sl+7ZLsveUg/JmG6j6NFjzWjRjb6Rn9MUWKGWDs5a8g="
     - secure: "RKDMtxH135S+urp/t92qAwe1L0oGDuheKSXBcjwU55v2OS1NX4GPzE64y9hCuYK43BkXB1Xv/mMnF55TYSjHHpnP4BUXW0/SIERrwM+JFQ1TiukocMNiRUZ0jzZ6sUl7wpwhDntXR//jb/6+gCxC7hdlYUQgQ6qjCQffBZT0RzY="
     - secure: "lqLNd52nASivxq6IFYsq7UIBbH2eHVcvqSbOGFkzDsWscBoeWl5BvM1uOGFRoU3kJB3L23tK3Dq8rfIMXUh6ghPQpMlLKNtQ1XiTIm0s5MqY4JqmRbSLb6eexof/sUE6CgtwTlYp6LTQzT0NLdlBsvyySq5EkfJqeMVH0K7uu5Y="
+    - KNOBS_TEST_DIR="knobs-test"

core/build.sbt

@@ -11,3 +11,5 @@ libraryDependencies ++= {
     "io.verizon.ermine" %% "parser" % scalazCrossVersioners.default(scalazVersion.value)("0.4.7")
   )
 }
+
+

core/src/main/scala/knobs/MutableConfig.scala

@@ -169,4 +169,3 @@ case class MutableConfig(root: String, base: BaseConfig) {
     Process.eval(subscribe(p, (k, v) => sig.set((k, v)))).flatMap(_ => sig.discrete).drop(1) // droping the previously initilized tuple of the signal.
   }
 }
-

core/src/main/scala/knobs/package.scala

@@ -19,6 +19,7 @@ import scalaz.concurrent.Task
 import scalaz.syntax.foldable._
 import scalaz.syntax.traverse._
 import scalaz.syntax.monad._
+import scalaz.syntax.std.option._
 import scalaz.std.list._
 import scalaz.stream._
 import scalaz.stream.merge.mergeN
@@ -95,8 +96,8 @@ package object knobs {
         (ds, _) = p
         seenp = seen + (path -> p)
         box = path.worth
-        r <- Foldable[List].foldLeftM(importsOf(box.resource, ds)(box.R).
-          filter(notSeen), seenp)(go)
+        imports <- importsOfM(box.resource, ds)(box.R)
+        r <- Foldable[List].foldLeftM(imports.filter(notSeen), seenp)(go)
       } yield r
     }
     Foldable[List].foldLeftM(paths, Map():Loaded)(go)
@@ -140,10 +141,11 @@ package object knobs {
           val pfxp = pfx + name + "."
           Foldable[List].foldLeftM(xs, m)(directive(pfxp, f, _, _))
         case Import(path) =>
-          val fp = f resolve path
-          files.get(fp.required) match {
-            case Some((ds, _)) => Foldable[List].foldLeftM(ds, m)(directive(pfx, fp, _, _))
-            case _ => Task.now(m)
+          interpolateEnv(f, path).map(f.resolve).flatMap { fp =>
+            files.get(fp.required) match {
+              case Some((ds, _)) => Foldable[List].foldLeftM(ds, m)(directive(pfx, fp, _, _))
+              case _ => Task.now(m)
+            }
           }
       }
     }
@@ -175,10 +177,32 @@ package object knobs {
   }
 
   /** Get all the imports in the given list of directives, relative to the given path */
+  @deprecated("Does not support interpolation of environment variables", "4.0.31")
   def importsOf[R:Resource](path: R, d: List[Directive]): List[KnobsResource] =
     resolveImports(path, d).map(_.required)
 
+  private [knobs] def importsOfM[R:Resource](path: R, d: List[Directive]): Task[List[KnobsResource]] =
+    resolveImportsM(path, d).map(_.map(_.required))
+
+  // nb There is nothing Tasky about this, but the callers go into Task, and MonadError changed
+  // incompatibly between Scalaz 7.1 and Scalaz 7.2, so we'll use the monad of least resistance
+  private [knobs] def interpolateEnv[R: Resource](f: R, s: String): Task[String] = {
+    def interpret(i: Interpolation): Task[String] = i match {
+      case Literal(x) => Task.now(x)
+      case Interpolate(name) =>
+        sys.env.get(name)
+          // added because lots of sys-admins think software is case unaware. Doh!
+          .orElse(sys.env.get(name.toLowerCase))
+          .cata(Task.now, Task.fail(ConfigError(f, s"""No such variable "$name". Only environment variables are interpolated in import directives.""")))
+    }
+    if (s contains "$") P.interp.parse(s).fold(
+      e => Task.fail(ConfigError(f, e)),
+      xs => xs.traverse(interpret).map(_.mkString)
+    ) else Task.now(s)
+  }
+
   /** Get all the imports in the given list of directives, relative to the given path */
+  @deprecated("Does not support interpolation of environment variables", "4.0.31")
   def resolveImports[R:Resource](path: R, d: List[Directive]): List[R] =
     d match {
       case Import(ref) :: xs => (path resolve ref) :: resolveImports(path, xs)
@@ -187,14 +211,22 @@ package object knobs {
       case _ => Nil
     }
 
+  /** Get all the imports in the given list of directives, relative to the given path */
+  private [knobs] def resolveImportsM[R: Resource](path: R, d: List[Directive]): Task[List[R]] =
+    d.traverseM {
+      case Import(ref) => interpolateEnv(path, ref).map(r => List(path.resolve(r)))
+      case Group(_, ys) => resolveImportsM(path, ys)
+      case _ => Task.now(Nil)
+    }
+
   /**
    * Get all the imports in the given list of directive, recursively resolving
    * imports relative to the given path by loading them.
    */
   def recursiveImports[R:Resource](path: R, d: List[Directive]): Task[List[R]] =
-    resolveImports(path, d).traverse(r =>
+    resolveImportsM(path, d).flatMap(_.traverseM(r =>
       implicitly[Resource[R]].load(Required(r)).flatMap(dds =>
-        recursiveImports(r, dds))).map(_.flatten)
+        recursiveImports(r, dds))))
 
   private [knobs] def loadOne(path: KnobsResource): Task[(List[Directive], Process[Task, Unit])] = {
     val box = path.worth
@@ -256,4 +288,3 @@ package object knobs {
     if (b) m else implicitly[Monad[M]].pure(())
 
 }
-

core/src/test/resources/import-from-env-missing-file.cfg

@@ -0,0 +1,3 @@
+x {
+  import "$(KNOBS_TEST_DIR)/nope-nope-nope.cfg"
+}

core/src/test/resources/import-from-env.cfg

@@ -0,0 +1,3 @@
+x {
+  import "$(KNOBS_TEST_DIR)/imported.cfg"
+}

core/src/test/resources/import-from-sys-prop.cfg

@@ -0,0 +1,3 @@
+x {
+  import "$(user.dir)/src/test/resources/mutant.cfg"
+}

core/src/test/resources/knobs-test/imported.cfg

@@ -0,0 +1 @@
+hello = "world"

core/src/test/scala/knobs/Test.scala

@@ -48,7 +48,7 @@ object Test extends Properties("Knobs") {
       db <- cfg.lookup[Boolean]("ag.q-e.i_u9.a")
       du <- cfg.lookup[Duration]("dur")
       fdu <- cfg.lookup[FiniteDuration]("dur")
-  } yield (aa == Some(1)) :| "int property" &&
+    } yield (aa == Some(1)) :| "int property" &&
           (ab == Some("foo")) :| "string property" &&
           (acx == Some(1)) :| "nested int" &&
           (acy == Some(true)) :| "nested bool" &&
@@ -58,8 +58,7 @@ object Test extends Properties("Knobs") {
           (db == Some(false)) :| "deep bool" &&
           (du == Some(5.seconds)) :| "duration property" &&
           (fdu == Some(5.seconds)) :| "finite duration property"
-
-  }
+    }
 
   lazy val interpTest: Task[Prop] =
     withLoad(List(Required(ClassPathResource("pathological.cfg")))) { cfg => for {
@@ -84,6 +83,38 @@ object Test extends Properties("Knobs") {
       _ => false
     ))
 
+  lazy val importFromEnvTest: Task[Prop] =
+    sys.env.get("KNOBS_TEST_DIR") match {
+      case Some("knobs-test") =>
+        withLoad(List(Required(ClassPathResource("import-from-env.cfg")))) { cfg => for {
+          aa <- cfg.lookup[String]("x.hello")
+        } yield aa == Some("world") }
+      case oops =>
+        // We could see to this ourselves by forking, except https://github.com/rickynils/scalacheck/issues/185
+        sys.error(s"Test assumes that KNOBS_TEST_DIR environment variable is set to 'knobs-test', was $oops")
+    }
+
+  lazy val importFromEnvMissingFileTest: Task[Prop] =
+    sys.env.get("KNOBS_TEST_DIR") match {
+      case Some("knobs-test") =>
+        load(List(Required(ClassPathResource("import-from-env-missing-file.cfg")))).attempt.map {
+          case scalaz.-\/(f: java.io.FileNotFoundException) => true
+          case _ => false
+        }
+      case oops =>
+        // We could see to this ourselves by forking, except https://github.com/rickynils/scalacheck/issues/185
+        sys.error(s"Test assumes that KNOBS_TEST_DIR environment variable is set to 'knobs-test', was $oops")
+    }
+
+  lazy val importFromSysPropTest: Task[Prop] =
+    load(List(Required(ClassPathResource("import-from-sys-prop.cfg")))).attempt.map(_.fold(
+      {
+        case ConfigError(_, msg) => msg.contains("""No such variable "user.dir". Only environment variables are interpolated in import directives.""")
+        case x => false
+      },
+      x => false
+    ))
+
   lazy val loadPropertiesTest: Task[Prop] =
     withLoad(List(Required(SysPropsResource(Prefix("path"))))) { cfg =>
       cfg.lookup[String]("path.separator").map(_.isDefined)
@@ -129,7 +160,7 @@ object Test extends Properties("Knobs") {
   lazy val classLoaderTest: Task[Prop] =
     load(List(Required(ClassPathResource("pathological.cfg", new java.net.URLClassLoader(Array.empty))))).attempt.map {
       case scalaz.-\/(f: java.io.FileNotFoundException) => true
-      case _ => false
+      case x => false
     }
 
   lazy val immutableConfigValueErrorTest: Task[Prop] = {
@@ -173,6 +204,12 @@ object Test extends Properties("Knobs") {
 
   property("import-as-ident") = importAsIdentTest.unsafePerformSync
 
+  property("import-from-env") = importFromEnvTest.unsafePerformSync
+
+  property("import-from-env-missing-file") = importFromEnvMissingFileTest.unsafePerformSync
+
+  property("import-from-sys-prop") = importFromSysPropTest.unsafePerformSync
+
   property("load-system-properties") = loadPropertiesTest.unsafePerformSync
 
   property("system-properties-negative") = propertiesNegativeTest.unsafePerformSync

docs/build.sbt

@@ -2,6 +2,7 @@ import com.typesafe.sbt.SbtSite.SiteKeys._
 import com.typesafe.sbt.SbtGhPages.GhPagesKeys._
 
 enablePlugins(DisablePublishingPlugin)
+disablePlugins(BinCompatPlugin)
 
 site.settings
 
@@ -16,3 +17,4 @@ ghpagesNoJekyll := false
 includeFilter in makeSite := "*.yml" | "*.md" | "*.html" | "*.css" | "*.png" | "*.jpg" | "*.gif" | "*.js" | "*.swf"
 
 git.remoteRepo := "[email protected]:Verizon/knobs.git"
+

docs/src/main/tut/index.md

@@ -116,7 +116,7 @@ To import the contents of another configuration file, use the `import` directive
 import "$(HOME)/etc/myapp.cfg"
 ```
 
-Absolute paths are imported as is. Relative paths are resolved with respect to the file they are imported from. It is an error for an `import` directive to name a file that doesn't exist, cannot be read, or contains errors.
+Absolute paths are imported as is. Relative paths are resolved with respect to the file they are imported from. It is an error for an `import` directive to name a file that doesn't exist, cannot be read, or contains errors. Only environment variables, not system properties or other bindings, are interpolated into import directives.
 
 #### File lookup semantics
 

project.sbt

@@ -30,10 +30,15 @@ lazy val zookeeper = project.dependsOn(core)
 
 lazy val docs = project.dependsOn(core, zookeeper)
 
+(binCompatVersion in ThisBuild) := Some(scalazCrossVersioner.value("4.0.30"))
+
 enablePlugins(DisablePublishingPlugin)
+binCompatVersion := None
 
 // Some tests set system properties, which results in a
 // ConcurrentModificationException while other tests are iterating
 // over sys.props.  For a stable build, we need to reduce test
 // concurrency to 1 across modules.
 concurrentRestrictions in Global += Tags.limit(Tags.Test, 1)
+
+

project/BinCompatPlugin.scala

@@ -0,0 +1,36 @@
+//: ----------------------------------------------------------------------------
+//: Copyright (C) 2017 Verizon.  All Rights Reserved.
+//:
+//:   Licensed under the Apache License, Version 2.0 (the "License");
+//:   you may not use this file except in compliance with the License.
+//:   You may obtain a copy of the License at
+//:
+//:       http://www.apache.org/licenses/LICENSE-2.0
+//:
+//:   Unless required by applicable law or agreed to in writing, software
+//:   distributed under the License is distributed on an "AS IS" BASIS,
+//:   WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+//:   See the License for the specific language governing permissions and
+//:   limitations under the License.
+//:
+//: ----------------------------------------------------------------------------
+import sbt._, Keys._
+import com.typesafe.tools.mima.plugin.MimaPlugin
+
+object BinCompatPlugin extends AutoPlugin {
+  object autoImport {
+    val binCompatVersion = settingKey[Option[String]]("version to check binary compatibility against")
+  }
+
+  import autoImport._
+  import MimaPlugin.autoImport._
+
+  override def requires = MimaPlugin && verizon.build.ScalazPlugin
+
+  override def trigger = allRequirements
+
+  override lazy val projectSettings = Seq(
+    mimaPreviousArtifacts := binCompatVersion.value
+      .fold(Set.empty[ModuleID])(v => Set(organization.value %% name.value % v))
+  )
+}

project/build.properties

@@ -14,4 +14,4 @@
 //:   limitations under the License.
 //:
 //: ----------------------------------------------------------------------------
-sbt.version=0.13.13
+sbt.version=0.13.15

project/plugins.sbt

@@ -22,9 +22,10 @@ resolvers += Resolver.url(
 addSbtPlugin("io.verizon.build" % "sbt-rig" % "2.0.29")
 
 // docs
-addSbtPlugin("com.typesafe.sbt"  % "sbt-site"    % "0.8.1")
-addSbtPlugin("com.typesafe.sbt"  % "sbt-ghpages" % "0.5.3")
-addSbtPlugin("org.tpolecat"      % "tut-plugin"  % "0.4.7")
-addSbtPlugin("com.eed3si9n"      % "sbt-unidoc"  % "0.3.2")
+addSbtPlugin("com.typesafe.sbt"  % "sbt-site"        % "0.8.1")
+addSbtPlugin("com.typesafe.sbt"  % "sbt-ghpages"     % "0.5.3")
+addSbtPlugin("com.typesafe"      % "sbt-mima-plugin" % "0.1.14")
+addSbtPlugin("org.tpolecat"      % "tut-plugin"      % "0.4.7")
+addSbtPlugin("com.eed3si9n"      % "sbt-unidoc"      % "0.3.2")
 
 scalacOptions += "-deprecation"