Skip to content
This repository has been archived by the owner on Nov 16, 2022. It is now read-only.

KEYCLOAK-13974 Dockerfile cleanup #172

Merged
merged 1 commit into from
May 4, 2020

Conversation

slaskawi
Copy link
Contributor

@slaskawi slaskawi commented Apr 20, 2020

KEYCLOAK-13974

A long while ago, we discussed permissions issue in our Keycloak Container (link1link2). The outcome of the discussion is that a binary just needs to be owned by a root group and we don't really need any extra user.

I'm not sure if this is the best moment, but perhaps we should do a permission clean up in our Dockerfile here as well? WDYT @stianst @abstractj @douglaspalmer @iankko @davidffrench ? If you think that's a good idea, I'll convert it into a proper Pull Request.

The image might be tested using: slaskawi/keycloak-operator:cleanup

@coveralls
Copy link

coveralls commented Apr 20, 2020

Coverage Status

Coverage remained the same at 41.176% when pulling bae5f1a on slaskawi:dockerfile_cleanup into bae3b1e on keycloak:master.

@iankko
Copy link

iankko commented Apr 22, 2020

@slaskawi

This is just a Preview PR to check if we want this change or not.

A long while ago, we discussed permissions issue in our Keycloak Container (link1link2). The outcome of the discussion is that a binary just needs to be owned by a root group and we don't really need any extra user.

I'm not sure if this is the best moment, but perhaps we should do a permission clean up in our Dockerfile here as well? WDYT @stianst @abstractj @douglaspalmer @iankko @davidffrench ? If you think that's a good idea, I'll convert it into a proper Pull Request.

+1 on switching this preview PR into a real one (it's even fixing a CVE, see this article for details).

Unsure if the Keycloak operator needs to run as privileged container already, or not (in order to be apply clusterwide settings). In any case weaking /etc/passwd permissions is discouraged in that article as dangerous, so better to avoid it IMHO

@slaskawi slaskawi changed the title Dockerfile cleanup KEYCLOAK-13974 Dockerfile cleanup Apr 27, 2020
@slaskawi
Copy link
Contributor Author

Converting this into a "real" Pull Request.

@slaskawi slaskawi marked this pull request as ready for review April 27, 2020 08:33
@slaskawi slaskawi added this to the 9.0.3 milestone Apr 27, 2020
Signed-off-by: Sebastian Laskawiec <slaskawi@redhat.com>
@slaskawi
Copy link
Contributor Author

Just in case, here's the build: https://travis-ci.org/github/keycloak/keycloak-operator/builds/680493735

@slaskawi
Copy link
Contributor Author

@stianst @abstractj @hmlnarik @mhajas @iankko @pb82 @davidffrench May I ask for a review? This should be a quick one.

Copy link
Contributor

@mhajas mhajas left a comment

Choose a reason for hiding this comment

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

Thanks for the PR @slaskawi! I tried slaskawi/keycloak-operator:cleanup and it works as expected.

Copy link
Contributor

@hmlnarik hmlnarik left a comment

Choose a reason for hiding this comment

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

Approving based on @mhajas 's approval

@slaskawi slaskawi merged commit 97045dc into keycloak:master May 4, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants