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
Show file tree
Hide file tree
Changes from 17 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 5 additions & 3 deletions .github/workflows/tests.yml
Original file line number Diff line number Diff line change
Expand Up @@ -35,12 +35,13 @@ jobs:
- name: Setup Bundler
run: |
bundle config path ~/vendor/bundle
bundle install

- name: Run ShellCheck
run: rake test:shellcheck
run: bundle exec rake test:shellcheck

- name: Run unit tests
run: rake test:unit
run: bundle exec rake test:unit
johrstrom marked this conversation as resolved.
Show resolved Hide resolved

- name: Run System Dashboard tests
run: cd apps/dashboard; bin/rake test:system
Expand Down Expand Up @@ -69,7 +70,8 @@ jobs:
uses: actions/cache@v2
with:
path: ~/vendor/bundle
key: ${{ runner.os }}-${{ matrix.ruby }}-gems-${{ hashFiles('apps/*/Gemfile.lock', 'Gemfile.lock') }}
# FIXME: Hard coded the ruby version here! to match unit tests cache.
key: ${{ runner.os }}-2.7.1-gems-${{ hashFiles('apps/*/Gemfile.lock', 'Gemfile.lock') }}

- name: Setup Bundler
run: |
Expand Down
64 changes: 64 additions & 0 deletions DEVELOPMENT.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,64 @@
# Developing Open-OnDemand

## Getting Started

These are instructions to build and interact with a full stack
development container. This container will create a duplicate user
with the same group and user id. Starting the container will prompt
you to set a password. This is only credentials for web access to the
container.

Pull down this source code and start the container.

```text
mkdir -p ~/ondemand
git clone ~/ondemand/src
johrstrom marked this conversation as resolved.
Show resolved Hide resolved
cd ~/ondemand/src
rake dev:start
```

See `rake --tasks` for all the `dev:` related tasks.

```
rake dev:exec # Bash exec into the development container
rake dev:restart # Restart development container
rake dev:start # Start development container
rake dev:stop # Stop development container
```

### Login to the container

Here's the important bit about user mapping with containers. Let's use the
example of `jessie` with `id` below. In creating the development container,
we added a user with the same. The password is for `dex` the IDP, and the
web only.

```
uid=1000(jessie) gid=1000(jessie) groups=1000(jessie)
```

Now you'll be able to access `http://localhost:8080/` where it'll redirect
you to `dex` the OpenID Connect provider within the container. Use the email
`<your username>@localhost`.


### Configuring the container

In starting the container, you may see the mount
`~/.config/ondemand/container:/etc/ood`. This mount allows us to
completely configure this Open-OnDemand container.

Create and edit files in the host's home directory and to mount in
new configurations.

Remove `~/.config/ondemand/container/static_user.yml` to reset your
container's password.

### Rebuilding the image

All the development tasks will use the `ood-dev:latest` image. If
you want to rebuild to a newer version use the rebuild task.

```text
rake dev:rebuild
```
3 changes: 3 additions & 0 deletions Dockerfile
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,9 @@ COPY ood_auth_map /opt/ood/ood_auth_map
COPY apps /opt/ood/apps
COPY Rakefile /opt/ood/Rakefile
COPY lib /opt/ood/lib
COPY Gemfile /opt/ood/Gemfile

RUN cd /opt/ood; bundle install

RUN source /opt/rh/ondemand/enable && \
rake -f /opt/ood/Rakefile -mj$CONCURRENCY build && \
Expand Down
1 change: 1 addition & 0 deletions Gemfile
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ source "https://rubygems.org"
git_source(:github) {|repo_name| "https://github.com/#{repo_name}" }

gem "rake"
gem "bcrypt"

group :test do
gem "rspec"
Expand Down
22 changes: 12 additions & 10 deletions Gemfile.lock
Original file line number Diff line number Diff line change
@@ -1,35 +1,37 @@
GEM
remote: https://rubygems.org/
specs:
bcrypt (3.1.16)
childprocess (3.0.0)
diff-lcs (1.4.4)
rake (13.0.3)
regexp_parser (1.8.2)
rake (13.0.6)
regexp_parser (2.1.1)
rspec (3.10.0)
rspec-core (~> 3.10.0)
rspec-expectations (~> 3.10.0)
rspec-mocks (~> 3.10.0)
rspec-core (3.10.0)
rspec-core (3.10.1)
rspec-support (~> 3.10.0)
rspec-expectations (3.10.0)
rspec-expectations (3.10.1)
diff-lcs (>= 1.2.0, < 2.0)
rspec-support (~> 3.10.0)
rspec-mocks (3.10.0)
rspec-mocks (3.10.2)
diff-lcs (>= 1.2.0, < 2.0)
rspec-support (~> 3.10.0)
rspec-support (3.10.0)
rubyzip (2.3.0)
rspec-support (3.10.2)
rubyzip (2.3.2)
selenium-webdriver (3.142.7)
childprocess (>= 0.5, < 4.0)
rubyzip (>= 1.2.2)
watir (6.17.0)
regexp_parser (~> 1.2)
selenium-webdriver (~> 3.6)
watir (6.19.1)
regexp_parser (>= 1.2, < 3)
selenium-webdriver (>= 3.142.7)

PLATFORMS
ruby

DEPENDENCIES
bcrypt
rake
rspec
watir
Expand Down
1 change: 1 addition & 0 deletions Rakefile
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ PASSENGER_APP_ENV = ENV["PASSENGER_APP_ENV"] || "production"
require "#{TASK_DIR}/packaging"
require "#{TASK_DIR}/test"
require "#{TASK_DIR}/docker"
require "#{TASK_DIR}/development"

def infrastructure
[
Expand Down
5 changes: 2 additions & 3 deletions docker/dev-entrypoint.sh
Original file line number Diff line number Diff line change
Expand Up @@ -9,12 +9,11 @@ APP_DEV_DIR="/home/$USER/ondemand/dev"
OOD_DEV_DIR="/var/www/ood/apps/dev/$USER"

sudo su root <<SETUP
mkdir -p $OOD_DEV_DIR
chmod 755 $OOD_DEV_DIR
mkdir -p $OOD_DEV_DIR
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.

cat "$OOD_STATIC_USER" >> /etc/ood/dex/config.yaml
Expand Down
1 change: 1 addition & 0 deletions lib/.rubocop.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
inherit_from: '../apps/dashboard/.rubocop.yml'
6 changes: 6 additions & 0 deletions lib/tasks/build_utils.rb
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
# frozen_string_literal: true

module BuildUtils
def image_tag
tag? ? numeric_tag : "#{numeric_tag}-#{git_hash}"
Expand Down Expand Up @@ -42,4 +44,8 @@ def dev_image_name
def image_name
"ood"
end

def user
@user ||= Etc.getpwnam(Etc.getlogin)
end
end
140 changes: 140 additions & 0 deletions lib/tasks/development.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,140 @@
# frozen_string_literal: true

namespace :dev do
require_relative 'build_utils'
require 'yaml'
require 'bcrypt'
include BuildUtils

def dev_container_name
'ood-dev' || ENV['OOD_DEV_CONTAINER_NAME'].to_s
end

def init_ood_portal
file = "#{config_directory}/ood_portal.yml"
return if File.exist?(file)

File.open(file, File::WRONLY|File::CREAT|File::EXCL) do |f|
f.write({
'servername': 'localhost',
'port': 8080,
'listen_addr_port': 8080,
'oidc_remote_user_claim': 'email',
'dex': {
'connectors': [{
'type': 'mockCallback',
'id': 'mock',
'name': 'Mock'
}]
}
}.to_yaml)
end
end

def init_ctr_user
file = "#{config_directory}/static_user.yml"
return if File.exist?(file)

require 'io/console'
puts 'Enter password:'
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.

enablePasswordDB: true
staticPasswords:
- email: "#{user.name}@localhost"
hash: "#{bcrypted}"
username: "#{user.name}"
userID: "71e63e31-7af3-41d7-add2-575568f4525f"
CONTENT

File.open(file, File::WRONLY | File::CREAT | File::EXCL) do |f|
f.write(content)
end
end

def container_rt_args
podman_runtime? ? podman_rt_args : docker_rt_args
end

def docker_rt_args
[].freeze
end

def podman_rt_args
[
'--userns', 'keep-id',
'--cap-add', 'sys_ptrace',
'--security-opt', 'label=disable'
].freeze
end

def config_directory
@config_directory ||= begin
base_dir = "#{user.dir}/.config/ondemand/container/config".tap { |dir| FileUtils.mkdir_p(dir) }
base_dir
end
end

def dev_mounts
[
'-v', "#{config_directory}:/etc/ood/config",
'-v', "#{user.dir}/ondemand:#{user.dir}/ondemand"
]
end

desc 'Start development container'
task :start => ['ensure_dev_files'] do
Rake::Task['package:dev_container'].invoke unless image_exists?("#{dev_image_name}:latest")

ctr_args = [container_runtime, 'run', '-p 8080:8080', '-p 5556:5556']
ctr_args.concat ["--name #{dev_container_name}"]
ctr_args.concat ['--rm', '--detach']
ctr_args.concat ['-e', 'OOD_STATIC_USER=/etc/ood/config/static_user.yml']
ctr_args.concat dev_mounts
ctr_args.concat container_rt_args

ctr_args.concat ["#{dev_image_name}:latest"]
sh ctr_args.join(' ')
end

desc 'Stop development container'
task :stop do
sh "#{container_runtime} stop #{dev_container_name}"
end

desc 'See the development container\'s logs'
task :logs do
sh "#{container_runtime} logs #{dev_container_name}"
end

desc 'Restart development container'
task :restart => [:stop, :start]

desc 'Rebuild the ood-dev:latest container'
task :rebuild => ['package:dev_container']

desc 'Bash exec into the development container'
task :exec do
ctr_args = [container_runtime, 'exec', '-it']
# home is set to /root? could be bug for me
ctr_args.concat ['-e', "HOME=#{user.dir}"]
ctr_args.concat ['--workdir', user.dir.to_s]
ctr_args.concat [dev_container_name, '/bin/bash']

sh ctr_args.join(' ')
end

task :bash => [:exec]

# let advanced users know this, not --tasks
task :ensure_dev_files do
[
:init_ood_portal,
:init_ctr_user
].each do |initer|
send(initer)
end
end
end
15 changes: 5 additions & 10 deletions lib/tasks/packaging.rb
Original file line number Diff line number Diff line change
Expand Up @@ -57,8 +57,8 @@ def tag_latest_container_cmd(image_name)
sh "git ls-files | #{tar} -c --transform 's,^,ondemand-#{version}/,' -T - | gzip > packaging/v#{version}.tar.gz"
end

desc "Build clean docker container from ood image"
task container: [:clean] do
desc "Build the ood image"
task :container do
sh build_cmd("Dockerfile", image_name) unless image_exists?("#{image_name}:#{image_tag}")
end

Expand All @@ -75,14 +75,9 @@ def tag_latest_container_cmd(image_name)

desc "Build container with Dockerfile.dev"
task dev_container: [:latest_container] do
if ENV['OOD_KEEP_USERNS'].to_s.empty?
extra = []
else
username = Etc.getlogin
extra = ["--build-arg", "USER=#{username}"]
extra.concat ["--build-arg", "UID=#{Etc.getpwnam(username).uid}"]
extra.concat ["--build-arg", "GID=#{Etc.getpwnam(username).uid}"]
end
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?


sh build_cmd("Dockerfile.dev", dev_image_name, extra_args: extra) unless image_exists?("#{dev_image_name}:#{image_tag}")
sh tag_latest_container_cmd(dev_image_name)
Expand Down
Loading