-
Notifications
You must be signed in to change notification settings - Fork 10
Refactor environment loading and handling for FrankenPHP + add more s… #397
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
Changes from all commits
b21817c
608a104
ba05a2a
55610b9
c849e5f
d391010
b41f811
a97be25
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -152,14 +152,19 @@ std::string GetLaravelEnvVariable(const std::string& env_key) { | |
| */ | ||
|
|
||
| using EnvGetterFn = std::string(*)(const std::string&); | ||
| EnvGetterFn envGetters[] = { | ||
|
|
||
| const std::vector<EnvGetterFn> completeEnvGetters = { | ||
| &GetSystemEnvVariable, | ||
| &GetFrankenEnvVariable, | ||
| &GetPhpEnvVariable, | ||
| &GetLaravelEnvVariable | ||
| }; | ||
|
|
||
| std::string GetEnvVariable(const std::string& env_key) { | ||
| const std::vector<EnvGetterFn> systemEnvGetters = { | ||
| &GetSystemEnvVariable, | ||
| }; | ||
|
|
||
| std::string GetEnvVariable(const std::vector<EnvGetterFn>& envGetters, const std::string& env_key) { | ||
| for (EnvGetterFn envGetter : envGetters) { | ||
| std::string env_value = envGetter(env_key); | ||
| if (!env_value.empty()) { | ||
|
|
@@ -169,8 +174,8 @@ std::string GetEnvVariable(const std::string& env_key) { | |
| return ""; | ||
| } | ||
|
|
||
| std::string GetEnvString(const std::string& env_key, const std::string default_value) { | ||
| std::string env_value = GetEnvVariable(env_key); | ||
| std::string GetEnvString(const std::vector<EnvGetterFn>& envGetters, const std::string& env_key, const std::string default_value) { | ||
ioaniftimesei marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| std::string env_value = GetEnvVariable(envGetters, env_key); | ||
| if (!env_value.empty()) { | ||
| return env_value; | ||
| } | ||
|
|
@@ -185,12 +190,16 @@ bool GetBoolFromString(const std::string& env, bool default_value) { | |
| return default_value; | ||
| } | ||
|
|
||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Let's rename this function to GetEnvBoolWithAllGetters or smth like this so it's more explicit what it does. Can remove the comment from the Includes.h after that. |
||
| bool GetEnvBool(const std::string& env_key, bool default_value) { | ||
| return GetBoolFromString(GetEnvVariable(env_key), default_value); | ||
| bool GetEnvBool(const std::vector<EnvGetterFn>& envGetters, const std::string& env_key, bool default_value) { | ||
ioaniftimesei marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| return GetBoolFromString(GetEnvVariable(envGetters, env_key), default_value); | ||
| } | ||
|
|
||
| unsigned int GetEnvNumber(const std::string& env_key, unsigned int default_value) { | ||
| std::string env_value = GetEnvVariable(env_key.c_str()); | ||
| bool GetEnvBoolWithAllGetters(const std::string& env_key, bool default_value) { | ||
| return GetBoolFromString(GetEnvVariable(completeEnvGetters, env_key), default_value); | ||
| } | ||
|
|
||
| unsigned int GetEnvNumber(const std::vector<EnvGetterFn>& envGetters, const std::string& env_key, unsigned int default_value) { | ||
ioaniftimesei marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| std::string env_value = GetEnvVariable(envGetters, env_key); | ||
| if (!env_value.empty()) { | ||
| try { | ||
| unsigned int number = std::stoi(env_value); | ||
|
|
@@ -203,26 +212,34 @@ unsigned int GetEnvNumber(const std::string& env_key, unsigned int default_value | |
| return default_value; | ||
| } | ||
|
|
||
| void LoadEnvironment() { | ||
| void LoadEnvironmentFromGetters(const std::vector<EnvGetterFn>& envGetters) { | ||
| auto& logLevelStr = AIKIDO_GLOBAL(log_level_str); | ||
| auto& logLevel = AIKIDO_GLOBAL(log_level); | ||
| if (GetEnvBool("AIKIDO_DEBUG", false)) { | ||
| if (GetEnvBool(envGetters, "AIKIDO_DEBUG", false)) { | ||
| logLevelStr = "DEBUG"; | ||
| logLevel = AIKIDO_LOG_LEVEL_DEBUG; | ||
| } else { | ||
| logLevelStr = GetEnvString("AIKIDO_LOG_LEVEL", "WARN"); | ||
| logLevelStr = GetEnvString(envGetters, "AIKIDO_LOG_LEVEL", "WARN"); | ||
| logLevel = Log::ToLevel(logLevelStr); | ||
| } | ||
|
|
||
| AIKIDO_GLOBAL(blocking) = GetEnvBool("AIKIDO_BLOCK", false) || GetEnvBool("AIKIDO_BLOCKING", false);; | ||
| AIKIDO_GLOBAL(disable) = GetEnvBool("AIKIDO_DISABLE", false); | ||
| AIKIDO_GLOBAL(collect_api_schema) = GetEnvBool("AIKIDO_FEATURE_COLLECT_API_SCHEMA", true); | ||
| AIKIDO_GLOBAL(localhost_allowed_by_default) = GetEnvBool("AIKIDO_LOCALHOST_ALLOWED_BY_DEFAULT", true); | ||
| AIKIDO_GLOBAL(trust_proxy) = GetEnvBool("AIKIDO_TRUST_PROXY", true); | ||
| AIKIDO_GLOBAL(disk_logs) = GetEnvBool("AIKIDO_DISK_LOGS", false); | ||
| AIKIDO_GLOBAL(blocking) = GetEnvBool(envGetters, "AIKIDO_BLOCK", false) || GetEnvBool(envGetters, "AIKIDO_BLOCKING", false); | ||
| AIKIDO_GLOBAL(disable) = GetEnvBool(envGetters, "AIKIDO_DISABLE", false); | ||
| AIKIDO_GLOBAL(collect_api_schema) = GetEnvBool(envGetters,"AIKIDO_FEATURE_COLLECT_API_SCHEMA", true); | ||
| AIKIDO_GLOBAL(localhost_allowed_by_default) = GetEnvBool(envGetters, "AIKIDO_LOCALHOST_ALLOWED_BY_DEFAULT", true); | ||
| AIKIDO_GLOBAL(trust_proxy) = GetEnvBool(envGetters, "AIKIDO_TRUST_PROXY", true); | ||
| AIKIDO_GLOBAL(disk_logs) = GetEnvBool(envGetters, "AIKIDO_DISK_LOGS", false); | ||
| AIKIDO_GLOBAL(sapi_name) = sapi_module.name; | ||
| AIKIDO_GLOBAL(token) = GetEnvString("AIKIDO_TOKEN", ""); | ||
| AIKIDO_GLOBAL(endpoint) = GetEnvString("AIKIDO_ENDPOINT", "https://guard.aikido.dev/"); | ||
| AIKIDO_GLOBAL(config_endpoint) = GetEnvString("AIKIDO_REALTIME_ENDPOINT", "https://runtime.aikido.dev/"); | ||
| AIKIDO_GLOBAL(report_stats_interval_to_agent) = GetEnvNumber("AIKIDO_REPORT_STATS_INTERVAL", 100); | ||
| } | ||
| AIKIDO_GLOBAL(token) = GetEnvString(envGetters, "AIKIDO_TOKEN", ""); | ||
| AIKIDO_GLOBAL(endpoint) = GetEnvString(envGetters, "AIKIDO_ENDPOINT", "https://guard.aikido.dev/"); | ||
| AIKIDO_GLOBAL(config_endpoint) = GetEnvString(envGetters, "AIKIDO_REALTIME_ENDPOINT", "https://runtime.aikido.dev/"); | ||
| AIKIDO_GLOBAL(report_stats_interval_to_agent) = GetEnvNumber(envGetters, "AIKIDO_REPORT_STATS_INTERVAL", 100); | ||
| } | ||
|
|
||
| void LoadEnvironment() { | ||
| LoadEnvironmentFromGetters(completeEnvGetters); | ||
| } | ||
|
|
||
| void LoadSystemEnvironment() { | ||
| LoadEnvironmentFromGetters(systemEnvGetters); | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -207,6 +207,14 @@ bool RequestProcessorInstance::ReportStats() { | |
| bool RequestProcessorInstance::RequestInit() { | ||
| std::string sapiName = sapi_module.name; | ||
|
|
||
| if (sapiName == "frankenphp") { | ||
| if (GetEnvBoolWithAllGetters("FRANKENPHP_WORKER", false)) { | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Calling GetEnvBoolWithAllGetters("FRANKENPHP_WORKER", ...) here can dereference AIKIDO_GLOBAL(server) via GetFrankenEnvVariable before server is initialized; guard AIKIDO_GLOBAL(server) or delay this check. Details✨ AI Reasoning 🔧 How do I fix it? Reply |
||
| AIKIDO_GLOBAL(isWorkerMode) = true; | ||
| AIKIDO_LOG_INFO("FrankenPHP worker warm-up request detected, skipping RequestInit\n"); | ||
| return true; | ||
| } | ||
| } | ||
|
|
||
| if (sapiName == "apache2handler" || sapiName == "frankenphp") { | ||
| // Apache-mod-php and FrankenPHP can serve multiple sites per process | ||
| // We need to reload config each request to detect token changes | ||
|
|
@@ -222,14 +230,6 @@ bool RequestProcessorInstance::RequestInit() { | |
| } | ||
| } | ||
|
|
||
| if (sapiName == "frankenphp") { | ||
| if (GetEnvBool("FRANKENPHP_WORKER", false)) { | ||
| AIKIDO_GLOBAL(isWorkerMode) = true; | ||
| AIKIDO_LOG_INFO("FrankenPHP worker warm-up request detected, skipping RequestInit\n"); | ||
| return true; | ||
| } | ||
| } | ||
|
|
||
| // Initialize the request processor only once(lazy) during RINIT because php-fpm forks the main process | ||
| // for workers and we need to load the library after the worker process is forked because | ||
| // the request processor is a go library that brings in the Go runtime, which (once initialized) | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,5 +1,5 @@ | ||
| Name: aikido-php-firewall | ||
| Version: 1.5.1 | ||
| Version: 1.5.2 | ||
| Release: 1 | ||
| Summary: Aikido PHP Extension | ||
| License: GPL | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.