Support for Development Containers (devcontainers)

[pablobm]

Fixes #5264

This adds configuration for Development Containers, aka devcontainers (https://containers.dev/). This allows running a development environment in a container, using Docker or other tools, more easily than done with Docker manually and with integration with IDEs.

Misc notes:

• Configuration is based on the one provided when running bin/rails devcontainer, with minimal changes to adapt to this project.
• Internally, the container manages versions of Ruby and Node.js using https://mise.jdx.dev. I have added a Docker volume to cache the related artifacts. Also the script ./devcontainer/start needs to run mise trust /workspaces/mise.local.toml to avoid an interactive prompt asking whether this file can be trusted.
• A few packages that we install explicitly in the normal Docker setup are already present here, reducing the size of the apt-get call. Hopefully I'm not missing any. Eg: the need for osmosis is not obvious as not covered by tests.
• A new GitHub Actions workflow runs this setup on CI.
• The documentation assumes VS Code, as there are many ways of running devcontainers, but VS Code seems to be the most popular one (unsurprisingly as this comes from Microsoft). The text allows for other options.
• I have copied the initial sections of DEVCONTAINER.md from DOCKER.md. There's a lot of duplication that I propose we address separately at DRY docs #6458
• The containerised environment makes things looks like we are running separate services away from localhost. WebMock needs to be told explicitly as :allow_localhost => true is not true in that setting.

Caveat: remote Selenium

From the point of view of the test suite, Selenium runs remotely, in a separate container. Unfortunately this causes two problems:

• Capybara doesn't appear to reset the browser properly between tests, and any changes in preferences are not applied. This affects test/system/embed_test.rb, which runs the browser with different language configurations.
• Parallel tests don't appear to be possible.

Note that these issues only affect tests run within the containerised setup. Classic, local environments run Selenium "locally" and don't have these issues.

As a workaround for setting preferences, this runs several Selenium instances (selenium-default, selenium-de, selenium-nolang), each with a set of preferences. This is not scalable, but right now it's ok as we only have three sets of preferences.

To run Selenium this way, some additional options are required in the driven_by call. To avoid duplication, the new test helper driven_by_selenium can be used, which sets things up correctly depending on the case.

As a workaround for parallel tests, we detect that we are in a container by checking if CAPYBARA_SERVER_PORT is set. If so, we don't call parallelize.

[tomhughes]

Is it possible to mount a persistent volume in the container for gems, npms etc so that it will only have to install new ones when it started?

[tomhughes]

Looking at it in more detail how exactly does the host/container split work? Is it mounting the checkout from the host to the /workspaces/... path in the container?

In which case I guess the gems and npms are already persistent? Certainly npms but where exactly is it installing gems to at the moment?

We'll need to replace DOCKER.md with some instructions on using this for sure.

The commit to get rid of test as a password can probably be pulled out to a separate PR as preparation for this then we probably want to squash this down to just a couple of commits once we're happy with it - maybe one to add devcontainer and the one to remove docker?

[pablobm]

NPM dependencies should be ok as they live in node_modules within the work directory.

Ruby dependencies are handled with https://mise.jdx.dev and go in /home/vscode/.local/share/mise/installs/ruby/. I have now added configuration for a persistent volume and it seems to work. Thank you for mentioning, I do forget these things exist 😳

I have added a todo list to the description. Will go through these items in the next few days, a bit at a time. If others are able to test that this works in different environments, that'd be grand!

[pablobm]

Found something very annoying while fixing test/system/embed_test.rb. I have left the relevant section of the PR untidy on purpose: discussing here before I clean it up properly, to see if people have thoughts. Commit: 7d992b9

From what I can tell, Rails+Capybara don't properly "reset" the remote Selenium when running a test with a different driven_by declaration. After trying a few things, what worked for me was to have different Selenium containers, one to be used for each configuration. This way we don't need to reset anything.

First issue I see is that this is not scalable. Not a problem right now as there are only three configurations. Could be a problem in the future if for any reason we want to test more.

[pablobm]

Another funny one. Relevant commit: cf421c0. The test test/system/select_language_test.rb fails with remote Selenium because there is no HTTPS, and therefore it can't set "secure" cookies.

I'm ignorant on this topic: should we bother with secure cookies? We force HTTPS anyway 🤔 Alternatively, we could have a setting in the JS that announces whether we are in production mode, and then set secure: true or not depending on that.

[tomhughes]

The point of the secure flag is to ensure that the browser will never send the cookie over an insecure connection.

[tomhughes]

That code should probably just be secure: location.protocol === 'https' or something.

[pablobm]

I'm reading a bit more. I think the explanation (or at least one explanation) is that an attacker could impersonate the HTTP version of the site before the redirection to HTTPS. Hence we can't be sure that we are setting the cookie securely.

I think location.protocol === 'https' would not work as it would not protect us from those edge cases that secure is supposed to be about. I'll put a variable to signal that we are in production and use that.

[tomhughes]

We already send Settings.server_protocol to the client as OSM.SERVER_PROTOCOL if you prefer to use that.

[pablobm]

Right, that's all tests passing under the devcontainer, on my Mac and on CI. Other environments yet to be tested.

I solved the issue with the cookies using OSM.SERVER_PROTOCOL as suggested. I also did a refactor to DRY up the cookie handling. Another candidate for a separate PR? Commit: 60af1a1

Still a few todo items to go through, plus the open questions. Non-devcontainers tests are failing because I'm missing a cleanup: I didn't want to commit too much time on that, just in case there are concerns about the caveats.

If you have opinions, I'd love to hear them!

[pablobm]

Correction: a bunch of errors on CI (devcontainer workflow), after a flawless one earlier... Oh my.

[pablobm]

Crisis averted. I had committed something silly.

[hlfan]

I think the cookies clean up doesn't need that much of investigation ahead of pulling it into its own PR.

[pablobm]

Cookies clean up extracted to #6430. Will remove from here when that is merged, as it's necessary for CI to pass.

[pablobm]

Selenium test/s3cr3t workaround extracted to #6431. Again, will remove from here when that is merged.

[pablobm]

@waldyrious @danieldegroot2 - Since you left reviews... 😉 Have you tried this setup? Did you encounter any issues?

[pablobm]

@hlfan - Potential clue of what's going on with the failing test for the language selector. I tested the devcontainer again today and it failed. This is an excerpt of the screenshot I got:

This suggests that the browser gives the focus to the first element in the list (Afrikaans) instead of leaving it on the input box. If you try this on your browser (open the selector, press tab to move the focus) then you won't be able to type in the box any more, which is consistent with the test failure.

As to what could possibly be moving the focus, I have no idea.

[danieldegroot2]

Have you tried this setup?

No, I personally don't use code.

[waldyrious]

I haven't used this either 😅

[gravitystorm]

I've tried this out today, using @devcontainers/cli and it worked for me. I successfully ran the entire test suite, and I got rails server working after a couple of small tweaks.

devcontainers-cli specific notes

The port forwarding is not supported by @devcontainers/cli ( see https://github.com/devcontainers/cli/issues/186 and https://github.com/devcontainers/cli/issues/22) but this can be worked around. First, launch the server bound to all IP addresses, not just localhost within the container, i.e. `bundle exec rails server -b 0.0.0.0`. Secondly, add the port forwarding to the compose file, i.e.

--- a/.devcontainer/compose.yaml
+++ b/.devcontainer/compose.yaml
@@ -6,6 +6,9 @@ services:
       context: ..
       dockerfile: .devcontainer/Dockerfile
 
+    ports:
+      - "3000:3000"
+
     volumes:
     - ../..:/workspaces:cached
     - mise-cache:/home/vscode/.local/share/mise

It would be great if cli had the port forwarding built in, since it would then be an easy substitute for having our standalone docker config - it will suit many people who might want to use Docker but who aren't interested in the IDE side of things.

My next plan is to test with DevPod to see how the IDE integration side of things works.

But from my side, this looks good to me. What are the current blockers, if any? Are there any other specific things you want tested?

Since this PR is additive, should we merge this and iron out any kinks later?

[hlfan]

What are the current blockers, if any?

Currently I don't think this is good to merge as there's a test that is failing consistently.

#6585 extends here as well, but I don't necessarily see that as blocking.

[pablobm]

Agreed with @hlfan - The test test/system/select_language_test.rb appeared to fail more consistently with the devcontainer than with other workflows. (However it appears to work better now? Go figure). For me that's the blocker.

Regarding the actual development environment: it did something weird for me last month (#6424 (comment)), then it's working again this week. I think we can live with that and improve it over time.

Something we could do is merge this, but disabling the workflow until we figure out what's going on with the language selector. I can create a separate issue to track that, including the detail I discovered at #6424 (comment)

[gravitystorm]

My next plan is to test with DevPod to see how the IDE integration side of things works.

I've not made much progress with this today, but that's to do with things on the DevPod side of things, nothing in our repo.

Something we could do is merge this, but disabling the workflow until we figure out what's going on with the language selector.

Yes please. I'd like to see if/how things work with CodeSpaces at least, in the meantime.

[pablobm]

Created new issue to track the flaky test for the language selector: #6712

[pablobm]

I have brought this out of draft now.

@gravitystorm: should the port forwarding config you shared above be included? I just played a bit with @devcontainers/cli and couldn't get the server to work despite your instructions. I stopped before I sank too much time on it. The test suite ran well.

If that's not an issue, then this might be ready to merge.

Re: disabling the GHA workflow, I don't know that there's a way to do it with code. Would a maintainer disable it on the GitHub UI please? https://docs.github.com/en/actions/how-tos/manage-workflow-runs/disable-and-enable-workflows

[gravitystorm]

@gravitystorm: should the port forwarding config you shared above be included?

No, I don't think so. The lack of port forwarding (and/or the subtly different "port publishing") is a devcontainers-cli limitation, and I'm not sure what else might break if we start including that stuff as a workaround. So best leave it aside for now.

[hlfan]

Re: disabling the GHA workflow, I don't know that there's a way to do it with code. Would a maintainer disable it on the GitHub UI please? https://docs.github.com/en/actions/how-tos/manage-workflow-runs/disable-and-enable-workflows

Done.

[tomhughes]

I haven't had any much luck trying to use this with codespaces...

First it took forever to startup and then when it did it said there were migrations pending and offered to run them but that failed without telling me why - running them manually revealed that they had run but the schema dump at the end was failing as it was using pg_dump 15 against a postgres 16 server.

After all that it's still not functional as far as I can see because there's no server running and no way that I can find to start one.

[mmd-osm]

Tried DevPod today w/ Docker as Provider. It took around 5 minutes to build and start up. Test cases were running ok:

[pablobm]

I haven't had any much luck trying to use this with codespaces

My opinion is that we merge this, for the purposes of development. Then we can have a separate PR with a different config for the purposes of using with codespaces, which is apparently a thing (example at thoughtbot/administrate#2948).

There could be a discussion along the lines of: "well if the codespaces config works better, why not use that". My answer at the moment is that I have no idea of what works better in which cases. I'm currently advocating to go with this PR because it follows Rails convention, in the sense that it works on top of what is provided by the generator bin/rails devcontainer. If after releasing this and gathering more feedback we think we'd be best served by a different approach, then we can change.

[tomhughes]

I was basically doing it because it was the easy way for me to test it but it sounds like I'll have to bite the bullet and see if I can get something working locally 😢