EGA-archive / LocalEGA

A federated storage for sensitive data
http://localega.readthedocs.io
Apache License 2.0
7 stars 16 forks source link

POSIX permissions #48

Closed silverdaz closed 5 years ago

silverdaz commented 5 years ago

Resolving #45 and #46

Describe the pull request:

Pull request long description:

Changes made:

Related issues:

45 and #46

Additional information:

It is worth solving #45 in another way and use the different driver options available from docker. The local driver can apparently use the same options as the mount command. So I tried gid=lega,setgid=lega,noexec,nodev before reverting to the simpler solution above. Worth noting though.

Mentions:

@blankdots @viklund

silverdaz commented 5 years ago

Would be nice to have that test in Travis, but I assume this could also be work for another PR, what do you say @silverdaz ?

Sure. I'll run the same, locally, first

we had a discussion about this last year and i think i mentioned maybe we should take a look at how OpenShift solves this "User" permissions problem as I think they might have something we can reuse (or i it just seems like that to me).

Yes. Sounds good to me. Not in that PR though. I'll address the test above and we can make another PR. Would you mind create an issue for that? As a note, I don't think it is worth spending a lot of effort on that, it's just Travis and test/environment development, and there is a simple trick for it. But it's important that the deployment teams have a good and clear picture on the topic. Well, ... those not using the S3 backend.

silverdaz commented 5 years ago

Hmmm.... I don't manage to reproduce the error.

So, let's see what differs from your environment with mine: 1) I have removed all the egarchive/lega-* images 2) I have re-build the egarchive/lega-base locally, (make -C images) 3) I bootstrapped (as above) 4) make up went on (forever) to fetch the external (:latest) images, and 5) I waited a bit for the ega keyserver to be ready. 6) I ran the simple.bats test and it passed.

Is it possible that my assumption of mounting the volume from the host into the container (using the lega user) only worked on OSX (thanks to the emulator), and that it is different on Linux?

Have you rebuild the image (step 2)? If not, that's the error, you would have pulled the old codebase in it, instead of the updates in that PR.

As you suggested, we can of course try on Travis. But I thought I would be able to reproduce the error locally first. I suspect that it is something else. Let me think more about it, and try some more.

blankdots commented 5 years ago

Ran the same steps that you ran, still the same issue, or maybe "I am holding it wrong". Only thing that actually helped was removing the user: lega from the compose file :)

I am not sure what causes it, but I know I mentioned before I had issue with that user in docker, that is why I used gosu, or you can take a look at: https://github.com/ncopa/su-exec it is lighter.

I assume to test on travis all you would need is another stage such as:

- stage: tests
      name: "Integration Tests POSIX"
      if: type = pull_request
      services: docker
      before_script:
        - pip3 install -r requirements.txt
        - git clone https://github.com/bats-core/bats-core.git
        - pushd bats-core && git checkout v1.1.0 && sudo ./install.sh /usr/local && popd
        - cd deploy
        - make -C images
        - make prepare
        - make bootstrap ARGS="--keyserver ega --archive-backend posix"
        - sudo chown -R travis private
        - make up OMIT="--scale res=0"
        - make preflight-check
      script:
        - cd ../tests
        - bats integration

or am I missing something ?

silverdaz commented 5 years ago

Ran the same steps that you ran, still the same issue, or maybe "I am holding it wrong". Only thing that actually helped was removing the user: lega from the compose file :)

It's actually a security issue, so let me think about it a bit more.

I am not sure what causes it, but I know I mentioned before I had issue with that user in docker, that is why I used gosu, or you can take a look at: https://github.com/ncopa/su-exec it is lighter.

I know already about gosu and su-exec

And yes, thanks for the travis snippet, I had figured it out already.

I'm closing the PR one more time, to take time to think about it and to remove confusion in the team.

silverdaz commented 5 years ago

Apparently, force-pushing on a closed PR forces us to create a new one, without being able to re-open that one.

So... head to PR #55