LPD-73846 Add clamav support #191
Conversation
|
looks good to me @anthony-chu |
| @@ -0,0 +1,3 @@ | |||
| hostname="clamav" | |||
| port=I"3310" | |||
There was a problem hiding this comment.
i'm wondering if it would be helpful to make the value of the port dynamically configured based on CLAMAV_PORT from ports.env. if we do, it seems like the ports.env file is only parsed in build.gradle, so the docker-liferay-bundle.gradle file wouldn't have access to it (at least, not that i know of; there might be a way). the only thought i had would be to parse it again as part of a task to configure this config file.
There was a problem hiding this comment.
@anthony-chu you should not need to configure this - it's all "in the box" (in the Docker network) and should not relate to the exposed host ports at all. See what we do for the elasticsearch service - it's a hard-coded port.
But to answer the more general question - you could parameterize this and do a string-replace during the Gradle copy operation that moves this to the Docker staging dir. See the Gradle docs and this example from Composer for ways to do it whenever it may be needed in the future.
drewbrokke
left a comment
drewbrokke
left a comment
There was a problem hiding this comment.
@anthony-chu looking good, just a few inline suggestions.
| @@ -0,0 +1,3 @@ | |||
| hostname="clamav" | |||
| port=I"3310" | |||
There was a problem hiding this comment.
@anthony-chu you should not need to configure this - it's all "in the box" (in the Docker network) and should not relate to the exposed host ports at all. See what we do for the elasticsearch service - it's a hard-coded port.
But to answer the more general question - you could parameterize this and do a string-replace during the Gradle copy operation that moves this to the Docker staging dir. See the Gradle docs and this example from Composer for ways to do it whenever it may be needed in the future.
| @@ -0,0 +1,3 @@ | |||
| hostname="clamav" | |||
| port=I"3310" | |||
| timeout=I"120000" | |||
There was a problem hiding this comment.
@anthony-chu if we have a conf file here, it will deploy no matter what, even if the antivirus is not enabled. This is why we instead put the configs as environment variables in liferay.{serviceName}.yaml files. See the liferay.elasticsearch.yaml file for an example.
|
|
||
| this.useClustering = this.useLiferay && this.clusterNodes > 0 | ||
|
|
||
| this.useClamAV = this.services.contains("clamav") |
There was a problem hiding this comment.
If this is not actually used for anything, let's just remove it.
3 participants
https://liferay.atlassian.net/browse/LPD-73846