Fix helm upgrade secrets#16676
Draft
sowmya-sl wants to merge 1 commit into
Draft
Conversation
openshift-ci
Bot
added
the
do-not-merge/work-in-progress
Contributor
|
Skipping CI for Draft Pull Request. |
openshift-ci
Bot
added
needs-rebase
Contributor
WalkthroughThis PR adds basic-auth Secret support to Helm install and upgrade flows, including secret creation from the URL form, secret selection in the upgrade form, backend propagation of the secret name, and matching Helm label and test-string updates. ChangesHelm basic-auth secret flow and text updates
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Suggested reviewers
🚥 Pre-merge checks | ✅ 12 | ❌ 3❌ Failed checks (3 warnings)
✅ Passed checks (12 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (2)
pkg/helm/handlers/handler_test.go (1)
138-139: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAssert the forwarded basic-auth Secret name in the handler fake.
The fake accepts
basicAuthSecretNamebut ignores it, so a regression inHandleUpgradeReleaseAsyncpassingreq.BasicAuthSecretNamewould not fail tests. Consider extending the helper with an expected value and checking it in the closure. As per coding guidelines, “Make sure the tests pass, and add any new tests as appropriate.”🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@pkg/helm/handlers/handler_test.go` around lines 138 - 139, The handler test fake for fakeUpgradeReleaseAsync currently accepts basicAuthSecretName but never verifies it, so it can miss regressions in HandleUpgradeReleaseAsync forwarding req.BasicAuthSecretName. Update the helper to take an expected basicAuthSecretName value and assert inside the returned closure that the received argument matches it, using the existing fakeUpgradeReleaseAsync symbol so the test fails if the handler stops passing the secret name correctly.Source: Coding guidelines
pkg/helm/actions/upgrade_release_test.go (1)
509-509: 📐 Maintainability & Code Quality | 🔵 Trivial | 🏗️ Heavy liftAdd coverage for non-empty and persisted auth Secret upgrades.
These call sites only pass
"", so the new selected Secret path, annotation fallback path, and missing/malformed Secret failure behavior are untested. A table case withhelm.openshift.io/auth-secretplus a matching basic-auth Secret would catch regressions in this PR’s core upgrade flow. As per coding guidelines, “Make sure the tests pass, and add any new tests as appropriate.”Also applies to: 605-605, 697-697
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@pkg/helm/actions/upgrade_release_test.go` at line 509, Add test coverage in UpgradeReleaseAsync call sites to exercise the persisted auth Secret flow instead of only passing an empty secret name. Update the relevant table-driven cases in upgrade_release_test.go to cover helm.openshift.io/auth-secret, the annotation fallback, and failure handling for missing or malformed basic-auth Secrets so the selected Secret path and upgrade behavior are validated. Use UpgradeReleaseAsync and the related auth Secret resolution logic as the main symbols to locate the affected tests and add the new cases.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@frontend/packages/helm-plugin/src/components/forms/install-upgrade/HelmInstallUpgradeForm.tsx`:
- Around line 118-128: Update HelmInstallUpgradeForm so upgrade flows are not
gated by an existing basicAuthSecretName; in the form logic around
showAuthSecret, secretMissing, and the selector rendering, allow users to open
the auth secret selector even when the persisted helm.openshift.io/auth-secret
annotation is absent. Then make the upgrade path let them choose or create a
Secret and persist that selection back into the upgrade state, using the
existing HelmActionType.Upgrade and useSecretResources(namespace) flow.
In
`@frontend/packages/helm-plugin/src/components/forms/url-chart/HelmCreateBasicAuthSecretModal.tsx`:
- Around line 73-76: The Helm basic-auth secret modal can currently create
Secrets that the backend will reject because it omits username when empty, while
GetUserCredentials expects both username and password. Update
HelmCreateBasicAuthSecretModal so the generated Secret always includes a
username field (and keep the form validation aligned with that requirement),
using the existing username/password handling in the modal’s secret payload to
prevent creating unusable auth Secrets.
- Around line 45-54: The modal close path in HelmCreateBasicAuthSecretModal’s
closeModal is blocked by the inProgress guard, so a successful create from
handleCreate cannot dismiss the dialog. Update the create flow so the modal can
close after a success, either by clearing inProgress before calling closeModal
or by allowing closeModal to bypass the guard for successful completion, and
make sure the same behavior is applied to the related create/submit path
referenced in handleCreate.
In `@pkg/helm/actions/upgrade_release.go`:
- Around line 163-173: The selected auth Secret is only applied inside the
rel.Chart.Metadata.Annotations nil-check, so releases without annotations skip
basicAuthSecretName and may fetch the chart unauthenticated. Move the
basicAuthSecretName handling in upgrade_release.go outside the
rel.Chart.Metadata.Annotations block in the upgrade flow, keeping the chart_url
annotation lookup conditional but always honoring the user-selected Secret
before falling back to helmAuthSecretAnnotation.
- Around line 210-219: Credential setup errors in the upgrade flow are only
logged and then execution continues, which can lead to an unauthenticated chart
lookup and stale auth-secret state. In `UpgradeRelease`, treat failures from
`GetUserCredentials` and `applyBasicAuthFromUserCredentials` as fatal by
returning the error immediately instead of falling through to `LocateChart`.
Keep the check around `auth_secret` in this block, and make sure any selected
Secret that is missing, malformed, or cannot be used by the registry client
stops the upgrade path before chart resolution.
---
Nitpick comments:
In `@pkg/helm/actions/upgrade_release_test.go`:
- Line 509: Add test coverage in UpgradeReleaseAsync call sites to exercise the
persisted auth Secret flow instead of only passing an empty secret name. Update
the relevant table-driven cases in upgrade_release_test.go to cover
helm.openshift.io/auth-secret, the annotation fallback, and failure handling for
missing or malformed basic-auth Secrets so the selected Secret path and upgrade
behavior are validated. Use UpgradeReleaseAsync and the related auth Secret
resolution logic as the main symbols to locate the affected tests and add the
new cases.
In `@pkg/helm/handlers/handler_test.go`:
- Around line 138-139: The handler test fake for fakeUpgradeReleaseAsync
currently accepts basicAuthSecretName but never verifies it, so it can miss
regressions in HandleUpgradeReleaseAsync forwarding req.BasicAuthSecretName.
Update the helper to take an expected basicAuthSecretName value and assert
inside the returned closure that the received argument matches it, using the
existing fakeUpgradeReleaseAsync symbol so the test fails if the handler stops
passing the secret name correctly.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository: openshift/coderabbit/.coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 1b166576-6636-476e-abca-97e4a270547d
📒 Files selected for processing (12)
frontend/packages/helm-plugin/locales/en/helm-plugin.jsonfrontend/packages/helm-plugin/src/components/forms/install-upgrade/HelmInstallUpgradeForm.tsxfrontend/packages/helm-plugin/src/components/forms/install-upgrade/HelmInstallUpgradePage.tsxfrontend/packages/helm-plugin/src/components/forms/url-chart/HelmCreateBasicAuthSecretModal.tsxfrontend/packages/helm-plugin/src/components/forms/url-chart/HelmURLChartForm.tsxfrontend/public/locales/en/public.jsonpkg/helm/actions/get_chart.gopkg/helm/actions/install_chart.gopkg/helm/actions/upgrade_release.gopkg/helm/actions/upgrade_release_test.gopkg/helm/handlers/handler_test.gopkg/helm/handlers/handlers.go
Comment on lines
+118
to
+128
| const showAuthSecret = helmAction === HelmActionType.Upgrade && !!basicAuthSecretName; | ||
| const secretResources = useSecretResources(namespace); | ||
| const autocompleteFilter = (strText: string, item: any): boolean => | ||
| fuzzy(strText, item?.props?.name); | ||
| const secretMissing = useMemo(() => { | ||
| if (!showAuthSecret || !secretResources[0]?.loaded) { | ||
| return false; | ||
| } | ||
| const secrets = secretResources[0]?.data ?? []; | ||
| return !secrets.some((s) => s?.metadata?.name === basicAuthSecretName); | ||
| }, [showAuthSecret, secretResources, basicAuthSecretName]); |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Upgrades without a persisted auth annotation cannot add one.
Because showAuthSecret requires an existing basicAuthSecretName, the selector is hidden for upgrades that don't already have helm.openshift.io/auth-secret. That prevents users from fixing an unauthorized upgrade by selecting or creating a Secret in the first place.
Suggested fix
- const showAuthSecret = helmAction === HelmActionType.Upgrade && !!basicAuthSecretName;
+ const showAuthSecret = helmAction === HelmActionType.Upgrade;Also applies to: 218-252
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@frontend/packages/helm-plugin/src/components/forms/install-upgrade/HelmInstallUpgradeForm.tsx`
around lines 118 - 128, Update HelmInstallUpgradeForm so upgrade flows are not
gated by an existing basicAuthSecretName; in the form logic around
showAuthSecret, secretMissing, and the selector rendering, allow users to open
the auth secret selector even when the persisted helm.openshift.io/auth-secret
annotation is absent. Then make the upgrade path let them choose or create a
Secret and persist that selection back into the upgrade state, using the
existing HelmActionType.Upgrade and useSecretResources(namespace) flow.
Comment on lines
+45
to
+54
| const closeModal = () => { | ||
| if (inProgress) { | ||
| return; | ||
| } | ||
| if (document.activeElement instanceof HTMLElement) { | ||
| document.activeElement.blur(); | ||
| } | ||
| onClose?.(); | ||
| closeOverlay(); | ||
| }; |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Successful creates never close the modal.
handleCreate() calls closeModal() while inProgress is still true, so the guard on Line 46 returns early and the overlay stays open after a successful create.
Suggested fix
- const closeModal = () => {
- if (inProgress) {
- return;
- }
+ const closeModal = (force = false) => {
+ if (inProgress && !force) {
+ return;
+ }
if (document.activeElement instanceof HTMLElement) {
document.activeElement.blur();
}
onClose?.();
closeOverlay();
@@
- closeModal();
+ closeModal(true);
// Keep form update separate so a parent callback failure cannot block modal close.
save?.(createdSecretName);Also applies to: 78-81
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@frontend/packages/helm-plugin/src/components/forms/url-chart/HelmCreateBasicAuthSecretModal.tsx`
around lines 45 - 54, The modal close path in HelmCreateBasicAuthSecretModal’s
closeModal is blocked by the inProgress guard, so a successful create from
handleCreate cannot dismiss the dialog. Update the create flow so the modal can
close after a success, either by clearing inProgress before calling closeModal
or by allowing closeModal to bypass the guard for successful completion, and
make sure the same behavior is applied to the related create/submit path
referenced in handleCreate.
Comment on lines
+73
to
+76
| stringData: { | ||
| ...(username ? { username } : {}), | ||
| password, | ||
| }, |
Contributor
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
The modal can create Secrets the backend cannot use.
The UI treats username as optional and omits that key entirely, but pkg/helm/actions/install_chart.go's GetUserCredentials rejects Secrets missing either username or password. A Secret created here without a username will fail the install/upgrade auth flow later. Based on learnings, English-only i18n additions are fine in this repo and do not need separate l10n updates.
Suggested fix
- const isCreateDisabled = !secretName.trim() || !password;
+ const isCreateDisabled = !secretName.trim() || !username.trim() || !password;
@@
- <FormGroup label={t('public~Secret username')} fieldId="helm-secret-username">
+ <FormGroup
+ label={t('public~Secret username')}
+ isRequired
+ fieldId="helm-secret-username"
+ >
<TextInput
@@
- <HelperTextItem>
- {t('helm-plugin~Optional username for OCI/HTTP(S) authentication.')}
- </HelperTextItem>
+ <HelperTextItem>{t('helm-plugin~Username for OCI/HTTP(S) authentication.')}</HelperTextItem>Also applies to: 122-137
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@frontend/packages/helm-plugin/src/components/forms/url-chart/HelmCreateBasicAuthSecretModal.tsx`
around lines 73 - 76, The Helm basic-auth secret modal can currently create
Secrets that the backend will reject because it omits username when empty, while
GetUserCredentials expects both username and password. Update
HelmCreateBasicAuthSecretModal so the generated Secret always includes a
username field (and keep the form validation aligned with that requirement),
using the existing username/password handling in the modal’s secret payload to
prevent creating unusable auth Secrets.
Source: Learnings
Comment on lines
+163
to
+173
| auth_secret := "" | ||
| // Before proceeding, check if chart URL is present as an annotation | ||
| if rel.Chart.Metadata.Annotations != nil { | ||
| if chart_url, ok := rel.Chart.Metadata.Annotations["chart_url"]; chartUrl == "" && ok { | ||
| chartUrl = chart_url | ||
| } | ||
| if basicAuthSecretName != "" { | ||
| auth_secret = basicAuthSecretName | ||
| } else if authSecret, ok := rel.Chart.Metadata.Annotations[helmAuthSecretAnnotation]; ok { | ||
| auth_secret = authSecret | ||
| } |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Apply the selected Secret even when release annotations are absent.
Line 169 only assigns basicAuthSecretName inside the rel.Chart.Metadata.Annotations != nil block, so releases without existing annotations drop the user-selected Secret and locate the chart unauthenticated.
Proposed fix
- auth_secret := ""
+ auth_secret := basicAuthSecretName
// Before proceeding, check if chart URL is present as an annotation
- if rel.Chart.Metadata.Annotations != nil {
+ if rel.Chart.Metadata != nil && rel.Chart.Metadata.Annotations != nil {
if chart_url, ok := rel.Chart.Metadata.Annotations["chart_url"]; chartUrl == "" && ok {
chartUrl = chart_url
}
- if basicAuthSecretName != "" {
- auth_secret = basicAuthSecretName
- } else if authSecret, ok := rel.Chart.Metadata.Annotations[helmAuthSecretAnnotation]; ok {
- auth_secret = authSecret
+ if auth_secret == "" {
+ if authSecret, ok := rel.Chart.Metadata.Annotations[helmAuthSecretAnnotation]; ok {
+ auth_secret = authSecret
+ }
}
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| auth_secret := "" | |
| // Before proceeding, check if chart URL is present as an annotation | |
| if rel.Chart.Metadata.Annotations != nil { | |
| if chart_url, ok := rel.Chart.Metadata.Annotations["chart_url"]; chartUrl == "" && ok { | |
| chartUrl = chart_url | |
| } | |
| if basicAuthSecretName != "" { | |
| auth_secret = basicAuthSecretName | |
| } else if authSecret, ok := rel.Chart.Metadata.Annotations[helmAuthSecretAnnotation]; ok { | |
| auth_secret = authSecret | |
| } | |
| auth_secret := basicAuthSecretName | |
| // Before proceeding, check if chart URL is present as an annotation | |
| if rel.Chart.Metadata != nil && rel.Chart.Metadata.Annotations != nil { | |
| if chart_url, ok := rel.Chart.Metadata.Annotations["chart_url"]; chartUrl == "" && ok { | |
| chartUrl = chart_url | |
| } | |
| if auth_secret == "" { | |
| if authSecret, ok := rel.Chart.Metadata.Annotations[helmAuthSecretAnnotation]; ok { | |
| auth_secret = authSecret | |
| } | |
| } | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@pkg/helm/actions/upgrade_release.go` around lines 163 - 173, The selected
auth Secret is only applied inside the rel.Chart.Metadata.Annotations nil-check,
so releases without annotations skip basicAuthSecretName and may fetch the chart
unauthenticated. Move the basicAuthSecretName handling in upgrade_release.go
outside the rel.Chart.Metadata.Annotations block in the upgrade flow, keeping
the chart_url annotation lookup conditional but always honoring the
user-selected Secret before falling back to helmAuthSecretAnnotation.
Comment on lines
+210
to
+219
| if auth_secret != "" { | ||
| klog.Infof("Found persisted auth secret %s for release %s/%s, applying credentials for upgrade", auth_secret, releaseNamespace, releaseName) | ||
| userCredentials, err := GetUserCredentials(coreClient, releaseNamespace, auth_secret) | ||
| if err != nil { | ||
| klog.Errorf("Failed to get user credentials for release upgrade %s/%s: %v", releaseNamespace, releaseName, err) | ||
| } else { | ||
| if err := applyBasicAuthFromUserCredentials(&client.ChartPathOptions, client, userCredentials); err != nil { | ||
| klog.Errorf("Failed to apply auth from secret %s for release %s/%s: %v", auth_secret, releaseNamespace, releaseName, err) | ||
| } | ||
| } |
Contributor
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Return credential setup failures instead of continuing unauthenticated.
If the persisted or selected Secret is missing, malformed, or registry-client setup fails, this logs the error but still calls LocateChart; that can produce a misleading 401 or even persist an auth-secret annotation that was never applied.
Proposed fix
if auth_secret != "" {
klog.Infof("Found persisted auth secret %s for release %s/%s, applying credentials for upgrade", auth_secret, releaseNamespace, releaseName)
userCredentials, err := GetUserCredentials(coreClient, releaseNamespace, auth_secret)
if err != nil {
- klog.Errorf("Failed to get user credentials for release upgrade %s/%s: %v", releaseNamespace, releaseName, err)
+ return nil, fmt.Errorf("failed to get user credentials for release upgrade %s/%s: %w", releaseNamespace, releaseName, err)
+ }
+ if err := applyBasicAuthFromUserCredentials(&client.ChartPathOptions, client, userCredentials); err != nil {
+ return nil, fmt.Errorf("failed to apply auth from secret %s for release %s/%s: %w", auth_secret, releaseNamespace, releaseName, err)
- } else {
- if err := applyBasicAuthFromUserCredentials(&client.ChartPathOptions, client, userCredentials); err != nil {
- klog.Errorf("Failed to apply auth from secret %s for release %s/%s: %v", auth_secret, releaseNamespace, releaseName, err)
- }
}
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| if auth_secret != "" { | |
| klog.Infof("Found persisted auth secret %s for release %s/%s, applying credentials for upgrade", auth_secret, releaseNamespace, releaseName) | |
| userCredentials, err := GetUserCredentials(coreClient, releaseNamespace, auth_secret) | |
| if err != nil { | |
| klog.Errorf("Failed to get user credentials for release upgrade %s/%s: %v", releaseNamespace, releaseName, err) | |
| } else { | |
| if err := applyBasicAuthFromUserCredentials(&client.ChartPathOptions, client, userCredentials); err != nil { | |
| klog.Errorf("Failed to apply auth from secret %s for release %s/%s: %v", auth_secret, releaseNamespace, releaseName, err) | |
| } | |
| } | |
| if auth_secret != "" { | |
| klog.Infof("Found persisted auth secret %s for release %s/%s, applying credentials for upgrade", auth_secret, releaseNamespace, releaseName) | |
| userCredentials, err := GetUserCredentials(coreClient, releaseNamespace, auth_secret) | |
| if err != nil { | |
| return nil, fmt.Errorf("failed to get user credentials for release upgrade %s/%s: %w", releaseNamespace, releaseName, err) | |
| } | |
| if err := applyBasicAuthFromUserCredentials(&client.ChartPathOptions, client, userCredentials); err != nil { | |
| return nil, fmt.Errorf("failed to apply auth from secret %s for release %s/%s: %w", auth_secret, releaseNamespace, releaseName, err) | |
| } | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@pkg/helm/actions/upgrade_release.go` around lines 210 - 219, Credential setup
errors in the upgrade flow are only logged and then execution continues, which
can lead to an unauthenticated chart lookup and stale auth-secret state. In
`UpgradeRelease`, treat failures from `GetUserCredentials` and
`applyBasicAuthFromUserCredentials` as fatal by returning the error immediately
instead of falling through to `LocateChart`. Keep the check around `auth_secret`
in this block, and make sure any selected Secret that is missing, malformed, or
cannot be used by the registry client stops the upgrade path before chart
resolution.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@frontend/packages/helm-plugin/locales/fr/helm-plugin.json`:
- Around line 153-154: The French Helm plugin translations have spacing
mismatches in the templated strings, which causes concatenated labels to render
incorrectly. Update the entries for the keys with appVersion and chartRepoName
in helm-plugin.json so they preserve the same leading and internal spacing as
the English source; specifically, keep the leading space before the App Version
fragment and add the missing space before the chartRepoName placeholder in the
translation values.
In `@frontend/packages/helm-plugin/locales/ja/helm-plugin.json`:
- Line 148: The Japanese locale entry for Repositories is still untranslated and
should be replaced with an appropriate Japanese string to match the other
locales. Update the locale value in the helm-plugin JSON for the Repositories
key, using the existing translation pattern from the ko and zh files as a guide
while keeping the same key and JSON structure.
In `@frontend/packages/helm-plugin/locales/ko/helm-plugin.json`:
- Line 126: The Korean translation for the Helm chart URL example is malformed
and should match the source text. Update the value in helm-plugin.json so the
OCI example uses the correct registry path from the original string, and remove
the duplicated mycharts/mychart segment while keeping the rest of the
translation unchanged.
In `@frontend/packages/helm-plugin/locales/zh/helm-plugin.json`:
- Line 158: Update the Chinese translation for the "Upgrade Helm release" entry
in helm-plugin.json so it uses the correct upgrade wording instead of creation
wording. Locate the localized string keyed by "Upgrade Helm release" in the zh
locale file and change the value from 创建 Helm 发行 to the appropriate 升级
equivalent to match the action label used by the Helm plugin UI.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository: openshift/coderabbit/.coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 0f27dcfe-24ac-4794-883c-a6837b9afc8e
📒 Files selected for processing (50)
frontend/packages/helm-plugin/console-extensions.jsonfrontend/packages/helm-plugin/integration-tests/features/helm-release.featurefrontend/packages/helm-plugin/integration-tests/features/helm/actions-on-helm-release-after-upgrade.featurefrontend/packages/helm-plugin/integration-tests/features/helm/actions-on-helm-release.featurefrontend/packages/helm-plugin/integration-tests/features/helm/helm-compatibility.featurefrontend/packages/helm-plugin/integration-tests/features/helm/helm-feature-flag.featurefrontend/packages/helm-plugin/integration-tests/features/helm/helm-installation-view.featurefrontend/packages/helm-plugin/integration-tests/features/helm/helm-navigation.featurefrontend/packages/helm-plugin/integration-tests/features/helm/helm-page-tabs.featurefrontend/packages/helm-plugin/integration-tests/features/helm/install-helm-chart.featurefrontend/packages/helm-plugin/integration-tests/features/helm/install-url-chart.featurefrontend/packages/helm-plugin/integration-tests/features/helm/topology-helm-release.featurefrontend/packages/helm-plugin/integration-tests/support/constants/static-text/helm-text.tsfrontend/packages/helm-plugin/integration-tests/support/pages/helm/helm-details-page.tsfrontend/packages/helm-plugin/integration-tests/support/pages/helm/helm-page.tsfrontend/packages/helm-plugin/integration-tests/support/pages/helm/upgrade-helm-release-page.tsfrontend/packages/helm-plugin/integration-tests/support/step-definitions/common/common.tsfrontend/packages/helm-plugin/integration-tests/support/step-definitions/helm/helm-compatibility.tsfrontend/packages/helm-plugin/integration-tests/support/step-definitions/helm/helm-navigation.tsfrontend/packages/helm-plugin/integration-tests/support/step-definitions/helm/helm-release.tsfrontend/packages/helm-plugin/integration-tests/support/step-definitions/helm/helm.tsfrontend/packages/helm-plugin/locales/en/helm-plugin.jsonfrontend/packages/helm-plugin/locales/es/helm-plugin.jsonfrontend/packages/helm-plugin/locales/fr/helm-plugin.jsonfrontend/packages/helm-plugin/locales/ja/helm-plugin.jsonfrontend/packages/helm-plugin/locales/ko/helm-plugin.jsonfrontend/packages/helm-plugin/locales/zh/helm-plugin.jsonfrontend/packages/helm-plugin/src/actions/add-resources.tsxfrontend/packages/helm-plugin/src/actions/creators.tsfrontend/packages/helm-plugin/src/catalog/utils/catalog-utils.tsxfrontend/packages/helm-plugin/src/components/__tests__/helm-release-mock-data.tsfrontend/packages/helm-plugin/src/components/details-page/HelmReleaseDetails.tsxfrontend/packages/helm-plugin/src/components/details-page/history/HelmReleaseHistory.tsxfrontend/packages/helm-plugin/src/components/details-page/overview/HelmReleaseOverview.tsxfrontend/packages/helm-plugin/src/components/details-page/overview/__tests__/HelmReleaseOverview.spec.tsxfrontend/packages/helm-plugin/src/components/forms/__tests__/HelmInstallUpgradeForm.spec.tsxfrontend/packages/helm-plugin/src/components/forms/install-upgrade/HelmInstallUpgradeForm.tsxfrontend/packages/helm-plugin/src/components/forms/url-chart/HelmCreateBasicAuthSecretModal.tsxfrontend/packages/helm-plugin/src/components/forms/url-chart/HelmURLChartForm.tsxfrontend/packages/helm-plugin/src/components/forms/url-chart/HelmURLChartInstallPage.tsxfrontend/packages/helm-plugin/src/components/forms/url-chart/HelmURLInstallForm.tsxfrontend/packages/helm-plugin/src/components/list-page/HelmReleaseList.tsxfrontend/packages/helm-plugin/src/components/list-page/HelmReleaseListPage.tsxfrontend/packages/helm-plugin/src/components/list-page/HelmReleaseListRow.tsxfrontend/packages/helm-plugin/src/components/list-page/HelmTabbedPage.tsxfrontend/packages/helm-plugin/src/models/helm.tsfrontend/packages/helm-plugin/src/topology/helmFilters.tsfrontend/packages/helm-plugin/src/utils/__tests__/helm-utils.spec.tsfrontend/packages/helm-plugin/src/utils/helm-utils.tsfrontend/public/locales/en/public.json
✅ Files skipped from review due to trivial changes (27)
- frontend/packages/helm-plugin/integration-tests/support/pages/helm/upgrade-helm-release-page.ts
- frontend/packages/helm-plugin/integration-tests/support/constants/static-text/helm-text.ts
- frontend/packages/helm-plugin/src/components/details-page/overview/tests/HelmReleaseOverview.spec.tsx
- frontend/packages/helm-plugin/integration-tests/support/step-definitions/helm/helm-compatibility.ts
- frontend/packages/helm-plugin/integration-tests/features/helm/helm-compatibility.feature
- frontend/packages/helm-plugin/integration-tests/features/helm/helm-page-tabs.feature
- frontend/packages/helm-plugin/integration-tests/features/helm/install-url-chart.feature
- frontend/packages/helm-plugin/integration-tests/features/helm/helm-feature-flag.feature
- frontend/packages/helm-plugin/src/utils/tests/helm-utils.spec.ts
- frontend/packages/helm-plugin/integration-tests/support/pages/helm/helm-page.ts
- frontend/packages/helm-plugin/integration-tests/support/step-definitions/common/common.ts
- frontend/packages/helm-plugin/src/models/helm.ts
- frontend/packages/helm-plugin/src/components/list-page/HelmReleaseListPage.tsx
- frontend/packages/helm-plugin/src/components/forms/tests/HelmInstallUpgradeForm.spec.tsx
- frontend/packages/helm-plugin/src/topology/helmFilters.ts
- frontend/packages/helm-plugin/src/catalog/utils/catalog-utils.tsx
- frontend/packages/helm-plugin/src/components/tests/helm-release-mock-data.ts
- frontend/packages/helm-plugin/src/components/forms/url-chart/HelmURLChartInstallPage.tsx
- frontend/packages/helm-plugin/integration-tests/features/helm/install-helm-chart.feature
- frontend/packages/helm-plugin/src/components/forms/url-chart/HelmURLInstallForm.tsx
- frontend/packages/helm-plugin/console-extensions.json
- frontend/packages/helm-plugin/src/components/list-page/HelmReleaseList.tsx
- frontend/packages/helm-plugin/src/components/details-page/history/HelmReleaseHistory.tsx
- frontend/packages/helm-plugin/integration-tests/features/helm/topology-helm-release.feature
- frontend/packages/helm-plugin/integration-tests/support/step-definitions/helm/helm.ts
- frontend/packages/helm-plugin/src/components/list-page/HelmTabbedPage.tsx
- frontend/packages/helm-plugin/integration-tests/features/helm/helm-installation-view.feature
🚧 Files skipped from review as they are similar to previous changes (4)
- frontend/public/locales/en/public.json
- frontend/packages/helm-plugin/src/components/forms/url-chart/HelmURLChartForm.tsx
- frontend/packages/helm-plugin/src/components/forms/url-chart/HelmCreateBasicAuthSecretModal.tsx
- frontend/packages/helm-plugin/src/components/forms/install-upgrade/HelmInstallUpgradeForm.tsx
sowmya-sl
force-pushed
the
fix-helm-upgrade-secrets
branch
from
7468da3 to
243b4d6
Compare
openshift-ci
Bot
added
component/sdk
sowmya-sl
force-pushed
the
fix-helm-upgrade-secrets
branch
from
243b4d6 to
05376cf
Compare
Contributor
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: sowmya-sl The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
…e microcopy - Add HelmCreateBasicAuthSecretModal for creating basic auth secrets during OCI/HTTP chart installation from URL - Extend HelmInstallUpgradeForm and HelmURLChartForm to support basic auth secret selection for authenticated chart repositories - Add upgrade support with basic auth secret propagation via chart annotations in get_chart.go, install_chart.go, and upgrade_release.go - Update handler to pass basicAuthSecretName through GetChartFromURL - Normalize UI microcopy: lowercase "release" in "Helm release", use "Helm Chart(s)" consistently across console-extensions.json, integration test features, step definitions, and page objects - Update i18n keys in helm-plugin and public locales Co-authored-by: Cursor <cursoragent@cursor.com>
sowmya-sl
force-pushed
the
fix-helm-upgrade-secrets
branch
from
05376cf to
955ad3f
Compare
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
5 participants
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.
Analysis / Root cause:
Solution description:
Screenshots / screen recording:
Test setup:
Test cases:
Browser conformance:
Additional info:
Reviewers and assignees:
Summary by CodeRabbit