search

cloudfoundry / nginx-buildpack

Cloud Foundry buildpack that provides NGINX
Apache License 2.0
30 stars 66 forks source link

Release notes for v1.0.0 suggest you can use {{ port }} with spaces, but you can't #10

Closed 36degrees closed 5 years ago

36degrees commented 5 years ago

What version of Cloud Foundry and CF CLI are you using? (i.e. What is the output of running cf curl /v2/info && cf version?

N/A

What version of the buildpack you are using?

v1.0.0 or above

If you were attempting to accomplish a task, what was it you were attempting to do?

Following the release notes to update from v0.0.5 to v1.0.3, I replaced {{.Port}} with {{ port }} in my nginx.conf

What did you expect to happen?

I expected the build to work

What was the actual behavior?

The build failed with:

          **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

The validator expects the string {{port}} with no spaces inside the curly braces.

Either the release notes should be updated to use {{port}} or the validator should be updated to allow optional spaces (if this would be templated correctly)

Please confirm where necessary:

  • [ ] I have included a log output
  • [ ] My log includes an error message
  • [ ] I have included steps for reproduction
cf-gitbot commented 5 years ago

We have created an issue in Pivotal Tracker to manage this:

https://www.pivotaltracker.com/story/show/162082773

The labels on this github issue will be updated when the story is started.

kardolus commented 5 years ago

@36degrees thanks! Good point. We decided to allow white spaces and updated the regex parsing in this commit: https://github.com/cloudfoundry/nginx-buildpack/commit/98abf0dbc139baed29620575423ccd01ca8447b3

sclevine commented 5 years ago

I don't think we should solve this by trying to use regex to parse go templates. Whitespace around port works, but other valid uses of port still fail.

cf-gitbot commented 5 years ago

We have created an issue in Pivotal Tracker to manage this:

https://www.pivotaltracker.com/story/show/162103278

The labels on this github issue will be updated when the story is started.

kardolus commented 5 years ago

@sclevine how should we be validating instead?

Ben16 commented 5 years ago

The presence of {{port}} (or any other valid template variant) is now verified without the use of a regex.

See the following commit: https://github.com/cloudfoundry/nginx-buildpack/commit/589a894105800a92bebdb5e67371bd0b79c5e7cd