openfaas / faasd

A lightweight & portable faas engine
https://store.openfaas.com/l/serverless-for-everyone-else
MIT License
2.97k stars 213 forks source link

Support request for monitoring with cAdvisor #340

Closed petertjmills closed 11 months ago

petertjmills commented 12 months ago

Due diligence

My actions before raising this issue

The compose-spec provides a read_only flag, for volumes, which would be trivial to implement and useful for running containers from the docker_compose.yaml file.

This is not currently implemented but can be trivially.

Why do you need this?

cAdvisor in their docker instructions use read-only volumes to mount the host systems folder. It's probably a good idea generally to allow containers to read but not write to some files, such as the secrets files. cAdvisor in particular needs access to containerd relevant files and should definitely not be able to write.

Who is this for?

POC for commercial implementation

Expected Behaviour

This should work by following the compose-spec and being able to add read_only to the volume configuration.

    volumes:
      - type: bind
        source: ./example/
        target: /var/lib/readonlydir
        read_only: true

Current Behaviour

This is currently not implemented but in supervisor.go /etc/resolv.conf and /etc/hosts are mounted as read only so it is possible and already works.

List All Possible Solutions and Workarounds

First solution:

Add a readOnly bool to the struct Mount in supervisor.go Set it from this ReadOnly field in ParseCompose(). In Start(), conditionally set the last option in the options array, based on if this flag is set or not; either "ro" or "rw"

Which Solution Do You Recommend?

First Solution

Steps to Reproduce (for bugs)

1. 2. 3. 4.

Your Environment

go version

containerd -version

uname -a

cat /etc/os-release

faasd version
alexellis commented 12 months ago

Hi Peter,

Thanks for your interest.

Let me first call you out here.

This is not currently implemented but can be trivially.

Nothing is trivial - it must be planned, scheduled, worked on by someone, tested, documented and maintained forever.

POC for commercial implementation

Would you like to talk to us about commercial support / consulting for faasd?

cAdvisor in their docker instructions use read-only volumes to mount the host systems folder

If it works with writable files/mounts, why do you need this? It's not clear other than you say "probably should".

Why do you need this?

You've not explained why you need cAdvisor and what you need from it. Are you trying to monitor faasd installations for disk space / free RAM?

Why do you want this to run as service in the faasd compose file? Take node-exporter for instance, they say not to run it in a container. You can run cadvisor with systemd directly on your host.

alexellis commented 12 months ago

/set title: Support request for monitoring with cAdvisor

petertjmills commented 12 months ago

Hi Alex,

Thank you for your fast response to this.

Apologies, I only mean it can be implemented trivially, as I have forked and implemented this with little prior Go experience. The bulk of the work is done by compose-go library, as the flag is already implemented there, it's just not used by faasd. I understand documentation and maintenance may also be required, I don't dispute that this isn't always trivial. I have raised this issue prior to a PR as per the contribution docs.

I probably should have put more time into phrasing it better. My use case right now, is to use cAdvisor, who only provide installation instructions through running as a container, afaik it isn't suggested anywhere that it can or should be installed on the host unlike node-exporter, therefore it would be ideal to run as a service container. I'd like to use it for metrics about other service containers, such as postgres or redis.

However this issue wasn't requesting cAdvisor support, it is just an example of a read-only volume use case. It is good security practice to have read-only volumes mounted to containers where possible as it reduces the attack surface of applications and can prevent containers from accidently corrupting or overwriting important files on the host filesystem. cAdvisor is a good example here as it mounts the root directory. If this is mounted read only a cAdvisor crash definitely can't do any harm to the host.

But the same could be said of any number of different containers you may want to use with faasd at a service level. For example if I wanted to mount a folder with a prometheus.yaml config, there isn't any reason that the prometheus container needs write access to that. Similarly with secrets, such as basic-auth-password, it's not a good idea to give write access to containers that shouldn't need it. All for the same reasons it's not a good idea to give all user accounts permissions to write to every folder.

I feel this is more of a security feature request rather than support question. Read only volumes are seen as best practice and used everywhere in the docker world and would be amazing to see support for them here.

I hope I've cleared up the use case for read only volumes a little. I have a PR ready to go if this sounds like a good idea.

Thanks

alexellis commented 11 months ago

I looked at your changes, and we can take a PR, if you also detail testing steps taken. But it doesn't sound like there's any specific thing that you can't do at the moment, so is this change more of a "nice to have"?

What more can you say about this commercial project for faasd?

petertjmills commented 11 months ago

I'll add a PR, thanks.

I partially agree. It is a nice to have, but many container platforms implement this as standard. I do feel it's more of a security consideration, and it's also relevant in many existing container projects as shown with this search. I haven't checked all ~9k results but it seems the vast majority of them are instances where projects are documenting the use of read only volumes.

The project is B2B faas on low cost hardware. Still POC stage.

alexellis commented 11 months ago

Let us know how you get on with the new release - https://github.com/openfaas/faasd/releases/tag/0.18.1