[cli] decouple apicast environment and 3scale configuration channel #590

mikz · 2018-02-13T12:50:58Z

THREESCALE_DEPLOYMENT_ENV controls the 3scale proxy configuration
channel (staging/production) and can be passed as -3 --channel from the CLI
APICAST_ENVIRONMENT controls the APIcast settings (caching, ...) and can be passed by -e --environment from the CLI
APICAST_LOADED_ENVIRONMENTS can control always loaded APIcast environments (internal use only)
update Test-APIcast to include [blackbox] forward ENV to APIcast Test-APIcast#3
production is still the default configuration channel, that loads the production environment configuration which configures the configuration loader to "boot". In short this makes APIcast to require THREESCALE_PORTAL_ENDPOINT or --configuration when started without any other params. That makes bin/apicast to not start because it is missing configuration.

Needed for 3scale/apicast-cloud-hosted#1

* THREESCALE_DEPLOYMENT_ENV controls the 3scale proxy configuration channel (staging/production) * APICAST_ENVIRONMENT controls the APIcast settings (caching, ...) * APICAST_LOADED_ENVIRONMENTS can control always loaded APIcast environments

* so the environment configuration is default value * the cli options are just overrides It should be possible to run apicast staging/production and get all the caching configurations already set.

and allow per test setting of environment and use new test configuration loader (that does nothing)

* gateway needs some endpoint that returns to for the "boot" loader * some tests used to run in lazy mode, so keep it that way

davidor · 2018-02-14T09:43:44Z

gateway/config/sandbox.lua

@@ -0,0 +1 @@
+staging.lua


Why sandbox and why it's the same as staging?

Because some configurations can still have THREESCALE_DEPLOYMENT_ENV=sandbox. It was the original value in AMP 2.0 IIRC. We alias it now to staging which makes more sense.

davidor · 2018-02-14T09:46:41Z

gateway/config/staging.lua

+    master_process = 'on',
+    lua_code_cache = 'on',
+    configuration_loader = 'lazy',
+    configuration_cache = 0,


Why not os.getenv('APICAST_CONFIGURATION_CACHE') or 0 ?

I think our staging should always reload configuration. And because of the order of options and context in the start.lua the CLI options take precedence. So setting APICAST_CONFIGURATION_CACHE env would override this anyway.

That makes the production.lua os.getenv a bit redundant, but I wanted it there as an example of what can be done. But if it is confusing I could change/remove it.

davidor · 2018-02-14T09:48:24Z

gateway/src/apicast/cli/environment.lua

@@ -148,7 +148,7 @@ function _M:add(env)

    local config = loadfile(path, 't', {
        print = print, inspect = require('inspect'), context = self._context,
-        tonumber = tonumber, tostring = tostring,
+        tonumber = tonumber, tostring = tostring, os = { getenv = resty_env.value },


I think that overriding getenv here can be a bit confusing.
Although I get that you're doing this to be able to use it in the environment files.

Yep. I think it is more useful to expose resty_env.value than plain resty_env.get or os.getenv.
And the only difference is that it does not return empty strings. Which is actually useful for default values.

davidor · 2018-02-14T09:51:51Z

t/configuration-loading-boot-with-config.t

-env_to_nginx(
-    'APICAST_CONFIGURATION_LOADER',
+env_to_apicast(
+    'APICAST_CONFIGURATION_LOADER' => 'boot'


Was this allowed before or it's supported since Test::APIcast 0.08 ?

This wasn't really working before Test::APIcast 0.08. env_to_nginx was not working in Test::APIcast::Blackbox at all.

davidor · 2018-02-14T09:53:05Z

gateway/http.d/init.conf

@@ -15,7 +15,7 @@ init_by_lua_block {
    -- This ENV variable is defined in the main nginx.conf.liquid and injected when including this partial.
    -- The content of the ENV variable is a Lua table, so when rendered it actually can run ipairs on it.
    for k,v in pairs({{ ENV }}) do
-      if type(k) == 'string' and k ~= 'APICAST_CONFIGURATION_LOADER' then
+      if type(k) == 'string' and not resty_env.value(k) then


I don't get this change. Can you explain it, please?

It sets only missing/empty variables.

This leaves the flexibility of overriding the ENV when starting nginx and treating the {{ ENV }} only as default values.

Because Test::APIcast runs APICAST_CONFIGURATION_LOADER=test bin/apicast and expects it to work. But in cases where APICAST_CONFIGURATION_LOADER=boot is already set in the ENV this would not apply and APICAST_CONFIGURATION_LOADER would have value boot during initialization. Basically ignoring what ENV it was started in.

So this change is for two reasons:

I think it makes sense for ENV of the shell to take priority than what is in the config file

Test::APIcast needs to start APIcast with APICAST_CONFIGURATION_LOADER=test when testing configuration with --test but then later start it with boot when the config is started for real (with no APICAST_CONFIGURATION_LOADER - but that is already hardcoded in the config as the default).

octobot assigned mikz Feb 13, 2018

mikz force-pushed the env-configuration-ineritance branch from 3ded468 to 751ab3f Compare February 13, 2018 12:57

mikz force-pushed the env-configuration-ineritance branch from af1754e to ea10dc5 Compare February 14, 2018 07:54

mikz added a commit that referenced this pull request Feb 14, 2018

[changelog] update changelog with CLI changes in #590

e196165

mikz added 3 commits February 14, 2018 09:05

[cli] define staging and production configuration in env

9558521

* so the environment configuration is default value * the cli options are just overrides It should be possible to run apicast staging/production and get all the caching configurations already set.

[t] update Test::APIcast

1e5e955

and allow per test setting of environment and use new test configuration loader (that does nothing)

[changelog] update changelog with CLI changes in #590

786a5c2

mikz force-pushed the env-configuration-ineritance branch from e196165 to 786a5c2 Compare February 14, 2018 08:05

mikz added 2 commits February 14, 2018 10:03

[libexec] respect APICAST_OPENRESTY_BINARY and TEST_NGINX_BINARY

ff34ff7

[ci] fix smoke tests

d2aff7f

* gateway needs some endpoint that returns to for the "boot" loader * some tests used to run in lazy mode, so keep it that way

mikz requested a review from davidor February 14, 2018 09:24

davidor reviewed Feb 14, 2018

View reviewed changes

davidor approved these changes Feb 14, 2018

View reviewed changes

mikz merged commit 775de4d into master Feb 14, 2018

mikz deleted the env-configuration-ineritance branch February 14, 2018 10:20

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[cli] decouple apicast environment and 3scale configuration channel #590

[cli] decouple apicast environment and 3scale configuration channel #590

mikz commented Feb 13, 2018 •

edited

Loading

davidor Feb 14, 2018

mikz Feb 14, 2018

davidor Feb 14, 2018

davidor Feb 14, 2018

mikz Feb 14, 2018

davidor Feb 14, 2018

mikz Feb 14, 2018

davidor Feb 14, 2018

mikz Feb 14, 2018

davidor Feb 14, 2018

mikz Feb 14, 2018 •

edited

Loading

		@@ -0,0 +1 @@
		staging.lua

[cli] decouple apicast environment and 3scale configuration channel #590

[cli] decouple apicast environment and 3scale configuration channel #590

Conversation

mikz commented Feb 13, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mikz Feb 14, 2018 • edited Loading

Choose a reason for hiding this comment

mikz commented Feb 13, 2018 •

edited

Loading

mikz Feb 14, 2018 •

edited

Loading