openfaas / faas-swarm

OpenFaaS provider for Docker Swarm
https://github.com/openfaas/faas
MIT License
79 stars 38 forks source link

Set ReadOnly during function create and update #26

Closed LucasRoesler closed 6 years ago

LucasRoesler commented 6 years ago

Description

Motivation and Context

Closed openfaas/faas#590

How Has This Been Tested?

Getting this posted so that others can check it out, I will be testing it soon with its companion PR for the faas-cli openfaas/faas-cli#439

Types of changes

Checklist:

LucasRoesler commented 6 years ago

It occurs to me that I did not add writable mount for /tmp when the ReadOnlyRootFilesystem is true. I assume that tmpfs is probably the most appropriate, perhaps someone with more experience or an opinion on Swarm can let me know?

    if request.ReadOnlyRootFilesystem {
        spec.TaskTemplate.ContainerSpec.Mounts = []mount.Mount{
            {
                Type:   mount.TypeTmpfs,
                Target: "/tmp",
            },
        }
    }

Per the docs https://docs.docker.com/storage/tmpfs/#specify-tmpfs-options, this should create a world writable folder with unlimited size. Perhaps we need an additional flag to allow a max size to be set?

Templum commented 6 years ago

@LucasRoesler Regarding an additional flag for max size. As far as I know for the K8S implementation we use the emptyDir and I'm not aware of a way how to limit an emptyDir so this could cause for differences in the behavior.

alexellis commented 6 years ago

Lucas thank you for your work co-coordinating this.

I saw this comment and wondered if it was from before we talked about prune?

I am not sure why so much of it is vendored, but it is what dep ensure created

Is prune enabled?

Thanks,

Alex

LucasRoesler commented 6 years ago

Just checked, prune is not enabled. I will create a PR for that and then rebase once it is merged.

alexellis commented 6 years ago

Hi @LucasRoesler I'm now seeing "conflicting files" such as:

vendor/github.com/openfaas/faas/.DEREK.yml
vendor/github.com/openfaas/faas/.travis.yml
vendor/github.com/openfaas/faas/BACKERS.md
vendor/github.com/openfaas/faas/CONTRIBUTING.md
vendor/github.com/openfaas/faas/DEV.md
vendor/github.com/openfaas/faas/README.md
vendor/github.com/openfaas/faas/TestDrive.md
vendor/github.com/openfaas/faas/api-docs/swagger.yml
vendor/github.com/openfaas/faas/build.sh
vendor/github.com/openfaas/faas/community.md
vendor/github.com/openfaas/faas/contrib/HACK.md
vendor/github.com/openfaas/faas/contrib/ci.sh
vendor/github.com/openfaas/faas/deploy_stack.sh
vendor/github.com/openfaas/faas/docker-compose.armhf.yml
vendor/github.com/openfaas/faas/docker-compose.yml
vendor/github.com/openfaas/faas/docs/README.md
vendor/github.com/openfaas/faas/gateway/Dockerfile
vendor/github.com/openfaas/faas/gateway/Dockerfile.arm64
vendor/github.com/openfaas/faas/gateway/Dockerfile.armhf
vendor/github.com/openfaas/faas/gateway/Gopkg.lock

This is probably due to the PR I just merged ( #27 )

Alex

LucasRoesler commented 6 years ago

I have rebased and I have bumped the version of faas again

alexellis commented 6 years ago

Merged, thanks @LucasRoesler for working on this.