Unify Docker image: remove separate SageMaker build
- Dominant language
- TypeScript
- Stars
- 481
- Forks
- 108
- Avg merge
- 8d 9h
- Merged PRs (30d)
- 7
Description
## Problem Statement
Graph Explorer maintains two Docker image variants built from the same Dockerfile — a main image and a SageMaker image differing only in build-time environment variable defaults. This doubles CI build/scan time, creates a confusing tag namespace (`sagemaker-*` vs regular), and forces the SageMaker lifecycle script to track a separate tag lineage.
On the client side, users must manually configure a "Public or Proxy Endpoint" URL and enable a "Using Proxy-Server" checkbox, even though the proxy server and UI are always co-located on the same origin. This creates unnecessary configuration friction — especially in SageMaker where the proxy URL must be derived from notebook metadata.
## Solution
Ship a single Docker image that works in all deployment contexts. The frontend uses relative asset paths and relative API fetches, eliminating build-time knowledge of the deployment path prefix. The client always routes database queries through the same-origin proxy server, removing the need for users to provide a proxy URL. The `NEPTUNE_NOTEBOOK` environment variable is preserved as a runtime convenience preset that configures port, logging, and SSL defaults.
## User Stories
1. As a user deploying on SageMaker, I want to pull the same image tag as everyone else, so that I don't have to track a separate `sagemaker-*` tag lineage.
2. As a user deploying on SageMaker with an existing lifecycle script, I want my deployment to keep working after an image upgrade, so that I don't have to change anything.
3. As a user creating a Connection, I want to only provide my database endpoint and auth settings, so that I don't have to figure out what "Public or Proxy Endpoint" means or what URL to put there.
4. As a user deploying behind a custom reverse proxy, I want the app to work without build-time path configuration, so that I can mount it at any URL prefix.
5. As a user editing an existing Connection, I want my saved connections to keep working after upgrade, so that I don't have to reconfigure them.
6. As a user deploying with ECS Fargate, I want to simplify my task definition environment variables, so that I don't need to specify proxy endpoint and proxy toggle vars.
7. As a user deploying with Docker, I want to pass only the database URL and auth settings, so that the setup is minimal.
8. As a CI maintainer, I want to build one image instead of two, so that build time and vulnerability scan coverage are halved.
9. As a user running with `NEPTUNE_NOTEBOOK=true`, I want the container to automatically use port 9250 and cloudwatch logging, so that I don't need to pass those explicitly.
10. As a user passing explicit `PROXY_SERVER_HTTP_PORT`, I want my value to take precedence over the `NEPTUNE_NOTEBOOK` defaults, so that I have full control.
11. As a user with `NEPTUNE_NOTEBOOK=true`, I want SSL to be forced off regardless of other settings, so that the Jupyter proxy works correctly.
12. As a developer, I want the `proxyConnection` concept removed from the codebase, so that there's one path for all database queries.
13. As a user viewing the Connection detail, I want to see my database endpoint clearly, so that I know which database I'm connected to.
14. As a user hitting a critical error, I want the "reload" link to take me back to the app root, so that I get a clean slate.
15. As a user who previously had `proxyConnection: false`, I want my saved database URL to be preserved on upgrade, so that I don't lose my connection.
16. As a user who previously had `proxyConnection: true`, I want my saved graph database URL to be preserved on upgrade, so that I don't lose my connection.
## Implementation Decisions
### Relative asset paths
- Vite `base` changes from `env.GRAPH_EXP_ENV_ROOT_FOLDER` to `"./"`.
- `index.html` gets a `` tag to anchor `document.baseURI` regardless of client-side routing. This future-proofs for a potential move from HashRouter to proper URL routes.
- `RELOAD_URL` simplifies to `"."`.
### Relative API fetches
- All client API calls (sparql, gremlin, openCypher, defaultConnection, logger, summary, etc.) use `new URL("../endpoint", document.baseURI)` instead of constructing absolute URLs from `connection.url`.
- The static files are served at `/explorer` and API routes at `/`. The `../` from the static directory resolves to the API root. This relationship is fixed in one place on the server.
- The two-path fallback in `defaultConnection.ts` (trying both root and `/proxy/9250/`) is removed in favor of a single relative fetch.
### Connection model simplification
- `proxyConnection` and `url` are removed from the `ConnectionConfig` type.
- `graphDbUrl` becomes the canonical required field for the database endpoint.
- `fetchDatabaseRequest` always sends the `graph-db-connection-url` header — no conditional based on `proxyConnection`.
- A read-time normalization handles legacy stored data: if `proxyConnection` was true, use `graphDbUrl`; if false, use `url`.
### Connection form simplification
- The "Public or Proxy Endpoint" text field is removed.
- The "Using Proxy-Server" checkbox is removed.
- "Graph Connection URL" is always visible (not gated behind the proxy checkbox).
- The IAM auth toggle is always visible (not gated behind the proxy checkbox).
### Dockerfile
- `ARG NEPTUNE_NOTEBOOK` and all conditional ENV logic are removed.
- `GRAPH_EXP_ENV_ROOT_FOLDER` is removed.
- `PROXY_SERVER_HTTP_PORT` and `LOG_STYLE` are NOT set as ENV vars — the app's built-in defaults (80, "default") apply when absent.
- `EXPOSE 80`, `EXPOSE 443`, `EXPOSE 9250` remain.
### Runtime NEPTUNE_NOTEBOOK handling (process-environment.sh)
- When `NEPTUNE_NOTEBOOK=true`:
- Writes `PROXY_SERVER_HTTP_PORT=9250` to `.env` (respects explicit env var override via dotenv's no-override behavior).
- Writes `LOG_STYLE=cloudwatch` to `.env` (same override behavior).
- Forces `PROXY_SERVER_HTTPS_CONNECTION=false` and `GRAPH_EXP_HTTPS_CONNECTION=false` (not overridable).
- Stops writing `NEPTUNE_NOTEBOOK` itself to `.env` — nothing downstream reads it.
- `PUBLIC_OR_PROXY_ENDPOINT` and `USING_PROXY_SERVER` handling is removed from the script. These env vars are silently ignored if still passed.
### CI: dual-tag publishing
- CI builds one image.
- Publishes under both regular tags and `sagemaker-*` tags during a transition period.
- The `--build-arg NEPTUNE_NOTEBOOK=true` step is removed.
### Lifecycle script update
- Pulls the regular tag (not `sagemaker-*`).
- Removes `PUBLIC_OR_PROXY_ENDPOINT` and `USING_PROXY_SERVER` from `docker run`.
### Documentation updates
- `docs/references/configuration.md`
- `docs/guides/deploy-to-ecs-fargate.md`
- `docs/guides/deploy-to-sagemaker.md`
- `docs/guides/deploy-with-docker.md`
- `docs/features/connections.md`
- `docs/architecture.md`
## Testing Decisions
A good test for this work exercises observable external behavior through the public interfaces — not internal wiring. Tests should verify what a user or calling code observes, not how the internals are structured.
### Modules to test
1. **Connection migration (read-time normalization)** — Given legacy stored connection data with various combinations of `proxyConnection`, `url`, and `graphDbUrl`, verify that the normalized output always produces the correct `graphDbUrl`. Prior art: `defaultConnection.test.ts`, `configuration.ts` normalization tests.
2. **Relative URL resolution** — Given various `document.baseURI` values (representing `/explorer/`, `/proxy/9250/explorer/`, custom prefix), verify that the API URL construction resolves to the correct absolute path. This can be a pure function test with injected base URIs. Prior art: unit tests in `connector/` and `core/`.
3. **process-environment runtime defaults** — Given various combinations of environment variables and `NEPTUNE_NOTEBOOK=true/false`, verify the correct values are written to `.env` and `defaultConnection.json`. Verify explicit env vars take precedence. Verify SSL is forced off when `NEPTUNE_NOTEBOOK=true`. Prior art: `process-environment.test.ts` (extensive existing test suite).
4. **Connection form UI** — Verify the form renders the correct fields (no proxy endpoint, no proxy toggle), validates that `graphDbUrl` is required, and submits the correct shape. Prior art: existing component tests using React Testing Library patterns in the codebase.
5. **fetchDatabaseRequest** — Verify that `graph-db-connection-url` header is always sent. Verify AWS headers are sent when IAM is enabled. Verify the function works without `proxyConnection` in the connection object. Prior art: `fetchDatabaseRequest.test.ts` (comprehensive existing suite).
## Out of Scope
- Removing `NEPTUNE_NOTEBOOK` entirely (kept for backward compat with deployed lifecycle scripts).
- Dropping the `sagemaker-*` tag publishing (deferred to a future release after transition period).
- Moving from HashRouter to proper URL routes (future work, but `` prepares for it).
- Changing the `graphDbUrl` field name (kept for backward compat with stored data).
- Renaming the "Graph Connection URL" UI label.
- Server-side route restructuring (API routes stay at root, static files stay at `/explorer`).
## Further Notes
- The ADR for this decision is committed at `docs/adr/0001-unify-docker-image-remove-sagemaker-variant.md` on branch `remove-sagemaker-docker-image`.
- Users passing `PUBLIC_OR_PROXY_ENDPOINT` or `USING_PROXY_SERVER` as env vars to Docker will see no errors — the vars are simply ignored. No deprecation warning is emitted.
- The `defaultConnection.json` file still works as a pre-configuration mechanism for operators. It just no longer includes proxy endpoint or proxy toggle fields.
Contributor guide
Assessment
This issue has not been assessed yet.