-
Notifications
You must be signed in to change notification settings - Fork 0
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
Bellon/getjerry customize branch test #2
base: master
Are you sure you want to change the base?
Changes from 70 commits
8bde676
43bd780
324f0b6
682e4e8
ff74a64
43749bb
e60a25e
f7ff383
b96e23c
42a3469
aaee599
0ad3169
1591e4d
34ba83f
b436eb6
218c791
331f64d
e5b19ac
f1a84c3
110ad03
fe01adb
b08be7b
c469855
9cbd369
70f1c43
d0aa19e
f19e14a
0673161
c82e24b
52f3ce3
32584cf
64a546a
6243f1b
e53eacb
5ed7200
c1b8f31
a7c5846
7b8fcc1
2b78518
3dd2d9f
da66a09
d7cb5ed
66afafa
afc53fa
92203c3
9fea405
3f43210
3042a9d
62fc791
ac6eb1d
cadfb69
5f4bf2b
17e9885
b849028
c835aac
5941d5a
5e0861e
75a9d6b
0bf6d70
ad1d54c
ef99a90
8cb996a
7bb2d32
1eeea19
d02a3f9
268f1ef
804ba45
fea0992
6a706eb
cd28a98
1064507
fb99077
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 | ||||
---|---|---|---|---|---|---|
|
@@ -72,7 +72,7 @@ VOLUME /data | |||||
|
||||||
WORKDIR /usr/share/bunkerweb/autoconf | ||||||
|
||||||
USER autoconf:autoconf | ||||||
USER root | ||||||
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. Critical: Running container as root poses security risks. Running the container as root significantly increases security risks and violates container security best practices. This change:
Consider reverting to non-root user: -USER root
+USER autoconf:autoconf If root access is absolutely necessary for specific operations, consider:
📝 Committable suggestion
Suggested change
|
||||||
|
||||||
HEALTHCHECK --interval=10s --timeout=10s --start-period=60s --retries=6 CMD /usr/share/bunkerweb/helpers/healthcheck-autoconf.sh | ||||||
|
||||||
|
Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
@@ -15,13 +15,22 @@ | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
from SwarmController import SwarmController | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
from IngressController import IngressController | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
from DockerController import DockerController | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
import uuid | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
from kubernetes import config | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
from kubernetes.leaderelection import leaderelection | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
from kubernetes.leaderelection.resourcelock.configmaplock import ConfigMapLock | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
from kubernetes.leaderelection import electionconfig | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
# Get variables | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
logger = setup_logger("Autoconf", getenv("LOG_LEVEL", "INFO")) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
swarm = getenv("SWARM_MODE", "no").lower() == "yes" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
kubernetes = getenv("KUBERNETES_MODE", "no").lower() == "yes" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
docker_host = getenv("DOCKER_HOST", "unix:///var/run/docker.sock") | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
wait_retry_interval = getenv("WAIT_RETRY_INTERVAL", "5") | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
namespace = getenv("NAMESPACE", "default") | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
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. 🛠️ Refactor suggestion Add validation for the namespace environment variable. The namespace value is used in leader election configuration but lacks validation. Empty or invalid namespace values could cause runtime errors. -namespace = getenv("NAMESPACE", "default")
+namespace = getenv("NAMESPACE", "default")
+if not namespace:
+ logger.error("Invalid NAMESPACE value, must not be empty")
+ _exit(1) 📝 Committable suggestion
Suggested change
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
# Authenticate using config file | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
config.load_incluster_config() | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Comment on lines
+32
to
+33
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. 🛠️ Refactor suggestion Move Kubernetes config loading into Kubernetes-specific code path The Kubernetes config loading is currently done globally, which could cause issues in non-Kubernetes modes. Consider moving it into the Kubernetes-specific code path. -# Authenticate using config file
-config.load_incluster_config()
+if kubernetes:
+ # Authenticate using config file
+ config.load_incluster_config()
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
if not wait_retry_interval.isdigit(): | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
logger.error("Invalid WAIT_RETRY_INTERVAL value, must be an integer") | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
@@ -38,19 +47,36 @@ def exit_handler(signum, frame): | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
signal(SIGINT, exit_handler) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
signal(SIGTERM, exit_handler) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
try: | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
# Instantiate the controller | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
if swarm: | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
logger.info("Swarm mode detected") | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
controller = SwarmController(docker_host) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
elif kubernetes: | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
logger.info("Kubernetes mode detected") | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
controller = IngressController() | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
else: | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
logger.info("Docker mode detected") | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
controller = DockerController(docker_host) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
# Wait for instances | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
def run_on_kubernetes_ha_mode(controller): | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
candidate_id = f'auto-{uuid.uuid4()}' | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
lock_name = "autoconfig" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
lock_namespace = namespace | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
config = electionconfig.Config( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
ConfigMapLock(lock_name, lock_namespace, candidate_id), | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
lease_duration=17, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
renew_deadline=15, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
retry_period=5, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
onstarted_leading=kubernetes_start(controller, candidate_id), | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
onstopped_leading=onstopped_leading(candidate_id)) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
logger.info(f'I am {candidate_id}') | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
# Enter leader election | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
leaderelection.LeaderElection(config).run() | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
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. 🛠️ Refactor suggestion Improve leader election configuration. The current implementation has several issues:
def run_on_kubernetes_ha_mode(controller):
candidate_id = f'auto-{uuid.uuid4()}'
- lock_name = "autoconfig"
+ lock_name = f"autoconfig-{getenv('DEPLOYMENT_NAME', 'default')}"
lock_namespace = namespace
+ lease_duration = int(getenv('LEASE_DURATION', '17'))
+ renew_deadline = int(getenv('RENEW_DEADLINE', '15'))
+ retry_period = int(getenv('RETRY_PERIOD', '5'))
config = electionconfig.Config(
ConfigMapLock(lock_name, lock_namespace, candidate_id),
- lease_duration=17,
- renew_deadline=15,
- retry_period=5,
+ lease_duration=lease_duration,
+ renew_deadline=renew_deadline,
+ retry_period=retry_period,
onstarted_leading=kubernetes_start(controller, candidate_id),
onstopped_leading=onstopped_leading(candidate_id)) 📝 Committable suggestion
Suggested change
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
def onstopped_leading(candidate_id): | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
logger.info(f'{candidate_id} is follower') | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
def kubernetes_start(controller, candidate_id): | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
logger.info(f'{candidate_id} is leader') | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
start(controller=controller) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
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. 🛠️ Refactor suggestion Improve error handling and callback implementation. The callback functions need improvements:
def onstopped_leading(candidate_id):
- logger.info(f'{candidate_id} is follower')
+ def callback():
+ logger.info(f'{candidate_id} is follower')
+ return callback
def kubernetes_start(controller, candidate_id):
- logger.info(f'{candidate_id} is leader')
- start(controller=controller)
+ def callback():
+ try:
+ logger.info(f'{candidate_id} is leader')
+ start(controller=controller)
+ except Exception as e:
+ logger.error(f"Error in leader callback: {e}")
+ logger.debug(format_exc())
+ return callback 📝 Committable suggestion
Suggested change
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
def start(controller): | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
import time | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
time.sleep(60000) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
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. Remove or justify the excessive sleep duration. The code includes an unexplained sleep of ~16.7 hours (60000 seconds) which could cause significant service startup delays. If a delay is necessary:
def start(controller):
- import time
- time.sleep(60000)
+ startup_delay = int(getenv('STARTUP_DELAY_SECONDS', '0'))
+ if startup_delay > 0:
+ logger.info(f"Waiting {startup_delay} seconds before startup...")
+ time.sleep(startup_delay) 📝 Committable suggestion
Suggested change
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
logger.info("Waiting for BunkerWeb instances ...") | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
instances = controller.wait(wait_retry_interval) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
logger.info("BunkerWeb instances are ready 🚀") | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
@@ -65,6 +91,25 @@ def exit_handler(signum, frame): | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Path(sep, "var", "tmp", "bunkerweb", "autoconf.healthy").write_text("ok") | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
logger.info("Processing events ...") | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
controller.process_events() | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
try: | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
# Instantiate the controller | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
if swarm: | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
logger.info("Swarm mode detected") | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
controller = SwarmController(docker_host) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
elif kubernetes: | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
logger.info("Kubernetes mode detected") | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
controller = IngressController() | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
else: | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
logger.info("Docker mode detected") | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
controller = DockerController(docker_host) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
if kubernetes: | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
run_on_kubernetes_ha_mode(controller=controller) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
else: | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
start(controller=controller) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
except: | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
logger.error(f"Exception while running autoconf :\n{format_exc()}") | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
sys_exit(1) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,9 @@ | ||
{% if USE_MODSECURITY == "yes" and MODSECURITY_CRS_VERSION == "3" and HTTP3 == "yes" +%} | ||
SecAction \ | ||
"id:900230,\ | ||
phase:1,\ | ||
nolog,\ | ||
pass,\ | ||
t:none,\ | ||
setvar:'tx.allowed_http_versions=HTTP/1.0 HTTP/1.1 HTTP/2 HTTP/2.0 HTTP/3 HTTP/3.0'" | ||
{% endif %} |
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
@@ -0,0 +1,4 @@ | ||||||
{% if USE_MODSECURITY == "yes" +%} | ||||||
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. Fix template syntax error The template syntax -{% if USE_MODSECURITY == "yes" +%}
+{% if USE_MODSECURITY == "yes" %} 📝 Committable suggestion
Suggested change
|
||||||
modsecurity on; | ||||||
modsecurity_rules_file /etc/nginx/http-modsecurity/modsecurity-rules.conf.modsec; | ||||||
{% endif %} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🟠 Code Vulnerability
last user should not be root (...read more)
Do not use
root
as the last user because your container runs with theroot
user. Always use a user with lower privileges.