From 7cc25b21f019fa4bbd6ab03a968bfa6c37485224 Mon Sep 17 00:00:00 2001 From: colin-lamed <9568290+colin-lamed@users.noreply.github.com> Date: Tue, 27 Jul 2021 11:28:56 +0100 Subject: [PATCH] BDOG-1421 Restore proxy config prefix --- CHANGELOG.md | 42 +++++++++++++++++++ .../src/main/resources/reference.conf | 4 +- .../uk/gov/hmrc/play/http/ws/WSRequest.scala | 28 ++++--------- 3 files changed, 53 insertions(+), 21 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 30c2effc..1fd3656d 100644 --- 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 index 632f8b6b..db63c583 100644 --- 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 index a2320f9b..459e1ac8 100644 --- 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) ) ) }