Enable MR Modular Architecture deployment #4450
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
WalkthroughThis change introduces and configures module federation support across the frontend and deployment manifests, including updates to webpack configs, error handling, Cypress test mocking, and Kubernetes manifests for modular architecture. It adds a multi-stage Dockerfile for the model registry UI/BFF, new configuration files, and enhanced test/deployment integration for module federation. Changes
Sequence Diagram(s)sequenceDiagram
participant Cypress Test
participant ModuleFederationMock
participant App
Cypress Test->>ModuleFederationMock: setupModuleFederationMocks()
ModuleFederationMock-->>Cypress Test: Intercept /_mf/** requests
Cypress Test->>App: Run test
App->>/_mf/modelRegistry/remoteEntry.js: Request remote entry
ModuleFederationMock-->>App: Respond with mock remote entry or 404
sequenceDiagram
participant Webpack
participant Env
participant ModuleFederationPlugins
Webpack->>Env: Read MF_CONFIG from EnvironmentPlugin
Webpack->>ModuleFederationPlugins: Integrate plugins into build
Env-->>Webpack: Provide module federation config (unless CYPRESS_TESTS=true)
Estimated code review effort3 (~40 minutes) Possibly related PRs
Suggested labels
Poem
✨ Finishing Touches
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Pull Request Overview
This PR enhances the model-registry frontend by integrating module federation into the Webpack build, introducing a multi-stage Dockerfile for UI and BFF components, and adding Kubernetes manifests (with kustomize overlays) to deploy the federated UI.
- Added a ConfigMap and injected its JSON into deployments for module federation
- Created Dockerfile stages for building and packaging the UI and Go BFF
- Introduced base and overlay Kubernetes manifests for the
model-registry-ui(service, deployment, RBAC, kustomization)
Reviewed Changes
Copilot reviewed 12 out of 12 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| manifests/core-bases/base/federation-configmap.yaml | Added ConfigMap for module federation settings |
| manifests/core-bases/base/deployment.yaml | Injected federation-config env var and declared volume |
| frontend/packages/model-registry/manifests/rhoai/model-registry-ui-deployment.yaml | Added overlay args patch for rhoai environment |
| frontend/packages/model-registry/manifests/rhoai/kustomization.yaml | Defined rhoai kustomize overlay |
| frontend/packages/model-registry/manifests/base/model-registry-ui-service.yaml | New Service for UI |
| frontend/packages/model-registry/manifests/base/model-registry-ui-service-account.yaml | Created ServiceAccount for UI |
| frontend/packages/model-registry/manifests/base/model-registry-ui-role.yaml | Kept commented RBAC references (deprecated) |
| frontend/packages/model-registry/manifests/base/model-registry-ui-deployment.yaml | Base Deployment for UI with probes and args |
| frontend/packages/model-registry/manifests/base/kustomization.yaml | Base kustomization referencing UI resources |
| frontend/packages/model-registry/Dockerfile | Multi-stage Dockerfile for UI and Go BFF |
| frontend/config/webpack.dev.js | Removed federation plugins from dev config |
| frontend/config/webpack.common.js | Added federation plugins and env var in common config |
Comments suppressed due to low confidence (3)
manifests/core-bases/base/deployment.yaml:144
- You declare a
federation-configvolume but never mount it into the container. Add a correspondingvolumeMountsentry under the container spec (e.g., mount path/etc/federation-config).
- name: federation-config
frontend/packages/model-registry/manifests/rhoai/model-registry-ui-deployment.yaml:1
- The base deployment already defines
args, so usingop: addhere will fail. Change the patch toop: replaceto override the existingargsarray.
- op: add
frontend/packages/model-registry/manifests/base/model-registry-ui-role.yaml:1
- [nitpick] This file contains entirely commented-out RBAC definitions. Consider removing or moving these to documentation to reduce clutter in the manifests directory.
# Leaving it here for reference, it should not be needed when using the user token.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🔭 Outside diff range comments (1)
frontend/packages/model-registry/manifests/base/model-registry-ui-deployment.yaml (1)
54-66: Fix YAML indentation issues.Static analysis detected incorrect indentation for the ports, args, and securityContext sections.
- - containerPort: 8080 - args: - - "--deployment-mode=federated" - - "--auth-method=user_token" - - "--auth-token-header=x-forwarded-access-token" - - "--auth-token-prefix=" - - "--port=8080" - securityContext: - allowPrivilegeEscalation: false - capabilities: - drop: - - ALL + - containerPort: 8080 + args: + - "--deployment-mode=federated" + - "--auth-method=user_token" + - "--auth-token-header=x-forwarded-access-token" + - "--auth-token-prefix=" + - "--port=8080" + securityContext: + allowPrivilegeEscalation: false + capabilities: + drop: + - ALL
🧹 Nitpick comments (6)
frontend/packages/model-registry/manifests/rhoai/model-registry-ui-deployment.yaml (1)
3-3: Fix trailing spaces.Static analysis detected trailing spaces on this line.
Apply this diff to remove trailing spaces:
- value: + value:frontend/packages/model-registry/manifests/base/kustomization.yaml (1)
10-13: Consider using a specific image tag instead of 'latest' for production deployments.Using the
latesttag can lead to deployment inconsistencies and makes rollbacks difficult. Consider using semantic versioning or commit-based tags for better deployment control.images: - name: model-registry-ui newName: quay.io/lferrnan/model-registry-ui-federated - newTag: latest + newTag: v1.0.0 # or commit hashfrontend/packages/model-registry/manifests/base/model-registry-ui-deployment.yaml (1)
46-52: Consider optimizing resource allocation.The resource limits and requests are identical (500m CPU, 2Gi memory). Consider setting lower requests to allow better resource utilization while maintaining the same limits for protection.
resources: limits: cpu: 500m memory: 2Gi requests: - cpu: 500m - memory: 2Gi + cpu: 100m + memory: 512Mifrontend/packages/model-registry/Dockerfile (2)
22-24: Remove one-off debugls -lato keep layers cleanThe extra layer provides no value once the image is stable and makes the history noisier.
-RUN ls -la /usr/src/app
19-29: Leverage Docker layer caching for faster rebuildsInstall deps before copying the whole source tree so that code changes don’t invalidate the npm cache on every build.
-COPY ${UI_SOURCE_CODE} /usr/src/app -# … -RUN npm ci --omit=optional -RUN npm run build:prod +# Copy manifest files first for better cache utilisation +COPY ${UI_SOURCE_CODE}/package*.json /usr/src/app/ +RUN npm ci --omit=optional + +# Now copy the rest of the sources and build +COPY ${UI_SOURCE_CODE} /usr/src/app +RUN npm run build:prodAlso add a
.dockerignoreto excludenode_modules, test artifacts and.gitto keep the context small.frontend/packages/model-registry/manifests/base/model-registry-ui-role.yaml (1)
1-78: Dead-code RBAC manifest: delete or move to docsThe entire file is commented out and therefore unused. Keeping inert manifests in the kustomize tree can confuse future maintainers and tooling.
• Drop the file entirely, or
• Move it todocs/with a short README explaining when it might be needed.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (12)
frontend/config/webpack.common.js(2 hunks)frontend/config/webpack.dev.js(1 hunks)frontend/packages/model-registry/Dockerfile(1 hunks)frontend/packages/model-registry/manifests/base/kustomization.yaml(1 hunks)frontend/packages/model-registry/manifests/base/model-registry-ui-deployment.yaml(1 hunks)frontend/packages/model-registry/manifests/base/model-registry-ui-role.yaml(1 hunks)frontend/packages/model-registry/manifests/base/model-registry-ui-service-account.yaml(1 hunks)frontend/packages/model-registry/manifests/base/model-registry-ui-service.yaml(1 hunks)frontend/packages/model-registry/manifests/rhoai/kustomization.yaml(1 hunks)frontend/packages/model-registry/manifests/rhoai/model-registry-ui-deployment.yaml(1 hunks)manifests/core-bases/base/deployment.yaml(2 hunks)manifests/core-bases/base/federation-configmap.yaml(1 hunks)
🧰 Additional context used
🧠 Learnings (9)
📓 Common learnings
Learnt from: ConorOM1
PR: opendatahub-io/odh-dashboard#4423
File: frontend/src/__tests__/cypress/cypress/fixtures/resources/yaml/model_registry.yaml:12-12
Timestamp: 2025-06-27T11:00:06.871Z
Learning: For ModelRegistry resources in `frontend/src/__tests__/cypress/cypress/fixtures/resources/yaml/model_registry.yaml`, the `oauthProxy: {}` empty configuration is sufficient to enable the OAuth proxy. The ModelRegistry operator enables the proxy by default when this key is present, without requiring explicit `enabled: true` flags or image specifications.
frontend/packages/model-registry/manifests/base/model-registry-ui-service-account.yaml (1)
Learnt from: ConorOM1
PR: opendatahub-io/odh-dashboard#4423
File: frontend/src/__tests__/cypress/cypress/fixtures/resources/yaml/model_registry.yaml:12-12
Timestamp: 2025-06-27T11:00:06.871Z
Learning: For ModelRegistry resources in `frontend/src/__tests__/cypress/cypress/fixtures/resources/yaml/model_registry.yaml`, the `oauthProxy: {}` empty configuration is sufficient to enable the OAuth proxy. The ModelRegistry operator enables the proxy by default when this key is present, without requiring explicit `enabled: true` flags or image specifications.
frontend/packages/model-registry/manifests/base/model-registry-ui-service.yaml (1)
Learnt from: ConorOM1
PR: opendatahub-io/odh-dashboard#4423
File: frontend/src/__tests__/cypress/cypress/fixtures/resources/yaml/model_registry.yaml:12-12
Timestamp: 2025-06-27T11:00:06.871Z
Learning: For ModelRegistry resources in `frontend/src/__tests__/cypress/cypress/fixtures/resources/yaml/model_registry.yaml`, the `oauthProxy: {}` empty configuration is sufficient to enable the OAuth proxy. The ModelRegistry operator enables the proxy by default when this key is present, without requiring explicit `enabled: true` flags or image specifications.
manifests/core-bases/base/federation-configmap.yaml (1)
Learnt from: ConorOM1
PR: opendatahub-io/odh-dashboard#4423
File: frontend/src/__tests__/cypress/cypress/fixtures/resources/yaml/model_registry.yaml:12-12
Timestamp: 2025-06-27T11:00:06.871Z
Learning: For ModelRegistry resources in `frontend/src/__tests__/cypress/cypress/fixtures/resources/yaml/model_registry.yaml`, the `oauthProxy: {}` empty configuration is sufficient to enable the OAuth proxy. The ModelRegistry operator enables the proxy by default when this key is present, without requiring explicit `enabled: true` flags or image specifications.
frontend/packages/model-registry/manifests/rhoai/kustomization.yaml (1)
Learnt from: ConorOM1
PR: opendatahub-io/odh-dashboard#4423
File: frontend/src/__tests__/cypress/cypress/fixtures/resources/yaml/model_registry.yaml:12-12
Timestamp: 2025-06-27T11:00:06.871Z
Learning: For ModelRegistry resources in `frontend/src/__tests__/cypress/cypress/fixtures/resources/yaml/model_registry.yaml`, the `oauthProxy: {}` empty configuration is sufficient to enable the OAuth proxy. The ModelRegistry operator enables the proxy by default when this key is present, without requiring explicit `enabled: true` flags or image specifications.
frontend/packages/model-registry/manifests/rhoai/model-registry-ui-deployment.yaml (1)
Learnt from: ConorOM1
PR: opendatahub-io/odh-dashboard#4423
File: frontend/src/__tests__/cypress/cypress/fixtures/resources/yaml/model_registry.yaml:12-12
Timestamp: 2025-06-27T11:00:06.871Z
Learning: For ModelRegistry resources in `frontend/src/__tests__/cypress/cypress/fixtures/resources/yaml/model_registry.yaml`, the `oauthProxy: {}` empty configuration is sufficient to enable the OAuth proxy. The ModelRegistry operator enables the proxy by default when this key is present, without requiring explicit `enabled: true` flags or image specifications.
frontend/packages/model-registry/manifests/base/model-registry-ui-deployment.yaml (1)
Learnt from: ConorOM1
PR: opendatahub-io/odh-dashboard#4423
File: frontend/src/__tests__/cypress/cypress/fixtures/resources/yaml/model_registry.yaml:12-12
Timestamp: 2025-06-27T11:00:06.871Z
Learning: For ModelRegistry resources in `frontend/src/__tests__/cypress/cypress/fixtures/resources/yaml/model_registry.yaml`, the `oauthProxy: {}` empty configuration is sufficient to enable the OAuth proxy. The ModelRegistry operator enables the proxy by default when this key is present, without requiring explicit `enabled: true` flags or image specifications.
frontend/packages/model-registry/manifests/base/model-registry-ui-role.yaml (1)
Learnt from: ConorOM1
PR: opendatahub-io/odh-dashboard#4423
File: frontend/src/__tests__/cypress/cypress/fixtures/resources/yaml/model_registry.yaml:12-12
Timestamp: 2025-06-27T11:00:06.871Z
Learning: For ModelRegistry resources in `frontend/src/__tests__/cypress/cypress/fixtures/resources/yaml/model_registry.yaml`, the `oauthProxy: {}` empty configuration is sufficient to enable the OAuth proxy. The ModelRegistry operator enables the proxy by default when this key is present, without requiring explicit `enabled: true` flags or image specifications.
frontend/packages/model-registry/manifests/base/kustomization.yaml (1)
Learnt from: ConorOM1
PR: opendatahub-io/odh-dashboard#4423
File: frontend/src/__tests__/cypress/cypress/fixtures/resources/yaml/model_registry.yaml:12-12
Timestamp: 2025-06-27T11:00:06.871Z
Learning: For ModelRegistry resources in `frontend/src/__tests__/cypress/cypress/fixtures/resources/yaml/model_registry.yaml`, the `oauthProxy: {}` empty configuration is sufficient to enable the OAuth proxy. The ModelRegistry operator enables the proxy by default when this key is present, without requiring explicit `enabled: true` flags or image specifications.
🧬 Code Graph Analysis (2)
frontend/config/webpack.dev.js (2)
frontend/config/webpack.common.js (2)
require(7-7)require(9-9)frontend/config/moduleFederation.js (2)
require(1-1)require(2-2)
frontend/config/webpack.common.js (2)
frontend/config/moduleFederation.js (2)
require(1-1)require(2-2)frontend/packages/model-registry/upstream/frontend/config/moduleFederation.js (1)
moduleFederationConfig(6-32)
🪛 YAMLlint (1.37.1)
frontend/packages/model-registry/manifests/rhoai/model-registry-ui-deployment.yaml
[error] 3-3: trailing spaces
(trailing-spaces)
frontend/packages/model-registry/manifests/base/model-registry-ui-deployment.yaml
[error] 16-16: trailing spaces
(trailing-spaces)
[warning] 54-54: wrong indentation: expected 8 but found 10
(indentation)
[warning] 56-56: wrong indentation: expected 8 but found 10
(indentation)
[warning] 65-65: wrong indentation: expected 12 but found 14
(indentation)
🔇 Additional comments (13)
frontend/packages/model-registry/manifests/base/model-registry-ui-service-account.yaml (1)
1-6: LGTM! ServiceAccount follows Kubernetes best practices.The ServiceAccount definition is minimal but appropriate for a base kustomize resource. The absence of a namespace allows overlays to specify the target namespace as needed.
frontend/packages/model-registry/manifests/rhoai/model-registry-ui-deployment.yaml (1)
4-6: Configuration arguments look correct.The container arguments appropriately configure the model registry UI for the RHOAI environment with disabled standalone mode and correct port mapping.
frontend/packages/model-registry/manifests/base/model-registry-ui-service.yaml (1)
1-17: Service configuration follows Kubernetes best practices.The Service correctly:
- Uses ClusterIP type for internal cluster access
- Maps port 8080 to targetPort 8080, consistent with the deployment configuration
- Includes appropriate labels and selectors
- Follows naming conventions
frontend/config/webpack.dev.js (1)
14-14: Good refactor to consolidate module federation configuration.Moving the module federation plugins to the common webpack configuration improves consistency across environments while keeping the dev config focused on development-specific settings.
frontend/packages/model-registry/manifests/rhoai/kustomization.yaml (1)
1-14: Kustomization follows standard patterns.The configuration correctly:
- References the base resources directory
- Applies the deployment patch with proper target specification
- Uses standard Kustomize structure and syntax
manifests/core-bases/base/deployment.yaml (1)
34-39: LGTM! Environment variable correctly configured from ConfigMap.The MODULE_FEDERATION_CONFIG environment variable is properly configured to read from the federation-config ConfigMap.
frontend/config/webpack.common.js (3)
6-6: LGTM! Webpack import added for EnvironmentPlugin.The webpack import is correctly added to support the EnvironmentPlugin for module federation configuration.
9-9: LGTM! Module federation imports added.The moduleFederationConfig and moduleFederationPlugins imports are correctly added to support module federation integration.
237-240: LGTM! Module federation configuration properly integrated.The EnvironmentPlugin correctly stringifies the moduleFederationConfig and injects it as MF_CONFIG environment variable, and the moduleFederationPlugins are properly spread into the plugins array.
frontend/packages/model-registry/manifests/base/model-registry-ui-deployment.yaml (3)
18-21: LGTM! Excellent security context configuration.The security context properly enforces RuntimeDefault seccomp profile and non-root user execution, following security best practices.
55-60: LGTM! Container arguments properly configured for federated deployment.The container arguments are correctly configured for federated deployment mode with appropriate authentication settings.
61-65: LGTM! Container security context follows security best practices.The container security context properly disables privilege escalation and drops all capabilities, following the principle of least privilege.
frontend/packages/model-registry/Dockerfile (1)
6-8: Verify Go base image tag exists before merge
golang:1.24.3does not exist at the time of writing (latest released minor is 1.23). The build will fail once this hits CI/registry.Would you run a quick
docker pull golang:1.24.3in CI to confirm the tag is available, or bump to the latest released tag?
frontend/packages/model-registry/manifests/base/model-registry-ui-deployment.yaml
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
frontend/packages/model-registry/manifests/rhoai/model-registry-ui-deployment.yaml (1)
3-3: Remove trailing whitespace aftervalue:
YAML lint flags line 3 for trailing spaces. While harmless to Kustomize, it is noise and fails strict linters in some CI setups.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (11)
frontend/config/webpack.common.js(2 hunks)frontend/config/webpack.dev.js(1 hunks)frontend/packages/model-registry/manifests/base/kustomization.yaml(1 hunks)frontend/packages/model-registry/manifests/base/model-registry-ui-deployment.yaml(1 hunks)frontend/packages/model-registry/manifests/base/model-registry-ui-role.yaml(1 hunks)frontend/packages/model-registry/manifests/base/model-registry-ui-service-account.yaml(1 hunks)frontend/packages/model-registry/manifests/base/model-registry-ui-service.yaml(1 hunks)frontend/packages/model-registry/manifests/rhoai/kustomization.yaml(1 hunks)frontend/packages/model-registry/manifests/rhoai/model-registry-ui-deployment.yaml(1 hunks)manifests/core-bases/base/deployment.yaml(2 hunks)manifests/core-bases/base/federation-configmap.yaml(1 hunks)
✅ Files skipped from review due to trivial changes (3)
- frontend/packages/model-registry/manifests/base/kustomization.yaml
- frontend/config/webpack.dev.js
- manifests/core-bases/base/federation-configmap.yaml
🚧 Files skipped from review as they are similar to previous changes (7)
- frontend/packages/model-registry/manifests/base/model-registry-ui-service-account.yaml
- manifests/core-bases/base/deployment.yaml
- frontend/packages/model-registry/manifests/base/model-registry-ui-service.yaml
- frontend/packages/model-registry/manifests/rhoai/kustomization.yaml
- frontend/config/webpack.common.js
- frontend/packages/model-registry/manifests/base/model-registry-ui-role.yaml
- frontend/packages/model-registry/manifests/base/model-registry-ui-deployment.yaml
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: ConorOM1
PR: opendatahub-io/odh-dashboard#4423
File: frontend/src/__tests__/cypress/cypress/fixtures/resources/yaml/model_registry.yaml:12-12
Timestamp: 2025-06-27T11:00:06.871Z
Learning: For ModelRegistry resources in `frontend/src/__tests__/cypress/cypress/fixtures/resources/yaml/model_registry.yaml`, the `oauthProxy: {}` empty configuration is sufficient to enable the OAuth proxy. The ModelRegistry operator enables the proxy by default when this key is present, without requiring explicit `enabled: true` flags or image specifications.
frontend/packages/model-registry/manifests/rhoai/model-registry-ui-deployment.yaml (1)
Learnt from: ConorOM1
PR: opendatahub-io/odh-dashboard#4423
File: frontend/src/__tests__/cypress/cypress/fixtures/resources/yaml/model_registry.yaml:12-12
Timestamp: 2025-06-27T11:00:06.871Z
Learning: For ModelRegistry resources in `frontend/src/__tests__/cypress/cypress/fixtures/resources/yaml/model_registry.yaml`, the `oauthProxy: {}` empty configuration is sufficient to enable the OAuth proxy. The ModelRegistry operator enables the proxy by default when this key is present, without requiring explicit `enabled: true` flags or image specifications.
🪛 YAMLlint (1.37.1)
frontend/packages/model-registry/manifests/rhoai/model-registry-ui-deployment.yaml
[error] 3-3: trailing spaces
(trailing-spaces)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: Lint
- GitHub Check: Cypress-Setup
frontend/packages/model-registry/manifests/rhoai/model-registry-ui-deployment.yaml
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
frontend/packages/model-registry/manifests/rhoai/model-registry-ui-deployment.yaml (1)
1-6: Patch still targets container by index and overwrites the wholeargsarraySame concerns raised in the previous review remain unaddressed:
•/containers/0is brittle if a side-car is ever injected ahead of the UI container.
•addon/argswill fail whenargsalready exists, or will obliterate existing flags.Please switch to either a strategic-merge patch keyed by
name: model-registry-uior JSON6902 ops that append to the existing array. See the earlier suggestions for concrete snippets.
🧹 Nitpick comments (1)
frontend/packages/model-registry/manifests/rhoai/model-registry-ui-deployment.yaml (1)
3-3: Remove trailing whitespace aftervalue:YAML-lint flags this line for trailing spaces. It’s harmless at runtime but noisy in CI linters.
- value: + value:
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (11)
frontend/config/webpack.common.js(2 hunks)frontend/config/webpack.dev.js(1 hunks)frontend/packages/model-registry/manifests/base/kustomization.yaml(1 hunks)frontend/packages/model-registry/manifests/base/model-registry-ui-deployment.yaml(1 hunks)frontend/packages/model-registry/manifests/base/model-registry-ui-role.yaml(1 hunks)frontend/packages/model-registry/manifests/base/model-registry-ui-service-account.yaml(1 hunks)frontend/packages/model-registry/manifests/base/model-registry-ui-service.yaml(1 hunks)frontend/packages/model-registry/manifests/rhoai/kustomization.yaml(1 hunks)frontend/packages/model-registry/manifests/rhoai/model-registry-ui-deployment.yaml(1 hunks)manifests/core-bases/base/deployment.yaml(1 hunks)manifests/core-bases/base/federation-configmap.yaml(1 hunks)
✅ Files skipped from review due to trivial changes (2)
- manifests/core-bases/base/deployment.yaml
- frontend/config/webpack.dev.js
🚧 Files skipped from review as they are similar to previous changes (8)
- frontend/packages/model-registry/manifests/base/model-registry-ui-service-account.yaml
- frontend/packages/model-registry/manifests/base/model-registry-ui-service.yaml
- frontend/packages/model-registry/manifests/base/kustomization.yaml
- frontend/config/webpack.common.js
- manifests/core-bases/base/federation-configmap.yaml
- frontend/packages/model-registry/manifests/base/model-registry-ui-deployment.yaml
- frontend/packages/model-registry/manifests/rhoai/kustomization.yaml
- frontend/packages/model-registry/manifests/base/model-registry-ui-role.yaml
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: ConorOM1
PR: opendatahub-io/odh-dashboard#4423
File: frontend/src/__tests__/cypress/cypress/fixtures/resources/yaml/model_registry.yaml:12-12
Timestamp: 2025-06-27T11:00:06.871Z
Learning: For ModelRegistry resources in `frontend/src/__tests__/cypress/cypress/fixtures/resources/yaml/model_registry.yaml`, the `oauthProxy: {}` empty configuration is sufficient to enable the OAuth proxy. The ModelRegistry operator enables the proxy by default when this key is present, without requiring explicit `enabled: true` flags or image specifications.
frontend/packages/model-registry/manifests/rhoai/model-registry-ui-deployment.yaml (1)
Learnt from: ConorOM1
PR: opendatahub-io/odh-dashboard#4423
File: frontend/src/__tests__/cypress/cypress/fixtures/resources/yaml/model_registry.yaml:12-12
Timestamp: 2025-06-27T11:00:06.871Z
Learning: For ModelRegistry resources in `frontend/src/__tests__/cypress/cypress/fixtures/resources/yaml/model_registry.yaml`, the `oauthProxy: {}` empty configuration is sufficient to enable the OAuth proxy. The ModelRegistry operator enables the proxy by default when this key is present, without requiring explicit `enabled: true` flags or image specifications.
🪛 YAMLlint (1.37.1)
frontend/packages/model-registry/manifests/rhoai/model-registry-ui-deployment.yaml
[error] 3-3: trailing spaces
(trailing-spaces)
lucferbux
force-pushed
the
rhoaieng-25523
branch
2 times, most recently
from
17694c4 to
6b372c8
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
frontend/packages/model-registry/manifests/rhoai/model-registry-ui-deployment.yaml (1)
1-6: Index-based, full-array replacement patch is still brittleThis JSON6902 patch keeps targeting
/containers/0/args, replacing the entireargsarray in one shot and relying on the container being at index 0. We already raised this in a prior review; nothing has changed.A safer pattern is either:
- strategic-merge patch keyed by
name: model-registry-ui, or- per-item JSON6902 ops that append via
/args/-so existing args survive and sidecars won’t break you.Example fix (option 2):
- - op: add - path: /spec/template/spec/containers/0/args - value: - - "--standalone-mode=false" - - "--default-platform=false" - - "--port=8080" + - op: add + path: /spec/template/spec/containers/0/args/- + value: "--standalone-mode=false" + - op: add + path: /spec/template/spec/containers/0/args/- + value: "--default-platform=false" + - op: add + path: /spec/template/spec/containers/0/args/- + value: "--port=8080"(Also remember to declare
patchType: json6902and target bykind/nameinkustomization.yaml.)
🧹 Nitpick comments (1)
frontend/packages/model-registry/manifests/rhoai/model-registry-ui-deployment.yaml (1)
3-3: Trailing whitespace – clean it upLine 3 has a trailing space after
value:flagged by YAMLlint. Remove it to keep manifests lint-clean.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (12)
frontend/config/webpack.common.js(2 hunks)frontend/config/webpack.dev.js(1 hunks)frontend/packages/model-registry/Dockerfile(1 hunks)frontend/packages/model-registry/manifests/base/kustomization.yaml(1 hunks)frontend/packages/model-registry/manifests/base/model-registry-ui-deployment.yaml(1 hunks)frontend/packages/model-registry/manifests/base/model-registry-ui-network-policy.yaml(1 hunks)frontend/packages/model-registry/manifests/base/model-registry-ui-service-account.yaml(1 hunks)frontend/packages/model-registry/manifests/base/model-registry-ui-service.yaml(1 hunks)frontend/packages/model-registry/manifests/rhoai/kustomization.yaml(1 hunks)frontend/packages/model-registry/manifests/rhoai/model-registry-ui-deployment.yaml(1 hunks)manifests/core-bases/base/deployment.yaml(1 hunks)manifests/core-bases/base/federation-configmap.yaml(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- frontend/packages/model-registry/manifests/base/model-registry-ui-network-policy.yaml
🚧 Files skipped from review as they are similar to previous changes (10)
- frontend/packages/model-registry/manifests/base/model-registry-ui-service-account.yaml
- frontend/config/webpack.dev.js
- frontend/packages/model-registry/manifests/base/kustomization.yaml
- frontend/packages/model-registry/manifests/base/model-registry-ui-deployment.yaml
- manifests/core-bases/base/deployment.yaml
- frontend/packages/model-registry/manifests/rhoai/kustomization.yaml
- frontend/packages/model-registry/manifests/base/model-registry-ui-service.yaml
- frontend/config/webpack.common.js
- manifests/core-bases/base/federation-configmap.yaml
- frontend/packages/model-registry/Dockerfile
🧰 Additional context used
🧠 Learnings (1)
frontend/packages/model-registry/manifests/rhoai/model-registry-ui-deployment.yaml (1)
Learnt from: ConorOM1
PR: opendatahub-io/odh-dashboard#4423
File: frontend/src/__tests__/cypress/cypress/fixtures/resources/yaml/model_registry.yaml:12-12
Timestamp: 2025-06-27T11:00:06.871Z
Learning: For ModelRegistry resources in `frontend/src/__tests__/cypress/cypress/fixtures/resources/yaml/model_registry.yaml`, the `oauthProxy: {}` empty configuration is sufficient to enable the OAuth proxy. The ModelRegistry operator enables the proxy by default when this key is present, without requiring explicit `enabled: true` flags or image specifications.
🪛 YAMLlint (1.37.1)
frontend/packages/model-registry/manifests/rhoai/model-registry-ui-deployment.yaml
[error] 3-3: trailing spaces
(trailing-spaces)
lucferbux
force-pushed
the
rhoaieng-25523
branch
2 times, most recently
from
9ddb103 to
692680e
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
frontend/packages/model-registry/manifests/rhoai/model-registry-ui-deployment.yaml (1)
1-6: Patch still targets container index 0 and overwritesargswholesaleThe JSON6902 op is unchanged from the earlier revision:
path: /spec/template/spec/containers/0/argsassumes the UI container will forever be at index 0 and replaces any existingargs, so a future side-car or a base manifest update will silently break this overlay.Recommended fix (same as previous review):
# strategic-merge (safer) apiVersion: apps/v1 kind: Deployment metadata: name: model-registry-ui spec: template: spec: containers: - name: model-registry-ui args: - "--standalone-mode=false" - "--default-platform=false" - "--port=8080"or append each flag via JSON6902 on
/args/-after specifyingpatchType: json6902inkustomization.yaml.Also remove the trailing space flagged by yamllint on line 3 while you are touching the file.
🧹 Nitpick comments (1)
frontend/packages/model-registry/manifests/rhoai/model-registry-ui-deployment.yaml (1)
3-3: Trailing whitespaceLine 3 has a superfluous trailing space that trips
yamllint—clean it up.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (13)
frontend/config/webpack.common.js(2 hunks)frontend/config/webpack.dev.js(1 hunks)frontend/packages/model-registry/Dockerfile(1 hunks)frontend/packages/model-registry/manifests/base/kustomization.yaml(1 hunks)frontend/packages/model-registry/manifests/base/model-registry-ui-deployment.yaml(1 hunks)frontend/packages/model-registry/manifests/base/model-registry-ui-network-policy.yaml(1 hunks)frontend/packages/model-registry/manifests/base/model-registry-ui-service-account.yaml(1 hunks)frontend/packages/model-registry/manifests/base/model-registry-ui-service.yaml(1 hunks)frontend/packages/model-registry/manifests/rhoai/kustomization.yaml(1 hunks)frontend/packages/model-registry/manifests/rhoai/model-registry-ui-deployment.yaml(1 hunks)frontend/src/utilities/const.ts(2 hunks)manifests/core-bases/base/deployment.yaml(1 hunks)manifests/core-bases/base/federation-configmap.yaml(1 hunks)
✅ Files skipped from review due to trivial changes (2)
- frontend/config/webpack.dev.js
- frontend/src/utilities/const.ts
🚧 Files skipped from review as they are similar to previous changes (10)
- frontend/packages/model-registry/manifests/base/model-registry-ui-service-account.yaml
- frontend/packages/model-registry/manifests/base/kustomization.yaml
- manifests/core-bases/base/federation-configmap.yaml
- frontend/config/webpack.common.js
- frontend/packages/model-registry/manifests/base/model-registry-ui-service.yaml
- manifests/core-bases/base/deployment.yaml
- frontend/packages/model-registry/manifests/base/model-registry-ui-network-policy.yaml
- frontend/packages/model-registry/manifests/rhoai/kustomization.yaml
- frontend/packages/model-registry/manifests/base/model-registry-ui-deployment.yaml
- frontend/packages/model-registry/Dockerfile
🧰 Additional context used
🧠 Learnings (1)
frontend/packages/model-registry/manifests/rhoai/model-registry-ui-deployment.yaml (1)
Learnt from: ConorOM1
PR: opendatahub-io/odh-dashboard#4423
File: frontend/src/__tests__/cypress/cypress/fixtures/resources/yaml/model_registry.yaml:12-12
Timestamp: 2025-06-27T11:00:06.871Z
Learning: For ModelRegistry resources in `frontend/src/__tests__/cypress/cypress/fixtures/resources/yaml/model_registry.yaml`, the `oauthProxy: {}` empty configuration is sufficient to enable the OAuth proxy. The ModelRegistry operator enables the proxy by default when this key is present, without requiring explicit `enabled: true` flags or image specifications.
🪛 YAMLlint (1.37.1)
frontend/packages/model-registry/manifests/rhoai/model-registry-ui-deployment.yaml
[error] 3-3: trailing spaces
(trailing-spaces)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Lint
frontend/packages/model-registry/manifests/rhoai/model-registry-ui-deployment.yaml
Outdated
Show resolved
Hide resolved
frontend/packages/model-registry/manifests/rhoai/kustomization.yaml
Outdated
Show resolved
Hide resolved
lucferbux
force-pushed
the
rhoaieng-25523
branch
2 times, most recently
from
03be2a2 to
8dd3ed6
Compare
|
@ederign @christianvogt @andrewballantyne (FYI) This is already ready to go, I would merge it right away, while I present the options to the architects, this is only for us to onboard the new images to konflux, then I can get back and tweak it depending if we want multiple containers in one pod or separate pods. Or even if we would want to move the manifests to other repos. |
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
frontend/packages/model-registry/manifests/base/model-registry-ui-service.yaml (1)
14-17: Consider referencing the container port by name instead of number.Using
targetPort: https(the container port name defined in the Deployment) rather than its numeric value improves readability and reduces drift if the numeric port is ever changed.- targetPort: 8443 + targetPort: httpsfrontend/packages/model-registry/manifests/base/model-registry-ui-deployment.yaml (1)
46-52: Requests equal limits → guaranteed QoS; double-check necessity.Setting CPU/memory requests equal to limits places the pod in Guaranteed QoS, which can starve other workloads and is usually reserved for critical system pods.
Consider loweringrequests(e.g. 250 m CPU / 1 Gi) unless the UI truly needs the full allocation at all times.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (11)
frontend/config/webpack.common.js(2 hunks)frontend/config/webpack.dev.js(1 hunks)frontend/packages/model-registry/Dockerfile(1 hunks)frontend/packages/model-registry/manifests/base/kustomization.yaml(1 hunks)frontend/packages/model-registry/manifests/base/model-registry-ui-deployment.yaml(1 hunks)frontend/packages/model-registry/manifests/base/model-registry-ui-network-policy.yaml(1 hunks)frontend/packages/model-registry/manifests/base/model-registry-ui-service-account.yaml(1 hunks)frontend/packages/model-registry/manifests/base/model-registry-ui-service.yaml(1 hunks)frontend/src/utilities/const.ts(2 hunks)manifests/core-bases/base/deployment.yaml(1 hunks)manifests/core-bases/base/federation-configmap.yaml(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- frontend/config/webpack.dev.js
🚧 Files skipped from review as they are similar to previous changes (8)
- manifests/core-bases/base/deployment.yaml
- frontend/packages/model-registry/manifests/base/model-registry-ui-service-account.yaml
- frontend/packages/model-registry/manifests/base/kustomization.yaml
- manifests/core-bases/base/federation-configmap.yaml
- frontend/config/webpack.common.js
- frontend/packages/model-registry/manifests/base/model-registry-ui-network-policy.yaml
- frontend/src/utilities/const.ts
- frontend/packages/model-registry/Dockerfile
🧰 Additional context used
🧠 Learnings (2)
frontend/packages/model-registry/manifests/base/model-registry-ui-service.yaml (1)
Learnt from: ConorOM1
PR: opendatahub-io/odh-dashboard#4423
File: frontend/src/__tests__/cypress/cypress/fixtures/resources/yaml/model_registry.yaml:12-12
Timestamp: 2025-06-27T11:00:06.871Z
Learning: For ModelRegistry resources in `frontend/src/__tests__/cypress/cypress/fixtures/resources/yaml/model_registry.yaml`, the `oauthProxy: {}` empty configuration is sufficient to enable the OAuth proxy. The ModelRegistry operator enables the proxy by default when this key is present, without requiring explicit `enabled: true` flags or image specifications.
frontend/packages/model-registry/manifests/base/model-registry-ui-deployment.yaml (1)
Learnt from: ConorOM1
PR: opendatahub-io/odh-dashboard#4423
File: frontend/src/__tests__/cypress/cypress/fixtures/resources/yaml/model_registry.yaml:12-12
Timestamp: 2025-06-27T11:00:06.871Z
Learning: For ModelRegistry resources in `frontend/src/__tests__/cypress/cypress/fixtures/resources/yaml/model_registry.yaml`, the `oauthProxy: {}` empty configuration is sufficient to enable the OAuth proxy. The ModelRegistry operator enables the proxy by default when this key is present, without requiring explicit `enabled: true` flags or image specifications.
🔇 Additional comments (2)
frontend/packages/model-registry/manifests/base/model-registry-ui-service.yaml (1)
8-9: Verify the service-CA generated secret is available before pod start-up.Both the Service annotation and the Deployment mount the
model-registry-ui-tlssecret. In OpenShift this secret is created asynchronously by the service-CA controller; the first pod start can fail if the secret isn’t ready.
Ensure your rollout strategy (e.g. aninitContainerwait, or pre-created secret in CI) accounts for this race.Also applies to: 74-76
frontend/packages/model-registry/manifests/base/model-registry-ui-deployment.yaml (1)
26-45: HTTPS health probes may fail without skipping TLS verification.The probes hit
https://<pod>:8443/healthcheck; if the container serves with a self-signed service-CA certificate, kubelet’s probe client will reject it.
Common fixes:
• Switchscheme: HTTPand useport: 8080for an internal plain-text health endpoint, or
• Add--health-probe-insecure-skip-tlsflag in the application if supported.
frontend/packages/model-registry/manifests/base/model-registry-ui-deployment.yaml
Outdated
Show resolved
Hide resolved
frontend/packages/model-registry/manifests/base/model-registry-ui-deployment.yaml
Outdated
Show resolved
Hide resolved
|
@christianvogt It seems that the issue was bigger than I expected, when we moved module federation from the dev config to common, it affected not only our prod but also cypress tests. I debugged it (I've left some of the exception handling cause I think it might be useful in case there are some issues) and hten built a solution with the help of Claude. I'm open to suggestions for that implementation. |
lucferbux
force-pushed
the
rhoaieng-25523
branch
from
f3b5074 to
e542b26
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
frontend/src/plugins/useAppExtensions.ts (1)
52-52: Optimize performance: Avoid spread operator in reduce accumulator.The spread operator in the reduce function creates O(n²) time complexity. Consider using a more efficient approach.
- setAppExtensions((prev) => - results.reduce((acc, r) => ({ ...acc, ...r.value }), { ...prev }), - ); + setAppExtensions((prev) => { + const newExtensions = { ...prev }; + results.forEach((r) => { + Object.assign(newExtensions, r.value); + }); + return newExtensions; + });frontend/src/__tests__/cypress/cypress/tests/mocked/moduleFederation/moduleFederation.cy.ts (1)
17-22: Consider more robust console error checking.The current approach to check console errors may be brittle since it depends on Cypress sinon setup and specific error message patterns.
Consider a more reliable approach:
- cy.window().then((win) => { - // If module federation is working correctly, these should not throw errors - expect(win.console.error).to.not.have.been.calledWith( - Cypress.sinon.match(/Failed to fetch dynamically imported module/), - ); - }); + // Check that the app loads without console errors by verifying DOM state + cy.get('[data-testid="app"]').should('be.visible'); + cy.get('[data-testid="app"]').should('not.contain', 'Error');MODULE_FEDERATION_CYPRESS_FIX.md (1)
96-99: Consider adding specific maintenance guidelines.The future considerations section is good but could be more specific about maintenance procedures.
Consider adding more specific guidelines:
## Future Considerations - When adding new federated modules, update the mock list in `e2e.ts` + - Add the new module name to the `cy.setupModuleFederationMocks()` call + - Create corresponding mock extensions if needed - Consider creating environment-specific module federation configs - Monitor for any new module federation runtime patterns that might need mocking +- Review and update mock responses when remote module APIs change +- Document any new environment variables or configuration options
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (18)
MODULE_FEDERATION_CYPRESS_FIX.md(1 hunks)frontend/config/moduleFederation.js(1 hunks)frontend/config/webpack.common.js(2 hunks)frontend/config/webpack.dev.js(1 hunks)frontend/package.json(1 hunks)frontend/packages/model-registry/Dockerfile(1 hunks)frontend/packages/model-registry/manifests/base/kustomization.yaml(1 hunks)frontend/packages/model-registry/manifests/base/model-registry-ui-deployment.yaml(1 hunks)frontend/packages/model-registry/manifests/base/model-registry-ui-network-policy.yaml(1 hunks)frontend/packages/model-registry/manifests/base/model-registry-ui-service-account.yaml(1 hunks)frontend/packages/model-registry/manifests/base/model-registry-ui-service.yaml(1 hunks)frontend/src/__tests__/cypress/cypress/support/e2e.ts(2 hunks)frontend/src/__tests__/cypress/cypress/tests/mocked/moduleFederation/moduleFederation.cy.ts(1 hunks)frontend/src/__tests__/cypress/cypress/utils/moduleFederationMock.ts(1 hunks)frontend/src/plugins/useAppExtensions.ts(1 hunks)frontend/src/utilities/const.ts(2 hunks)manifests/core-bases/base/deployment.yaml(1 hunks)manifests/core-bases/base/federation-configmap.yaml(1 hunks)
✅ Files skipped from review due to trivial changes (2)
- frontend/package.json
- frontend/src/tests/cypress/cypress/utils/moduleFederationMock.ts
🚧 Files skipped from review as they are similar to previous changes (13)
- frontend/packages/model-registry/manifests/base/model-registry-ui-service-account.yaml
- frontend/config/moduleFederation.js
- frontend/src/tests/cypress/cypress/support/e2e.ts
- frontend/config/webpack.common.js
- frontend/config/webpack.dev.js
- manifests/core-bases/base/deployment.yaml
- frontend/src/utilities/const.ts
- frontend/packages/model-registry/manifests/base/model-registry-ui-network-policy.yaml
- frontend/packages/model-registry/manifests/base/model-registry-ui-service.yaml
- manifests/core-bases/base/federation-configmap.yaml
- frontend/packages/model-registry/manifests/base/model-registry-ui-deployment.yaml
- frontend/packages/model-registry/manifests/base/kustomization.yaml
- frontend/packages/model-registry/Dockerfile
🧰 Additional context used
🧠 Learnings (3)
📓 Common learnings
Learnt from: ConorOM1
PR: opendatahub-io/odh-dashboard#4423
File: frontend/src/__tests__/cypress/cypress/fixtures/resources/yaml/model_registry.yaml:12-12
Timestamp: 2025-06-27T11:00:06.871Z
Learning: For ModelRegistry resources in `frontend/src/__tests__/cypress/cypress/fixtures/resources/yaml/model_registry.yaml`, the `oauthProxy: {}` empty configuration is sufficient to enable the OAuth proxy. The ModelRegistry operator enables the proxy by default when this key is present, without requiring explicit `enabled: true` flags or image specifications.
frontend/src/__tests__/cypress/cypress/tests/mocked/moduleFederation/moduleFederation.cy.ts (2)
Learnt from: christianvogt
PR: opendatahub-io/odh-dashboard#4381
File: frontend/src/__tests__/cypress/tsconfig.json:9-9
Timestamp: 2025-06-19T20:38:32.485Z
Learning: In the ODH Dashboard project, the `frontend/src/__tests__/cypress/tsconfig.json` file intentionally has an empty `files` array to disable type checking for Cypress test files. This is part of the monorepo structure where Cypress was separated into its own package but type checking is deliberately disabled for it.
Learnt from: ConorOM1
PR: opendatahub-io/odh-dashboard#4444
File: frontend/src/__tests__/cypress/cypress/tests/e2e/modelRegistry/testRegisterModel.cy.ts:139-195
Timestamp: 2025-07-02T10:12:55.016Z
Learning: In ODH Dashboard Cypress tests marked with `@NonConcurrent` tag, tests are intentionally designed to run sequentially to simulate realistic user workflows. For model registry tests, this allows testing scenarios like adding versions to existing models rather than creating isolated test scenarios, which better reflects how users would actually interact with the system.
MODULE_FEDERATION_CYPRESS_FIX.md (2)
Learnt from: christianvogt
PR: opendatahub-io/odh-dashboard#4381
File: frontend/src/__tests__/cypress/tsconfig.json:9-9
Timestamp: 2025-06-19T20:38:32.485Z
Learning: In the ODH Dashboard project, the `frontend/src/__tests__/cypress/tsconfig.json` file intentionally has an empty `files` array to disable type checking for Cypress test files. This is part of the monorepo structure where Cypress was separated into its own package but type checking is deliberately disabled for it.
Learnt from: ConorOM1
PR: opendatahub-io/odh-dashboard#4444
File: frontend/src/__tests__/cypress/cypress/tests/e2e/modelRegistry/testRegisterModel.cy.ts:139-195
Timestamp: 2025-07-02T10:12:55.016Z
Learning: In ODH Dashboard Cypress tests marked with `@NonConcurrent` tag, tests are intentionally designed to run sequentially to simulate realistic user workflows. For model registry tests, this allows testing scenarios like adding versions to existing models rather than creating isolated test scenarios, which better reflects how users would actually interact with the system.
🧬 Code Graph Analysis (1)
frontend/src/plugins/useAppExtensions.ts (2)
frontend/src/utilities/const.ts (1)
MF_CONFIG(44-44)frontend/src/utilities/allSettledPromises.ts (1)
allSettledPromises(3-30)
🪛 Biome (1.9.4)
frontend/src/plugins/useAppExtensions.ts
[error] 52-52: Avoid the use of spread (...) syntax on accumulators.
Spread syntax should be avoided on accumulators (like those in .reduce) because it causes a time complexity of O(n^2).
Consider methods such as .splice or .push instead.
(lint/performance/noAccumulatingSpread)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Lint
🔇 Additional comments (5)
frontend/src/plugins/useAppExtensions.ts (2)
26-35: LGTM: Good error handling for module loading failures.The
.catchhandler ensures that individual module loading failures don't break the entire extension loading process, which is essential for graceful degradation.
46-65: LGTM: Comprehensive error handling for module federation initialization.The nested try-catch structure correctly handles both synchronous errors from
initRemotesand asynchronous errors from the promise chain. Thefinallyblock ensuressetLoaded(true)is called regardless of success or failure, which is crucial for preventing the app from hanging in a loading state.frontend/src/__tests__/cypress/cypress/tests/mocked/moduleFederation/moduleFederation.cy.ts (2)
1-46: LGTM: Comprehensive test coverage for module federation scenarios.The test suite covers the three key scenarios: normal operation, failure handling, and disabled module federation. The structure is clear and follows good Cypress testing practices.
37-42: Verifyprocess.envoverride inonBeforeLoadactually takes effectSetting
process.envat runtime in theonBeforeLoadhook may not override values in your app bundle, since webpack’s DefinePlugin inlines env vars at build time. We didn’t find any other tests using this pattern.• File:
frontend/src/__tests__/cypress/cypress/tests/mocked/moduleFederation/moduleFederation.cy.ts
Lines: 37–42Please confirm via manual testing or browser console that
window.process.env.CYPRESS_TESTS = 'true'is visible to your application code. If it isn’t, consider using Cypress’s built-in environment variables (e.g.Cypress.env('CYPRESS_TESTS', 'true')) or another injection strategy.MODULE_FEDERATION_CYPRESS_FIX.md (1)
1-108: LGTM: Excellent comprehensive documentation.This documentation provides a thorough explanation of the module federation testing solution, covering the problem, multi-layered approach, testing instructions, and future considerations. The structure is clear and the code examples are helpful for understanding the implementation.
lucferbux
force-pushed
the
rhoaieng-25523
branch
from
e542b26 to
5146216
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
frontend/src/plugins/useAppExtensions.ts (1)
52-52: Consider optimizing accumulator performance.The spread syntax in the reducer creates O(n^2) time complexity due to object recreation on each iteration. While the performance impact is likely minimal given the small number of federated modules, consider this optimization:
- results.reduce((acc, r) => ({ ...acc, ...r.value }), { ...prev }), + results.reduce((acc, r) => Object.assign(acc, r.value), { ...prev }),This maintains the same functionality while improving performance by mutating the accumulator instead of creating new objects.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
MODULE_FEDERATION_CYPRESS_FIX.md(1 hunks)frontend/config/moduleFederation.js(1 hunks)frontend/package.json(1 hunks)frontend/src/__tests__/cypress/cypress/support/e2e.ts(2 hunks)frontend/src/__tests__/cypress/cypress/utils/moduleFederationMock.ts(1 hunks)frontend/src/plugins/useAppExtensions.ts(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- frontend/package.json
🚧 Files skipped from review as they are similar to previous changes (4)
- frontend/config/moduleFederation.js
- frontend/src/tests/cypress/cypress/support/e2e.ts
- frontend/src/tests/cypress/cypress/utils/moduleFederationMock.ts
- MODULE_FEDERATION_CYPRESS_FIX.md
🧰 Additional context used
🧠 Learnings (1)
📓 Common learnings
Learnt from: christianvogt
PR: opendatahub-io/odh-dashboard#4381
File: frontend/src/__tests__/cypress/tsconfig.json:9-9
Timestamp: 2025-06-19T20:38:32.485Z
Learning: In the ODH Dashboard project, the `frontend/src/__tests__/cypress/tsconfig.json` file intentionally has an empty `files` array to disable type checking for Cypress test files. This is part of the monorepo structure where Cypress was separated into its own package but type checking is deliberately disabled for it.
Learnt from: ConorOM1
PR: opendatahub-io/odh-dashboard#4444
File: frontend/src/__tests__/cypress/cypress/tests/e2e/modelRegistry/testRegisterModel.cy.ts:139-195
Timestamp: 2025-07-02T10:12:55.016Z
Learning: In ODH Dashboard Cypress tests marked with `@NonConcurrent` tag, tests are intentionally designed to run sequentially to simulate realistic user workflows. For model registry tests, this allows testing scenarios like adding versions to existing models rather than creating isolated test scenarios, which better reflects how users would actually interact with the system.
🧬 Code Graph Analysis (1)
frontend/src/plugins/useAppExtensions.ts (2)
frontend/src/utilities/const.ts (1)
MF_CONFIG(44-44)frontend/src/utilities/allSettledPromises.ts (1)
allSettledPromises(3-30)
🪛 Biome (1.9.4)
frontend/src/plugins/useAppExtensions.ts
[error] 52-52: Avoid the use of spread (...) syntax on accumulators.
Spread syntax should be avoided on accumulators (like those in .reduce) because it causes a time complexity of O(n^2).
Consider methods such as .splice or .push instead.
(lint/performance/noAccumulatingSpread)
🔇 Additional comments (2)
frontend/src/plugins/useAppExtensions.ts (2)
26-35: Excellent error handling for module federation failures.The addition of the
.catchhandler provides robust error handling for remote module loading failures. This is particularly important for the Cypress testing scenario where modules may be unavailable or mocked. The fallback to an empty array maintains consistent behavior and prevents crashes.
44-65: Robust error handling implementation for module federation.The comprehensive error handling strategy effectively addresses the Cypress testing challenges mentioned in the PR objectives. The multi-layered approach catches both synchronous initialization errors and asynchronous loading failures, ensuring the application remains stable when remote modules are unavailable or mocked.
Key strengths:
- Synchronous errors from
initRemotesare caught by the inner try-catch- Asynchronous errors from promise chain are handled by
.catchsetLoaded(true)is guaranteed to execute via.finallyand catch blocks- Appropriate use of
console.warnfor non-critical failures
lucferbux
force-pushed
the
rhoaieng-25523
branch
from
5146216 to
0273530
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
frontend/config/moduleFederation.js(1 hunks)frontend/package.json(1 hunks)frontend/src/__tests__/cypress/cypress/support/e2e.ts(2 hunks)frontend/src/__tests__/cypress/cypress/utils/moduleFederationMock.ts(1 hunks)frontend/src/plugins/useAppExtensions.ts(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- frontend/src/tests/cypress/cypress/utils/moduleFederationMock.ts
🚧 Files skipped from review as they are similar to previous changes (3)
- frontend/src/tests/cypress/cypress/support/e2e.ts
- frontend/config/moduleFederation.js
- frontend/package.json
🧰 Additional context used
🪛 Biome (1.9.4)
frontend/src/plugins/useAppExtensions.ts
[error] 52-52: Avoid the use of spread (...) syntax on accumulators.
Spread syntax should be avoided on accumulators (like those in .reduce) because it causes a time complexity of O(n^2).
Consider methods such as .splice or .push instead.
(lint/performance/noAccumulatingSpread)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Cypress-Setup
- GitHub Check: Lint
🔇 Additional comments (1)
frontend/src/plugins/useAppExtensions.ts (1)
27-35: Excellent error handling implementation.The addition of comprehensive error handling with graceful fallback to an empty extensions array is well-implemented. This ensures module federation failures don't break the application and provides useful debugging information through console warnings.
|
Ok, I'm planning on creating an overlay, need to think how given circular dependencies but I'll sort this out so we don't inmediatly ship it downstream. But overall this is the layout we talked about @christianvogt @andrewballantyne I think I still need the service, so I'll expose the port int he main dashboard service, @christianvogt not sure if you have any other idea to tell module federation the target to get the bff. |
openshift-ci
bot
removed
the
do-not-merge/work-in-progress
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
manifests/core-bases/base/kustomization.yaml (1)
22-22: Consider switching toconfigMapGeneratorfor the federation settingsHard-coding
federation-configmap.yamlas a regular resource works, but it forfeits two nice Kustomize features:
- automatic name hashing to force rollouts when the data changes, and
- easier environment-specific overrides through literals or file substitutions.
Refactoring to something like
configMapGenerator: - name: federation-config files: - federation.json=federation-config.jsonwould give you those benefits and remove one file from source control.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
frontend/config/moduleFederation.js(1 hunks)frontend/package.json(1 hunks)frontend/src/__tests__/cypress/cypress/support/e2e.ts(2 hunks)frontend/src/__tests__/cypress/cypress/utils/moduleFederationMock.ts(1 hunks)frontend/src/plugins/useAppExtensions.ts(2 hunks)manifests/core-bases/base/deployment.yaml(2 hunks)manifests/core-bases/base/kustomization.yaml(1 hunks)manifests/core-bases/base/service.yaml(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- manifests/core-bases/base/service.yaml
🚧 Files skipped from review as they are similar to previous changes (6)
- frontend/config/moduleFederation.js
- frontend/src/plugins/useAppExtensions.ts
- manifests/core-bases/base/deployment.yaml
- frontend/src/tests/cypress/cypress/utils/moduleFederationMock.ts
- frontend/src/tests/cypress/cypress/support/e2e.ts
- frontend/package.json
🧰 Additional context used
🧠 Learnings (1)
manifests/core-bases/base/kustomization.yaml (1)
Learnt from: ConorOM1
PR: #4423
File: frontend/src/tests/cypress/cypress/fixtures/resources/yaml/model_registry.yaml:12-12
Timestamp: 2025-06-27T11:00:06.871Z
Learning: For ModelRegistry resources in frontend/src/__tests__/cypress/cypress/fixtures/resources/yaml/model_registry.yaml, the oauthProxy: {} empty configuration is sufficient to enable the OAuth proxy. The ModelRegistry operator enables the proxy by default when this key is present, without requiring explicit enabled: true flags or image specifications.
lucferbux
force-pushed
the
rhoaieng-25523
branch
6 times, most recently
from
7a2fc47 to
548200a
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
manifests/modular-architecture/kustomization.yaml (1)
1-20: Add missing trailing newline & consider GA apiVersion
yamllintflags the absence of a newline at EOF, which can break some tooling.
While you’re touching the file, upgradingapiVersionfrom the still-betakustomize.config.k8s.io/v1beta1to the GAv1is a low-risk improvement.-apiVersion: kustomize.config.k8s.io/v1beta1 +apiVersion: kustomize.config.k8s.io/v1 @@ name: odh-dashboard +manifests/modular-architecture/deployment.yaml (1)
3-3: Remove trailing spaces aftervalue:This is a minor formatting issue, but cleaning it avoids future diff noise and keeps yamllint quiet.
- value: + value:
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (17)
frontend/config/moduleFederation.js(1 hunks)frontend/config/webpack.common.js(2 hunks)frontend/config/webpack.dev.js(1 hunks)frontend/package.json(1 hunks)frontend/packages/model-registry/Dockerfile(1 hunks)frontend/packages/model-registry/package.json(1 hunks)frontend/src/__tests__/cypress/cypress/support/e2e.ts(2 hunks)frontend/src/__tests__/cypress/cypress/utils/moduleFederationMock.ts(1 hunks)frontend/src/plugins/useAppExtensions.ts(2 hunks)manifests/core-bases/base/deployment.yaml(1 hunks)manifests/core-bases/base/kustomization.yaml(1 hunks)manifests/core-bases/base/service.yaml(1 hunks)manifests/modular-architecture/deployment.yaml(1 hunks)manifests/modular-architecture/federation-configmap.yaml(1 hunks)manifests/modular-architecture/kustomization.yaml(1 hunks)manifests/modular-architecture/service.yaml(1 hunks)manifests/odh/kustomization.yaml(1 hunks)
🧠 Learnings (2)
📓 Common learnings
Learnt from: ConorOM1
PR: opendatahub-io/odh-dashboard#4423
File: frontend/src/__tests__/cypress/cypress/fixtures/resources/yaml/model_registry.yaml:12-12
Timestamp: 2025-06-27T11:00:06.871Z
Learning: For ModelRegistry resources in `frontend/src/__tests__/cypress/cypress/fixtures/resources/yaml/model_registry.yaml`, the `oauthProxy: {}` empty configuration is sufficient to enable the OAuth proxy. The ModelRegistry operator enables the proxy by default when this key is present, without requiring explicit `enabled: true` flags or image specifications.
manifests/modular-architecture/deployment.yaml (1)
Learnt from: ConorOM1
PR: #4423
File: frontend/src/tests/cypress/cypress/fixtures/resources/yaml/model_registry.yaml:12-12
Timestamp: 2025-06-27T11:00:06.871Z
Learning: For ModelRegistry resources in frontend/src/__tests__/cypress/cypress/fixtures/resources/yaml/model_registry.yaml, the oauthProxy: {} empty configuration is sufficient to enable the OAuth proxy. The ModelRegistry operator enables the proxy by default when this key is present, without requiring explicit enabled: true flags or image specifications.
🪛 YAMLlint (1.37.1)
manifests/modular-architecture/deployment.yaml
[error] 3-3: trailing spaces
(trailing-spaces)
[warning] 38-38: wrong indentation: expected 6 but found 4
(indentation)
manifests/modular-architecture/kustomization.yaml
[error] 20-20: no new line character at the end of file
(new-line-at-end-of-file)
✅ Files skipped from review due to trivial changes (7)
- manifests/modular-architecture/service.yaml
- manifests/odh/kustomization.yaml
- manifests/core-bases/base/deployment.yaml
- frontend/config/webpack.common.js
- manifests/modular-architecture/federation-configmap.yaml
- frontend/src/tests/cypress/cypress/utils/moduleFederationMock.ts
- frontend/package.json
🚧 Files skipped from review as they are similar to previous changes (8)
- manifests/core-bases/base/service.yaml
- frontend/config/moduleFederation.js
- manifests/core-bases/base/kustomization.yaml
- frontend/config/webpack.dev.js
- frontend/src/tests/cypress/cypress/support/e2e.ts
- frontend/src/plugins/useAppExtensions.ts
- frontend/packages/model-registry/package.json
- frontend/packages/model-registry/Dockerfile
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: ConorOM1
PR: opendatahub-io/odh-dashboard#4423
File: frontend/src/__tests__/cypress/cypress/fixtures/resources/yaml/model_registry.yaml:12-12
Timestamp: 2025-06-27T11:00:06.871Z
Learning: For ModelRegistry resources in `frontend/src/__tests__/cypress/cypress/fixtures/resources/yaml/model_registry.yaml`, the `oauthProxy: {}` empty configuration is sufficient to enable the OAuth proxy. The ModelRegistry operator enables the proxy by default when this key is present, without requiring explicit `enabled: true` flags or image specifications.
manifests/modular-architecture/deployment.yaml (1)
Learnt from: ConorOM1
PR: #4423
File: frontend/src/tests/cypress/cypress/fixtures/resources/yaml/model_registry.yaml:12-12
Timestamp: 2025-06-27T11:00:06.871Z
Learning: For ModelRegistry resources in frontend/src/__tests__/cypress/cypress/fixtures/resources/yaml/model_registry.yaml, the oauthProxy: {} empty configuration is sufficient to enable the OAuth proxy. The ModelRegistry operator enables the proxy by default when this key is present, without requiring explicit enabled: true flags or image specifications.
🪛 YAMLlint (1.37.1)
manifests/modular-architecture/deployment.yaml
[error] 3-3: trailing spaces
(trailing-spaces)
[warning] 38-38: wrong indentation: expected 6 but found 4
(indentation)
manifests/modular-architecture/kustomization.yaml
[error] 20-20: no new line character at the end of file
(new-line-at-end-of-file)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Cypress-Setup
- GitHub Check: Lint
🔇 Additional comments (1)
manifests/modular-architecture/deployment.yaml (1)
5-6: Clarify image reference
image: model-registry-uiwill pull from Docker Hub unless an image transformer overrides it. To prevent accidental pulls of an unintended image, either:
- Specify the fully-qualified reference (
quay.io/lferrnan/model-registry-ui-federated:latest), or- Add an
images:transformer in the kustomization that rewrites this placeholder.Please verify that such a transformer already exists; otherwise downstream clusters may fail to pull.
lucferbux
force-pushed
the
rhoaieng-25523
branch
5 times, most recently
from
d678294 to
76dc178
Compare
lucferbux
force-pushed
the
rhoaieng-25523
branch
from
76dc178 to
10b91b9
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
manifests/modular-architecture/deployment.yaml (1)
11-11: Strip trailing whitespaceLine 11 contains trailing spaces flagged by YAML-lint. They are harmless to the parser but fail CI linting.
Remove the extra spaces to keep the manifest clean.manifests/modular-architecture/README.md (1)
7-20: Specify language for fenced code blockMarkdown-lint warns (
MD040). Addingyamlafter the opening back-ticks improves syntax highlighting and satisfies the linter.-``` +```yaml
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (15)
frontend/config/moduleFederation.js(1 hunks)frontend/config/webpack.common.js(2 hunks)frontend/config/webpack.dev.js(1 hunks)frontend/package.json(1 hunks)frontend/packages/model-registry/Dockerfile(1 hunks)frontend/packages/model-registry/package.json(1 hunks)frontend/src/__tests__/cypress/cypress/support/e2e.ts(2 hunks)frontend/src/__tests__/cypress/cypress/utils/moduleFederationMock.ts(1 hunks)frontend/src/plugins/useAppExtensions.ts(2 hunks)manifests/core-bases/base/service.yaml(1 hunks)manifests/modular-architecture/README.md(1 hunks)manifests/modular-architecture/deployment.yaml(1 hunks)manifests/modular-architecture/federation-configmap.yaml(1 hunks)manifests/modular-architecture/kustomization.yaml(1 hunks)manifests/modular-architecture/service.yaml(1 hunks)
🧠 Learnings (2)
📓 Common learnings
Learnt from: CR
PR: opendatahub-io/odh-dashboard#0
File: .cursor/rules/cypress-e2e.mdc:0-0
Timestamp: 2025-07-21T11:49:57.416Z
Learning: Applies to frontend/src/__tests__/cypress/cypress/tests/e2e/**/ : Group related tests in feature directories under tests/e2e.
Learnt from: christianvogt
PR: opendatahub-io/odh-dashboard#4381
File: frontend/src/__tests__/cypress/tsconfig.json:9-9
Timestamp: 2025-06-19T20:38:32.485Z
Learning: In the ODH Dashboard project, the `frontend/src/__tests__/cypress/tsconfig.json` file intentionally has an empty `files` array to disable type checking for Cypress test files. This is part of the monorepo structure where Cypress was separated into its own package but type checking is deliberately disabled for it.
Learnt from: CR
PR: opendatahub-io/odh-dashboard#0
File: .cursor/rules/cypress-e2e.mdc:0-0
Timestamp: 2025-07-21T11:49:57.416Z
Learning: Applies to frontend/src/__tests__/cypress/cypress/**/*.{ts,tsx} : All linting errors must be fixed: use object destructuring, proper formatting, no unused variables/imports, no any types unless necessary.
manifests/modular-architecture/deployment.yaml (1)
Learnt from: ConorOM1
PR: #4423
File: frontend/src/tests/cypress/cypress/fixtures/resources/yaml/model_registry.yaml:12-12
Timestamp: 2025-06-27T11:00:06.871Z
Learning: For ModelRegistry resources in frontend/src/__tests__/cypress/cypress/fixtures/resources/yaml/model_registry.yaml, the oauthProxy: {} empty configuration is sufficient to enable the OAuth proxy. The ModelRegistry operator enables the proxy by default when this key is present, without requiring explicit enabled: true flags or image specifications.
🪛 YAMLlint (1.37.1)
manifests/modular-architecture/deployment.yaml
[error] 11-11: trailing spaces
(trailing-spaces)
🪛 markdownlint-cli2 (0.17.2)
manifests/modular-architecture/README.md
7-7: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
✅ Files skipped from review due to trivial changes (2)
- frontend/config/webpack.common.js
- frontend/src/tests/cypress/cypress/utils/moduleFederationMock.ts
🚧 Files skipped from review as they are similar to previous changes (11)
- manifests/core-bases/base/service.yaml
- frontend/config/moduleFederation.js
- frontend/packages/model-registry/package.json
- manifests/modular-architecture/service.yaml
- frontend/src/tests/cypress/cypress/support/e2e.ts
- manifests/modular-architecture/kustomization.yaml
- frontend/config/webpack.dev.js
- manifests/modular-architecture/federation-configmap.yaml
- frontend/packages/model-registry/Dockerfile
- frontend/src/plugins/useAppExtensions.ts
- frontend/package.json
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: CR
PR: opendatahub-io/odh-dashboard#0
File: .cursor/rules/cypress-e2e.mdc:0-0
Timestamp: 2025-07-21T11:49:57.416Z
Learning: Applies to frontend/src/__tests__/cypress/cypress/tests/e2e/**/ : Group related tests in feature directories under tests/e2e.
Learnt from: christianvogt
PR: opendatahub-io/odh-dashboard#4381
File: frontend/src/__tests__/cypress/tsconfig.json:9-9
Timestamp: 2025-06-19T20:38:32.485Z
Learning: In the ODH Dashboard project, the `frontend/src/__tests__/cypress/tsconfig.json` file intentionally has an empty `files` array to disable type checking for Cypress test files. This is part of the monorepo structure where Cypress was separated into its own package but type checking is deliberately disabled for it.
Learnt from: CR
PR: opendatahub-io/odh-dashboard#0
File: .cursor/rules/cypress-e2e.mdc:0-0
Timestamp: 2025-07-21T11:49:57.416Z
Learning: Applies to frontend/src/__tests__/cypress/cypress/**/*.{ts,tsx} : All linting errors must be fixed: use object destructuring, proper formatting, no unused variables/imports, no any types unless necessary.
manifests/modular-architecture/deployment.yaml (1)
Learnt from: ConorOM1
PR: #4423
File: frontend/src/tests/cypress/cypress/fixtures/resources/yaml/model_registry.yaml:12-12
Timestamp: 2025-06-27T11:00:06.871Z
Learning: For ModelRegistry resources in frontend/src/__tests__/cypress/cypress/fixtures/resources/yaml/model_registry.yaml, the oauthProxy: {} empty configuration is sufficient to enable the OAuth proxy. The ModelRegistry operator enables the proxy by default when this key is present, without requiring explicit enabled: true flags or image specifications.
🪛 YAMLlint (1.37.1)
manifests/modular-architecture/deployment.yaml
[error] 11-11: trailing spaces
(trailing-spaces)
🪛 markdownlint-cli2 (0.17.2)
manifests/modular-architecture/README.md
7-7: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🔇 Additional comments (1)
manifests/modular-architecture/deployment.yaml (1)
45-48: Double-checkproxy-tlsvolume exists in the parent specThe new container mounts the
proxy-tlsvolume, but this patch doesn’t add the volume definition.
If the base Deployment is ever refactored and drops that volume, this mount will break the rollout.
Please verify the volume remains present upstream or add a defensive patch here.
andrewballantyne
approved these changes
Jul 23, 2025
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: andrewballantyne The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
openshift-merge-bot
bot
merged commit 250e496
into
opendatahub-io:main
40 checks passed
6 tasks
d0w
pushed a commit
to d0w/odh-dashboard
that referenced
this pull request
Jul 29, 2025
This was referenced Jul 30, 2025
openshift-merge-bot bot
pushed a commit
that referenced
this pull request
Aug 11, 2025
* add feature flag (#4431) * Add model metrics for KServe refactor (#4346) * added lmeval improvements (#4438) * serving runtime version status label and tests (#4398) * Rhoaieng 26502 (#4440) * Enable pipelines tests affected by RHOAIENG-24702 * enable test affected by RHOAIENG-27515 * Lint fixes * Add feature flag for Feature store (#4437) * Fixed bug where storage class select wouldn't appear in NIM Modal (#4427) * Enhance ManageNIMServingModal to support OpenShift default storage class and improve storage class selection logic - Added support for OpenShift default storage class in ManageNIMServingModal. - Updated logic to prefer ODH default storage class over OpenShift default when both are available. - Introduced a new prop `showDefaultWhenNoConfig` in StorageClassSelect to control visibility and enablement of storage class options based on configuration availability. - Added comprehensive tests for ManageNIMServingModal and StorageClassSelect to cover new functionality and edge cases. * removed debug logging * removed extension on ts import * fixed awkward logic * fix: PVC resizing doesn't trigger a redeployment (#4334) * fix: PVC resizing doesn't trigger a redeployment * one annotation solution * updated tests * fixed lint * NIM isolation * generic version * fix: lint * deleted comment * Serving refactor: Add tokens and hardware accelerators (#4441) * Add tokens and hardware accelerators * Remove unused kserve auth function * Cypress e2e Test - Verify a model version can be registered via versions view (#4444) * add test for registering via version view * add page object with timeout * enhance test * add empty state object, update test * update test object * use common mr login function * create versions page object * remove duplicate v2 check * add database checks * update login pattern to standard approach * sanitise queries * add uid to object storage model * update and apply nav method * add feature flag tag * fixed provider filter in resources (#4443) * fixed provider filter in resources * updated mocks for start basic workbench card * made new component hidden from enabled page * removed deprecated usage of useQueryParams * Allow empty requested/limit for CPU and memory when hardware profile is disabled (#4407) * Serving refactor: Show deployments of all projects in global page (#4451) * Show deployments of all projects in global page * Fix padding issue * Fix spacing issue * Fix deployment status bug (#4446) * Cypress e2e Test - Verify model registry permissions can be managed (#4456) * add new registry permissions test * lint fix * new line lint * Change "Launch application" to "Open application" (#4409) * change launch application to open application * change link * remove readme change * Update Model Registry Plugin with latest upstream changes. (#4455) * Update model-registry to 21197e57471e2a7fc6a09e178147540e8e218a94 * Revert changes * Update model-registry to 6545c37be18e2ead22e2d0ba83ae7874c3bd8680 * Revert overriden files * Pipeline failure feedback 2 (#4436) * moved over changes from pipeline-failure-feedback manually into newest branch; also adjusted title and description upon erroring out also: now only showing errors that have actual messages (reduce noise) * working on finishing this up...now that i have an actual cluster/backend to use again :) * removing the ignoreTimeout; not using anymore per UX * cleaning up code * removing timeout hacks that sped up development * using server timeout directly now * adjusted comment; next: remove the new css * adding an overrideTimeout * linter fix + removing unneeded timeOut alert * fixed unit test * (using cursor) added a div with an id for the mock tests; they are passing locally now * used cursor to add section to ignore ChunkLoadErrors * linter fix * style (scss) changes removed * Add toast notification upon successful datascience pipeline configuration (#4350) * add polling * fix linking * remove unused util * fix lint * protect against null array * change to use notificationwatchercontext * fix autolint * add test * add testids * change error msg * remove comment * fix linting * adjust redundant const * change time check * use dspipelinekind metadata namespace * use dspaloaded and server timeout functions * fix lint * add polling * fix linking * remove unused util * fix lint * protect against null array * change to use notificationwatchercontext * fix autolint * add test * add testids * change error msg * remove comment * fix linting * adjust redundant const * change time check * use dspipelinekind metadata namespace * adjust notifications to not show during modal * remove info notification * fix imports * check for undefined * fix tests * fix notifs * add cypress tests * e2e test (#4403) * Microcopy updates for "project-scoped" items (#4448) * microcopy updates * ChunkLoadError fix * microcopy updates * Remove list and listitem imports * Update test with global-scoped name * li ul changes * lint error fix * adjust the content tags * Update manageAcceleratorProfiles.cy.ts * edit label microcopy (#4458) * split frontend lint task into two separate runs (#4461) * run type checking in all packages with turbo (#4381) * run type checking in all packages with turbo * remove type-check from unit test script as it is run separately * Updates and enhancements for 'Verify the filters on Resources page' e2e test (#4463) * enhance test with backend validation * use inline types * check namespace * implement rhoai specific checks * add self-managed step * add navigate method * update tags and apply lint * Add more approvers (#4464) * Fix type errors for array in es2021 (#4467) * Add stopping status to model deployments (#4421) * RHOAIENG-29339: Filter out outdated notebook image tags and only validate UI version dropdown for images with 2+ non-outdated versions\n\nThis change updates the Cypress workbench image/version test to:\n- Filter out outdated notebook image tags (using the opendatahub.io/image-tag-outdated annotation)\n- Only validate the UI version dropdown when there are 2 or more non-outdated versions for an image\n- Skip UI validation for images with only 1 non-outdated version, matching current UI behavior\n\nThis ensures the test is robust and cluster-agnostic, and resolves failures due to outdated or single-version images.\n\nJira: https://issues.redhat.com/browse/RHOAIENG-29339 (#4471) * Quarantine model serving tests due to [Product Bug: RHOAIENG-29340] * Small change for quarantine branch RHOAIENG-29340 * Remove accidentally added web-ui directory from index * Update Hardware profile admin page (#4400) * Update hardware profile CRD, add local queue and workload priority class options * Added kueue fields to the details popover in HWP select * easier to use feature flags via a button in the header (#4237) * moved over a lot; the button is showing again. need to control when/how the banner shows again tho * better; need a break: next: compare tests (since files changed....)" * tweaks re pr review * tests are at lest running..... * yay passing :) * lint/prettier fixes * unit test fix * linter fix (unused import) * lint fix (added space) * fixing linter test error; (test runs locally just fine of course) * fixing merge error * Retrieve default local queue name from DSC (#4462) * Remove deleted crd from kustomization file (#4478) * Temporarily restore dashboard HWP crd (#4480) * Serving refactor: Add modelmesh as a platform shortcircuit option (#4459) * Add modelmesh as a platform shortcircuit option * Fix eslint oom * Add dependencies to package.json * Update icon * Internal model serving endpoint (#4475) * Update Label url to Internal * Updated Internal label test in cypress * Upgrade cluster storage modal (#4457) * upgrade cluster storage to include model context * remove isGeneralPurpose useEffect * remove redundant annotation setting * feat: Enhance the dashboard to support deploying NIM with a PVC (#4447) * v1 * fix: lint * hide on edit * refactor: DEFAULT_MODEL_PATH * model pvc dropdown * deletion logic * fix: lint * fix for current tests * fix:lint * lint * defaultMockInferenceServiceData * added testid's * tests + info * coderabbitai * deleted comment * no model is selected + relativeTime * Change how model deployment links appear (#4442) * Change how model deployment links appear * Cypress fixes * Adjustments from feedback * Move instances of isModelStopped into a util * Move duplicated code into shared resource * Rebase fixes * Integrate stop start changes * Small fixes * Fix issue with metrics link * Quick fix for stopping --------- Co-authored-by: Griffin-Sullivan <[email protected]> * Remove Product Bug references and @Bug tags from model serving tests (#4488) - Removed '[Product Bug: RHOAIENG-29340]' from describe blocks - Removed '@Bug' tags from all test configurations - Fixed linter formatting issues for tag arrays - Tests are now unquarantined and ready for execution * RHOAIENG-25339: Update application of hardware profiles in all workloads (#4413) * conditionally set resources * added unit tests * bring back lost changes * fixed unit tests * fixed mock tests * remove logic to unset resources for harware profiles * added new annotation * added new annotation to the serving runtime * Revert "fixed mock tests" This reverts commit 805f14d6294b429f5c4e0647f1606fdcc7141f8c. * fix lint * added a check for acc profile * fix mock tests * fixed more tests * fix mode serving mock test * added a better check * fixed tests * removed console * fixed unit test * Feature flag modal re-organization (#4481) * starting on separating feature flags into groups * comments and scaffolding * starting with cursor...added tabs * better * better * better * better * flags redistributed onto tabs now; easier to find the active ones * no longer having indeterminate checkboxes * cleanup * cleanup * restoring package.json * added scrolling to *just* the content of the tab that needs it (tried to use patternfly but capability not there) * did find a patternfly way; with chatgpt (better than cursor in this case) * working on test * removed debugging code * removing PageSection (review feedback) * linter fix (removing unused import) * Create a set of Cursor rules for Cypress e2e Development (#4485) * chore: initial changes for Cypress E2E rules and setup * docs(cypress-e2e): update rules for linting scripts, cy.get usage, and tagging per PR feedback * docs(cypress-e2e): clarify lint/fix commands for root, frontend, and backend per feedback * docs(cypress-e2e): update lint/fix instructions to frontend-only for E2E tests * Feature Store Initial Setup (#4487) * Add types, mocks, feature store project context, empty state and project selector for feature store. (#5) * Add types and mocks for feast * Add feature store project context and project selector * feat(RHOAIENG-28417):added feature store code block common component * feat(RHOAIENG-28415):added single label and tags with overflow and show all tags component * feat(RHOAIENG-28413): added feature store side navigation routes * feat(RHOAIENG-28413):uncommented the feature store flag check for routes * feat(RHOAIENG-28413):rearranged the nav items below experiments in the navbar and minor content changes in name * feat(RHOAIENG-28413):added core loader in the routes for feature store * fix(context):added featurestore context in app file * removed unused component * fix(feast-context): moved the feast context to the feast routes level * style: addressed review comments (text and spacing) --------- Co-authored-by: Pushpa Padti <[email protected]> * Add missing frontmatter to cypress-e2e.mdc rule file - Added description, globs, and alwaysApply fields to match jira-creation.mdc format - Enables proper rule configuration and LLM-driven relevance detection - Addresses reviewer feedback about missing frontmatter section * Fix globs field to be empty array instead of null - Changed 'globs:' to 'globs: []' to ensure it parses as empty array - Prevents type errors in rule-runner which expects a list - Addresses PR reviewer feedback * Fix bug when stopping model with confirmation modal (#4492) * Update cluster storage connected resources (#4469) * Update cluster storage connected resources * update ConnectedResourcesProps * RHOAIENG-27574: Filter Hardware profiles if kueue disabled (#4490) * added filter logic * added unit tests * fixed lint * moved the filter logic * enum * renamed * updated tests * removed unnecessary const * fix dupl issie * RHOAIENG-30110: The workload priority is not maintained when switching between local queue and not selectors. (#4496) * fixed the bug * added gutter * added unit tests * Kueue extras (#4489) * Set default hwp namespace in notebooks to dashboard namespace, remove non-legacy hwp from model mesh, move hwp annotations from serving runtime to inference service * Small fix * Additional fixes * Add test IDs for deployed model names in ModelMesh and plugin components (#4499) * Add test IDs for deployed model names in ModelMesh and plugin components - Add 'deployed-model-name' test ID to InferenceServiceTableRow for ModelMesh deployments - Add 'deployed-model-name' test ID to DeploymentsTableRow for plugin-based deployments - Fix syntax error in testSingleModelAdminCreation.cy.ts test file - Update Cypress tests to use new test ID selectors Supports RHOAIENG-29747 and RHOAIENG-29977 * Fix mock tests by restoring metrics-link test IDs while adding model name selectors - Restore original metrics-link-{name} test IDs for backward compatibility with existing tests - Add data-model-name='deployed-model-name' attributes as additional selectors - Update findModelServerDeployedName method to use new generic selector - Add findModelDeployedName method for table-based model selection This fixes the mock test failures while maintaining both specific (metrics link) and general (model name) test selectors. * Revert PR changes to InferenceServiceTableRow.tsx * Update test selector to use data-testid instead of data-model-name * Add deployed-model-name test ID to InferenceServiceTableRow - Add data-testid='deployed-model-name' to both Link and span elements - Preserve existing metrics-link functionality - Update page object to use new test ID selector - Add uncaught exception handler for JavaScript parsing errors - Support both loaded and unloaded model states * Update model serving tests: remove exception handling and fix project deletion timeout - Remove unnecessary Cypress.on('uncaught:exception') handling from all model serving tests - Set 5-minute timeout for project deletion due to RHOAIENG-19969 - Add TODO comments to review timeout once RHOAIENG-19969 is resolved - Support proper test retries by waiting for project deletion completion - Clean up test code to be more focused and maintainable * Address review comments: use findByTestId and remove redundant data-model-name - Update findModelDeployedName() in page object to use findByTestId('deployed-model-name') for consistency - Remove redundant data-model-name attribute from InferenceServiceTableRow component - Keep only data-testid='deployed-model-name' for test selection Addresses review feedback from Griffin-Sullivan on PR #4499 * Update frontend/src/__tests__/cypress/cypress/pages/modelServing.ts Co-authored-by: Griffin Sullivan <[email protected]> * Remove unused findModelDeployedName method from page object - Method was not used anywhere in the codebase - Clean up unnecessary code for better maintainability * Rename findModelServerName to findModelMetricsLink for clarity - Renamed findModelServerName(name: string) to findModelMetricsLink(name: string) in ModelServingSection - This method actually finds metrics links using metrics-link-${name}, not model server names - Updated all test file references to use the new method name - Kept the original findModelServerName() method for input fields unchanged - Addresses naming confusion between different findModelServerName methods Updated files: - testSingleModelContributorCreation.cy.ts - testSingleModelAdminCreation.cy.ts - testDeployOCIModel.cy.ts - testModelServingTolerations.cy.ts --------- Co-authored-by: Griffin Sullivan <[email protected]> * Remove dashboard hardware profiles CRD (#4500) * Add 'Last Deployed' column to model deployments (#4468) * Add 'Last Deployed' column to model deployments * Adjust extension and modify for model mesh * Add generic component for last deployed * Cypress fix * Fix sorting for models refactor * Adjust to only show timestamp for 'Started' models * fixed filtering behavior for model registry page (#4482) * fixed filtering behavior for model registry page * fixed tests * added filtering for model version table * removed file extension * upversion dashboard (#4503) * Workbench Test Maintenance - implement dynamic notebook image selection with backend fallback (#4502) * feat: implement dynamic notebook image selection with backend fallback - Replace hardcoded 'jupyter' image validation with dynamic selection - Add selectNotebookImageWithBackendFallback utility function: - Try UI selection first (createSpawnerPage.findNotebookImage) - Fall back to backend query if UI element not found - Uses opendatahub.io/notebook-image-name annotation for display name - Update workbench tests to use scoped validation (notebookRow.shouldHaveNotebookImageName) - Centralize all OC commands in imageStreams.ts utility per Cypress E2E rules - Handle environment variables (APPLICATIONS_NAMESPACE) internally in utilities - Fix async/sync mixing issues in Cypress command chaining Resolves: RHOAIENG-30224 * feat: complete dynamic image selection for testWorkbenchVariables.cy.ts - Replace all hardcoded 'code-server' validations with dynamic approach - Both tests now use selectNotebookImageWithBackendFallback for UI-first selection - All 4 workbench instances now validate using getImageStreamDisplayName - Handle complex test structure with multiple workbenches per test - Proper async chaining for nested workbench creation flows - All 6 workbench test files now consistently use dynamic image selection All workbench tests (11 total) now passing with dynamic image selection: ✅ testWorkbenchControlSuite.cy.ts (2 tests) ✅ testWorkbenchCreation.cy.ts (2 tests) ✅ testWorkbenchImages.cy.ts (1 test) ✅ testWorkbenchNegativeTests.cy.ts (2 tests) ✅ testWorkbenchStatus.cy.ts (1 test) ✅ testWorkbenchVariables.cy.ts (2 tests) - COMPLETED ✅ workbenches.cy.ts (1 test) Resolves: RHOAIENG-30224 * refactor: remove unused helper functions from imageStreams.ts Address CodeRabbit AI feedback by removing dead code: - Remove verifyNotebookImageDisplayName (unused) - Remove validateNotebookRowImageName (unused) - Remove shouldHaveNotebookImageNameFromImageStream (unused) - Remove selectTestImageStream (unused) All tests use getImageStreamDisplayName directly with manual validation, making these helper functions redundant. This cleanup reduces code duplication and maintains only the functions actually used: ✅ getImageStreamDisplayName - Core function used in all tests ✅ selectNotebookImageWithBackendFallback - Main selection function ✅ getAvailableNotebookImageStreams - Used by fallback logic ✅ getNotebookImageNames - Used by testWorkbenchImages.cy.ts File size reduced from 273 lines to ~150 lines with no functionality lost. * test: add @Workbenches tag to workbenches.cy.ts test Add missing @Workbenches tag to the 'Verify users can create a workbench and connect an existent PersistentVolume' test case. This ensures the test is included when running workbench tests with tag filtering (e.g., --env grepTags='@Workbenches'). * Add back hardware profiles crd (#4507) * Add back hardware profiles crd * Add description * Deploy a Model from a PVC (#4449) * deploy a model from pvc * existing uri bug fix * simplify useEffect and hide uri when pvc found * Refactor: Avoid destructuring from process.env (#4510) * add documentation on extensibility (#4501) * add documentation on extensibility * add link under 'developer readmes' * add cypress tests for missing links (#4513) * add cypress tests for help links * Refactor: Avoid destructuring from process.env --------- Co-authored-by: Lucas Fernandez <[email protected]> * Serving refactor: Backport nim platform (#4493) * Add refactor platform card * Update NIM context to be generic * Updating naming * Add tests * import consistency updates * cleaner isPlatformEnabled * Feature Store Context and API setup (#4509) * Feature Store Context and API setup: * Backend proxy setup * Routes * Context * Hooks * API * Unit tests * remove title icon * import updates and few changes * adding unit tests * PR review updates * Invalid project selector * adding prefix in proxy * Add fix to image streams notebook enabling annotation (#4522) * Splitting feature store types.ts (#4529) * Add skeleton for accelerator profile while loading (#4454) * change loading checks and update components * add state for component * fix lint * Update AcceleratorProfileSelectField.tsx * fix loading state issues * remove imports * add test case * DevFeatureFlag removal (#4517) * Status refresh for the Model Registry's Deployments (#4466) * MR status refresh * fix lint: imports * address comments * Globals page exports routes with metrics too (#4519) * Globals page exports routes with metrics too * better metric link logic * Initial folder structure for llama stack ui (RAG/Gen AI v3) (#4535) * feat: integrate model registry into the deployment (#4450) * PVC Serving E2E Test (#4494) * Add E2E test for deploying from PVC * pr fixes * remove feature flag flip * PVC serving microcopy updates (#4516) * PVC serving microcopy updates * Add HelperTextItem * change to popover * warning title change * fix: update title and label text in Enable NIM dialog to reference personal key (#4536) Signed-off-by: Olga Lavtar <[email protected]> * Refactor RegisterCatlogModel and Remove useEffect (#4420) * Refactor RegisterCatlogModel and remove useEffect * model name fix for cypress test * simplify the hook * mapping suggestion fix Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> * remove timeout from test Co-authored-by: Robert Sun <[email protected]> * timeout removal Co-authored-by: Robert Sun <[email protected]> * timeout removal Co-authored-by: Robert Sun <[email protected]> * remove the prefilled values test --------- Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> Co-authored-by: Robert Sun <[email protected]> * aggregate get/list/watch permissions to project admins/contributors (#4497) * chore: e2e Test Maintenance - Jul 24th 25' (#4538) * RHOAIENG-30472: Standardize deleteKueueResources function options to match deleteOpenShiftProject pattern - Add standardized options parameter with timeout, wait, and ignoreNotFound properties - Implement same pattern and defaults as deleteOpenShiftProject for consistency - Fix async/sync code mixing issues that caused Cypress errors - Update test files to use new options parameter syntax with proper comma placement - Maintain backward compatibility while improving code consistency * Add exception handler for webpack-dev-server fallback errors in E2E tests - Ignore 'Unexpected token <' errors that occur due to webpack-dev-server fallback - Prevents E2E tests from failing due to these development environment issues - Added alongside existing ChunkLoadError handler for consistent error handling * Add quarantine steps to model serving E2E tests - Add quarantine steps to testModelStopStart.cy.ts - Add quarantine steps to testSingleModelAdminCreation.cy.ts - Add quarantine steps to testSingleModelContributorCreation.cy.ts - Fix formatting issues with tag arrays per prettier requirements * Update frontend/src/__tests__/cypress/cypress/utils/oc_commands/distributedWorkloads.ts Co-authored-by: Noam Manos <[email protected]> * Improve robustness of Kueue resource deletion commands - Replace && with || : ; to ensure all deletion attempts run independently - Prevents cleanup from stopping if one resource doesn't exist - Addresses review feedback from @manosnoam in PR #4538 Co-authored-by: Noam Manos <[email protected]> --------- Co-authored-by: Noam Manos <[email protected]> * Add version labels to refactored code (#4523) * Add version labels to refactored code * Add generic component * Modify servingruntime to servingdetails, create shared version component * Added entire repo for llama-stack-modular-ui (#4541) * Added entire repo for llama-stack-modular-ui Signed-off-by: Agnieszka Gancarczyk <[email protected]> * Removed .github Signed-off-by: Agnieszka Gancarczyk <[email protected]> * Added .eslintignore Signed-off-by: Agnieszka Gancarczyk <[email protected]> * Added gh actions Signed-off-by: Agnieszka Gancarczyk <[email protected]> * Fix for lint errors Signed-off-by: Agnieszka Gancarczyk <[email protected]> * Removed gh actions Signed-off-by: Agnieszka Gancarczyk <[email protected]> * Ignoring tests in llama-stack-modular-ui directory Signed-off-by: Agnieszka Gancarczyk <[email protected]> --------- Signed-off-by: Agnieszka Gancarczyk <[email protected]> * [feat] Add real amd mock endpoints for Query and Chat completion (#4543) This commit contains the following: 1. Mock and real endpoints for query. 2. Allows query when no DB is present. 3. Allows query when DB is present. Signed-off-by: Varsha Prasad Narsing <[email protected]> Co-authored-by: Matias Schimuneck <[email protected]> * Add area/llama-stack-modular-ui label for PRs (#4551) * Serving refactor global models page backport (#4531) * Add global models page backports * Remove unused backport file * Quarantine distributed workloads test - add bug tag and JIRA reference RHOAIENG-30510 (#4550) * Unquarantine serving tests from RHOAIENG-30376 (#4554) * add llama stack github action workflow (#4546) * add llama stack workflow * fix package.json in llama-stack-modular-ui * change the workflow name * feat: Add comprehensive OpenAPI documentation for Llama Stack Modular… (#4547) * feat: Add OpenAPI documentation for Llama Stack Modular UI BFF - Add comprehensive OpenAPI 3.0 specification for BFF endpoints - Include /api/v1/query endpoint for RAG and chat completion - Add vector database management endpoints - Include model management endpoints - Fix CORS configuration for healthcheck endpoint to enable Swagger UI - Document only public-facing models and remove authentication requirements - Add common schemas and responses in separate lib/common.yaml file The OpenAPI documentation can be visualized using Swagger UI or any OpenAPI viewer. Run the BFF with: make run STATIC_ASSETS_DIR=../frontend/dist MOCK_LS_CLIENT=true ALLOWED_ORIGINS="*" * docs: Improve OpenAPI specification clarity and security definitions - Add explicit security: [] to healthcheck and config endpoints for public access - Clean up proxy endpoint descriptions by removing redundant authentication text - Improve formatting consistency in OpenAPI specification - Clarify which endpoints are public vs require authentication --------- Co-authored-by: Matias Schimuneck <[email protected]> * Bump form-data in /frontend/packages/model-registry/upstream/frontend (#4521) Bumps [form-data](https://github.com/form-data/form-data) from 4.0.3 to 4.0.4. - [Release notes](https://github.com/form-data/form-data/releases) - [Changelog](https://github.com/form-data/form-data/blob/master/CHANGELOG.md) - [Commits](https://github.com/form-data/form-data/compare/v4.0.3...v4.0.4) --- updated-dependencies: - dependency-name: form-data dependency-version: 4.0.4 dependency-type: indirect ... Signed-off-by: dependabot[bot] <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: Manaswini Das <[email protected]> * config: enable typecheck for source files, disable only for test files (#4562) - Enable typecheck linter for regular source code files - Disable typecheck only for test files (*_test.go) to avoid Ginkgo framework errors - Maintain proper type checking for production code while allowing test framework functions * Removed all code related to chatbot openshift auth and chatbot sharing visuals (#4561) * Removed all code related to oAuth Signed-off-by: Agnieszka Gancarczyk <[email protected]> * Fixed eslint Signed-off-by: Agnieszka Gancarczyk <[email protected]> * Fixed eslint Signed-off-by: Agnieszka Gancarczyk <[email protected]> * Fixed eslint Signed-off-by: Agnieszka Gancarczyk <[email protected]> * Fixed eslint Signed-off-by: Agnieszka Gancarczyk <[email protected]> * Fixed eslint Signed-off-by: Agnieszka Gancarczyk <[email protected]> * Fixed eslint Signed-off-by: Agnieszka Gancarczyk <[email protected]> * Fixed eslint Signed-off-by: Agnieszka Gancarczyk <[email protected]> --------- Signed-off-by: Agnieszka Gancarczyk <[email protected]> * exclude llama-stack-modular-ui in tsconfig.json (#4566) * Removed out of date documentation files (#4567) * make rwo tag always show (#4524) * Quarantine model serving tests for Product Bug RHOAIENG-30799 - Added [Product Bug: RHOAIENG-30799] to describe blocks for all model serving tests - Added @Bug tag to all affected test cases - Quarantined tests in: - testSingleModelAdminCreation.cy.ts - testSingleModelContributorCreation.cy.ts - testModelPvcDeployment.cy.ts - testModelStopStart.cy.ts - testMultiModelAdminCreation.cy.ts - testDeployOCIModel.cy.ts - testSingleServingRuntimeCreation.cy.ts * module federation documentation (#4556) * Feature view api (#4553) * Feature View * Table * Routes * API Hooks * Empty state * Ensuring API availability for feature store * Feature view tags overflow fix * empty owner placeholder * Coderabit updates * Updating feature view online offline label * cypress tests * Update failed unit test * Test updates * Uncommenting mock status * Port start / stop models to the plugin (#4533) * Model Registry - ODH Enablement (#4540) * Lucas' test MR container * Update params.env * Update deployment.yaml * Update deployment.yaml * Fix issue with overlay * Get target ports referenced by name * Revert namespace * Avoid using labels in this overlay --------- Co-authored-by: Andrew Ballantyne <[email protected]> Co-authored-by: Lucas Fernandez <[email protected]> * Serving refactor global cypress test fixes (#4555) * Add global models page backports * Remove unused backport file * fixes * small test update * Remove comments * Fix more tests * Fix last deployed tooltip mouseenter * feat: adapt base images to red hat and migrate from ubi8 to ubi9 (#4565) * Cypress e2e Test - Verify models can be deployed from model registry (#4564) * add new deploy test * increase timeout * handle empty registry condition * deploy secret * handle single registry condition * improve timeout mechanism * minor enhancements * combine mr utils * add page object * lint * remove initial validation * move clickRegisterModelButton to new util * update tests * follow up missing changes in commit (#4576) * Upgrade mod arch upstream (#4579) * Update model-registry to b4c43a0dfc6864f9a630a011e71f48c48b695df1 * Revert changes that override upstream * RHOAIENG-28909: Add Storage Class Provisioner "disk.csi.azure.com" to Storage Enums (#4528) * added disk.csi.azure.com * added RWOP * added unit tests * added RWX * rwo only * New LMEval E2E tests for TrustyAI (#4511) * New LMEval E2E tests for TrustyAI This commit adds comprehensive LMEval tests and related E2E utilities: - Add 2 scenarios for LMEval using static and dynamic models - Add LMEval job utilities for configuration and job verification - Add ModelTestSetup utility for any e2e test setup - Add YAML support to ESLint: eslint-plugin-yml, yaml-eslint-parser - Update LMEval form components with missing test IDs Signed-off-by: manosnoam [email protected] * Fix CodeRabbitAI comments Signed-off-by: manosnoam <[email protected]> * Multiple fixes to address Reviewers comments Signed-off-by: manosnoam <[email protected]> * fix(lm-eval): Dynamically detect port for InferenceService connections LMEval jobs were failing with ConnectionRefusedError because they were connecting to InferenceService models on the default port 80, while the ServingRuntime was configured to listen on a different container port. This fix fetches the ServingRuntime associated with each InferenceService and extracts the containerPort from its definition. The port is then used to construct the correct internal cluster URL. Signed-off-by: manosnoam <[email protected]> * Update LMEval mock tests and page object Signed-off-by: manosnoam <[email protected]> * Split LMEval tests into static and dynamic test files Signed-off-by: manosnoam <[email protected]> * Fix CodeRabbitAI suggestions Signed-off-by: manosnoam <[email protected]> * Rebase lmeval_e2e branch Signed-off-by: manosnoam <[email protected]> * Quarantine testLMEvalDynamic due to bug RHOAIENG-30642 Signed-off-by: manosnoam <[email protected]> * Fix accessibility violations (A11y) with plain buttons in LMEval form Signed-off-by: manosnoam <[email protected]> * Add testId to LMEvalTaskSection Signed-off-by: manosnoam <[email protected]> * Rename lmEval.ts to lmEvalFormPage.ts Signed-off-by: manosnoam <[email protected]> * Fix e2e.ts before hook to skip test suites based on tags Signed-off-by: manosnoam <[email protected]> * Update LMEval E2E page objects and tests - Remove complex dropdown logic, enhance model selection - Move cleanup function, remove fallback project name - Enhance YAML parsing with error handling - Login before test, to avoid session race conditions with devFeatureFlags - Remove Tag for bug RHOAIENG-30642 - Update README.md Signed-off-by: manosnoam <[email protected]> --------- Signed-off-by: manosnoam [email protected] Signed-off-by: manosnoam <[email protected]> * E2E Test for Token Authentication (#4572) * add E2E test to validate token authentication * copy tokens from UI * remove comment * Serving refactor: fix project details tab cypress tests (#4580) * Make test ids more consistent * Update project test too * Feature store entities (#4568) * feat(RHOAIENG-28420):added constants and feature store route for entitiy details page * feat(RHOAIENG-28420,RHOAIENG-28421):added hooks for feature store entities * feat(reusable-components):added a common timestamp component,scrollable links popover and import changes * feat(reusable-toolbar):added a common toolbar component * feat(entities-detail-page):added entitities detail and list view component * feat(entities-table):added entity table component and row component * fix(lint):addressed linting errors for relative imports * fix(jest-test-cases):added hook test cases and fixed test cases for error utils and api integration * fix(components): added route fixes, empty state and error state for entity details page * feat(cypress):added cypress mocks and test cases * fix:added common route function * fix(review):added spec file for utility functions and rearranged the options in the filter to match the column * Remove bug tags from model serving tests (#4585) * add bug tag * add description tag * remove bug tags * lint fix * add ticket to oci test * Add dashboard support for the pipelines Kubernetes native API (#4484) * add section * store * adjust types * remove console * add crd connection * begin kfp upstream changes * revert kfp changes * remove dataid word * begin kfp upstream changes * add support for new versions * add mock tests * fix tests and add k8s name checking * fix lint * adjust tests and add auto scroll * fix unit test * add deel equal * add component for k8s checkbox * change to smooth * Add deploy button, dropdown button, and connection autofill button as extension in MR midstream (#4453) * Add deploy button and dropdown button as extension in MR midstream * Add autofill connection button as extension when registering model * rebase * Update the extensions * Rebase and update * Update plugin context for the MR app to unblock midstream standalone mode * address feedback * Revert change to mandatoryNamespace Signed-off-by: Mike Turley <[email protected]> * Attempt to give lint job more memory Signed-off-by: Mike Turley <[email protected]> --------- Signed-off-by: Mike Turley <[email protected]> Co-authored-by: Mike Turley <[email protected]> * Update MR e2e test `testArchiveModels' (#4586) * update test * add should_be_enabled * add page object * linter fixes * lint * add selectors * Add edit model modal to refactor (#4573) * consolidate hooks * deprecate hooks * replace hooks with default hook * fixed tests * remove auto patch code for admin page * delete deprecated hooks * clean up * fixed issue * updated comment * Feature store toolbar component (#4593) * Feature store toolbar component * solving linting * fix:resolved linting issues --------- Co-authored-by: Claudia Alphonse <[email protected]> * Doc upload in chatbot (#4612) * Converted to new designs Signed-off-by: Agnieszka Gancarczyk <[email protected]> * Removed some eslint ignore statements Signed-off-by: Agnieszka Gancarczyk <[email protected]> * Added document upload functionality Signed-off-by: Agnieszka Gancarczyk <[email protected]> * Refactor Signed-off-by: Agnieszka Gancarczyk <[email protected]> * test fix * test fix --------- Signed-off-by: Agnieszka Gancarczyk <[email protected]> Co-authored-by: pnaik1 <[email protected]> * Increasing memory allocation to 8GB for lint tests (#4616) * feat: Add OpenAPI serving to BFF (#4560) - Add OpenAPI handler with JSON, YAML, and Swagger UI endpoints - Support external references in OpenAPI specification - Integrate OpenAPI routes with existing routing system - Preserve static file serving functionality with STATIC_ASSETS_DIR - Add comprehensive API documentation endpoints: * /openapi.json - OpenAPI specification in JSON format * /openapi.yaml - OpenAPI specification in YAML format * /swagger-ui - Interactive Swagger UI interface * /openapi - Redirect to Swagger UI The BFF now serves both frontend static files and provides comprehensive OpenAPI documentation for the API endpoints. * Link refactor deploy buttons to existing deploy modal (#4542) * Link refactor deploy buttons to existing deploy modal * Adjustments from feedback * Add toolbar for refactored global models page * Small bug fixes * Refine some areas * Remove references to refresh * Clean up code and fix deploy button in global no models * Kueue microcopy changes (#4620) * Adds user_token auth mode to llama stack module (#4596) * Updates authentication to follow pattern used by model registry. Now supports user_token mode. * fix: properly disable authentication if auth_method=disabled * refactor: optimize TokenClientFactory performance and make API path prefix configurable - Move TokenClientFactory creation from per-request to app-level initialization - Add configurable APIPathPrefix to EnvConfig with default /api/v1 - Add --api-path-prefix command-line flag and API_PATH_PREFIX env var - Replace hardcoded ApiPathPrefix constants with dynamic path generation methods - Update middleware to use app.tokenFactory instead of recreating per request - Update isAPIRoute function to be App method using configurable prefix - Update all tests to work with new configurable approach - Maintain backward compatibility with default /api/v1 prefix This improves performance by eliminating per-request object creation and provides flexibility for different deployment scenarios. --------- Co-authored-by: Akram Ben Aissi <[email protected]> * E2E test for model deployment with Kserve Raw (#4583) * Cypress E2E test for model deployment with Kserve Raw -20579 * refactored code * Addressed review comments * Add [Product Bug: RHOAIENG-31261] and Bug Tag to Quarantine the test * update checkInferenceServiceState utility method to check deployment mode * Update deployment mode conditions * remove getDeploymentModeLength() function. * refactored code * refactored code * fixed tests * add feature page (#4574) * add feature page * feat:added test cases,tooltip, common filter changes * feat:added fature views api in the odh cypress file * fix:reused the generic filter component and removed the unused featurestoolbar * changed the test id for the toolbar --------- Co-authored-by: Claudia Alphonse <[email protected]> * Unquarantine missed serving test from RHOAIENG-30799 bug (#4617) * Feature services (#4571) * Feature service * Added project col * Coderabbit comments * Tests added * Feature view links * feature service dates * Feature service toolbar * fixing lint * Lint fix * fixing tests * Fixing tests 2 * Replace project column * Adding back type and mocks * typo fix * TruncatedText truncation for tooltips in HardwareProfileSelection helper description (#4578) * fix tooltip * adjust other overflow * update truncate text * adjust truncation options * add scroll : * use panel * add back in support for no truncation * adjust text * fix truncation for simpleselect * change back to truncated text * fix mouse enter hook * Dspa caching rebased (#4621) * dspa cache enablement; squashing all the commits: (rebasing on derek's branch: story-21309) * using cacheEnabled (instead of cache: {enabled: <value>}) * autoscrolling when the alert shows up now so alert is not hidden *turning off autoscroll unless user interacted with it * added 'Loading...' text * renamed View->Manage for the modal * added toast * added tests * added to cypress mock test * playing with titles/text sizes * adjusting layout depending on the context * alert looks better and takes up entire width now, no matter the layout mode * linter fix (import order ) * fixing tests * updated tests; did lint fixes; removed the lint:changed from package.json * fixing import * working on package files.... * restoring package-lock.json * restored base package files * starting on review comments: fixed one layout issue and using 'useNotificaton' instead of the context * fixing button alignment in the modal * small tweaks: style removed, test code adjusted per review * fixed import * linter fix * test fix * fixing test (to use useNotification hook instead of registerNotification) * removing tag that cursor added * fixing more tests * adding in a StackItem to add a margin without adding explicit css; tested that the auto-scroll to view the alert is still functional * Refractor ChatbotMain.tsx file (#4624) * remove @app alias * create separate folder * add hooks * add unit test for hooks * Rhoaieng 28422 - Entities tab - feature views (#4626) * feat:added entity param for the feature views tab * feat:added cypress test for feature views tab and updated usefeature views hook spec test * feat:added feature view tab for entities * feat:added classname and a fixed test case * [feat] Allow context message to be an input from the user (#4603) This change allows users to input context message from the user instead of hardcoding it. To test the change, use the mock client and run the command: curl -X POST http://localhost:8080/api/v1/query \ -H "Content-Type: application/json" \ -d '{ "content": "What is machine learning?", "vector_db_ids": ["my-vector-db-identifier"], "llm_model_id": "ollama/llama3.2-3b", "system_prompt": "You are a technical consultant. Provide concise, actionable insights based on the available context." }' Signed-off-by: Varsha Prasad Narsing <[email protected]> * Feature service details (#4628) * Feature service * Added project col * Feature service toolbar * Replace project column * Feature service details * tests update * Coderabbit comments * Fallback for no tags * Fix case sensitivity: rename featureServiceDetails.tsx to FeatureServiceDetails.tsx * Details page empty states * Empty message update * solving tests * resolving conflicts * revert * revert comment * Package and feature flag setup for model training (#4615) * serving refactor: cypress project details model fixes (#4601) * Add checking runtimeTemplates for deploy button * Fix accessibility error * More consistent testids * Add testid for project details table too * Add missing testid for change platform button * Minor fix to make deploy button a link in the section overview * fix type and short circuit some template api calls * Fix deploy button not having a project in global * Fix templates load state * final fixes * Add PR review guidelines and best practices (#4486) * Add PR review guidelines and best practices * Fix indentation, wording, add to PR template * Fix Markdown formatting issues * Fix MD007 issues * Add more details about when to and when not to use useCallback * Add code examples * make content clearer and add examples * Serving refactor cypress: Re-add deploy from registry button link (#4623) * Make and use nav back to registry button * Fix gap being gone when aria-disabled true * chore(deps): Upgrade PatternFly to 6.3.0 release (#4435) fix lint chore(deps): bump versions to latest prerelease candidates chore(deps): bump versions to latest prerelease candidates chore(deps): bump versions to latest prerelease candidates, update table components chore(deps): fix lint chore(deps): fix lint chore(deps): bump to stable release versions chore(deps): add isHamburgerButton prop for page toggle animation chore(deps): fix tabs bug, add TODO for table, remove BarsIcon as child of PageToggleButton chore(deps): remove BarsIcon from imports in Header.tsx chore(deps): apply usePageInsets prop instead of util classes to match page margins chore(deps): bump to latest patch release versions chore(deps): revert hasAnimations on opt-in components chore(deps): revert hasAnimations on opt-in components chore(deps): revert hasAnimations on opt-in components chore(deps): revert Header.tsx to apply isHamburgerButton * test: create unit tests for internal api handlers and cmd package (#4619) * Feature view tab for features (#4631) * Add CY_RETRY option for Cypress test retry configuration - Add new CY_RETRY environment variable to control test retry behavior - Document all Cypress environment configuration in testing.md - Reference main documentation from cypress README to avoid duplication Signed-off-by: manosnoam <[email protected]> * Fix uncaught exception handler conflict in e2e.ts Multiple uncaught:exception handlers were interfering in e2e.ts, causing timeout errors to be ignored instead of failing tests. For example, pipelines/topology.ts had a timeout error handler that was overriding the global one, causing tests to run for 1.5+ hours (until Jenkins killed them). Fixed by consolidating into a single uncaught exception handler. Signed-off-by: manosnoam <[email protected]> * Add support for both process.env and --env variables for all CY_ vars Signed-off-by: manosnoam <[email protected]> * Make modular architecture workspace enabled so it works with plugins (#4595) * Move 'Use existing settings' to top of Hardware Profile select list (#4627) * change order * add unit test * move to new describe block * add not exist assertion * Add params.env to modular architecture overlay so the operator can inject the image (#4575) * fix: Update NIM enable test to be more resilient (#4590) * feat: implement robust NIM application enablement test with automatic manifest application - Add comprehensive NIM application enablement test with automatic manifest application - Create dedicated nimCommands.ts file for NIM-specific functions - Implement robust NIM card detection and enable button handling - Add proper error handling for cases where NIM is already enabled - Improve test reliability with proper timeouts and element detection - Add NIM manifest fixture for testing - Resolve async/sync mixing issues and improve scrolling behavior - Add @SanitySet3 tag for test categorization - Fix linting issues and improve code formatting * fix: resolve linting issues in NIM test files - Fix unnecessary else after return in NIMCard.ts - Fix prettier formatting issues in testEnableNIM.cy.ts - Fix indentation and spacing in nimCommands.ts - Add proper function name for test function to resolve func-names warning - Ensure all files pass eslint and prettier checks * refactor: simplify NIM test by removing already-enabled checks - Remove all logic around checking if NIM was already enabled - Simplify executeNIMTestSteps function to return void instead of boolean - Remove unnecessary conditional logic and skip statements - Clean up the test flow since deleteNIMAccount in before() handles cleanup - Make the test more straightforward and focused on the enablement flow * refactor: improve NIM test with better step logging and page object encapsulation - Replace cy.log() with cy.step() for test steps to improve test reporting - Add reload() method to ExplorePage class for better encapsulation - Use explorePage.reload() instead of direct cy.reload() calls - Keep informative cy.log() calls for context that don't represent test steps - Improve test readability and maintainability * refactor: move direct DOM selectors to NIMCard page object methods - Add findDrawerPanel() method to NIMCard class for drawer panel selector - Add findActionList() method to NIMCard class for action list selector - Add findEnableButton() method to NIMCard class for enable button selector - Replace direct cy.get() calls with page object methods in test - Improve encapsulation and follow mandatory coding guidelines for UI interactions --------- Co-authored-by: Noam Manos <[email protected]> * Model serving fix cypress platform selection errors (#4602) * Reuse same platform error alert * Add error alert for details page too * Verify Required type and add system instruction (#4638) * add required file * add required type and system instructions * add source toggle button * fix test * microcopy change for pipeline caching section; no functional changes (#4643) * microcopy change for pipeline caching section; no functional changes * updated relevant test that checked the text * RHOAIENG 29677 -Feature Views details section (#4635) * feat:updated mocks and cypress tests for feature services,views,entities * feat:added apis getfeatureviewbyname, added routes for data sources, and updated jest test files * feat:added hooks changes for feature store * feat:updated props in featurestorecodeblock, and constant changes for feature store objects * feat:added filter utils function to handle feature views, and types and utils files * feat:added filtering changes for project column for entities * feat:added filtering changes for project column for features * feat:added filtering changes for project column for feature services * feat:added filtering changes for project column for feature views and removed the unused toolbar for feature views * feat:added feature views details tab, details page and the consuming tab component * feat:added feature view tabs component, and feature view schema and transformation component * feat:added feature view lineage tab and feature view materialization tab component * fix:test cases fixes * fix(review-comments):resolved code rabbit and other review comments * fix(review-comments):added fallback for not interaction code block * fix(store-type-filter-feature-view):added custom filtering fix for the feature view for the store type filter * fix(review-comments): added feature view materialization component with sorting, replaced the table less header for entity details part with flex * fix:added compact variant for materialization component * fix:added minor textual changes and test id changes * cypress config file * cypress config file added to tsconfig for cypress * Migrate /images POST, PUT, and DELETE endpoints to frontend (#4470) * PVC Model Path Helper Text Update (#4651) * helper text * pull out connected resources * Add reviewers in llama-stack-modular-ui (rag-playground) (#4625) * Add ChristianZaccaria as reviewer in llama-stack-modular-ui (rag-playground) * Add Bobbins228 and Ygnas as reviewers in rag-playground * Fix the pipelines run metrics table (#4652) * Ignore store_session_info when displaying pipeline run metrics This is an implementation detail not directly set by the user, so it does not need to be displayed. Signed-off-by: mprahl <[email protected]> * Fix displaying pipeline run metrics with zero values If a metric had a `0` value, the code would call `toString` on `boolValue` which is undefined and caused the UI to crash until the pipeline run was archived. Signed-off-by: mprahl <[email protected]> --------- Signed-off-by: mprahl <[email protected]> * Update start / stop models with UX improvements (#4645) * remove model name (#4653) * Update Tekton output-image tags to version v2.35.1-odh (#4430) Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com> * updated tekton (#4432) * upversion dashboard (#4434) * Upversion Dashboard --------- Signed-off-by: Olga Lavtar <[email protected]> Signed-off-by: Agnieszka Gancarczyk <[email protected]> Signed-off-by: Varsha Prasad Narsing <[email protected]> Signed-off-by: dependabot[bot] <[email protected]> Signed-off-by: manosnoam [email protected] Signed-off-by: manosnoam <[email protected]> Signed-off-by: Mike Turley <[email protected]> Signed-off-by: mprahl <[email protected]> Co-authored-by: Katelynn Perry <[email protected]> Co-authored-by: Griffin Sullivan <[email protected]> Co-authored-by: Purva Naik <[email protected]> Co-authored-by: Fede Alonso <[email protected]> Co-authored-by: Pushpa Padti <[email protected]> Co-authored-by: Theia <[email protected]> Co-authored-by: mtalvi <[email protected]> Co-authored-by: Emily Samoylov <[email protected]> Co-authored-by: Conor O'Malley <[email protected]> Co-authored-by: Robert Sun <[email protected]> Co-authored-by: Juntao Wang <[email protected]> Co-authored-by: Ashley McEntee <[email protected]> Co-authored-by: Derek Xu <[email protected]> Co-authored-by: Lucas Fernandez <[email protected]> Co-authored-by: Jill Pelavin <[email protected]> Co-authored-by: Arsheen Taj Syed <[email protected]> Co-authored-by: Christian Vogt <[email protected]> Co-authored-by: Anthony Coughlin <[email protected]> Co-authored-by: Anthony Coughlin <[email protected]> Co-authored-by: Nana Nosirova <[email protected]> Co-authored-by: Sri Sai Sowjanya Darna <[email protected]> Co-authored-by: Griffin-Sullivan <[email protected]> Co-authored-by: Anish Surti <[email protected]> Co-authored-by: Claudia Alphonse <[email protected]> Co-authored-by: Lucas Fernandez <[email protected]> Co-authored-by: Dipanshu Gupta <[email protected]> Co-authored-by: Eder Ignatowicz <[email protected]> Co-authored-by: olavtar <[email protected]> Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> Co-authored-by: Noam Manos <[email protected]> Co-authored-by: Agnieszka Gancarczyk <[email protected]> Co-authored-by: Varsha <[email protected]> Co-authored-by: Matias Schimuneck <[email protected]> Co-authored-by: Andrew Ballantyne <[email protected]> Co-authored-by: Matias Schimuneck <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: Akram Ben Aissi <[email protected]> Co-authored-by: Andrew Ballantyne <[email protected]> Co-authored-by: Mike Turley <[email protected]> Co-authored-by: Anish Surti <[email protected]> Co-authored-by: Alex Creasy <[email protected]> Co-authored-by: Jenny <[email protected]> Co-authored-by: IAN MILLER <[email protected]> Co-authored-by: Gary Harden <[email protected]> Co-authored-by: Christian Zaccaria <[email protected]> Co-authored-by: Matt Prahl <[email protected]> Co-authored-by: odh-devops-app[bot] <140140902+odh-devops-app[bot]@users.noreply.github.com> Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
antowaddle
pushed a commit
to antowaddle/odh-dashboard
that referenced
this pull request
Aug 15, 2025
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
Closes https://issues.redhat.com/browse/RHOAIENG-25523
This pull request introduces significant updates to the frontend build system, module federation integration, and deployment configurations for the
model-registrypackage. The changes include adding module federation support in the Webpack configuration, creating Dockerfile and Kubernetes manifests for themodel-registryUI, and integrating a federation configuration for seamless service communication.Webpack Configuration Updates:
webpack.EnvironmentPluginandmoduleFederationPluginsinwebpack.common.jsand removing unused references inwebpack.dev.js. [1] [2] [3] [4] [5]Dockerfile and Build Pipeline:
model-registrypackage, defining separate stages for building the UI (Node.js) and backend (Go) components, and packaging them in a minimal distroless image.Kubernetes Manifests for Model Registry:
model-registryUI, including deployment, service, service account, and role configurations. These enable secure and scalable deployment of the UI component. [1] [2] [3] [4] [5]Federation Configuration:
federation-configConfigMap and updated deployment configurations to include module federation settings, enabling integration with other services in the Open Data Hub ecosystem. [1] [2] [3]Environment-Specific Customizations:
kustomization.yamlfile for therhoaienvironment to apply deployment-specific patches, such as enabling standalone mode and platform-specific arguments. [1] [2]Module Federation Testing
The ODH Dashboard application has been updated to use Module Federation, which allows dynamic loading of remote modules at runtime. However, this caused Cypress tests to fail because:
public-cypresswithout a backend/_mf/endpoints to serve remote entriesSolution
We've implemented a multi-layered approach to handle module federation in tests:
1. Build-time Module Federation Disabling
The webpack module federation configuration now checks for test environment variables:
2. Updated Build Scripts
The Cypress build scripts now set the
CYPRESS_TESTS=trueenvironment variable:{ "cypress:server:build": "ODH_DIST_DIR=./public-cypress POLL_INTERVAL=9999999 FAST_POLL_INTERVAL=9999999 WS_HOSTNAME=localhost:9002 CYPRESS_TESTS=true npm run build", "cypress:server:build:coverage": "COVERAGE=true CYPRESS_TESTS=true npm run cypress:server:build", "cypress:server:dev": "POLL_INTERVAL=9999999 FAST_POLL_INTERVAL=9999999 ODH_PORT=9001 WS_HOSTNAME=localhost:9002 CYPRESS_TESTS=true npm run start:dev" }3. Runtime Error Handling
Enhanced the
useAppExtensions.tshook to handle module federation failures gracefully:4. Cypress Mocking Layer
Created comprehensive mocking for module federation in tests:
/_mf/**/remoteEntry.jsrequestsThe mocking is implemented in:
cypress/utils/moduleFederationMock.ts- Reusable mock utilitiescypress/support/e2e.ts- Automatic setup for all tests5. Custom Cypress Command
Added a new Cypress command for easy module federation mocking:
How Has This Been Tested?
Deploying the whole application in modular architecture, there's a current deployment in:
https://oauth-openshift.apps.ui-monarch.dev.datahub.redhat.com/
Images used:
quay.io/lferrnan/odh-dashboard:mod-arch-integrationquay.io/lferrnan/model-registry-ui-federated:latestTest Impact
Request review criteria:
Self checklist (all need to be checked):
If you have UI changes:
After the PR is posted & before it merges:
mainSummary by CodeRabbit
New Features
Improvements
Testing
Documentation
Chores