Conversation
Phase 1 integrations (cherry-picked from update-docs branch): - GitHub Actions: Production-grade CI/CD pipeline with change detection, matrix builds, auto-merge, and quality gates - config.toml: Centralized configuration for cluster, versions, and fuses - docker-bake.hcl: Multi-target builds (ops, dev-portal variants) - UI/packages: Complete Backstage implementation (frontend + backend) - load-test-gateway.js: K6 load testing script All changes are additive with zero conflicts with main. Related: PR #77
Phase 2 integration: Task organization improvements Add `internal: true` flags to implementation-level tasks to declutter `task --list` output. User-facing tasks remain public (deploy, destroy, quality:check, etc.), while implementation details are hidden. Changes: - Taskfile.yaml: Mark `deploy-core` as internal - Task/bootstrap.yaml: Mark 10 tasks as internal (cilium:deploy, it:bootstrap, cert-manager:deploy, external-secrets:deploy, it:eso:apply-resources, vault:generate-secrets, vault:deploy, it:deploy-secret-and-certs, argocd:deploy, gateway:deploy) - Task/stacks.yaml: Mark `deploy` as internal Result: `task --list` now shows ~15 user-facing commands instead of 40+ Related: PR #77
📝 WalkthroughSummary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings. WalkthroughRepository-wide configuration, CI/CD workflows, task visibility, Backstage UI/theme and dependencies, observability integrations, a load test script, and documentation were updated — including new auto-merge and refactored CI workflows, Grafana UI integration, multiple dependency bumps, and added internal flags to several tasks. Changes
Sequence Diagram(s)sequenceDiagram
actor PR as Pull Request
participant GH as GitHub API
participant GHA as GitHub Actions (auto-merge)
participant AM as Auto-Merge Service
PR->>GHA: Trigger auto-merge workflow (PR target or workflow_run)
GHA->>GH: Resolve PR context (payload or workflow_run -> fetch PR)
GH-->>GHA: Return PR data (draft, state, mergeable)
alt PR found and valid
GHA->>GHA: Run guards (not draft, open, mergeable_state)
alt Guards pass
GHA->>AM: Enable auto-merge (squash) using repo token
AM-->>GHA: Confirmation enabled
else Guards fail
GHA-->>PR: Skip enabling (draft/closed/behind)
end
else No PR context
GHA-->>PR: Fail step (no PR found)
end
sequenceDiagram
participant PR as Pull Request
participant GHA as GitHub Actions (ci)
participant PD as Path Detector
participant Jobs as CI Jobs (docs-pr, core-ci, docker-build)
participant CI as ci-green aggregator
PR->>GHA: Trigger CI workflow
GHA->>PD: Detect changed paths (docs, values, base, targets)
PD-->>GHA: Classification (docs-only / full / targets)
alt Docs-only
GHA->>Jobs: Run docs-pr
Jobs-->>GHA: Docs job result
else Full run (base/targets/values changed)
GHA->>Jobs: Run core-ci (lint/security/validate)
GHA->>Jobs: Run docker-build (matrix parallel)
Jobs-->>GHA: Core and build results
end
GHA->>CI: Aggregate job statuses
alt Any failed/cancelled
CI-->>PR: CI failed
else All passed
CI-->>PR: CI green
end
Possibly related PRs
Suggested labels
Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (2 inconclusive)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches🧪 Generate unit tests (beta)
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. Comment |
|
Caution Docstrings generation - FAILED No docstrings were generated. |
There was a problem hiding this comment.
Actionable comments posted: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
UI/packages/backend/package.json (1)
18-59: Fix Prettier line length violation on line 32.The backend correctly uses the
createBackend()API and TypeScript ~5.8.0 meets requirements. However, line 32 exceeds the 88-character Prettier limit:"@backstage/plugin-catalog-backend-module-scaffolder-entity-model": "^0.2.15",This line is 89 characters. Split it across multiple lines or adjust formatting to comply with the project's Prettier configuration.
🤖 Fix all issues with AI agents
In @.github/labeler.yml:
- Around line 44-46: The glob "K8s/**/argocd*" won't match files under an
"argocd/" directory because "*" doesn't cross slashes; update the labeler rule
that currently uses the pattern "K8s/**/argocd*" to also include a recursive
directory pattern such as "K8s/**/argocd/**" (or replace with a pattern that
covers both file-prefix and directory cases) so files under an argocd/ directory
are matched; look for the "K8s/**/argocd*" entry in .github/labeler.yml and add
or swap in the recursive form.
In @.github/workflows/auto-merge.yaml:
- Around line 55-66: The workflow currently treats mergeable_state 'unknown' as
acceptable and may enable auto-merge prematurely; update the logic so auto-merge
is only enabled for explicit safe states (e.g., require
steps.ctx.outputs.mergeable_state == 'clean' rather than != 'behind'), and
ensure the earlier output assignment from pr.mergeable_state does not default to
'unknown' in a way that bypasses the check (use the actual pr.mergeable_state
and/or an explicit equality check for 'clean' when evaluating the Enable
auto-merge step while keeping the existing status == 'ready' check).
In @merge-conflict.md:
- Line 242: Add a blank line immediately before each markdown table that
currently starts at the spots referenced (tables at lines 242, 248, and 260) so
they render correctly, and update the fenced code block beginning at line 340 by
adding a language identifier (e.g., "plaintext") after the opening triple
backticks to ensure proper syntax highlighting for the block that shows the
asset path move.
In @Task/bootstrap.yaml:
- Line 67: The change marked the task vault:generate-secrets as internal: true
which will break consumers that invoke that task directly; revert/remove the
internal: true flag from the vault:generate-secrets task (or alternatively
update all external docs that reference vault:generate-secrets to reflect the
change), and also wrap or split long YAML lines in Task/bootstrap.yaml that
exceed the 88-char limit (e.g., long command strings, descriptions or
annotations) by using YAML folded style (>) or breaking long strings into
shorter sequenced lines so all lines comply with the Prettier 88-character rule.
In @Task/stacks.yaml:
- Line 82: The deploy task in Task/stacks.yaml was marked internal (internal:
true), which hides the documented public command `task stacks:deploy`; remove
the `internal: true` entry from the `deploy` task (or if the intent is to make
it private, instead update the README to remove or change the `task
stacks:deploy` documentation), ensuring references to the `deploy` task name
remain unchanged so users can invoke `task stacks:deploy` as documented.
In @UI/packages/app/src/theme/primer.ts:
- Around line 83-101: The theme currently uses Material-UI v5 syntax
components.MuiCssBaseline.styleOverrides['@global'], which is incompatible with
our MUI v4 dependency; change the theme shape to use
overrides.MuiCssBaseline['@global'] instead and move the existing '@global'
object (the ':root', '[data-theme-mode="dark"]', html and '#root' entries) under
overrides → MuiCssBaseline → '@global' so MUI v4 will apply the global CSS
properly.
🧹 Nitpick comments (8)
UI/packages/app/src/theme/primer.ts (1)
56-56: Line exceeds 88 character limit.This line is approximately 130 characters. Per coding guidelines, line length must be 88 characters.
♻️ Suggested fix
- fontFamily: '-apple-system, BlinkMacSystemFont, "Segoe UI", Helvetica, Arial, sans-serif, "Apple Color Emoji", "Segoe UI Emoji"', + fontFamily: [ + '-apple-system', + 'BlinkMacSystemFont', + '"Segoe UI"', + 'Helvetica', + 'Arial', + 'sans-serif', + '"Apple Color Emoji"', + '"Segoe UI Emoji"', + ].join(', '),UI/packages/app/src/components/catalog/EntityPage.tsx (3)
94-125: Consider iframe security and maintainability improvements.A few concerns with the Grafana iframe integration:
- Hardcoded dashboard UID (
fRIvzUZMz) - if the dashboard is recreated or imported elsewhere, this breaks silently.- No
sandboxattribute - consider addingsandbox="allow-scripts allow-same-origin"to restrict iframe capabilities.- Warning message exposes internal paths (line 105-106) - the file path
K8s/backstage/backstage/templates/cm-tpl.yamlmay reveal internal infrastructure details.♻️ Suggested improvements
+ // Consider moving to app-config for maintainability + const LOKI_DASHBOARD_UID = 'fRIvzUZMz'; + return ( <iframe title="Container Logs" src={src} className={classes.iframe} + sandbox="allow-scripts allow-same-origin" /> );For the warning message, consider a more generic message:
- message='The "grafana.domain" configuration is missing in app-config. Please check your K8s/backstage/backstage/templates/cm-tpl.yaml and ensure DNS_SUFFIX is set.' + message='The "grafana.domain" configuration is missing in app-config. Please configure Grafana integration in your deployment settings.'
127-158: Same concerns apply to EntityKubernetesMetrics.This component has the same issues as
EntityLokiLogs: hardcoded dashboard UID (GlXkUBGiz), no iframe sandbox, and exposed internal paths in warning message.Consider extracting common iframe logic into a reusable component to reduce duplication.
♻️ Extract common Grafana iframe component
interface GrafanaIframeProps { title: string; dashboardUid: string; dashboardPath: string; queryParams: string; } const GrafanaIframe = ({ title, dashboardUid, dashboardPath, queryParams }: GrafanaIframeProps) => { const classes = useStyles(); const { entity } = useEntity(); const config = useApi(configApiRef); const grafanaUrl = config.getOptionalString('grafana.domain'); if (!grafanaUrl) { return ( <WarningPanel title="Integration Disabled" message='The "grafana.domain" configuration is missing.' /> ); } const src = `${grafanaUrl}${dashboardPath}${queryParams}`; return ( <iframe title={title} src={src} className={classes.iframe} sandbox="allow-scripts allow-same-origin" /> ); };
332-334: Extract inline styles to useStyles.The repeated inline
style={{ height: '100%', width: '100%', minHeight: '700px' }}for TopologyPage containers should be added to theuseStyleshook for consistency.♻️ Suggested refactor
const useStyles = makeStyles({ iframe: { width: '100%', height: '800px', border: 'none', }, + topologyContainer: { + height: '100%', + width: '100%', + minHeight: '700px', + }, });Then use
className={classes.topologyContainer}instead of inline styles.Also applies to: 386-388, 419-421
load-test-gateway.js (2)
17-17: Document that insecure TLS is for testing only.The
insecureSkipTLSVerify: truesetting is appropriate for load testing against local or staging environments with self-signed certificates. Consider adding a comment clarifying that this script is intended for non-production testing only.📝 Suggested documentation addition
thresholds: { http_req_duration: ['p(95)<200'], // Goal: p95 should be under 200ms }, + // Skip TLS verification for testing against self-signed certificates + // WARNING: Only use this script against non-production environments insecureSkipTLSVerify: true, };
6-12: Verify target system capacity for aggressive stress testing.The test configuration uses 300 constant virtual users for 2 minutes with no sleep between requests (maximum pressure). This is an extremely aggressive stress test that will generate thousands of requests per second. Ensure your target environment can handle this load without cascading failures that could affect other systems. Consider starting with fewer VUs (e.g., 50-100) and gradually increasing to find the breaking point.
Also applies to: 27-27
.github/workflows/auto-merge.yaml (1)
19-19: Consider adding explanatory comments for complex conditional logic.The repository check and workflow_run fallback logic (lines 19, 26-39) are correct but complex. Adding inline comments would improve maintainability, especially for the dual trigger paths (pull_request_target vs workflow_run).
📝 Suggested documentation improvements
auto-merge: runs-on: ubuntu-latest + # Only run for PRs from the same repo (not forks) or when triggered by workflow completion if: github.event.pull_request.head.repo.full_name == github.repository || github.event.workflow_run.event == 'pull_request' steps: - name: Get PR context id: ctx uses: actions/github-script@v8 with: script: | + // For pull_request_target events, use the payload directly + // For workflow_run events, fetch the PR from the completed workflow const pr = context.payload.pull_request ? context.payload.pull_request : await (async () => {Also applies to: 26-39
.github/workflows/ci.yaml (1)
129-175: Matrix build job is well-designed, butjlumbroso/free-disk-space@mainpins to a moving target.Pinning to
@main(line 146) is risky—it can break builds unexpectedly if the action changes.🔧 Pin to a specific version or commit SHA
- name: Free Disk Space (Ubuntu) - uses: jlumbroso/free-disk-space@main + uses: jlumbroso/free-disk-space@v1.3.1 with: tool-cache: false
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (19)
.github/ISSUE_TEMPLATE/bug_report.yml.github/ISSUE_TEMPLATE/config.yml.github/ISSUE_TEMPLATE/documentation.yml.github/ISSUE_TEMPLATE/feature_request.yml.github/labeler.yml.github/pull_request_template.md.github/workflows/auto-merge.yaml.github/workflows/ci.yaml.github/workflows/docs.yamlTask/bootstrap.yamlTask/stacks.yamlTaskfile.yamlUI/packages/app/package.jsonUI/packages/app/src/components/catalog/EntityPage.tsxUI/packages/app/src/theme/primer.tsUI/packages/backend/DockerfileUI/packages/backend/package.jsonload-test-gateway.jsmerge-conflict.md
🧰 Additional context used
📓 Path-based instructions (6)
**/*.{js,ts,tsx,jsx,json,css,scss,yaml,yml,md}
📄 CodeRabbit inference engine (AGENTS.md)
Line length must be 88 characters (Prettier configuration)
Files:
load-test-gateway.jsTask/stacks.yamlmerge-conflict.mdUI/packages/app/src/components/catalog/EntityPage.tsxTaskfile.yamlTask/bootstrap.yamlUI/packages/app/package.jsonUI/packages/app/src/theme/primer.tsUI/packages/backend/package.json
**/*.{js,ts,tsx,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
Use single quotes for quote style (Prettier configuration)
Files:
load-test-gateway.jsUI/packages/app/src/components/catalog/EntityPage.tsxUI/packages/app/src/theme/primer.ts
**/*.{yaml,yml}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{yaml,yml}: All workloads must specify CPU/memory requests and limits using appropriate units (m for CPU, Mi/Gi for memory)
YAML files must extend yamllint default configuration with document-start and comments disabled and relaxed braces, brackets, quoted-strings
All Kubernetes resources must include required labels:owner,business-unit,environment, andapp.kubernetes.io/part-of
PriorityClasses are required on all workloads
No hardcoded secrets in manifests - use Vault and External Secrets Operator for secret management
Files:
Task/stacks.yamlTaskfile.yamlTask/bootstrap.yaml
**/*.md
📄 CodeRabbit inference engine (AGENTS.md)
**/*.md: Markdown files should have prose wrap enabled (Prettier configuration)
Markdown line length should be 92 characters (markdownlint configuration)
Unordered lists in Markdown must use dashes as list markers (markdownlint configuration)
Markdown files may only use specific HTML elements:<details>,<summary>,<img>(markdownlint configuration)
Files:
merge-conflict.md
UI/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Backstage developer portal must use React with TypeScript and Yarn workspaces
Files:
UI/packages/app/src/components/catalog/EntityPage.tsxUI/packages/app/src/theme/primer.ts
Taskfile.yaml
📄 CodeRabbit inference engine (AGENTS.md)
Use Task (go-task) for automation and orchestration of common development tasks
Files:
Taskfile.yaml
🧠 Learnings (8)
📚 Learning: 2025-12-19T21:34:55.885Z
Learnt from: CR
Repo: rou-cru/idp-blueprint PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-19T21:34:55.885Z
Learning: Applies to Taskfile.yaml : Use Task (go-task) for automation and orchestration of common development tasks
Applied to files:
Task/stacks.yaml.github/labeler.ymlTaskfile.yamlTask/bootstrap.yaml
📚 Learning: 2025-12-19T21:34:55.885Z
Learnt from: CR
Repo: rou-cru/idp-blueprint PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-19T21:34:55.885Z
Learning: Applies to **/applicationset*.{yaml,yml} : ArgoCD ApplicationSets must use the app-of-appsets pattern for bulk deployments
Applied to files:
Task/stacks.yaml.github/labeler.yml
📚 Learning: 2025-12-19T21:34:55.885Z
Learnt from: CR
Repo: rou-cru/idp-blueprint PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-19T21:34:55.885Z
Learning: Applies to **/{application,applicationset}*.{yaml,yml} : ArgoCD sync waves must be properly configured in all ApplicationSet and Application resources
Applied to files:
Task/stacks.yaml.github/labeler.yml
📚 Learning: 2025-12-19T21:34:55.885Z
Learnt from: CR
Repo: rou-cru/idp-blueprint PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-19T21:34:55.885Z
Learning: Applies to **/*.{yaml,yml} : No hardcoded secrets in manifests - use Vault and External Secrets Operator for secret management
Applied to files:
.github/labeler.ymlTask/bootstrap.yaml
📚 Learning: 2025-12-19T21:34:55.885Z
Learnt from: CR
Repo: rou-cru/idp-blueprint PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-19T21:34:55.885Z
Learning: Applies to **/kustomization.{yaml,yml} : Use Kustomize for Infrastructure as Code base organization and component modularity
Applied to files:
.github/labeler.ymlTask/bootstrap.yaml
📚 Learning: 2025-12-19T21:34:55.885Z
Learnt from: CR
Repo: rou-cru/idp-blueprint PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-19T21:34:55.885Z
Learning: Applies to **/cilium/**/*.{yaml,yml} : Cilium components are exempt from resource requirements (CPU/memory requests and limits)
Applied to files:
Task/bootstrap.yaml
📚 Learning: 2025-12-19T21:34:55.885Z
Learnt from: CR
Repo: rou-cru/idp-blueprint PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-19T21:34:55.885Z
Learning: Applies to UI/**/*.{ts,tsx} : Backstage developer portal must use React with TypeScript and Yarn workspaces
Applied to files:
UI/packages/app/package.jsonUI/packages/backend/package.json
📚 Learning: 2025-12-19T21:34:55.885Z
Learnt from: CR
Repo: rou-cru/idp-blueprint PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-19T21:34:55.885Z
Learning: Use Playwright MCP to check visual rendering and browser debugging for Backstage and Astro Documentation changes when available
Applied to files:
UI/packages/app/package.json
🧬 Code graph analysis (1)
UI/packages/app/src/theme/primer.ts (1)
UI/packages/app/src/App.tsx (1)
UnifiedThemeProvider(83-85)
🪛 LanguageTool
merge-conflict.md
[uncategorized] ~20-~20: The official name of this software platform is spelled with a capital “H”.
Context: ...ws (Complete Replacement) Location: .github/workflows/, .github/dependabot.yml, ...
(GITHUB)
[uncategorized] ~20-~20: The official name of this software platform is spelled with a capital “H”.
Context: ...nt) Location: .github/workflows/, .github/dependabot.yml, .github/labeler.yml,...
(GITHUB)
[uncategorized] ~20-~20: The official name of this software platform is spelled with a capital “H”.
Context: .../workflows/, .github/dependabot.yml, .github/labeler.yml, .github/pull_request_tem...
(GITHUB)
[uncategorized] ~20-~20: The official name of this software platform is spelled with a capital “H”.
Context: ...dependabot.yml, .github/labeler.yml, .github/pull_request_template.md`, Issue templa...
(GITHUB)
[uncategorized] ~35-~35: The official name of this software platform is spelled with a capital “H”.
Context: ...e management** Action: Copy entire .github/ directory from update-docs → main...
(GITHUB)
[uncategorized] ~37-~37: The official name of this software platform is spelled with a capital “H”.
Context: ...in **Risk:** None.mainhas minimal.github/` content. --- #### 1.2 Configuration...
(GITHUB)
[uncategorized] ~173-~173: The official name of this software platform is spelled with a capital “H”.
Context: ...e 1: Direct Integrations (Safe) 1. Copy .github/ directory from update-docs → main...
(GITHUB)
🪛 markdownlint-cli2 (0.18.1)
merge-conflict.md
242-242: Tables should be surrounded by blank lines
(MD058, blanks-around-tables)
248-248: Tables should be surrounded by blank lines
(MD058, blanks-around-tables)
260-260: Tables should be surrounded by blank lines
(MD058, blanks-around-tables)
340-340: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🔍 Remote MCP
Now let me search for more specific information about the auto-merge workflow and GitHub Actions best practices.Based on my research, I now have sufficient context to provide a comprehensive summary of the additional facts relevant to this PR review.
Summary of Additional Context for PR Review
Load Testing with k6
k6 is optimized for minimal resource consumption and designed for running high-load performance tests such as spike, stress, or soak tests. The load test script added in this PR configures 300 virtual users for 2 minutes with a stress testing scenario. A stress test simulates the traffic that the application is expected to experience at the highest point of the day or season, focusing on the highest amount of traffic that the application experiences. The p95 latency threshold of 200ms is a reasonable performance requirement for an API endpoint.
Backstage Dependency Updates - Breaking Changes
Backstage v1.35.0 removed support for the old backend system. This is a significant breaking change that affects multiple backend plugins being updated in this PR. Breaking changes in packages that have reached version >=1.0.0 will only be done when necessary and with the goal of having minimal impact, and when possible, there will always be a deprecation path for a breaking change.
Key plugin updates with potential impacts:
- New Backstage installations will now be based on TypeScript version 5.8, while internal development moves up to version 5.6, and it is recommended to upgrade the TypeScript version to at least 5.6.
- Backstage-UI has updates, one of which is breaking: if you were using the Cell component with text-specific props, you should now instead use the CellText component, as plain Cell is now meant to be used with generic child elements instead.
@backstage/backend-defaults: v0.13.1 → v0.14.0 (minor bump, but paired with breaking changes in dependent plugins)@backstage/plugin-scaffolder-backend: v3.0.1 → v3.1.0 (minor bump)@backstage/plugin-kubernetes-backend: v0.20.4 → v0.21.0 (minor bump)
Grafana Plugin Integration
The PR adds a new dependency @backstage-community/plugin-grafana (^0.12.0) and integrates it into the EntityPage component with conditional rendering based on grafana.domain configuration. This plugin enables Kubernetes metrics and container logs visualization through Grafana's Loki and metrics endpoints.
Auto-Merge Workflow Requirements
For auto-merge to be enabled on a pull request, the target repository must have Allow auto-merge enabled in settings, and the pull request base must have a branch protection rule with at least one requirement enabled. The workflow uses the peter-evans/enable-pull-request-automerge@v3 action with squash merge strategy and includes conditional checks for draft status, mergeable state, and PR openness. If you use the default GITHUB_TOKEN, the merge will not trigger further workflow runs; if you want to trigger further workflow runs, you will need to use a repo scoped Personal Access Token (PAT).
[::web_search::]
⏰ 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: Build portal
🔇 Additional comments (35)
Taskfile.yaml (1)
39-39: Markingdeploy-coreas internal enforces its existing design intent.The task description on line 38 states "do not run directly," and adding
internal: truemakes this constraint explicit and enforceable. No external references to this task exist, confirming the change poses no breaking risk.UI/packages/app/src/theme/primer.ts (2)
34-38: LGTM! Text palette properly defined.The text color tokens align with GitHub Primer dark palette and are correctly consumed by components like
LogoFull.tsxviatheme.palette.text.primary.
177-346: Component overrides provide comprehensive dark theme coverage.The added overrides for Typography, TableCell, Link, Chip, Tab, Dialog components, etc., consistently apply Primer dark colors. The color values match the defined palette.
UI/packages/app/package.json (3)
18-18: New Grafana plugin dependency added.The
@backstage-community/plugin-grafanaat version^0.12.0enables the Grafana dashboard integration used inEntityPage.tsx. Ensure the backend has corresponding Grafana configuration (grafana.domain) set up.
23-49: Backstage dependency updates look appropriate.These are minor/patch version bumps aligning with Backstage ecosystem updates. The versions appear consistent across the Backstage package family.
50-50: Breaking change in@backstage/ui0.10.0 does not require action in this codebase.The version bump includes a refactoring of the Cell component to a generic wrapper, with text-specific props moved to a new CellText component. However, the Cell component is not used in this codebase—only the CSS stylesheet from
@backstage/uiis imported. No migration is required.Likely an incorrect or invalid review comment.
UI/packages/app/src/components/catalog/EntityPage.tsx (4)
74-84: LGTM! Helper functions for relation checks.The
hasSubcomponentsandhasApishelpers correctly check entity relations using proper nullish coalescing and optional chaining.
243-250: LGTM! Conditional rendering for subcomponents.The
EntitySwitchwithhasSubcomponentsproperly gates theEntityHasSubcomponentsCardrendering.
305-318: LGTM! API route now conditional.Adding
if={hasApis}to the API route improves UX by hiding the tab when no APIs are associated with the entity.
284-290: Logs and Metrics routes added consistently.The new
/logsand/metricsroutes are added toserviceEntityPage,websiteEntityPage, and partially todefaultEntityPage(logs only). This provides good observability integration.Note:
defaultEntityPageonly has/logsbut not/metrics- verify if this is intentional.Also applies to: 353-359, 414-416
UI/packages/backend/Dockerfile (1)
53-53: Correct fix for yarn cache cleanup.The previous command attempted to
rm -rfthe output ofyarn cache clean, which is incorrect since that command doesn't output a directory path. The updated command properly invokesyarn cache cleanto clear the cache, improving build correctness.merge-conflict.md (2)
170-199: Well-structured execution plan with appropriate risk management.The three-phase execution plan effectively balances risk and value:
- Phase 1 safely integrates non-conflicting changes through direct file copies
- Phase 2 correctly identifies manual review requirements to preserve operational logic while improving UX
- Phase 3 appropriately defers high-conflict structural changes to avoid disruption
The inclusion of specific verification steps for each phase demonstrates good operational discipline.
202-231: Comprehensive verification and prioritization strategy.The verification steps cover all critical integration points (CI, config, UI, load testing), and the priority-based summary table (P0-P3) provides clear guidance for sequencing work. This approach should minimize integration risks while maximizing value delivery.
.github/workflows/auto-merge.yaml (2)
4-5: Verify pull_request_target security posture.Using
pull_request_targetprovides the necessary write permissions for auto-merge, and the repository check at line 19 prevents forks from triggering the workflow. However, ensure this aligns with your security policy, aspull_request_targetgrants elevated permissions. The current implementation is safe since it only makes GitHub API calls without checking out or executing code from the PR.Based on external tools context: Your repository should have "Allow auto-merge" enabled in settings and branch protection rules configured for this workflow to function properly.
Also applies to: 19-19
69-69: Document GITHUB_TOKEN limitation for triggering downstream workflows.The workflow uses the default
GITHUB_TOKENto enable auto-merge. Be aware that merges performed with this token will not trigger additional workflow runs (e.g., post-merge CI, deployments). If you need to trigger downstream workflows after merge, you'll need to use a repo-scoped Personal Access Token (PAT) instead.Based on external tools context: This is a known GitHub limitation designed to prevent infinite workflow loops.
.github/workflows/docs.yaml (1)
55-55: Disregard this review—the premise is incorrect.Version 22 is already in place and appropriate. No downgrade from version 24 occurred; version 24 is never used in the workflows. Node.js 22 is an LTS release and aligns with the project's dependency requirements (UI/package.json supports
20 || 22).Likely an incorrect or invalid review comment.
load-test-gateway.js (1)
20-26: No changes needed—the load test is correctly written for this environment.The catalog endpoint in this dev/test environment does not require authentication. The
app-config.yamlcontains noauth:section (standard Backstage configurations with authentication enabled always include one), and the SECURITY.md explicitly documents that this environment has "No authentication on some services." The script appropriately makes an unauthenticated GET request and expects a 200 response.Likely an incorrect or invalid review comment.
.github/workflows/ci.yaml (3)
83-98: Docs-only job looks correct.The conditional
needs.changes.outputs.docs == 'true' && needs.changes.outputs.full == 'false'properly gates this job to run only for documentation-only changes, avoiding unnecessary CI runs.
177-189:ci-greengate job correctly aggregates status from all dependent jobs.The
if: always()combined with checkingneeds.*.resultforfailureorcancelledis a common pattern for required status checks. This ensures CI fails if any upstream job fails, even if some jobs were skipped.
17-81: Path-based change detection implementation is sound and current.The
changesjob correctly usesdorny/paths-filter@v3(the current latest version as of January 2026) to detect changed areas and dynamically compute build targets. The logic for forcing all targets whenbasechanges is correct, and thejqcommand on line 67 properly handles edge cases including empty arrays and unmatched filters.Note: While
dorny/paths-filter@v3remains officially active, some organizations usestep-security/paths-filteras a more secure drop-in replacement if security policies require it..github/pull_request_template.md (1)
1-13: Clean and practical PR template.The template covers essential sections: summary, testing approach, and checklist items. The structure encourages contributors to document changes and verify quality gates before merging.
.github/ISSUE_TEMPLATE/config.yml (1)
4-4: Documentation URL updated consistently.The URL migration to
idp-blueprint.roura.xyzaligns with the broader PR changes across all issue templates..github/ISSUE_TEMPLATE/feature_request.yml (1)
22-22: URL update is consistent with the documentation migration..github/ISSUE_TEMPLATE/bug_report.yml (1)
22-22: URL update is consistent with other templates..github/ISSUE_TEMPLATE/documentation.yml (1)
22-22: Documentation URLs updated consistently.Both the prerequisites checkbox and the placeholder example URL have been updated to the new domain, maintaining consistency across all issue templates.
Also applies to: 70-70
.github/labeler.yml (10)
52-53:IT/cilium/**/*looks consistent with the new layout.
Nothing else to flag here.
59-61: Vault globs look good; confirm you really want both IT- and K8s-scoped matches.
IT/vault/**/*+K8s/**/vault/**/*is clear and consistent.
67-71: External Secrets globs look good and align with “no hardcoded secrets” expectations.
Nice to see bothexternal-secret*.yamlandsecret-store*.yamlcovered.
76-79: Kyverno/policies relabeling toK8s/policies/**/*seems correct for the new structure.
No concerns, assumingK8s/policies/is now the canonical location.
84-88: Observability relabeling toK8s/observability/**/*is straightforward.
93-96: CI/CD relabeling toK8s/cicd/**/*is straightforward.
110-113: Gateway relabeling toIT/gateway/**/*looks consistent.
119-122: File pathIT/k3d-cluster.yamlis correct. The file exists with the correct.yamlextension and matches the reference in the labeler configuration.
101-105: Current glob patterns adequately cover cert-manager configurations; no cert-manager manifests found in K8s/.The K8s/ directory contains only references to cert-manager (namespace exclusions, RBAC rules, comments, and dependencies) but no cert-manager manifest files. The existing patterns at lines 101-105 are sufficient.
22-22: No action needed—filename case and glob patterns are correct.The file
.github/pull_request_template.mdexists with the exact casing shown in the labeler and will correctly match the glob pattern on line 22. Note that this file will also trigger thedocumentationlabel via the**/*.mdpattern on line 9, which is expected since it is both a GitHub configuration file and a markdown document.
| ### A. Documentation Structure Changes | ||
|
|
||
| #### A.1 Directory Reorganization | ||
| | Old Path (`main`) | New Path (`update-docs`) | Action | |
There was a problem hiding this comment.
Fix markdown formatting issues for better rendering.
The document has minor markdown formatting issues that should be corrected:
- Tables at lines 242, 248, and 260 need blank lines before them
- The fenced code block at line 340 needs a language identifier
📋 Proposed formatting fixes
Fix 1: Add blank line before table at line 242
#### A.1 Directory Reorganization
+
| Old Path (`main`) | New Path (`update-docs`) | Action |Fix 2: Add blank line before table at line 248
#### A.2 Concept Files → Implementation Files
+
| File in `main` (old path) | File in `update-docs` (new path) | Similarity | Action |Fix 3: Add blank line before table at line 260
#### A.3 Component Files → Implementation/Components Files
+
| File in `main` | File in `update-docs` | Similarity | Action |Fix 4: Add language identifier to code block
-```
+```plaintext
assets/diagrams/concepts/backbone-loops.* → assets/diagrams/implementation/backbone-loops.*Also applies to: 248-248, 260-260, 340-351
🧰 Tools
🪛 markdownlint-cli2 (0.18.1)
242-242: Tables should be surrounded by blank lines
(MD058, blanks-around-tables)
🤖 Prompt for AI Agents
In @merge-conflict.md at line 242, Add a blank line immediately before each
markdown table that currently starts at the spots referenced (tables at lines
242, 248, and 260) so they render correctly, and update the fenced code block
beginning at line 340 by adding a language identifier (e.g., "plaintext") after
the opening triple backticks to ensure proper syntax highlighting for the block
that shows the asset path move.
|
|
||
| cilium:deploy: | ||
| desc: "Install Cilium as CNI" | ||
| internal: true |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's examine the Task/bootstrap.yaml file structure
head -100 Task/bootstrap.yaml | cat -nRepository: rou-cru/idp-blueprint
Length of output: 3748
🏁 Script executed:
# Check file for yamllint configuration and inspect the modified lines context
tail -n +50 Task/bootstrap.yaml | head -n 250 | cat -nRepository: rou-cru/idp-blueprint
Length of output: 10103
🏁 Script executed:
# Run the verification script from the review comment to check for external references
rg -nP --type yaml --type md --type sh 'task.*(bootstrap:)?(cilium:deploy|it:bootstrap|cert-manager:deploy|external-secrets:deploy|it:eso:apply-resources|vault:generate-secrets|vault:deploy|it:deploy-secret-and-certs|argocd:deploy|gateway:deploy)\b' -g '!Task/bootstrap.yaml' -g '!Taskfile.yaml'Repository: rou-cru/idp-blueprint
Length of output: 234
🏁 Script executed:
# Check the line length compliance (88 chars per guidelines)
awk 'length > 88 {print NR": "length" chars: "$0}' Task/bootstrap.yaml | head -20Repository: rou-cru/idp-blueprint
Length of output: 1303
🏁 Script executed:
# Verify the exact context of the modified lines
sed -n '60,70p; 75,90p; 85,100p; 100,115p; 120,130p; 140,155p; 220,235p; 235,250p; 270,285p' Task/bootstrap.yaml | cat -nRepository: rou-cru/idp-blueprint
Length of output: 5203
Do not approve: Breaking changes detected with vault:generate-secrets and line length violations.
The verification found that vault:generate-secrets is explicitly referenced in external documentation (K8s/cicd/argo-workflows/PIPELINE-README.md, lines 109 and 323), instructing users to run this task directly. Marking it internal: true will break those documented workflows. The other tasks (cilium:deploy, cert-manager:deploy, external-secrets:deploy, etc.) appear safe to mark internal, but this change cannot proceed as-is.
Additionally, the file has multiple line length violations exceeding the 88-character Prettier limit (lines 33: 141 chars, 44: 94 chars, 130: 120 chars, 224: 132 chars, 285-288: 103-139 chars).
Actions required:
- Remove
internal: truefromvault:generate-secretstask, or update external documentation to reflect the API change - Fix line length violations to comply with coding guidelines
🤖 Prompt for AI Agents
In @Task/bootstrap.yaml at line 67, The change marked the task
vault:generate-secrets as internal: true which will break consumers that invoke
that task directly; revert/remove the internal: true flag from the
vault:generate-secrets task (or alternatively update all external docs that
reference vault:generate-secrets to reflect the change), and also wrap or split
long YAML lines in Task/bootstrap.yaml that exceed the 88-char limit (e.g., long
command strings, descriptions or annotations) by using YAML folded style (>) or
breaking long strings into shorter sequenced lines so all lines comply with the
Prettier 88-character rule.
|
|
||
| deploy: | ||
| desc: "Deploy all application stacks via ArgoCD ApplicationSets (feature-gated)" | ||
| internal: true |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Search for direct invocations of stacks:deploy
rg -n --type yaml --type md --type sh 'task.*stacks:deploy' -g '!Taskfile.yaml'Repository: rou-cru/idp-blueprint
Length of output: 137
🏁 Script executed:
sed -n '78,88p' README.mdRepository: rou-cru/idp-blueprint
Length of output: 454
Do not mark the deploy task as internal—it breaks a documented public API.
The README explicitly documents task stacks:deploy as a user-callable command: "reapply stacks only (GitOps layer)." Marking the task as internal: true will prevent users from invoking it, breaking the documented behavior without corresponding documentation updates.
Either remove the internal: true change or update the README to reflect that this command is no longer available to users.
🤖 Prompt for AI Agents
In @Task/stacks.yaml at line 82, The deploy task in Task/stacks.yaml was marked
internal (internal: true), which hides the documented public command `task
stacks:deploy`; remove the `internal: true` entry from the `deploy` task (or if
the intent is to make it private, instead update the README to remove or change
the `task stacks:deploy` documentation), ensuring references to the `deploy`
task name remain unchanged so users can invoke `task stacks:deploy` as
documented.
| '@global': { | ||
| ':root': { | ||
| '--bui-bg': '#0d1117', | ||
| }, | ||
| '[data-theme-mode="dark"]': { | ||
| '--bui-bg': '#0d1117', | ||
| }, | ||
| html: { | ||
| WebkitFontSmoothing: 'auto', | ||
| height: '100%', | ||
| width: '100%', | ||
| backgroundColor: '#0d1117', | ||
| }, | ||
| '#root': { | ||
| height: '100%', | ||
| width: '100%', | ||
| backgroundColor: '#0d1117', | ||
| }, | ||
| }, |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cat UI/packages/app/package.json | jq '.dependencies["@material-ui/core"]'Repository: rou-cru/idp-blueprint
Length of output: 74
🌐 Web query:
Material-UI v4 @global JSS syntax documentation
💡 Result:
Material‑UI v4 uses JSS’s @global via theme overrides or withStyles/makeStyles. Two common patterns:
-
Theme overrides (e.g. CssBaseline):
const theme = createMuiTheme({
overrides: {
MuiCssBaseline: {
'@global': {
html: { WebkitFontSmoothing: 'auto' },
body: { margin: 0 }
}
}
}
});
—see docs. [1] (v4.mui.com) -
withStyles / makeStyles global block:
const GlobalCss = withStyles({
'@global': {
'.my-class': { color: 'red' },
'html, body': { height: '100%' }
}
})(() => null);
—example and jss-plugin-global info. [2][3] (stackoverflow.com)
Notes:
- v4 includes the jss-plugin-global plugin by default (so @global works). [3] (v4.mui.com)
If you want, I can paste a minimal runnable example (ThemeProvider+CssBaseline or a GlobalCss component).
Citations:
- 1: https://v4.mui.com/customization/globals/?utm_source=openai
- 2: https://stackoverflow.com/questions/56263634/how-to-create-deterministic-styles-when-using-createstyles?utm_source=openai
- 3: https://v4.mui.com/zh/styles/advanced/?utm_source=openai
🏁 Script executed:
sed -n '75,110p' UI/packages/app/src/theme/primer.tsRepository: rou-cru/idp-blueprint
Length of output: 1043
🏁 Script executed:
sed -n '1,85p' UI/packages/app/src/theme/primer.ts | tail -50Repository: rou-cru/idp-blueprint
Length of output: 1810
🌐 Web query:
Material-UI v4 vs v5 styleOverrides vs overrides syntax difference
💡 Result:
Short summary — main syntax/behaviour differences between MUI v4 and v5
-
Theme shape
- v4: theme keys were props, overrides, and typography/palette/etc. (e.g. theme.overrides.MuiButton.root). (v4.mui.com)
- v5: everything lives under theme.components with component keys; use defaultProps, styleOverrides, and variants (e.g. theme.components.MuiButton.styleOverrides.root). (mui.com)
-
Names
- overrides → styleOverrides
- props → defaultProps
- (both moved into theme.components). (mui.com)
-
Styling engine and selectors
- v4 used JSS; nested/local-rule syntax like "&$focused" and "$someClass" was common. (v4.mui.com)
- v5 uses Emotion (default) so local-rule ($) JSS selectors don’t work — use global class names (.Mui-focused, .Mui-disabled, etc.) or standard CSS selectors. Migration guide shows exact replacements. (mui.com)
-
Dynamic overrides / callbacks
- v4 theme.overrides were static objects.
- v5 styleOverrides can be a callback receiving ({ ownerState, theme }) so you can style based on props/state; also supports using theme.unstable_sx inside styleOverrides. (mui.com)
-
Variants API
- v5 exposes a structured variants API inside components for adding/overriding prop-based variants (array of { props, style }). (v4 had more ad‑hoc approaches). (mui.com)
-
Migration notes (practical)
- Run the v4→v5 codemods. Replace theme.overrides → theme.components[Key].styleOverrides and theme.props → theme.components[Key].defaultProps. Convert any "$" JSS selectors to ".Mui-..." global selectors and update nested selectors per the migration guide. (mui.com)
If you want, I can convert a specific v4 theme.overrides snippet to the v5 styleOverrides form.
Citations:
- 1: https://v4.mui.com/customization/globals/?utm_source=openai
- 2: https://mui.com/material-ui/customization/theme-components/?utm_source=openai
- 3: https://mui.com/material-ui/migration/v5-style-changes/?utm_source=openai
- 4: https://v4.mui.com/customization/globals/?utm_source=openai
- 5: https://mui.com/material-ui/migration/v5-style-changes/?utm_source=openai
- 6: https://mui.com/blog/callback-support-in-style-overrides/?utm_source=openai
- 7: https://mui.com/material-ui/customization/theme-components/?utm_source=openai
- 8: https://mui.com/material-ui/migration/migration-v4/?utm_source=openai
Incorrect Material-UI theme syntax for v4.
The code uses components.MuiCssBaseline.styleOverrides['@global'], which is Material-UI v5 syntax. The project has MUI v4 (^4.12.2) installed, which does not support styleOverrides under components. MUI v4 requires overrides.MuiCssBaseline['@global'] instead.
Update the theme structure to use the v4-compatible syntax:
overrides: {
MuiCssBaseline: {
'@global': { ... }
}
}
🤖 Prompt for AI Agents
In @UI/packages/app/src/theme/primer.ts around lines 83 - 101, The theme
currently uses Material-UI v5 syntax
components.MuiCssBaseline.styleOverrides['@global'], which is incompatible with
our MUI v4 dependency; change the theme shape to use
overrides.MuiCssBaseline['@global'] instead and move the existing '@global'
object (the ':root', '[data-theme-mode="dark"]', html and '#root' entries) under
overrides → MuiCssBaseline → '@global' so MUI v4 will apply the global CSS
properly.
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
|
|
This pull request has been automatically marked as stale because it has not had recent activity in the last 30 days. It will be closed in 7 days if no further activity occurs. If you're still working on this PR, please:
Thank you for your contribution! |
|
This pull request has been automatically closed due to inactivity. If you'd like to continue this work, please:
Thank you for your contribution! |




No description provided.