Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(conf) fallback for lack of value for var placeholders #1606

Merged
merged 1 commit into from
Sep 8, 2016

Conversation

thibaultcha
Copy link
Member

In the case when anonymous_reports=on + socket.toip is not
resolving (ex: no internet connection) Nginx would be started with an
invalid configuration file because Kong's nginx config template would have an untouched
${{SYSLOG_REPORTS}} placeholder left. See prefix_handker.lua's compile_conf() function.

This ensures that by default, some of those variables (holding the directive name + value) are at least removed from the template before starting.

@thibaultcha thibaultcha changed the title fix(template) provide hard-coded defaults fix(template) fallback for lack of value for var placeholders Sep 8, 2016
@thibaultcha thibaultcha force-pushed the fix/template-defaults branch from 5b2b5ed to bf0cbb4 Compare September 8, 2016 00:46
@thibaultcha thibaultcha changed the title fix(template) fallback for lack of value for var placeholders fix(conf) fallback for lack of value for var placeholders Sep 8, 2016
`anonymous_reports=on` + `socket.toip` not resolving (ex: no internet
connection) would make Kong use an invalid Nginx template (with
variable placeholders untouched).
@Tieske
Copy link
Member

Tieske commented Sep 8, 2016

lgtm

@Tieske Tieske added pr/ready (but hold merge) No more concerns, but do not merge yet (probably a conflict of interest with another PR or release) and removed pr/status/needs review labels Sep 8, 2016
@thibaultcha thibaultcha merged commit 7a9e7b2 into next Sep 8, 2016
@thibaultcha thibaultcha deleted the fix/template-defaults branch September 8, 2016 19:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pr/ready (but hold merge) No more concerns, but do not merge yet (probably a conflict of interest with another PR or release)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants