BDOG-1421 Restore proxy config prefix

hmrc · Jul 27, 2021 · 7cc25b2 · 7cc25b2
1 parent 5716a6c
commit 7cc25b2
Show file tree

Hide file tree

Showing 3 changed files with 53 additions and 21 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -1,3 +1,45 @@
+## Version 13.9.0
+
+| Change | Complexity | Fix  |
+|--------------------------------------------------------------|------------|-----------------------------------------------|
+| WSProxy changes  | Minor  | Optional change |
+
+### WSProxy
+`WSProxy` has been deprecated, the behaviour is avilable on `WSRequest`.
+`WSProxyConfiguration.apply` has been deprecated, use `WSProxyConfiguration.buildWsProxyServer` instead.
+
+There are some differences with `WSProxyConfiguration.buildWsProxyServer`:
+  * configPrefix is fixed to `proxy`.
+  * `proxy.proxyRequiredForThisEnvironment` has been replaced with `proxy.enabled`, but note, it
+      defaults to false (rather than true).
+  * nonProxyHosts can be configured by `proxy.nonProxyHosts`. It defaults to not apply the proxy to
+  * internal calls (platform and localhost).
+    Given the addition of nonProxyHosts, the reason for `proxy.enabled` is to avoid configuring the proxy.
+
+What this means:
+  You will **not** require two HttpClient implementations for using a proxy. A single HttpClient will be sufficient. When enabled by `proxy.enabled`, you can configure which hosts the proxy applies to with `proxy.nonProxyHosts`. By default this excludes `localhost`.
+
+
+```scala
+@Singleton
+class DefaultHttpClient @Inject()(
+  config: Configuration,
+  override val wsClient: WSClient,
+  override protected val actorSystem: ActorSystem
+) extends uk.gov.hmrc.http.HttpClient
+     with WSHttp {
+
+  override lazy val configuration: Config = config.underlying
+
+  override val hooks: Seq[HttpHook] = Seq.empty
+
+  override def wsProxyServer: Option[WSProxyServer] =
+    WSProxyConfiguration.buildWsProxyServer(config)
+}
+```
+
+
+
 ## Version 13.0.0
 
 | Change | Complexity | Fix  |

diff --git a/http-verbs-common/src/main/resources/reference.conf b/http-verbs-common/src/main/resources/reference.conf
@@ -19,5 +19,5 @@ http-verbs.retries {
   ssl-engine-closed-already.enabled = false
 }
 
-bootstrap.http.proxy.enabled = false
-bootstrap.http.proxy.nonProxyHosts = [ "*.service", "*.mdtp", "localhost" ]
+proxy.enabled = false
+proxy.nonProxyHosts = [ "*.service", "*.mdtp", "localhost" ]
diff --git a/http-verbs-common/src/main/scala/uk/gov/hmrc/play/http/ws/WSRequest.scala b/http-verbs-common/src/main/scala/uk/gov/hmrc/play/http/ws/WSRequest.scala
@@ -31,7 +31,7 @@ trait WSRequest extends WSRequestBuilder {
       .foldLeft(wsClient.url(url).withHttpHeaders(headers: _*))(_ withProxyServer _)
 }
 
-@deprecated("WSProxy is not required. Behaviour has been inlined into WSRequest", "5.8.0")
+@deprecated("WSProxy is not required. Behaviour has been inlined into WSRequest", "13.9.0")
 trait WSProxy extends WSRequest {
 
   def wsProxyServer: Option[WSProxyServer]
@@ -43,10 +43,9 @@ trait WSProxy extends WSRequest {
     }
 }
 
-// - Provide a ProxyWireMockSupport for testing? requires removing "localhost" from nonProxyHosts and running a mock?
 object WSProxyConfiguration {
 
-  @deprecated("Use buildWsProxyServer instead. See docs for differences.", "5.8.0")
+  @deprecated("Use buildWsProxyServer instead. See docs for differences.", "13.9.0")
   def apply(configPrefix: String, configuration: Configuration): Option[WSProxyServer] = {
     val proxyRequired =
       configuration.getOptional[Boolean](s"$configPrefix.proxyRequiredForThisEnvironment").getOrElse(true)
@@ -64,26 +63,17 @@ object WSProxyConfiguration {
     else None
   }
 
-  /** Replaces `apply`. The differences are:
-    * - configPrefix is fixed to "bootstrap.http.proxy".. For typical usage, this means that configuration "proxy."
-    *   changes to "bootstrap.http.proxy.".
-    * - "proxy.proxyRequiredForThisEnvironment" has been replaced with "bootstrap.http.proxy.enabled", but note, it
-    *   defaults to false (rather than true).
-    * - nonProxyHosts can be configured by "bootstrap.http.proxy.nonProxyHosts". It defaults to not apply the proxy to
-    *   internal calls (platform and localhost).
-    * Given the addition of nonProxyHosts, the reason for "bootstrap.http.proxy.enabled" is to avoid configuring the proxy.
-    */
   def buildWsProxyServer(configuration: Configuration): Option[WSProxyServer] =
-    if (configuration.get[Boolean]("bootstrap.http.proxy.enabled")) {
+    if (configuration.get[Boolean]("proxy.enabled")) {
       import scala.collection.JavaConverters.iterableAsScalaIterableConverter
       Some(
         DefaultWSProxyServer(
-          protocol      = Some(configuration.get[String]("bootstrap.http.proxy.protocol")), // this defaults to https, do we need it to be required in configuration?
-          host          = configuration.get[String]("bootstrap.http.proxy.host"),
-          port          = configuration.get[Int]("bootstrap.http.proxy.port"),
-          principal     = configuration.getOptional[String]("bootstrap.http.proxy.username"),
-          password      = configuration.getOptional[String]("bootstrap.http.proxy.password"),
-          nonProxyHosts = Some(configuration.underlying.getStringList("bootstrap.http.proxy.nonProxyHosts").asScala.toSeq)
+          protocol      = Some(configuration.get[String]("proxy.protocol")),
+          host          = configuration.get[String]("proxy.host"),
+          port          = configuration.get[Int]("proxy.port"),
+          principal     = configuration.getOptional[String]("proxy.username"),
+          password      = configuration.getOptional[String]("proxy.password"),
+          nonProxyHosts = Some(configuration.underlying.getStringList("proxy.nonProxyHosts").asScala.toSeq)
         )
       )
     }