Lint issues in dockerfile_exec_\*.sh

[wesley-dean-flexion]

When attempting to start a govready container from govready/govready-q (DockerHub), the following line is in my logs:

13:54:11.643: [APP/PROC/WEB.0] dockerfile_exec.sh: line 104: [: too many arguments

Without going in too deeply (i.e., I haven't determined if the gunicorn or uwsgi version is being used), I looked at line 104 in both files; turns out they're the same, so it doesn't really matter.

This is a link to the gunicorn version:

govready-q/deployment/docker/dockerfile_exec_gunicorn.sh

Line 104 in f85b457

if [ $DB_BEFORE_090 = "True" ]

So, when it's running, it appears that DB_BEFORE_090 is not set which is causing the runtime error.

I linted the dockerfile_gunicorn_exec.sh file with shellcheck and received the following lint errors:

In dockerfile_exec_gunicorn.sh line 1:
# This is the main entry point, i.e. process zero, of the
^-- SC2148: Tips depend on target shell and yours is unknown. Add a shebang.


In dockerfile_exec_gunicorn.sh line 16:
cat /proc/mounts | egrep -v "^proc|^cgroup| /proc| /dev| /sys"
    ^----------^ SC2002: Useless cat. Consider 'cmd < file | ..' or 'cmd file | ..' instead.
                   ^---^ SC2196: egrep is non-standard and deprecated. Use grep -E instead.


In dockerfile_exec_gunicorn.sh line 37:
ADDRESS=$(echo $ADDRESS | sed s/:80$//; )
          ^--------------------------^ SC2001: See if you can use ${variable//search/replace} instead.
               ^------^ SC2086: Double quote to prevent globbing and word splitting.

Did you mean: 
ADDRESS=$(echo "$ADDRESS" | sed s/:80$//; )


In dockerfile_exec_gunicorn.sh line 44:
	"host": $(echo ${ADDRESS} | jq -R .),
                       ^--------^ SC2086: Double quote to prevent globbing and word splitting.

Did you mean: 
	"host": $(echo "${ADDRESS}" | jq -R .),


In dockerfile_exec_gunicorn.sh line 46:
	"secret-key": $(echo ${SECRET_KEY-} | jq -R .),
                             ^------------^ SC2086: Double quote to prevent globbing and word splitting.

Did you mean: 
	"secret-key": $(echo "${SECRET_KEY-}" | jq -R .),


In dockerfile_exec_gunicorn.sh line 47:
	"syslog": $(echo ${SYSLOG-} | jq -R .),
                         ^--------^ SC2086: Double quote to prevent globbing and word splitting.

Did you mean: 
	"syslog": $(echo "${SYSLOG-}" | jq -R .),


In dockerfile_exec_gunicorn.sh line 50:
	"db": $(echo ${DBURL-} | jq -R .)
                     ^-------^ SC2086: Double quote to prevent globbing and word splitting.

Did you mean: 
	"db": $(echo "${DBURL-}" | jq -R .)


In dockerfile_exec_gunicorn.sh line 56:
	cat local/environment.json \
            ^--------------------^ SC2002: Useless cat. Consider 'cmd < file | ..' or 'cmd file | ..' instead.


In dockerfile_exec_gunicorn.sh line 57:
	| jq ".$1 = $(echo $2 | jq -R .)" \
                           ^-- SC2086: Double quote to prevent globbing and word splitting.

Did you mean: 
	| jq ".$1 = $(echo "$2" | jq -R .)" \


In dockerfile_exec_gunicorn.sh line 64:
if [ ! -z "${EMAIL_HOST-}" ]; then
     ^-- SC2236: Use -n instead of ! -z.


In dockerfile_exec_gunicorn.sh line 71:
if [ ! -z "${MAILGUN_API_KEY-}" ]; then
     ^-- SC2236: Use -n instead of ! -z.


In dockerfile_exec_gunicorn.sh line 76:
if [ ! -z "${BRANDING-}" ]; then
     ^-- SC2236: Use -n instead of ! -z.


In dockerfile_exec_gunicorn.sh line 81:
if [ ! -z "${PROXY_AUTHENTICATION_USER_HEADER-}" ]; then
     ^-- SC2236: Use -n instead of ! -z.


In dockerfile_exec_gunicorn.sh line 87:
if [ ! -z "${GR_PDF_GENERATOR-}" ]; then
     ^-- SC2236: Use -n instead of ! -z.


In dockerfile_exec_gunicorn.sh line 92:
if [ ! -z "${GR_IMG_GENERATOR-}" ]; then
     ^-- SC2236: Use -n instead of ! -z.


In dockerfile_exec_gunicorn.sh line 104:
if [ $DB_BEFORE_090 = "True" ]
     ^------------^ SC2086: Double quote to prevent globbing and word splitting.

Did you mean: 
if [ "$DB_BEFORE_090" = "True" ]


In dockerfile_exec_gunicorn.sh line 140:
if [ ! -z "${FIRST_RUN-}" ]; then
     ^-- SC2236: Use -n instead of ! -z.

For more information:
  https://www.shellcheck.net/wiki/SC2148 -- Tips depend on target shell and y...
  https://www.shellcheck.net/wiki/SC2086 -- Double quote to prevent globbing ...
  https://www.shellcheck.net/wiki/SC2196 -- egrep is non-standard and depreca...

[davidpofo]

@wesley-dean-flexion The image uploaded to docker hub is currently 8 months old and doesn't represent the current state of govready. In the installing guide you can build a container of your own if you desire. Efforts will be made to keep our hub image to be updated after some upcoming key releases.

[wesley-dean-flexion]

The lint issues are present in the main branch of the repository on GitHub pulled today; that the image on DockerHub fails is not the issue.

[its-a-lisa-at-work]

@davidpofo @gregelin do ya'll have an estimate on when the key releases when will be out?

[gregelin]

@wesley-dean-flexion @its-a-lisa-at-work We will work on fixing this week. I'm just reading through this issue now and need to spend some time looking at what's going on with the linting.

As you probably saw in our docs, we replaced the single container approach with a more robust container stack. This was a bit of debate internally of having a pre-built container vs building a stack. The reason the stack won out was that in more than 50% of the time, the container deployed ends up being custom to the deployment. Either the organization wants to build on their base image or there is some other customization. We can certainly help creating a deployment that works for you so you don't have fight any gotchas in our code alone.

For reference here is our new deployment information.

• https://github.com/GovReady/govready-deployments - for simple/test deployments
• https://github.com/GovReady/govready-q/tree/main/dev_env - for dev deployments and doing development

(We didn't update the Docker Hub content in a timely fashion after we made this change. At the end of last week we updated Docker Hub to mark it deprecated.)

(Note: This reply has been updated because the original version unintentionally came off as snarky.)

[davidpofo]

@wesley-dean-flexion @its-a-lisa-at-work I made a branch & PR which addresses all but one warning from shellcheck SC2153: Possible misspelling: EMAIL_PORT may not be assigned, but EMAIL_HOST is. EMAIL_PORT is properly assigned it is just too close to EMAIL_HOST spelling. Is this acceptable?

[gregelin]

@wesley-dean-flexion, here is current status of this ticket:

1. line 104: [: too many arguments / if [ $DB_BEFORE_090 = "True" ]
   status: In progress
   details: Will be removing outdated check related to long database migration from 0.8.6 to 0.9.0

2. Lint errors on dockerfile_gunicorn_exec.sh
   status: fixed
   details: See addressing shell script issues  #1670 and above issue

3. (implicit) complexity of gunicorn vs uwsgi
   status: to be fixed
   details: We only use Gunicorn. Need to refactor to remove legacy code. Will create a new ticket before closing this ticket

4. (implicit) Documentation
   status: fixing
   details: Cleaning up information in deployment/docker/README.md

5. (implicit) Getting a Docker container for GovReady-Q
   status: to be fixed
   details: Will create documentation and maybe example to make it easier to build stand alone GovReady-Q container.