refactor(nginx-templates): reduce redundancy #11354
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
samugi
force-pushed
the
feat/nginx-conf-templates
branch
2 times, most recently
from
de175a4
to
6f135a0
Compare
samugi
changed the title
samugi
changed the title
samugi
force-pushed
the
feat/nginx-conf-templates
branch
6 times, most recently
from
f3a7103
to
4bb916c
Compare
samugi
changed the title
samugi
force-pushed
the
feat/nginx-conf-templates
branch
3 times, most recently
from
e68eb19
to
e72ce3a
Compare
samugi
force-pushed
the
feat/nginx-conf-templates
branch
3 times, most recently
from
0b3a3f6
to
505902c
Compare
samugi
force-pushed
the
feat/nginx-conf-templates
branch
from
505902c
to
2dae3a5
Compare
|
As discussed, let's postpone any additional changes to a separate PR if needed. We are also leaving the |
samugi
force-pushed
the
feat/nginx-conf-templates
branch
from
2dae3a5
to
372f12d
Compare
StarlightIbuki
approved these changes
Aug 14, 2023
bungle
reviewed
Aug 14, 2023
This comment was marked as duplicate.
This comment was marked as duplicate.
Sorry, something went wrong.
bungle
requested changes
Aug 14, 2023
samugi
force-pushed
the
feat/nginx-conf-templates
branch
from
fe7cb7d
to
b7b4c9c
Compare
samugi
force-pushed
the
feat/nginx-conf-templates
branch
3 times, most recently
from
0a09167
to
640fad5
Compare
bungle
reviewed
Aug 15, 2023
bungle
requested changes
Aug 15, 2023
bungle
approved these changes
Aug 15, 2023
samugi
force-pushed
the
feat/nginx-conf-templates
branch
3 times, most recently
from
2930641
to
e0c4e17
Compare
This commit tries to unify as much of the configuration as possible in a single place to reduce duplication and consequently the chances of forgetting to update one of the files. * nginx_kong.lua is compiled into nginx-kong.conf: chunks that are only meant for the test environment are rendered, if needed. * When the template is rendered, nginx-kong.conf is dynamically loaded in the http section using the include directive. refactor(templates): apply PR suggestions - use nginx include instead of rendering files in the templates using inline lua expressions - rename test-flags to nginx-conf-flags - cleanup of non needed changes - adapt tests Co-authored-by: Aapo Talvensaari <aapo.talvensaari@gmail.com>
samugi
force-pushed
the
feat/nginx-conf-templates
branch
from
e0c4e17
to
d37f96f
Compare
|
Looks good! Merge on green! |
samugi
force-pushed
the
feat/nginx-conf-templates
branch
from
15c5163
to
16feece
Compare
bungle
approved these changes
Aug 16, 2023
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary
Currently integration tests use dedicated templates that are manually kept in sync with the base nginx_kong.lua and nginx_kong_stream.lua configurations (should a change be made to those files it has to be manually replicated in custom_nginx.template and default_nginx.template). This is error prone and can lead to inconsistencies between the standard config and the one used in tests.
This PR tries to unify as much of the configuration as possible in a single place to reduce duplication and consequently the chances of forgetting to update one of the files, which may result in running tests with a configuration that differs from the "production" one.
Implementation
nginx_kong.luais compiled intonginx-kong.conf, while all the.luafiles infixtures/template_injectare compiled into.test.confconfiguration files in the prefix folder. During compilation, chunks that are only meant for the tests are rendered, if needed (this is controlled by thetestflag in the template environment). Config flags are set in the template rendering environment based on the --conf-template-flags input of the start cmd.When the template is rendered,
nginx-kong.confand the other partial configs are loaded in thehttpsection using theincludedirective. A similar approach is followed for the stream configuration.Checklist
Full changelog
Issue reference
KAG-1962