Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Docker setup for pyrdp #66

Merged
merged 13 commits into from
Feb 1, 2019
Merged

Docker setup for pyrdp #66

merged 13 commits into from
Feb 1, 2019

Conversation

Bromulux
Copy link

No description provided.

Etienne Lacroix added 4 commits January 14, 2019 11:10
The certificate cloner and the Man-in-the-middle features are both working in the docker.
Pyrdp-player is now working in a container
pyRDP can now be use with docker compose
The user in the container is no longer "root", but "developer". This simplifies the use of the pyrdp-player because the "xhost +local:docker" command is no longer required.
Copy link
Member

@obilodeau obilodeau left a comment

Choose a reason for hiding this comment

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

Good but some more work required. For the documentation, I guess I wasn't clear: I would like the README to cover how to use docker (not docker-compose) and the docker-compose is more as an example (no need to talk about it in the README).

Dockerfile Outdated Show resolved Hide resolved
Dockerfile Outdated Show resolved Hide resolved
README.md Show resolved Hide resolved
docker-compose.yml Outdated Show resolved Hide resolved
docker-compose.yml Outdated Show resolved Hide resolved
General improvements have been made in the documentation and some points have been clarified
Dockerfile Show resolved Hide resolved
Etienne Lacroix and others added 3 commits January 21, 2019 14:56
The Dockerfile now takes advantage of caching. Plus, some typos have been corrected in the doc. The user in the container is now "pyrdp" instead of "developer" to simplify.
Copy link
Member

@obilodeau obilodeau left a comment

Choose a reason for hiding this comment

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

Another round of review done. There are only very small changes to do left. The documentation is very good by the way 👍

Dockerfile Outdated Show resolved Hide resolved
Dockerfile Show resolved Hide resolved
Dockerfile Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
docker-compose.yml Show resolved Hide resolved
Etienne Lacroix added 3 commits January 23, 2019 11:36
The assignment of QT_X11_NO_MITSHM is now done with the run command and explained in the README. This prevents the user from modifying the Dockerfile.
QT_X11_NO_MITSHM=1 added to the environment section
sudo was removed from all the docker commands, QT_X11_NO_MITSHM=1 was added to the run options and some minor typos were corrected in the README
Copy link
Member

@obilodeau obilodeau left a comment

Choose a reason for hiding this comment

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

LGTM!

README.md Outdated Show resolved Hide resolved
-The run command example in the README is more explicit. 
-The build command now uses the image name.
@obilodeau
Copy link
Member

@xshill can you take another look when you have a minute?

Corrections of the README so the installation commands are more specific.
@xshill xshill merged commit 67a2d2e into GoSecure:master Feb 1, 2019
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.

None yet

3 participants