fecgov / fec-proxy

Reverse proxy app to combine multiple applications.
Other
8 stars 6 forks source link

Build warnings when deploying #169

Closed lbeaufort closed 3 years ago

lbeaufort commented 4 years ago

What we're after: This work has the potential to be problematic if/when something happens to the proxy.

Completion criteria:

Build warnings when deploying:

Downloaded app package (15.5K)
-----> Staticfile Buildpack version 1.5.5
-----> Installing nginx
    Using nginx version 1.17.9
-----> Installing nginx 1.17.9
    Download [https://buildpacks.cloudfoundry.org/dependencies/nginx-static/nginx-1.17.9-linux-x64-cflinuxfs3-d122d80d.tgz]
    **WARNING** nginx 1.17.x will no longer be available in new buildpacks released after 2020-05-01.
    See: https://nginx.org/
-----> Root folder /tmp/app
-----> Copying project files into public
-----> Configuring nginx
    **WARNING** overriding nginx.conf is deprecated and highly discouraged, as it breaks the functionality of the Staticfile and Staticfile.auth configuration directives. Please use the NGINX buildpack available at: https://github.com/cloudfoundry/nginx-buildpack

From the above error message,

Please use the NGINX buildpack available at: https://github.com/cloudfoundry/nginx-buildpack

More context on why using staticfile buildpack and custom nginx.conf (like we do): https://github.com/cloudfoundry/staticfile-buildpack/issues/144#issuecomment-379567823

lbeaufort commented 4 years ago

https://github.com/cloudfoundry/staticfile-buildpack/issues/144 https://docs.cloudfoundry.org/buildpacks/staticfile/index.html#configure https://github.com/cloudfoundry/staticfile-buildpack/releases https://github.com/cloudfoundry/staticfile-buildpack/commit/2082dafa5c37468e706bb777aa109ac1de2c9601

lbeaufort commented 4 years ago

Need to follow https://docs.cloudfoundry.org/buildpacks/nginx/index.html#templating

Downloading nginx_buildpack...
Downloaded nginx_buildpack (8.9M)
Cell 666f4c22-43e5-4020-95f1-36eef13541b8 creating container for instance f193e3c0-823a-4990-9926-85694a03b5ed
Cell 666f4c22-43e5-4020-95f1-36eef13541b8 successfully created container for instance f193e3c0-823a-4990-9926-85694a03b5ed
Downloading app package...
Downloaded app package (16.2K)
-----> Nginx Buildpack version 1.1.7
-----> Supplying nginx
-----> No nginx version specified - using mainline => 1.17.9
-----> Installing nginx 1.17.9
       Download [https://buildpacks.cloudfoundry.org/dependencies/nginx/nginx-1.17.9-linux-x64-cflinuxfs3-9efdb551.tgz]
       **WARNING** nginx 1.17.x will no longer be available in new buildpacks released after 2020-05-01.
       See: https://nginx.org/
       **ERROR** nginx.conf file must be configured to respect the value of `{{port}}`
       **ERROR** Could not validate nginx.conf: no {{port}} in nginx.conf
Failed to compile droplet: Failed to run all supply scripts: exit status 14
Exit status 223
Cell 666f4c22-43e5-4020-95f1-36eef13541b8 stopping instance f193e3c0-823a-4990-9926-85694a03b5ed
Cell 666f4c22-43e5-4020-95f1-36eef13541b8 destroying container for instance f193e3c0-823a-4990-9926-85694a03b5ed
Cell 666f4c22-43e5-4020-95f1-36eef13541b8 successfully destroyed container for instance f193e3c0-823a-4990-9926-85694a03b5ed

Deleting app proxy in org fec-beta-fec / space dev as lbeaufort@fec.gov...
OK
Renaming app proxy-venerable to proxy in org fec-beta-fec / space dev as lbeaufort@fec.gov...
OK
error: BuildpackCompileFailed
lbeaufort commented 4 years ago

can validate with brew install nginx and nginx -t -c /Users/lbeaufort/dev/fec-proxy/nginx.conf but I'm not sure this is specific to the way the cf buildpack needs it

lbeaufort commented 4 years ago

Example conf files: https://github.com/cloud-gov/cf-subpath-proxy/blob/master/nginx.conf https://github.com/cloudfoundry/nginx-buildpack/blob/master/fixtures/mainline/nginx.conf https://github.com/alphagov/paas-ip-authentication-route-service/blob/master/nginx.conf https://github.com/18F/federalist-proxy/blob/staging/nginx.conf

From https://stackoverflow.com/questions/55733063/cloud-foundry-updating-to-nginx-buildpack-500-error-during-deploy:

Your nginx.conf file still has embedded ruby in it. You need to remove all that. The nginx buildpack isn't running your config through erb. If you need that stuff, you'd have to run erb on your config file manually, perhaps in a .profile file.

https://github.com/cloudfoundry/staticfile-buildpack/issues/144#issuecomment-379567823 are all true but:

  1. It makes apps incompatible with future versions of the buildpack that may come with a different nginx.conf that requires different template parameters from the rest of the buildpack.

➡️ should at worst cause app staging failures in case of new versions of the buildpack

  1. It has the potential to unexpectedly break other documented buildpack features, including basic auth via the Staticfile.auth file.

➡️ As far as we know apply only to the case in which we also have static content (as long as we use it as a pure reverse proxy you should be good)

  1. It is most often used for use cases outside of hosting static content that are not intended to be covered by the staticfile buildpack.

➡️ this is how we're using it

lbeaufort commented 4 years ago

I’m thinking we need to accept this risk for now, since we’d need to completely re-write the proxy application and we don’t have that expertise, and might be riskier than just leaving it as-is until more people address this and we can look at their implementation.

Changes that fail with buildpack errors: https://github.com/fecgov/fec-proxy/pull/174/files

Starting app proxy in org fec-beta-fec / space dev as lbeaufort@fec.gov...
Downloading nginx_buildpack...
Downloaded nginx_buildpack
Cell b701ee4f-885a-4c07-80c1-036e3d494e0a creating container for instance 67f4a8e1-d0e7-4713-8ae9-0f445f8cf81c
Cell b701ee4f-885a-4c07-80c1-036e3d494e0a successfully created container for instance 67f4a8e1-d0e7-4713-8ae9-0f445f8cf81c
Downloading app package...
Downloaded app package (16.9K)
-----> Nginx Buildpack version 1.1.7
-----> Supplying nginx
-----> No nginx version specified - using mainline => 1.17.9
-----> Installing nginx 1.17.9
       Download [https://buildpacks.cloudfoundry.org/dependencies/nginx/nginx-1.17.9-linux-x64-cflinuxfs3-9efdb551.tgz]
       **WARNING** nginx 1.17.x will no longer be available in new buildpacks released after 2020-05-01.
       See: https://nginx.org/
       **ERROR** Could not parse tmp config file: template: conf:53: function "route" not defined
       **ERROR** Could not validate nginx.conf: template: conf:53: function "route" not defined
Failed to compile droplet: Failed to run all supply scripts: exit status 14
Exit status 223
Cell b701ee4f-885a-4c07-80c1-036e3d494e0a stopping instance 67f4a8e1-d0e7-4713-8ae9-0f445f8cf81c
Cell b701ee4f-885a-4c07-80c1-036e3d494e0a destroying container for instance 67f4a8e1-d0e7-4713-8ae9-0f445f8cf81c
Cell b701ee4f-885a-4c07-80c1-036e3d494e0a successfully destroyed container for instance 67f4a8e1-d0e7-4713-8ae9-0f445f8cf81c

Deleting app proxy in org fec-beta-fec / space dev as lbeaufort@fec.gov...
OK
Renaming app proxy-venerable to proxy in org fec-beta-fec / space dev as lbeaufort@fec.gov...
OK
error: BuildpackCompileFailed
patphongs commented 4 years ago

@lbeaufort From what I can gage, we use cloud.gov's staticfile_buildpack v1.5.5. That version utilizes nginx version 1.17.9. The latest version of staticfile_buildpack v1.5.7 is using nginx version 1.17.10. As long as cloud.gov does not upgrade their staticfile_buildpack to a future version that is incompatible with the version of nginx, I see this risk as low. We should be cognizant of when cloud.gov upgrades their staticfile_buildpack to ensure that there are no issues with the upgraded nginx version that is used.

ChrisMcGowan commented 4 years ago

Hi folks - so 1.5.7 is the current static-buildpack level - you can track new releases here :

https://github.com/cloudfoundry/staticfile-buildpack/releases

From our cloud-foundry standpoint - new releases of buildpacks come in new releases of cf-deployment:

https://github.com/cloudfoundry/cf-deployment/blob/master/cf-deployment.yml - look down under the releases: section of the file.

lbeaufort commented 3 years ago

Closing in favor of #200 - the version warnings no longer appear, but we should move over to the NGINX buildpack.