Skip to content

Conversation

@MikolajKasprzak
Copy link
Contributor

@MikolajKasprzak MikolajKasprzak commented Oct 21, 2025

📝 Description

This ADR was created to address a problems with complexity and permissions problems in manager app due to using Monolithic container with Apache + Django.

✨ Type of Change

Select the type of change your PR introduces:

  • 🐞 Bug fix – Non-breaking change which fixes an issue
  • 🚀 New feature – Non-breaking change which adds functionality
  • 🔨 Refactor – Non-breaking change which refactors the code base
  • 💥 Breaking change – Changes that break existing functionality
  • 📚 Documentation update
  • 🔒 Security update
  • 🧪 Tests
  • 🚂 CI

🧪 Testing Scenarios

Describe how the changes were tested and how reviewers can test them too:

  • ✅ Tested manually
  • 🤖 Ran automated end-to-end tests

✅ Checklist

Before submitting the PR, ensure the following:

  • 🔍 PR title is clear and descriptive
  • 📝 For internal contributors: If applicable, include the JIRA ticket number (e.g., ITEP-123456) in the PR title. Do not include full URLs
  • 💬 I have commented my code, especially in hard-to-understand areas
  • 📄 I have made corresponding changes to the documentation
  • ✅ I have added tests that prove my fix is effective or my feature works

Signed-off-by: Kasprzak, Mikolaj <[email protected]>

- **Author(s)**: [Mikolaj Kasprzak](https://github.com/MikolajKasprzak)
- **Date**: 2025-10-21
- **Status**: Proposed
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Don't forget to change status to Accepted before merge

Copy link
Contributor

@jdanieck jdanieck left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@MikolajKasprzak @saratpoluri @ltalarcz Please take a look at my comments. While I am not blocking this PR since everyone else approves, I am advising against accepting the ADR as-is. I suggest we prioritize tech-debt cleanup first and continue refining the ADR, focusing solely on the separation between the HTTP server and the Manager service. I’m asking you to reconsider the approach.


### Key Problems:

1. **Security vulnerabilities**: Apache required root privileges for initialization, creating unnecessary attack surface
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Apache does not need to run as root inside a container. By using a non-privileged port and writable PID/log paths, the container can run entirely as non-root (daemon) while still serving requests and loading all modules.

Switching to NGINX would face the same issue: the master process normally starts as root to bind to port 80 and write the PID file, then drops privileges to the nginx user. Non-root operation is possible if you use a non-privileged port (e.g., 8080) and writable directories.

Example non-root Apache Dockerfile

FROM httpd:2.4

COPY ./index.html /usr/local/apache2/htdocs/index.html

# Change Apache to listen on a non-root port
RUN sed -i 's/^Listen 80/Listen 8080/' /usr/local/apache2/conf/httpd.conf

# Add PidFile directive to use /tmp and make logs directory writable by daemon user
RUN echo "PidFile /tmp/httpd.pid" >> /usr/local/apache2/conf/httpd.conf \
    && chown -R daemon:daemon /usr/local/apache2/logs

# Switch to non-root user after all configuration changes
USER daemon
EXPOSE 8080

CMD ["httpd-foreground", "-DFOREGROUND", "-f", "/usr/local/apache2/conf/httpd.conf"]

Testing

Example index.html:

$ cat index.html 
hello world!

The run.sh convenience test script:

#!/bin/bash

docker build . -t non-root-apache
if [ "$(docker ps -aq -f name=non-root-apache)" ]; then
  docker rm -f non-root-apache >/dev/null 2>&1 || true
fi
docker run -dit --name non-root-apache -p 8080:8080 non-root-apache
docker logs non-root-apache 
curl localhost:8080
ps aux | grep httpd

Result:

$ ./run.sh 
[+] Building 0.0s (9/9) FINISHED                                                                                                                                                                                                                                                                                   docker:default
 => [internal] load build definition from Dockerfile                                                                                                                                                                                                                                                                         0.0s
 => => transferring dockerfile: 666B                                                                                                                                                                                                                                                                                         0.0s
 => [internal] load metadata for docker.io/library/httpd:2.4                                                                                                                                                                                                                                                                 0.0s
 => [internal] load .dockerignore                                                                                                                                                                                                                                                                                            0.0s
 => => transferring context: 2B                                                                                                                                                                                                                                                                                              0.0s
 => [internal] load build context                                                                                                                                                                                                                                                                                            0.0s
 => => transferring context: 31B                                                                                                                                                                                                                                                                                             0.0s
 => [1/4] FROM docker.io/library/httpd:2.4                                                                                                                                                                                                                                                                                   0.0s
 => CACHED [2/4] COPY ./index.html /usr/local/apache2/htdocs/index.html                                                                                                                                                                                                                                                      0.0s
 => CACHED [3/4] RUN sed -i 's/^Listen 80/Listen 8080/' /usr/local/apache2/conf/httpd.conf                                                                                                                                                                                                                                   0.0s
 => CACHED [4/4] RUN echo "PidFile /tmp/httpd.pid" >> /usr/local/apache2/conf/httpd.conf     && chown -R daemon:daemon /usr/local/apache2/logs                                                                                                                                                                               0.0s
 => exporting to image                                                                                                                                                                                                                                                                                                       0.0s
 => => exporting layers                                                                                                                                                                                                                                                                                                      0.0s
 => => writing image sha256:5c40698332da9cf13b01895ae1b4c4380d814c5774f010602a8b6ed67e9039ac                                                                                                                                                                                                                                 0.0s
 => => naming to docker.io/library/non-root-apache                                                                                                                                                                                                                                                                           0.0s
e9a5b9bb2eab519b4c62e1bbeaaf494f09bf44f0bd58b35703dcd43e5854c8cf

AH00558: httpd: Could not reliably determine the server's fully qualified domain name, using 172.17.0.2. Set the 'ServerName' directive globally to suppress this message
AH00558: httpd: Could not reliably determine the server's fully qualified domain name, using 172.17.0.2. Set the 'ServerName' directive globally to suppress this message
[Thu Oct 23 10:34:35.363237 2025] [mpm_event:notice] [pid 1:tid 1] AH00489: Apache/2.4.65 (Unix) configured -- resuming normal operations
[Thu Oct 23 10:34:35.363348 2025] [core:notice] [pid 1:tid 1] AH00094: Command line: 'httpd -D FOREGROUND -D FOREGROUND -f /usr/local/apache2/conf/httpd.conf'

hello world!

daemon   2837032 23.0  0.0   6212  4480 pts/0    Ss+  12:34   0:00 httpd -DFOREGROUND -DFOREGROUND -f /usr/local/apache2/conf/httpd.conf
daemon   2837239  0.0  0.0 1997396 2600 pts/0    Sl+  12:34   0:00 httpd -DFOREGROUND -DFOREGROUND -f /usr/local/apache2/conf/httpd.conf
daemon   2837240  0.0  0.0 1997452 3240 pts/0    Sl+  12:34   0:00 httpd -DFOREGROUND -DFOREGROUND -f /usr/local/apache2/conf/httpd.conf
daemon   2837244  0.0  0.0 1997396 2600 pts/0    Sl+  12:34   0:00 httpd -DFOREGROUND -DFOREGROUND -f /usr/local/apache2/conf/httpd.conf
jdanieck 2837332  0.0  0.0   6548  1920 pts/1    S+   12:34   0:00 grep httpd

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm fully aware that apache can be run without root permissions. Read it more carefully and read init script and you will see that apache in container is run as non user. Root user is used to configure apache and creating non root user (during init script) for that because apache is installed from apt package.

The problem is that there is no point in creating image similar to official httpd based on ubuntu 22:04 that manager use.
The image is already very big does resemble a monolithic approach rather than following container best practices. Why lost time on creating that image when we can create separate container that use nginx/httpd image?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The immediate goal was to remove the root user for this release with minimal changes, and that’s exactly what I propose — it’s a meaningful security and compliance improvement worth the effort. Broader architectural redesigns can be addressed later at lower-priority. Let’s take this discussion offline as I see it requires more discussion.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Read it more carefully

@MikolajKasprzak , please review the CODE_OF_CONDUCT.md, and I hope you'll apply it in your communication next time.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry if I offended you - no harm was intended. I wanted to clarify that Apache requires root privileges for initialization (in the context of our current solution), whereas @jdanieck's comment suggests he interpreted the documentation as saying Apache needs to run as root.


### New Architecture:

#### Docker Compose Deployment:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please use mermaid diagrams to depict all deployments mentioned in the doc

  • the original deployment (in the context)
  • proposed compose deployment
  • proposed k8s deployments
  • (optionally) alternatives


- **Author(s)**: [Mikolaj Kasprzak](https://github.com/MikolajKasprzak)
- **Date**: 2025-10-21
- **Status**: `Accepted`
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The primary concern is that the Manager service runs as root, which can be fixed without major architectural changes. We should first clean up technical debt - simplify/remove init scripts, fix permissions, and remove legacy dependencies - similar to what was done for the Controller in PR #235.

Long-term separation of the HTTP server and Manager service makes sense, so this ADR should focus solely on that aspect.

- **Pros**: Industry standard, security best practices, clean separation, excellent performance, Kubernetes-native
- **Cons**: Requires container architecture changes, initial migration effort, slightly more complex Pod spec

### Option B: Pure Kubernetes Ingress (Considered)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I’d like us to explore a more Kubernetes-native approach before making a decision — we might even need a POC. What specific challenges are we seeing with WebSocket/MQTT support? There are many Ingress controller implementations, so perhaps it’s just a matter of choosing the right one.

Also, in recent Kubernetes versions, the Gateway API
was introduced as a more advanced alternative to Ingress, offering richer routing capabilities. It might be worth considering for our use case.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can use Gateway IP it provides the same capabilities as old Ingress approach.

I can extend description of this section. We always want to use Kubernetes ingress, we just need second nginx as side container or different approach to host our static files. You shouldn't use Ingress for that.

Challenges with configuring timeouts and production ready config for websockets connections nothing impossible or time consuming.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants