Skip to content

feat(factory): detect devcontainer.json when no devfile is present - #1050

Draft
rohanKanojia wants to merge 12 commits into
eclipse-che:mainfrom
rohankanojia-forks:devcontainer-detection
Draft

rohanKanojia wants to merge 12 commits into
eclipse-che:mainfrom
rohankanojia-forks:devcontainer-detection

Conversation

@rohanKanojia

@rohanKanojia rohanKanojia commented Sep 3, 2026 •

Copy link
Copy Markdown

What does this PR do?

⚠️ Currently WIP

Related to eclipse-che/che#23458

Adds devcontainer.json detection to the factory URL resolver. When a factory URL points to a repo with no devfile but has .devcontainer/devcontainer.json (or .devcontainer.json), che-server returns a generated devfile that merges devcontainer commands with the default devfile.

The generated devfile adds five commands:

  • start-devcontainer (manual command, not postStart): builds the devcontainer image via rootless podman, starts the nested container, runs lifecycle hooks, and publishes terminal runtime for the che-code extension
  • rebuild-devcontainer: force rebuild (removes existing container/image)
  • rebuild-devcontainer-no-cache: rebuild without cache
  • show-devcontainer-log: tail the build/run log
  • clean-devcontainer-images: remove the nested container and its image
Screenshot From 2026-09-03 18-36-09

The detection runs in URLFactoryBuilder.createFactoryFromDevfile() after all devfile locations are exhausted. The fallback chain is:

devfile.yaml / .devfile.yaml → .devcontainer/devcontainer.json / .devcontainer.json → default empty devfile

Key script behaviors:

  • Runs all devcontainer lifecycle hooks (initializeCommand, onCreateCommand, updateContentCommand, postCreateCommand, postStartCommand, postAttachCommand)

Screenshot/screencast of this PR

Screenshot From 2026-09-03 18-37-03 Screenshot From 2026-09-03 18-37-17

What issues does this PR fix or reference?

It's related to eclipse-che/che#23458

Prerequisites:

  • OpenShift 4.20+ cluster with Eclipse Che installed
  • Container run capabilities enabled

Steps:

Detailed test plan https://gist.github.com/rohanKanojia/5bfdcb4bbea318f5949dafc495960a05

  1. Deploy Che Server to an OpenShift Cluster >= 4.20
  2. Ensure Nested containers functionality is enabled:
oc patch checluster/eclipse-che -n eclipse-che \
  --type='merge' -p \
  '{"spec":{"devEnvironments":{"disableContainerRunCapabilities":false}}}'
  1. Build and deploy the custom che-server image, or patch the CheCluster CR to use a pre-built image with this change
oc patch checluster eclipse-che -n eclipse-che --type=merge -p '{
  "spec": { "components": { "cheServer": { "deployment": { "containers": [{
    "name": "che",
    "image": "quay.io/rokumar/che-server:latest",
    "imagePullPolicy": "Always"
  }] } } } }
}'
  1. Open a repo with a devcontainer.json but no devfile, e.g. : https://github.com/coder/envbuilder-starter-devcontainer

PR Checklist

As the author of this Pull Request I made sure that:

Release Notes

Reviewers

Reviewers, please comment how you tested the PR when approving it.

@openshift-ci

openshift-ci Bot commented Sep 3, 2026

Copy link
Copy Markdown

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

@openshift-ci

openshift-ci Bot commented Sep 3, 2026

Copy link
Copy Markdown

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: rohanKanojia

The full list of commands accepted by this bot can be found here.

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

When a factory URL points to a repo with no devfile, probe for
.devcontainer/devcontainer.json (then .devcontainer.json). If found,
merge devcontainer commands and events with the default devfile and
return a generated factory.

The generated devfile adds five commands to the default devfile:
- start-devcontainer: builds and runs the devcontainer via rootless
  podman, wires terminal profile and lifecycle hooks (postStart event)
- rebuild-devcontainer: force rebuild with REBUILD=1
- rebuild-devcontainer-no-cache: rebuild without cache
- show-devcontainer-log: tail the devcontainer build/run log
- clean-devcontainer-images: prune all podman images

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Signed-off-by: Rohan Kumar <rohaan@redhat.com>
- Validate fetched content looks like JSON before treating as devcontainer
  detection (rejects empty responses and HTML error pages)
- Use continue instead of break when rawFileLocation returns null
- Escalate DevfileException logging from debug to warn
- Remove Kubernetes service-account mount and env vars from nested container
- Replace opt-in REUSE_CONTAINER with fingerprint-based auto-reuse via
  che.devcontainer.config label (sha256 of config)
- Escape Go template syntax (use jq instead of --format '{{}}') to avoid
  DWO variable replacement errors
- Mark devcontainer CLI runtime install as temporary (tracking PR eclipse-che#267)

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@rohanKanojia
rohanKanojia force-pushed the devcontainer-detection branch from 27a3b17 to 03fecc1 Compare September 8, 2026 19:10
rohanKanojia and others added 2 commits September 18, 2026 16:12
Base64-encode start-devcontainer.sh so flattening cannot rewrite bash
${VAR:-default} syntax. Rethrow SCM auth during the probe, pin the
devcontainer CLI, and drop leftover editor wiring now owned by the
extension.

Signed-off-by: Rohan Kumar <rohaan@redhat.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
Expose resolved config path and on-disk file hash so the editor can detect devcontainer.json changes separately from resolved-config reuse.

Co-authored-by: Cursor <cursoragent@cursor.com>
…rity

KEEP_ID_OK is set only on the path that creates the container, so every reuse
left it at 0 and ran the fallback `chmod -R a+rwX` over the whole project --
on a container that already has uid parity and does not need it. On a large
repository that is a full tree walk on every workspace start, and it widens
permissions on files that were correctly owned to begin with.

A reused container keeps the user-namespace mapping it was created with, so ask
the container instead of trusting a variable the create path never set.

The mapping is read as JSON through jq rather than with --format: UidMap is the
JSON key but not the Go field name, and the template form fails with "can't
evaluate field UidMap in type *define.InspectIDMappings" on podman 5.8.

Signed-off-by: Rohan Kumar <rohaan@redhat.com>
The first iteration ships the devfile commands only, with no editor extension,
so nothing reads the runtime description the script published. Publishing state
that no consumer reads is dead weight in review and in the script.

Removed: runtime.json and everything that fed it -- publish_runtime and the
raw-file config fingerprint. The resolved-config fingerprint stays: it decides
container reuse and labels the container.

The setup lock stays as well. It serialises concurrent runs, which has nothing
to do with the editor.

The closing summary pointed at an editor action that does not exist yet; it now
points at the podman exec line it already prints.

This is a change of scope, not of design. The runtime description and the
extension that consumes it are on a separate branch for the next iteration,
where both sides can be reviewed together.

Signed-off-by: Rohan Kumar <rohaan@redhat.com>
@coderabbitai

coderabbitai Bot commented Sep 24, 2026

Copy link
Copy Markdown

Important

Draft PR not reviewed

Draft PRs are not automatically reviewed by default.

  • Trigger a manual review

To automatically review draft PRs, update your CodeRabbit configuration:

reviews:
  auto_review:
    drafts: true

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

rohanKanojia and others added 3 commits September 24, 2026 21:07
The PID written into the lock file was only read by the editor extension,
which is not part of this iteration. flock still serializes concurrent
runs; only the published PID and its teardown are removed.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01CidWf1g9fie6vS1rQDRcNF
Writes terminal.integrated.profiles.linux into che-code's machine settings
so a shell can be opened inside the dev container from the terminal
dropdown. Machine rather than workspace settings because the setting is
restricted and would be discarded in an untrusted workspace.

defaultProfile is left alone: the default terminal stays in UDI, where the
cluster credentials are.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01CidWf1g9fie6vS1rQDRcNF
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.

1 participant