feat(#4181): migrate K8s credentials to app-config.yaml - #4184
feat(#4181): migrate K8s credentials to app-config.yaml#4184fullsend-ai-coder[bot] wants to merge 4 commits into
Conversation
Migrate kserve-kubeflow-connector-backend from KubeConfig.loadFromDefault() to config-based K8s authentication following the OCM plugin pattern. Changes: - Remove unused baseUrl from ModelCatalogConfig (entity provider uses DiscoveryService.getBaseUrl() exclusively) - Merge ConnectorConfig into ReconcilerConfig (eliminate redundant intermediate type) - Add K8s connection fields (url, serviceAccountToken, skipTLSVerify, caData, clusterName) to ReconcilerConfig - Implement loadFromOptions() when both url and serviceAccountToken are present in app-config; fall back to loadFromDefault() for local dev (KUBECONFIG, ~/.kube/config, K8S_TOKEN env var) - Add kubernetesPluginRef lookup in plugin.ts to resolve K8s credentials from kubernetes.clusterLocatorMethods config (D7 precedence chain) - Use safeGetOptionalString wrapper for all optional config reads to handle empty env var substitution edge case - Replace console.log/error with LoggerService in setupInformer - Update config.d.ts with K8s fields (@visibility secret for tokens) - Add K8s RBAC example YAML (examples/k8s-rbac.yaml) with minimum required permissions (InferenceServices, Routes, ServiceAccounts) - Update app-config.yaml with K8s connection fields and commented-out kubernetesPluginRef alternative Note: existing unit tests in catalog-backend-module-model-catalog have pre-existing Babel parser failures unrelated to this change. Closes #4181 Assisted-by: Claude Opus 4.6
Missing ChangesetsThe following package(s) are changed by this PR but do not have a changeset:
See CONTRIBUTING.md for more information about how to add changesets. Changed Packages
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #4184 +/- ##
==========================================
- Coverage 58.12% 58.12% -0.01%
==========================================
Files 2422 2422
Lines 96493 96486 -7
Branches 26884 26882 -2
==========================================
- Hits 56084 56078 -6
+ Misses 40207 40206 -1
Partials 202 202
*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:
|
|
/fs-fix run 'yarn prettier:fix' workspace/ai-integrations to fix the failures in CI that manifest from running 'yarn prettier:check' |
|
🤖 Finished Fix · ✅ Success · Started 8:06 PM UTC · Completed 8:18 PM UTC |
Remove trailing blank line flagged by prettier:check CI gate. Addresses review feedback on #4184 Assisted-by: Claude Opus 4.6
🔧 Fix agent — iteration 1 (human-triggered)Fixed prettier formatting in app-config.yaml by removing a trailing blank line. yarn prettier:check now passes. Fixed (1):
Tests: passed Updated by fullsend fix agent |
|
/fs-review |
|
🤖 Finished Review · ✅ Success · Started 8:42 PM UTC · Completed 9:01 PM UTC |
ReviewFindingsMedium
Low
Previous runReviewFindingsMedium
Low
Labels: PR modifies K8s authentication mechanisms (service account tokens, TLS config, credential precedence) and has a fail-open finding on partial config fallback. Previous run (2)ReviewFindingsMedium
Low
Labels: Feature PR adding K8s config-based authentication to the ai-integrations workspace kserve connector plugin. |
![fullsend-ai-review[bot]](https://avatars.githubusercontent.com/in/3478501?s=60&v=4){kind=small}
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
fullsend-ai-review
Bot
added
requires-manual-review
…~/.kube/config; add namespace comments to k8s rbac example
… baseUrl in test Medium — direct config fallback applied url and serviceAccountToken independently. If only one was present, the partial credential passed to setupInformer where loadFromDefault() could silently connect to the wrong cluster. Now validates both fields together (matching the kubernetesPluginRef path) and logs a warning when only one is present. Low — test fixture had stale baseUrl field removed from ModelCatalogConfig in this PR. Silently ignored but misleading. Removed. Signed-off-by: gabemontero <gmontero@redhat.com> Assisted-by: Claude Opus 4.6 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Fullsend review — addressedMedium — direct config fallback fail-open (plugin.ts:155): Low — stale Acknowledged but skipped:
Commit: 737e4ae |
|
Remove trailing blank line flagged by prettier:check CI gate. Addresses review feedback on redhat-developer#4184 Assisted-by: Claude Opus 4.6
|
/fs-review |
|
🤖 Finished Review · ✅ Success · Started 3:39 PM UTC · Completed 4:01 PM UTC Commit: |
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
Second fullsend review — triage and responseReviewed all 9 findings from the second fullsend review. Three fixed, six acknowledged with rationale. Fixed (commit c85d843)
No change (with rationale)
|
|
/fs-review |
|
🤖 Finished Review · ✅ Success · Started 6:42 PM UTC · Completed 7:02 PM UTC Commit: |
1 participant
Migrate kserve-kubeflow-connector-backend from KubeConfig.loadFromDefault() to config-based K8s authentication following the OCM plugin pattern.
Changes:
DiscoveryService.getBaseUrl() exclusively)
intermediate type)
caData, clusterName) to ReconcilerConfig
are present in app-config; fall back to loadFromDefault() for local
dev (KUBECONFIG, ~/.kube/config, K8S_TOKEN env var)
from kubernetes.clusterLocatorMethods config (D7 precedence chain)
handle empty env var substitution edge case
required permissions (InferenceServices, Routes, ServiceAccounts)
kubernetesPluginRef alternative
Note: existing unit tests in catalog-backend-module-model-catalog have pre-existing Babel parser failures unrelated to this change.
Assisted-by: Claude Opus 4.6
Closes #4181
Post-script verification
agent/4181-k8s-config-migration)c91e22821f3dd7e2daf287e349aeec360eb34c7d..HEAD)