-
Notifications
You must be signed in to change notification settings - Fork 737
HELM-763: Add secrets for Helm release upgrade #16787
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
6eac525
1c9f554
1724ee0
a90ac5e
03abe52
cdb0587
453eaf5
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -6,7 +6,6 @@ | |
| "**Helm Charts** are packages for deploying an Application or components of a larger Application.": "**Helm Charts** are packages for deploying an Application or components of a larger Application.", | ||
| "A description for the Helm Chart repository.": "A description for the Helm Chart repository.", | ||
| "A display name for the Helm Chart repository.": "A display name for the Helm Chart repository.", | ||
| "A secret with \"username\" and \"password\" keys for OCI/HTTP(S) authentication": "A secret with \"username\" and \"password\" keys for OCI/HTTP(S) authentication", | ||
| "A unique name for the Helm Chart repository.": "A unique name for the Helm Chart repository.", | ||
| "A unique name for the Helm Release.": "A unique name for the Helm Release.", | ||
| "Add a Helm Chart Repository to extend the Developer Catalog": "Add a Helm Chart Repository to extend the Developer Catalog", | ||
|
|
@@ -39,9 +38,11 @@ | |
| "Complete the form to create a Helm release. The Helm chart authors might have provided some default values.": "Complete the form to create a Helm release. The Helm chart authors might have provided some default values.", | ||
| "Configure Helm release": "Configure Helm release", | ||
| "Create": "Create", | ||
| "Create authentication Secret": "Create authentication Secret", | ||
| "Create Helm Chart Repository": "Create Helm Chart Repository", | ||
| "Create Helm Release": "Create Helm Release", | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. cap of "Helm Release" inconsistent throughout this file. capped in some strings ("Create Helm Release," "Upgrade Helm Release") and lowercase in others ("Configure Helm release," "Unique name for Helm release."). same issue as "Helm Chart". confirm correct casing and apply it globally. ideally we case it like we did in previous PRs where we discussed these same issues. perhaps keep a note somewhere?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Changed to Helm release overall. |
||
| "Create ProjectHelmChartRepository": "Create ProjectHelmChartRepository", | ||
| "Create Secret": "Create Secret", | ||
| "Created": "Created", | ||
| "Delete": "Delete", | ||
| "Delete Helm Release": "Delete Helm Release", | ||
|
|
@@ -55,6 +56,7 @@ | |
| "Edit Helm Chart Repository": "Edit Helm Chart Repository", | ||
| "Edit ProjectHelmChartRepository": "Edit ProjectHelmChartRepository", | ||
| "Errors in the form data.": "Errors in the form data.", | ||
| "Failed to create Secret.": "Failed to create Secret.", | ||
| "False": "False", | ||
| "Filter by status": "Filter by status", | ||
| "For more information on the chart, refer to this <2>README</2>": "For more information on the chart, refer to this <2>README</2>", | ||
|
|
@@ -100,6 +102,7 @@ | |
| "None": "None", | ||
| "of": "of", | ||
| "Partner": "Partner", | ||
| "Password or token for OCI/HTTP(S) authentication.": "Password or token for OCI/HTTP(S) authentication.", | ||
| "per page": "per page", | ||
| "Please enter a URL that is less then 2048 characters.": "Please enter a URL that is less then 2048 characters.", | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Typo: "less then" should be "less than." also, not allowed to use "please" |
||
| "Pods": "Pods", | ||
|
|
@@ -126,7 +129,12 @@ | |
| "Rollback to Revision {{revision}}": "Rollback to Revision {{revision}}", | ||
| "Save": "Save", | ||
| "Scope type": "Scope type", | ||
| "Secret \"{{secretName}}\" was not found in this namespace. Select an existing secret or create a new one.": "Secret \"{{secretName}}\" was not found in this namespace. Select an existing secret or create a new one.", | ||
| "Secret for Basic authentication": "Secret for Basic authentication", | ||
| "Secret name": "Secret name", | ||
| "Secret password or token": "Secret password or token", | ||
| "Secret username": "Secret username", | ||
| "Secret with \"{{username}}\" and \"{{password}}\" keys for OCI/HTTP(S) authentication.": "Secret with \"{{username}}\" and \"{{password}}\" keys for OCI/HTTP(S) authentication.", | ||
| "Select": "Select", | ||
| "Select a Project to view its details<1></1>.": "Select a Project to view its details<1></1>.", | ||
| "Select a secret": "Select a secret", | ||
|
|
@@ -146,6 +154,7 @@ | |
| "The repository name cannot exceed than 2048 characters.": "The repository name cannot exceed than 2048 characters.", | ||
| "The version of chart to install.": "The version of chart to install.", | ||
| "This Helm Chart is provided by a trusted partner and has been verified for ease of integration.": "This Helm Chart is provided by a trusted partner and has been verified for ease of integration.", | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. make this active voice --> "A trusted partner provides this Helm Chart and has verified it for ease of integration." |
||
| "This name is reserved. Choose a different name.": "This name is reserved. Choose a different name.", | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. say why name is reserved? for ex., "This name is reserved for system use. Choose a different name."? |
||
| "TLS Client config": "TLS Client config", | ||
| "To install a Helm chart, enter the chart URL - Open Container Initiative (OCI) URL or HTTP/HTTPS tar file and version.": "To install a Helm chart, enter the chart URL - Open Container Initiative (OCI) URL or HTTP/HTTPS tar file and version.", | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. use a colon instead of a hyphen here? E.g., "enter the chart URL: an OCI URL or HTTP/HTTPS tar file, plus the version." As written, hyphen makes this hard to parse on first read. also, tar --> .tar or TAR
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Wordings changed. |
||
| "True": "True", | ||
|
|
@@ -154,6 +163,7 @@ | |
| "Unable to load Helm Release history": "Unable to load Helm Release history", | ||
| "Unable to load Helm Releases": "Unable to load Helm Releases", | ||
| "Unique name for Helm release.": "Unique name for Helm release.", | ||
| "Unique name of the Secret": "Unique name of the Secret", | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 2 things: should "Secret" be lowercase to match "secret" elsewhere in the file? should this read "A unique name for the Secret." to match the pattern used for the other "unique name" strings (lines 10, 11)?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. All secrets are now Secrets. Changed for. |
||
| "Unknown error submitting": "Unknown error submitting", | ||
| "Update helm chart repository in the namespace.": "Update helm chart repository in the namespace.", | ||
| "Update the helm chart repository.": "Update the helm chart repository.", | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. cap of "Helm Chart" is inconsistent across file (line 8 "Helm Chart repository" and line 39 "Helm chart authors"). use correct casing and apply consistently. ideally, use same casing as last PR discussion of this same issue. |
||
|
|
@@ -163,6 +173,7 @@ | |
| "Upgrade by selecting a new chart version or manually changing YAML.": "Upgrade by selecting a new chart version or manually changing YAML.", | ||
| "Upgrade Helm Release": "Upgrade Helm Release", | ||
| "URL": "URL", | ||
| "Username for OCI/HTTP(S) authentication.": "Username for OCI/HTTP(S) authentication.", | ||
| "Values from your current release are merged with the new chart's defaults. Review the YAML or form before upgrading.": "Values from your current release are merged with the new chart's defaults. Review the YAML or form before upgrading.", | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. make this active voice --> "The new chart's defaults merge with values from your current release. Review the YAML or form before upgrading."
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Comments on existing text (i.e., not in green highlight) should probably be captured in a Jira ticket and applied in a new PR, otherwise we may have trouble getting the changes in this PR merged. (Your call, @sowmya-sl.) |
||
| "Version": "Version" | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -2,6 +2,7 @@ import type { ReactNode, FC } from 'react'; | |
| import { useMemo } from 'react'; | ||
| import { TextInputTypes, Grid, GridItem, Button, Alert } from '@patternfly/react-core'; | ||
| import type { FormikProps } from 'formik'; | ||
| import * as fuzzy from 'fuzzysearch'; | ||
| import type { JSONSchema7 } from 'json-schema'; | ||
| import * as _ from 'lodash'; | ||
| import { Trans, useTranslation } from 'react-i18next'; | ||
|
|
@@ -14,11 +15,18 @@ import { FormHeader } from '@console/shared/src/components/form-utils/FormHeader | |
| import { CodeEditorField } from '@console/shared/src/components/formik-fields/CodeEditorField'; | ||
| import { DynamicFormField } from '@console/shared/src/components/formik-fields/DynamicFormField'; | ||
| import { InputField } from '@console/shared/src/components/formik-fields/InputField'; | ||
| import { ResourceDropdownField } from '@console/shared/src/components/formik-fields/ResourceDropdownField'; | ||
| import { SyncedEditorField } from '@console/shared/src/components/formik-fields/SyncedEditorField'; | ||
| import type { EditorType } from '@console/shared/src/components/synced-editor/editor-toggle'; | ||
| import type { HelmChart, HelmActionConfigType } from '../../../types/helm-types'; | ||
| import { HelmActionType } from '../../../types/helm-types'; | ||
| import { helmActionString } from '../../../utils/helm-utils'; | ||
| import { | ||
| useBasicAuthSecretDropdown, | ||
| CREATE_SECRET_KEY, | ||
| NONE_SECRET_KEY, | ||
| } from '../url-chart/useBasicAuthSecretDropdown'; | ||
| import { useSecretResources } from '../url-chart/useSecretResources'; | ||
| import HelmChartVersionDropdown from './HelmChartVersionDropdown'; | ||
| import { useHelmReadmeModalLauncher } from './HelmReadmeModal'; | ||
|
|
||
|
|
@@ -35,6 +43,8 @@ export type HelmInstallUpgradeFormData = { | |
| formData: any; | ||
| formSchema: JSONSchema7; | ||
| editorType: EditorType; | ||
| basicAuthSecretName?: string; | ||
| isURLInstall?: boolean; | ||
| }; | ||
|
|
||
| interface HelmInstallUpgradeFormProps { | ||
|
|
@@ -68,11 +78,33 @@ const HelmInstallUpgradeForm: FC< | |
| chartIndexEntry, | ||
| annotatedName, | ||
| providerName, | ||
| setFieldValue, | ||
| }) => { | ||
| const { t } = useTranslation('helm-plugin'); | ||
| const { handleSecretChange } = useBasicAuthSecretDropdown({ | ||
| namespace, | ||
| currentSecretName: values.basicAuthSecretName, | ||
| setFieldValue, | ||
| }); | ||
| const { chartName, chartVersion, chartReadme, formData, formSchema, editorType } = values; | ||
| const { type: helmAction, title, subTitle } = helmActionConfig; | ||
| const helmReadmeModalLauncher = useHelmReadmeModalLauncher({ readme: chartReadme }); | ||
| const showAuthSecret = values.isURLInstall; | ||
| const secretResources = useSecretResources(namespace); | ||
| const autocompleteFilter = (strText: string, item: any, key?: string): boolean => | ||
| fuzzy(strText, item?.props?.name || (typeof item === 'string' ? item : key) || ''); | ||
| const secretMissing = useMemo(() => { | ||
| if ( | ||
| !showAuthSecret || | ||
| !values.basicAuthSecretName || | ||
| values.basicAuthSecretName === NONE_SECRET_KEY || | ||
| !secretResources[0]?.loaded | ||
| ) { | ||
| return false; | ||
| } | ||
| const secrets = secretResources[0]?.data ?? []; | ||
| return !secrets.some((s) => s?.metadata?.name === values.basicAuthSecretName); | ||
| }, [showAuthSecret, secretResources, values.basicAuthSecretName]); | ||
| const isSubmitDisabled = | ||
| (helmAction === HelmActionType.Upgrade && !dirty) || | ||
| isSubmitting || | ||
|
|
@@ -162,6 +194,48 @@ const HelmInstallUpgradeForm: FC< | |
| providerName={providerName} | ||
| /> | ||
| </GridItem> | ||
| {showAuthSecret && ( | ||
| <GridItem xl={5} lg={4} md={12}> | ||
| <ResourceDropdownField | ||
| name="basicAuthSecretName" | ||
| label={t('Secret for Basic authentication')} | ||
| resources={secretResources} | ||
| dataSelector={['metadata', 'name']} | ||
| fullWidth | ||
| placeholder={ | ||
| helmAction === HelmActionType.Upgrade ? t('None') : t('Select a secret') | ||
| } | ||
| showBadge | ||
| autocompleteFilter={autocompleteFilter} | ||
| actionItems={[ | ||
| { | ||
| actionTitle: t('None'), | ||
| actionKey: NONE_SECRET_KEY, | ||
| }, | ||
| { | ||
| actionTitle: t('Create Secret'), | ||
| actionKey: CREATE_SECRET_KEY, | ||
| }, | ||
| ]} | ||
| onChange={handleSecretChange} | ||
| helpText={t( | ||
| 'Secret with "{{username}}" and "{{password}}" keys for OCI/HTTP(S) authentication.', | ||
| { username: 'username', password: 'password' }, | ||
| )} | ||
| /> | ||
| {secretMissing && ( | ||
| <Alert | ||
| variant="warning" | ||
| isInline | ||
| isPlain | ||
| title={t( | ||
| 'Secret "{{secretName}}" was not found in this namespace. Select an existing secret or create a new one.', | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. make this active voice and drop "create new" redundancy. --> "Secret "{{secretName}}" doesn't exist in this namespace. Select an existing secret or create one." |
||
| { secretName: values.basicAuthSecretName }, | ||
| )} | ||
| /> | ||
| )} | ||
| </GridItem> | ||
| )} | ||
| </Grid> | ||
| </FormSection> | ||
| {!chartError && | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should this be "Create authentication secret" (lowercase) to match how "secret" is treated elsewhere in this file (like "Select a secret" line 140, as one ex)?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have made all secrets as Secret in #16785