add --address option to trigger service (#7090)

* factor --address, --http-port, --port-file options from http-json to cli-opts

- enabling reuse in trigger service

* use cli-opts for address and http-port options in Trigger service

* mark ServiceConfig and some defaults private

* use --address option to set up server

* document Setter

* test --address option is parsed

* missing (c) headers

* add changelog

CHANGELOG_BEGIN
- [Trigger Service] Accepts a new ``--address`` option to listen for HTTP connections on
  interfaces other than localhost, such as ``0.0.0.0`` for all addresses.
  See `issue #7090 <https://github.com/digital-asset/daml/pull/7090>`__.
CHANGELOG_END

ledger-service/cli-opts/BUILD.bazel

@@ -17,5 +17,6 @@ da_scala_library(
         "//ledger/ledger-api-common",
         "@maven//:com_github_scopt_scopt_2_12",
         "@maven//:io_netty_netty_handler",
+        "@maven//:org_scalaz_scalaz_core_2_12",
     ],
 )

ledger-service/cli-opts/src/main/scala/cliopts/Http.scala

@@ -0,0 +1,57 @@
+// Copyright (c) 2020 Digital Asset (Switzerland) GmbH and/or its affiliates. All rights reserved.
+// SPDX-License-Identifier: Apache-2.0
+
+package com.daml.cliopts
+
+import scalaz.syntax.id._
+import scalaz.syntax.std.option._
+
+import java.io.File
+import java.nio.file.Path
+
+object Http {
+  val defaultAddress: String = java.net.InetAddress.getLoopbackAddress.getHostAddress
+
+  /** Add options for setting up an HTTP server to `parser`.
+    *
+    * @param defaultHttpPort
+    *     If set, http-port is optional, with default given; otherwise the option is required.
+    * @param portFile
+    *     If set, there will be an optional port-file option; otherwise it will be absent.
+    */
+  def serverParse[C](parser: scopt.OptionParser[C], serviceName: String)(
+      address: Setter[C, String],
+      httpPort: Setter[C, Int],
+      defaultHttpPort: Option[Int],
+      portFile: Option[Setter[C, Option[Path]]],
+  ): Unit = {
+    import parser.opt
+
+    opt[String]("address")
+      .action((x, c) => address(_ => x, c))
+      .optional()
+      .text(
+        s"IP address that $serviceName service listens on. Defaults to ${defaultAddress: String}.")
+
+    opt[Int]("http-port")
+      .action((x, c) => httpPort(_ => x, c))
+      .into(o => if (defaultHttpPort.isDefined) o.optional() else o.required())
+      .text(
+        s"$serviceName service port number. " +
+          defaultHttpPort.cata(p => s"Defaults to ${p: Int}. ", "") +
+          "A port number of 0 will let the system pick an ephemeral port." +
+          (if (portFile.isDefined) " Consider specifying `--port-file` option with port number 0."
+           else ""))
+
+    portFile foreach { setPortFile =>
+      opt[File]("port-file")
+        .action((x, c) => setPortFile(_ => Some(x.toPath), c))
+        .optional()
+        .text(
+          "Optional unique file name where to write the allocated HTTP port number. " +
+            "If process terminates gracefully, this file will be deleted automatically. " +
+            s"Used to inform clients in CI about which port $serviceName listens on. " +
+            "Defaults to none, that is, no file gets created.")
+    }
+  }
+}

ledger-service/cli-opts/src/main/scala/cliopts/package.scala

@@ -0,0 +1,13 @@
+// Copyright (c) 2020 Digital Asset (Switzerland) GmbH and/or its affiliates. All rights reserved.
+// SPDX-License-Identifier: Apache-2.0
+
+package com.daml
+
+package object cliopts {
+
+  /** A lens-style setter.  When you want nested structures to be possible, this
+    * is vastly superior to the more obvious `(B, T) => T`, because unlike that
+    * one, this form permits nesting via trivial composition.
+    */
+  type Setter[T, B] = (B => B, T) => T
+}

ledger-service/cli-opts/src/main/scala/ledger/api/tls/TlsConfigurationCli.scala

@@ -1,7 +1,10 @@
 // Copyright (c) 2020 Digital Asset (Switzerland) GmbH and/or its affiliates. All rights reserved.
 // SPDX-License-Identifier: Apache-2.0
 
-package com.daml.ledger.api.tls
+package com.daml
+package ledger.api.tls
+
+import cliopts.Setter
 
 import java.nio.file.Paths
 
@@ -9,7 +12,7 @@ import scala.util.Try
 
 object TlsConfigurationCli {
   def parse[C](parser: scopt.OptionParser[C], colSpacer: String)(
-      setter: (TlsConfiguration => TlsConfiguration, C) => C): Unit = {
+      setter: Setter[C, TlsConfiguration]): Unit = {
     def enableSet(tlsUp: TlsConfiguration => TlsConfiguration, c: C) =
       setter(tlsc => tlsUp(tlsc copy (enabled = true)), c)
 

ledger-service/http-json/src/main/scala/com/digitalasset/http/Config.scala

@@ -4,7 +4,6 @@
 package com.daml.http
 
 import java.io.File
-import java.net.InetAddress
 import java.nio.file.Path
 import java.util.concurrent.TimeUnit
 
@@ -24,7 +23,7 @@ import scala.util.Try
 private[http] final case class Config(
     ledgerHost: String,
     ledgerPort: Int,
-    address: String = InetAddress.getLoopbackAddress.getHostAddress,
+    address: String = com.daml.cliopts.Http.defaultAddress,
     httpPort: Int,
     portFile: Option[Path] = None,
     applicationId: ApplicationId = ApplicationId("HTTP-JSON-API-Gateway"),

ledger-service/http-json/src/main/scala/com/digitalasset/http/Main.scala

@@ -3,7 +3,6 @@
 
 package com.daml.http
 
-import java.io.File
 import java.nio.file.{Path, Paths}
 
 import akka.actor.ActorSystem
@@ -143,28 +142,14 @@ object Main extends StrictLogging {
         .required()
         .text("Ledger port number")
 
-      opt[String]("address")
-        .action((x, c) => c.copy(address = x))
-        .optional()
-        .text(
-          s"IP address that HTTP JSON API service listens on. Defaults to ${Config.Empty.address: String}.")
+      import com.daml.cliopts
 
-      opt[Int]("http-port")
-        .action((x, c) => c.copy(httpPort = x))
-        .required()
-        .text(
-          "HTTP JSON API service port number. " +
-            "A port number of 0 will let the system pick an ephemeral port. " +
-            "Consider specifying `--port-file` option with port number 0.")
-
-      opt[File]("port-file")
-        .action((x, c) => c.copy(portFile = Some(x.toPath)))
-        .optional()
-        .text(
-          "Optional unique file name where to write the allocated HTTP port number. " +
-            "If process terminates gracefully, this file will be deleted automatically. " +
-            "Used to inform clients in CI about which port HTTP JSON API listens on. " +
-            "Defaults to none, that is, no file gets created.")
+      cliopts.Http.serverParse(this, serviceName = "HTTP JSON API")(
+        address = (f, c) => c copy (address = f(c.address)),
+        httpPort = (f, c) => c copy (httpPort = f(c.httpPort)),
+        defaultHttpPort = None,
+        portFile = Some((f, c) => c copy (portFile = f(c.portFile))),
+      )
 
       opt[String]("application-id")
         .action((x, c) => c.copy(applicationId = ApplicationId(x)))

triggers/service/BUILD.bazel

@@ -38,6 +38,7 @@ da_scala_library(
         "//language-support/scala/bindings",
         "//ledger-api/rs-grpc-akka",
         "//ledger-api/rs-grpc-bridge",
+        "//ledger-service/cli-opts",
         "//ledger/ledger-api-client",
         "//ledger/ledger-api-common",
         "//libs-scala/scala-utils",
@@ -110,6 +111,7 @@ da_scala_test(
         "//daml-lf/data",
         "//language-support/scala/bindings-akka",
         "//ledger-api/rs-grpc-bridge",
+        "//ledger-service/cli-opts",
         "//ledger/caching",
         "//ledger/ledger-api-common",
         "//ledger/participant-integration-api",

triggers/service/src/main/scala/com/digitalasset/daml/lf/engine/trigger/ServiceConfig.scala

@@ -6,16 +6,18 @@ package com.daml.lf.engine.trigger
 import java.nio.file.{Path, Paths}
 import java.time.Duration
 
+import com.daml.cliopts
 import com.daml.platform.services.time.TimeProviderType
 import scalaz.Show
 
 import scala.concurrent.duration
 import scala.concurrent.duration.FiniteDuration
 
-case class ServiceConfig(
+private[trigger] final case class ServiceConfig(
     // For convenience, we allow passing in a DAR on startup
     // as opposed to uploading it dynamically.
     darPath: Option[Path],
+    address: String,
     httpPort: Int,
     ledgerHost: String,
     ledgerPort: Int,
@@ -74,10 +76,10 @@ object JdbcConfig {
   private val indent: String = List.fill(8)(" ").mkString
 }
 
-object ServiceConfig {
-  val DefaultHttpPort: Int = 8088
+private[trigger] object ServiceConfig {
+  private val DefaultHttpPort: Int = 8088
   val DefaultMaxInboundMessageSize: Int = RunnerConfig.DefaultMaxInboundMessageSize
-  val DefaultMinRestartInterval: FiniteDuration = FiniteDuration(5, duration.SECONDS)
+  private val DefaultMinRestartInterval: FiniteDuration = FiniteDuration(5, duration.SECONDS)
   val DefaultMaxRestartInterval: FiniteDuration = FiniteDuration(60, duration.SECONDS)
 
   @SuppressWarnings(Array("org.wartremover.warts.NonUnitStatements")) // scopt builders
@@ -89,10 +91,12 @@ object ServiceConfig {
       .action((f, c) => c.copy(darPath = Some(Paths.get(f))))
       .text("Path to the dar file containing the trigger.")
 
-    opt[Int]("http-port")
-      .optional()
-      .action((t, c) => c.copy(httpPort = t))
-      .text(s"Optional HTTP port. Defaults to ${DefaultHttpPort}.")
+    cliopts.Http.serverParse(this, serviceName = "Trigger")(
+      address = (f, c) => c.copy(address = f(c.address)),
+      httpPort = (f, c) => c.copy(httpPort = f(c.httpPort)),
+      defaultHttpPort = Some(DefaultHttpPort),
+      portFile = None,
+    )
 
     opt[String]("ledger-host")
       .required()
@@ -156,6 +160,7 @@ object ServiceConfig {
       args,
       ServiceConfig(
         darPath = None,
+        address = cliopts.Http.defaultAddress,
         httpPort = DefaultHttpPort,
         ledgerHost = null,
         ledgerPort = 0,

triggers/service/src/main/scala/com/digitalasset/daml/lf/engine/trigger/ServiceMain.scala

@@ -83,7 +83,7 @@ object ServiceMain {
         val system: ActorSystem[Message] =
           ActorSystem(
             Server(
-              "localhost",
+              config.address,
               config.httpPort,
               ledgerConfig,
               restartConfig,

triggers/service/src/test/scala/com/digitalasset/daml/lf/engine/trigger/ServiceConfigTest.scala

@@ -0,0 +1,19 @@
+// Copyright (c) 2020 Digital Asset (Switzerland) GmbH and/or its affiliates. All rights reserved.
+// SPDX-License-Identifier: Apache-2.0
+
+package com.daml.lf.engine.trigger
+
+import org.scalatest.{Matchers, OptionValues, WordSpec}
+
+class ServiceConfigTest extends WordSpec with Matchers with OptionValues {
+  "parse" should {
+    import ServiceConfig.parse
+    import com.daml.cliopts.Http.defaultAddress
+    val baseOpts = Array("--ledger-host", "localhost", "--ledger-port", "9999")
+
+    "read address" in {
+      parse(baseOpts).value.address should ===(defaultAddress)
+      parse(baseOpts ++ Seq("--address", "0.0.0.0")).value.address should ===("0.0.0.0")
+    }
+  }
+}