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

Improve dev tooling #1305

Merged
merged 22 commits into from
Aug 4, 2021
Merged

Improve dev tooling #1305

merged 22 commits into from
Aug 4, 2021

Conversation

johrstrom
Copy link
Contributor

@johrstrom johrstrom commented Jul 26, 2021

I went to make my development image on my personal machine over the weekend and found that what I ran at work was a far to much of a snowflake to replicate. So I put work into the dev namespace build/start/stop development containers.

Here's the progress made. Note that the develoment.md should explain things.

  • creates a user with the same name & uid:gid inside the container
  • documentation
  • prompts to set a password
  • podman support
  • docker support
  • only build a container if there isn't ood-dev:latest already (but can rebuild through a parameter)
  • get rid of the mock connector. Even if ktrout user doesn't exist, there could be something there that makes it less secure.

@johrstrom johrstrom marked this pull request as ready for review July 27, 2021 02:04
@johrstrom
Copy link
Contributor Author

I have to work on docker support to pass -u & -g flags but I think this is ready for review.

fix podman
add dev:logs task
simplify config mount point
Copy link
Contributor

@msquee msquee left a comment

Choose a reason for hiding this comment

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

@johrstrom This is awesome, I pulled down this PR with GitPod and really had no issues booting the container.

Once support is added for passing -u and -g to Docker then I will push a PR GitPod support!

I noticed that when using the Mock Connector it uses the default username kilgore (Dex's default mock user) and I had to rake dev:exec into the container and run:

useradd kilgore
/opt/ood/nginx_stage/sbin/nginx_stage pun --user kilgore

@johrstrom
Copy link
Contributor Author

Yea it's like a much cooler version of our user ood. I can login as Kilgore Trout if I create the user. Actually by not having the user is more secure, given you can just click through. We need to get rid of that connector.

PRs welcome on the docker support.

Comment on lines 78 to 80
extra = ["--build-arg", "USER=#{user.name}"]
extra.concat ["--build-arg", "UID=#{user.uid}"]
extra.concat ["--build-arg", "GID=#{user.uid}"]
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@msquee do we need something special here for gitpod?

DEVELOPMENT.md Outdated Show resolved Hide resolved
plain_password = $stdin.noecho(&:gets).chomp
bcrypted = BCrypt::Password.create(plain_password)

content = <<~CONTENT
Copy link
Contributor

Choose a reason for hiding this comment

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

I wonder if we should make it possible to define a Dex user via ood-portal-generator so you don't have to write out a second file for the Dex local user. I believe right now we hardcode in a default local user, which I think was just not thinking people or even developers might want to choose a different user besides the default we provide

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think we need a way to provide development containers with such a feature. Enabling it in ood-portal-generator - I'm not sure. I've been told someone kept that ood user around during the basic auth days and it went sideways for them. I mean they accidentally kept it and someone else started using it.

I think there could be a discourse topic where someone wanted to do something similar for a proof of concept deployment. So maybe we should provide a way to generate a single user with a new password. For proof of concept deployments and this container.

Copy link
Contributor

Choose a reason for hiding this comment

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

I think the biggest argument for using ood-portal-generator for generating the user is we already manage the Dex YAML config with ood-portal-generator so having ood-portal-generator make the config and then modifying it later without ood-portal-generator seems messy and will require subsequent executions of ood-portal-generator to require the --force flag. I think taking the work here for generating a bcrypt password and supporting something similar with ood-portal-generator shouldn't be too big a change and pretty easy to maintain backwards compatibility.

cd $OOD_DEV_DIR
ln -s $APP_DEV_DIR gateway

/opt/ood/ood-portal-generator/sbin/update_ood_portal
/opt/ood/ood-portal-generator/sbin/update_ood_portal --force

if [ -n "$OOD_STATIC_USER" ] && [ -f "$OOD_STATIC_USER" ]; then
Copy link
Contributor

Choose a reason for hiding this comment

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

If we could define a custom static user with ood-portal-generator, this extra logic could be removed. It's not great in my opinion to have this file managed by ood-portal-generator but then due to lack of flexibility, we completely overwrite it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think I added this --force here because we mount in /opt/etc/ood/config so the sha file doesn't exist the very first time you run.

--force may actually suite this use case though. We persist the ood_portal.yml on the host file system, mounted into the container. And keep the ood-portal.conf ephemeral, regenerated every time we start the container up, so we actually don't care what it's previous state was.

Maybe --rpm is the right flag here? But I'm happy to force here in this edge case, and keep the update_ood_portal the way it is, if nothing else, to just keep the attack surface small.

@johrstrom
Copy link
Contributor Author

Just added docker runtime flags to boot the container as --user. No idea what's going on with your images Trey.

…er-cache

option for e2e tests because it doesn't need all the app gems.
@msquee
Copy link
Contributor

msquee commented Jul 29, 2021

@johrstrom From a bare install on my Mac I run into this error:

groupadd: GID '20' already exists The command '/bin/sh -c groupadd -g $GID $USER && useradd -u $UID --create-home --gid $USER $USER && echo "$USER ALL=(ALL) NOPASSWD:ALL" >>/etc/sudoers.d/$USER' returned a non-zero code: 4 rake aborted!

Full trace:

Error: No such image: ood-dev:latest
docker tag ood:2.0.13-43dc74b ood:latest
Error: No such image: ood-dev:2.0.13-43dc74b
docker build --build-arg VERSION=v2.0.13-43dc74b -t ood-dev:2.0.13-43dc74b -f Dockerfile.dev . --build-arg USER=mario --build-arg UID=501 --build-arg GID=20
Sending build context to Docker daemon  30.47MB
Step 1/8 : FROM ood:latest
 ---> 5c7130301ca7
Step 2/8 : ARG USER=ood
 ---> Using cache
 ---> e3c6fbf9b462
Step 3/8 : ARG UID=1000
 ---> Using cache
 ---> d7c71f925ed4
Step 4/8 : ARG GID=1000
 ---> Using cache
 ---> 320be8890bf3
Step 5/8 : RUN dnf reinstall -y httpd-tools &&     dnf install -y         strace lua python3 &&     dnf clean all && rm -rf /var/cache/dnf/*
 ---> Using cache
 ---> ec6f9031142d
Step 6/8 : RUN groupadd -g $GID $USER &&     useradd -u $UID --create-home --gid $USER $USER &&     echo "$USER ALL=(ALL) NOPASSWD:ALL" >>/etc/sudoers.d/$USER
 ---> Running in 32d1a00ae954
groupadd: GID '20' already exists
The command '/bin/sh -c groupadd -g $GID $USER &&     useradd -u $UID --create-home --gid $USER $USER &&     echo "$USER ALL=(ALL) NOPASSWD:ALL" >>/etc/sudoers.d/$USER' returned a non-zero code: 4
rake aborted!
Command failed with status (4): [docker build --build-arg VERSION=v2.0.13-4...]
/Users/mario/Code/jeff-dev/lib/tasks/packaging.rb:82:in `block (2 levels) in <top (required)>'
/Users/mario/Code/jeff-dev/lib/tasks/development.rb:91:in `block (2 levels) in <top (required)>'
/Users/mario/.vendor/bundle/ruby/2.7.0/gems/rake-13.0.6/exe/rake:27:in `<top (required)>'
/Users/mario/.rvm/gems/ruby-2.7.1/bin/ruby_executable_hooks:24:in `eval'
/Users/mario/.rvm/gems/ruby-2.7.1/bin/ruby_executable_hooks:24:in `<main>'
Tasks: TOP => package:dev_container
(See full trace by running task with --trace)

@johrstrom
Copy link
Contributor Author

Thanks for the heads up. Yea there was a bug there in the useradd command as well. I pushed a change that'll only create the group if the GID doesn't already exist.

@treydock
Copy link
Contributor

OMG, I haven't gotten around to testing this in docker yet. Could it be something with adding a user with the same uid/gid? Do you have the same experience with package:dev_container off of the master branch?

This is my buildah output on Linux:

localhost/ood-dev                                   latest           31450274265c   24 hours ago    1.95 GB
localhost/ood-dev                                   2.0.13-7de665e   31450274265c   24 hours ago    1.95 GB
localhost/ood                                       latest           610e11a9feb8   24 hours ago    1.91 GB
localhost/ood                                       2.0.13-7de665e   610e11a9feb8   24 hours ago    1.91 GB

Building off master gives me reasonable images:

ood-dev                                          2.0.13-1930523            7f8883cb284d   7 seconds ago        1.73GB
ood-dev                                          latest                    7f8883cb284d   7 seconds ago        1.73GB
ood                                              2.0.13-1930523            7464b62197dc   About a minute ago   1.69GB
ood                                              latest                    7464b62197dc   About a minute ago   1.69GB

@johrstrom
Copy link
Contributor Author

I can only speculate as to what your issues would be @treydock especially if you can build the container on master. There was an update to the entrypoint, but I'm at a loss to see how that would spill over to become a 99 GB file. I'm going to 🙏 it's just a blip and you can bounce your docker deamon to get rid of it.

@msquee
Copy link
Contributor

msquee commented Jul 29, 2021

I can only speculate as to what your issues would be @treydock especially if you can build the container on master. There was an update to the entrypoint, but I'm at a loss to see how that would spill over to become a 99 GB file. I'm going to 🙏 it's just a blip and you can bounce your docker deamon to get rid of it.

To add some confidence to this, my image builds have been consistently under 2GB on multiple platforms

@treydock
Copy link
Contributor

I had to force restart my Docker daemon because I was unable to CTRL+C a build that was just stuck exporting the image layers. So a restart of Docker daemon doesn't solve my issue.

$ docker --version
Docker version 20.10.7, build f0df350

@treydock
Copy link
Contributor

Thinking maybe some stale images were the cause I did docker image prune --all to force delete pretty much every image on my laptop and that didn't solve the issue either.

@treydock
Copy link
Contributor

Updated Docker Desktop on my Mac and Docker version is still the same, and as expected the behavior didn't change.

@treydock
Copy link
Contributor

The main ood image builds just fine, it's the ood-dev one that seems to have issues.

@treydock
Copy link
Contributor

I went through the always painful process of rebooting my laptop and the issue persists. My ondemand directory I'm at is only ~87MB on disk, so it's not like I have some huge file stuck in the build root.

@treydock
Copy link
Contributor

So this works:

docker build -t ood-dev:2.0.13-af6ecfb -f Dockerfile.dev  .

If I remove the --build-arg flags, and build with defaults, the build is successful. This is what full build command looks like on my system from Rake task (flags re-ordered thinking maybe build-arg after path was breaking things)

docker build --build-arg USER=tdockendorf --build-arg UID=623703847 --build-arg GID=758353319 -t ood-dev:2.0.13-af6ecfb -f Dockerfile.dev .

If I change UID and GID to be 1001 then I also can build the image, so this is successful

docker build --build-arg USER=tdockendorf --build-arg UID=1001 --build-arg GID=1001 -t ood-dev:2.0.13-af6ecfb -f Dockerfile.dev  .

Something about my really large UID and or GID is breaking things very badly. So far removing just the GID arg with high number UID doesn't work but this does work:

docker build --build-arg USER=tdockendorf --build-arg GID=758353319 -t ood-dev:2.0.13-af6ecfb -f Dockerfile.dev  .

So looks like my UID is the problem. These are the UID/GID I get on my OHTECH managed device.

So when I build the ondemand RPM build box that actually runs as the UID of person running Docker, I don't build the container with my UID/GID inside the container. Rather all commands are run through a wrapper script that changes the UID/GID of the running command to match runtime arguments, not build time arguments. It's far from ideal but I've never had problems like this and don't have to build a container with my UID/GID , the container is instead portable and can be pushed to Docker hub and the runtime behavior of who executes commands changes based on the person running Docker commands.

@treydock
Copy link
Contributor

Forgot to add, noticed --build-arg VERSION getting passed but that argument isn't used in Dockerfile.dev so maybe forgot to use or not needed?

@johrstrom
Copy link
Contributor Author

Thanks for the updates. Version is used in ood and I think we just keep passing it in the rake tasks.

To your actual issue, we'll have to think of something. You may be able to use userns-remap and map yourself to a 1000:1000 user but that's a whole extra step - and something folks will find out after they build a 100GB image (or try to). The whole idea of this was to make it super simple so we'll just have to account for this use case somehow.

https://docs.docker.com/engine/security/userns-remap/

@johrstrom
Copy link
Contributor Author

@treydock your issue should be fixed. It was as simple as adding -l to useradd. Some thing's wrong with updating the lastlog files. 🤷 . The sed against the mailbox is just to avoid the noise Setting mailbox file permissions: Invalid argument so we won't create a mailbox at all.

moby/moby#5419

To your point about portability - that's the kind of the point here, it's not portable. It's your container. You want your files mounting in your ~/ondemand/dev directory so you can read & write them with the same IDs.

@johrstrom
Copy link
Contributor Author

Are we ready for this? I think we can defer the removal of the mock container.

@msquee
Copy link
Contributor

msquee commented Aug 4, 2021

@johrstrom I'm happy with everything in this PR! I'm finishing up a few things on PR #1311 but we are good to merge this in.

@treydock
Copy link
Contributor

treydock commented Aug 4, 2021

The recent changes worked for me, was able to build the container images and launch dev environment and login.

@johrstrom johrstrom merged commit fd9f650 into master Aug 4, 2021
@johrstrom johrstrom deleted the dev-runtime branch August 4, 2021 18:09
This was referenced Aug 4, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants