feat(x2a): RHDH 1.10 support#3321
Conversation
|
Important This PR includes changes that affect public-facing API. Please ensure you are adding/updating documentation for new features or behavior. Changed Packages
|
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| url: '${AAP_URL:-https://aap.example.com}', | ||
| orgName: '${AAP_ORG_NAME:-MyOrganization}', | ||
| username: '${AAP_USERNAME}', | ||
| password: '${AAP_PASSWORD}', |
|
The plugins and conversions can be executed in RHDH 1.10 and OCP: 
|
|
The DCM flow with 
|
mareklibra
force-pushed
the
localRhdhExport
branch
from
a449e8e to
f3dbd86
Compare
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #3321 +/- ##
==========================================
+ Coverage 52.43% 53.58% +1.15%
==========================================
Files 2251 2253 +2
Lines 85725 85814 +89
Branches 24111 24117 +6
==========================================
+ Hits 44948 45983 +1035
+ Misses 39164 38387 -777
+ Partials 1613 1444 -169
*This pull request uses carry forward flags. Click here to find out more. Continue to review full report in Codecov by Harness.
🚀 New features to boost your workflow:
|
mareklibra
force-pushed
the
localRhdhExport
branch
2 times, most recently
from
5e25e9c to
7b88287
Compare
eloycoto
left a comment
eloycoto
left a comment
There was a problem hiding this comment.
As mention in private, I do not have to the instance.
I'm also a bit worried about the current NODE_TLS disabled inside the plug-in.
| // Set it here so Node.js TLS layer skips server cert verification. | ||
| const cluster = kc.getCurrentCluster(); | ||
| if (cluster?.skipTLSVerify && !process.env.NODE_TLS_REJECT_UNAUTHORIZED) { | ||
| process.env.NODE_TLS_REJECT_UNAUTHORIZED = '0'; |
There was a problem hiding this comment.
I'm not sure about this, if you change this value, means that the whole NODE_TLS is disabled for all request that the module do.
If we add this, we're allowing that any attacker load any URL with invalid certificate (Scaffolder) without real constraints, no? We have a way to skip the CERT issues in k8s in place, this maybe should be change to be honest.
There was a problem hiding this comment.
Right, it turns off the validation for the whole node process.
I was not able to make it working just for the single session, no matter documentation states so. There might be different version library resolved at runtime.
This workaround is based on the assumption that the skipTLSVerify is used in dev clusters only. Maybe its too strong assumption.
There was a problem hiding this comment.
This workaround is based on the assumption that the skipTLSVerify is used in dev clusters only. Maybe its too strong assumption.
I would like to say that this does not happen in production, but..
| * limitations under the License. | ||
| */ | ||
|
|
||
| export function navigateTo(url: string) { |
There was a problem hiding this comment.
Because JS is totally functional programming, shoudn't we use onNavigate function, so it passed as the argument, and it's more FP friendly? It cannot change based on external parameters and it's clear for testing.
I'm worried that the a global mutation will change the DownloadStaticPublicFile, this way seems more FP friendly.
export const DownloadStaticPublicFile = ({
onNavigate = (url) => { globalThis.location.href = url; }
}: DownloadStaticPublicFileProps) => {
// ...
useEffect(() => {
discoveryApi.getBaseUrl('x2a').then(baseUrl => {
onNavigate(`${baseUrl}/static/${filePath}`);
});
}, [discoveryApi, filePath, onNavigate]);
There was a problem hiding this comment.
The only reason for adding the function is in simplifying tests, there are changes around jsdom in Jest 30 (newly used).
Both approaches, the suggested and the one in PR, work well.
Considering how the navigateTo is implemented and called, I do not see the described risk as relevant. It's not a global mutation.
To move forward, changing it. It's a cosmetic change.
| } | ||
| } | ||
|
|
||
| function copyDirSync(src, dest) { |
There was a problem hiding this comment.
why not? https://nodejs.org/api/fs.html#fscpsyncsrc-dest-options we're over complicated this.
| @@ -0,0 +1,189 @@ | |||
| #!/usr/bin/env node | |||
There was a problem hiding this comment.
Why this file is needed? Not sure If I get why
There was a problem hiding this comment.
Do you mean the whole export-to-rhdh-repo.js?
It's simplification for local RHDH testing. With every change, a dev needs to:
- Build each plugin as a dynamic plugin
- Copy the output into the correct directory in the RHDH repo
- Configure RHDH's app-config with all x2a-specific settings (ok, maybe just once)
Doing this manually for 5 plugins every time a change occurs is tedious. The script reduces this to a single yarn enable-in-rhdh-repo command. We used that in the Orchestrator, its implementatino here is highly motivated by the one there.
I could keep it for myself but I believe others can benefit from it as well.
There was a problem hiding this comment.
ok, maybe it is good it you can document this, or telling how to do it! because I didn't know about it!
|
|
||
| const source = x2aDynamicPlugins.frontend[pluginKey]; | ||
| if (!config.dynamicPlugins.frontend[pluginKey]) { | ||
| config.dynamicPlugins.frontend[pluginKey] = source; |
There was a problem hiding this comment.
There are a few merge here, and I'm thinking, can we use something like this?
https://lodash.com/docs/#merge
I'm worried that a new config is added, and we miss here, (I have no clue why we need this too), and we miss adding here the conditional
There was a problem hiding this comment.
The merge would not work here since it overwrites existing leaf values in target.
We need to set defaults without affecting user customizations in app-config.local.yaml. Using lodash.merge would destroy any user-edited values.
But it brought me to the idea with deepDefaults(), it's slightly different from the lodash version around merging of arrays.
Reimplemented.
Bump all @backstage/* dependencies from 1.45.3 to 1.49.4 across all plugins (x2a, x2a-backend, x2a-common, x2a-node, x2a-mcp-extras, scaffolder-backend-module-x2a) and app packages. Frontend: - Replace legacy frontend (packages/app) with New Frontend System (NFS) using createApp from @backstage/frontend-defaults, PageBlueprint, NavContentBlueprint, TranslationBlueprint, and SignInPageBlueprint with GitHub/GitLab/Bitbucket auth providers - Add plugins/x2a/src/alpha.tsx with NFS plugin definition (createFrontendPlugin, x2aTranslationsModule) including FormFieldBlueprint for RepoAuthentication and RulesAcceptance scaffolder field extensions - Delete legacy app files (Root, EntityPage, SearchPage, apis.ts, App.test.tsx, setupTests.ts) that are no longer needed with NFS - Add plugins/x2a/app-config.yaml for RHDH dynamic plugin wiring (scaffolder field extensions, translation resources, app icons, dynamic routes) - Integrate RHDH theme via @red-hat-developer-hub/backstage-plugin-theme - Add app.extensions config for NFS language settings (en/de/es/fr/it) - Refactor DownloadStaticPublicFile to accept an onNavigate callback prop for Jest 30 jsdom compatibility Backend: - Replace deprecated scaffolder-backend-module-bitbucket with scaffolder-backend-module-bitbucket-cloud - Add TLS certificate error diagnostics in makeK8sClient with actionable hints for NODE_EXTRA_CA_CERTS and cluster CA setup - Remove global NODE_TLS_REJECT_UNAUTHORIZED bypass - Update CatalogService mock with new required methods (queryLocations, streamLocations, updateLocation) Testing and tooling: - Move e2e tests to workspace-level e2e-tests/ with multi-locale Playwright config (per-locale ports for en/de/es/fr/it) - Upgrade @playwright/test to 1.60.0 - Add Jest 30 + @backstage/cli-defaults devDependencies - Add --registry flag to build-dynamic-plugins.sh - Add enable-in-rhdh-repo scripts (export-to-rhdh-repo.js, update-config-in-rhdh-repo.js, config-for-rhdh-repo.yaml) for local RHDH development and testing Cleanup: - Remove variant="gridItem" props per Backstage 1.49 breaking changes - Remove x2a-dcr references from the app (RHDH 1.9-only workaround) - Add dist-dynamic to eslint ignorePatterns in all plugin configs - Add resolutions for NFS packages and zod to fix schema/type conflicts - Document TLS certificate handling in README Signed-off-by: Marek Libra <marek.libra@gmail.com>
mareklibra
force-pushed
the
localRhdhExport
branch
from
7b88287 to
0d5d497
Compare
|
|
Changes are in. |
3 participants
Fixes: FLPATH-4033
Upgrade x2a workspace to Backstage 1.49.4 for RHDH 1.10
Bump all @backstage/* dependencies from 1.45.3 to 1.49.4 manifest
levels across all plugins (x2a, x2a-backend, x2a-common, x2a-node,
x2a-mcp-extras, scaffolder-backend-module-x2a) and app packages.
Key changes:
packages/app-legacy (legacy frontend) and create new packages/app
using the New Frontend System (NFS) with PageBlueprint,
NavContentBlueprint, and TranslationBlueprint
(createFrontendPlugin, x2aTranslationsModule)
sidebar navigation into the NFS app
scaffolder-backend-module-bitbucket-cloud
plugin-scaffolder, and zod to fix schema/type conflicts
(queryLocations, streamLocations, updateLocation)
helper for Jest 30 jsdom compatibility
TODO:
@backstage/authin an OCP setup via a latest 1.49.z overlays build