From 35822f507b784ec2af3f82a335302352de1a7921 Mon Sep 17 00:00:00 2001 From: Nancy Butler <42977925+mantis-toboggan-md@users.noreply.github.com> Date: Mon, 28 Oct 2024 11:55:27 -0700 Subject: [PATCH 1/3] add codemirror error reporting wire in codemirror validation to rke2 addon config update addon config validation status when list of available addons changes --- pkg/eks/components/CruEKS.vue | 2 ++ shell/components/CodeMirror.vue | 28 ++++++++++++++++++- shell/components/YamlEditor.vue | 3 +- .../provisioning.cattle.io.cluster/rke2.vue | 15 ++++++++-- .../tabs/AddOnConfig.vue | 11 ++++++-- 5 files changed, 53 insertions(+), 6 deletions(-) diff --git a/pkg/eks/components/CruEKS.vue b/pkg/eks/components/CruEKS.vue index ca08cb9e538..41bb5f5ae14 100644 --- a/pkg/eks/components/CruEKS.vue +++ b/pkg/eks/components/CruEKS.vue @@ -120,6 +120,8 @@ export default defineComponent({ if (this.value.id) { const liveNormanCluster = await this.value.findNormanCluster(); + debugger; + this.normanCluster = await store.dispatch(`rancher/clone`, { resource: liveNormanCluster }); // ensure any fields editable through this UI that have been altered in aws are shown here - see syncUpstreamConfig jsdoc for details if (!this.isNewOrUnprovisioned) { diff --git a/shell/components/CodeMirror.vue b/shell/components/CodeMirror.vue index b15a8d976af..f7c8cb91ffb 100644 --- a/shell/components/CodeMirror.vue +++ b/shell/components/CodeMirror.vue @@ -5,7 +5,7 @@ import { _EDIT, _VIEW } from '@shell/config/query-params'; export default { name: 'CodeMirror', - emits: ['onReady', 'onInput', 'onChanges', 'onFocus'], + emits: ['onReady', 'onInput', 'onChanges', 'onFocus', 'validationChanged'], props: { /** @@ -39,6 +39,7 @@ export default { codeMirrorRef: null, loaded: false, removeKeyMapBox: false, + hasLintErrors: false, }; }, @@ -76,6 +77,11 @@ export default { Object.assign(out, this.options); + // parent components control lint with a boolean; if linting is enabled, we need to override that boolean with a custom error handler to wire lint errors into dashboard validation + if (this.options.lint) { + out.lint = { onUpdateLinting: this.handleLintErrors }; + } + return out; }, @@ -104,7 +110,25 @@ export default { } }, + watch: { + hasLintErrors(neu) { + this.$emit('validationChanged', !neu); + } + }, + methods: { + /** + * Codemirror yaml linting uses js-yaml parse + * it does not distinguish between warnings and errors so we will treat all yaml lint messages as errors + * other codemirror linters (eg json) will report from, to, severity where severity may be 'warning' or 'error' + * only 'error' level linting will trigger a validation event from this component + */ + handleLintErrors(diagnostics = []) { + const hasLintErrors = diagnostics.filter((d) => !d.severity || d.severity === 'error').length > 0; + + this.hasLintErrors = hasLintErrors; + }, + focus() { if ( this.$refs.codeMirrorRef ) { this.$refs.codeMirrorRef.cminstance.focus(); @@ -118,6 +142,8 @@ export default { }, onReady(codeMirrorRef) { + this.$emit('validationChanged', true); + this.$nextTick(() => { codeMirrorRef.refresh(); this.codeMirrorRef = codeMirrorRef; diff --git a/shell/components/YamlEditor.vue b/shell/components/YamlEditor.vue index 05f695b3f79..4d2d0a0da25 100644 --- a/shell/components/YamlEditor.vue +++ b/shell/components/YamlEditor.vue @@ -13,7 +13,7 @@ export const EDITOR_MODES = { }; export default { - emits: ['update:value', 'newObject', 'onInput', 'onReady', 'onChanges'], + emits: ['update:value', 'newObject', 'onInput', 'onReady', 'onChanges', 'validationChanged'], components: { CodeMirror, @@ -236,6 +236,7 @@ export default { @onInput="onInput" @onReady="onReady" @onChanges="onChanges" + @validationChanged="$emit('validationChanged', $event)" /> (base = base && v)); - return validRequiredPools && base; + const hasAddonConfigErrors = Object.values(this.addonConfigValidation).filter((v) => v === false).length > 0; + + return validRequiredPools && base && !hasAddonConfigErrors; }, currentCluster() { if (this.mode === _EDIT) { @@ -1566,6 +1569,8 @@ export default { * 2) We're ready to cache any values the user provides for each addon */ async initAddons() { + this.addonConfigValidation = {}; + for (const chartName of this.addonNames) { const entry = this.chartVersions[chartName]; @@ -2134,7 +2139,11 @@ export default { } } } - } + }, + + addonConfigValidationChanged(configName, isValid) { + this.addonConfigValidation[configName] = isValid; + }, } }; @@ -2431,6 +2440,7 @@ export default { :label="labelForAddon($store, v.name, false)" :weight="9" :showHeader="false" + :error="addonConfigValidation[v.name]===false" @active="showAddons(v.name)" > diff --git a/shell/edit/provisioning.cattle.io.cluster/tabs/AddOnConfig.vue b/shell/edit/provisioning.cattle.io.cluster/tabs/AddOnConfig.vue index 720bd3dc53d..7bd01d50052 100644 --- a/shell/edit/provisioning.cattle.io.cluster/tabs/AddOnConfig.vue +++ b/shell/edit/provisioning.cattle.io.cluster/tabs/AddOnConfig.vue @@ -7,7 +7,7 @@ import { labelForAddon } from '@shell/utils/cluster'; import { _EDIT } from '@shell/config/query-params'; export default { - emits: ['additional-manifest-changed', 'update-questions', 'update-values'], + emits: ['additional-manifest-changed', 'update-questions', 'update-values', 'validationChanged'], components: { Banner, @@ -60,7 +60,13 @@ export default { isEdit() { return this.mode === _EDIT; } - } + }, + + methods: { + handleValidationChanged(e) { + this.$emit('validationChanged', e); + } + }, }; @@ -97,6 +103,7 @@ export default { :editor-mode="mode === 'view' ? 'VIEW_CODE' : 'EDIT_CODE'" :hide-preview-buttons="true" @update:value="$emit('update-values', addonVersion.name, $event)" + @validationChanged="handleValidationChanged" />
From 08831df700888b3687d40ec8bc4fd6c1f3b63d86 Mon Sep 17 00:00:00 2001 From: Nancy Butler <42977925+mantis-toboggan-md@users.noreply.github.com> Date: Wed, 8 Jan 2025 10:34:54 -0800 Subject: [PATCH 2/3] added e2e test for addon config validation; --- cypress/e2e/po/components/addon-config.po.ts | 12 +++++++++ .../e2e/po/components/create-edit-view.po.ts | 4 +++ .../create/cluster-create-rke2-custom.po.ts | 5 ++++ .../pages/manager/cluster-manager.spec.ts | 25 +++++++++++++++++++ pkg/eks/components/CruEKS.vue | 2 -- shell/components/CodeMirror.vue | 1 + .../tabs/AddOnConfig.vue | 1 + 7 files changed, 48 insertions(+), 2 deletions(-) create mode 100644 cypress/e2e/po/components/addon-config.po.ts diff --git a/cypress/e2e/po/components/addon-config.po.ts b/cypress/e2e/po/components/addon-config.po.ts new file mode 100644 index 00000000000..fc9c4760fa2 --- /dev/null +++ b/cypress/e2e/po/components/addon-config.po.ts @@ -0,0 +1,12 @@ +import ComponentPo from '@/cypress/e2e/po/components/component.po'; +import YamlEditorPo from '~/cypress/e2e/po/components/yaml-editor.po'; + +export default class AddonConfigPo extends ComponentPo { + constructor(selector = '.dashboard-root') { + super(selector); + } + + yamlEditor() :YamlEditorPo { + return new YamlEditorPo(this.self().find('[data-testid="addon-yaml-editor"]')); + } +} diff --git a/cypress/e2e/po/components/create-edit-view.po.ts b/cypress/e2e/po/components/create-edit-view.po.ts index 169e8a6ffbb..b94c8091ac8 100644 --- a/cypress/e2e/po/components/create-edit-view.po.ts +++ b/cypress/e2e/po/components/create-edit-view.po.ts @@ -17,4 +17,8 @@ export default class CreateEditViewPo extends ComponentPo { nextPage() { return new AsyncButtonPo(this.self().find('.cru-resource-footer .role-primary')).click(); } + + saveButtonPo() :AsyncButtonPo { + return new AsyncButtonPo(this.self().find('.cru-resource-footer .role-primary')); + } } diff --git a/cypress/e2e/po/edit/provisioning.cattle.io.cluster/create/cluster-create-rke2-custom.po.ts b/cypress/e2e/po/edit/provisioning.cattle.io.cluster/create/cluster-create-rke2-custom.po.ts index ff4685a09d4..6690cffd94d 100644 --- a/cypress/e2e/po/edit/provisioning.cattle.io.cluster/create/cluster-create-rke2-custom.po.ts +++ b/cypress/e2e/po/edit/provisioning.cattle.io.cluster/create/cluster-create-rke2-custom.po.ts @@ -4,6 +4,7 @@ import ClusterManagerCreatePagePo from '@/cypress/e2e/po/edit/provisioning.cattl import TabbedPo from '@/cypress/e2e/po/components/tabbed.po'; import RegistriesTabPo from '@/cypress/e2e/po/components/registries-tab.po'; import NetworkTabPo from '@/cypress/e2e/po/components/network-tab.po'; +import AddonConfigPo from '@/cypress/e2e/po/components/addon-config.po'; /** * Create page for an RKE2 custom cluster @@ -44,4 +45,8 @@ export default class ClusterManagerCreateRke2CustomPagePo extends ClusterManager network(): NetworkTabPo { return new NetworkTabPo(); } + + calicoAddonConfig(): AddonConfigPo { + return new AddonConfigPo(); + } } diff --git a/cypress/e2e/tests/pages/manager/cluster-manager.spec.ts b/cypress/e2e/tests/pages/manager/cluster-manager.spec.ts index 10dd81531d9..e68b4adc715 100644 --- a/cypress/e2e/tests/pages/manager/cluster-manager.spec.ts +++ b/cypress/e2e/tests/pages/manager/cluster-manager.spec.ts @@ -311,6 +311,31 @@ describe('Cluster Manager', { testIsolation: 'off', tags: ['@manager', '@adminUs editCreatedClusterPage.nameNsDescription().description().self().should('have.value', rke2CustomName); }); + it('will disbable saving if an addon config has invalid data', () => { + clusterList.goTo(); + + clusterList.checkIsCurrentPage(); + clusterList.createCluster(); + + createRKE2ClusterPage.waitForPage(); + + createRKE2ClusterPage.rkeToggle().set('RKE2/K3s'); + + createRKE2ClusterPage.selectCustom(0); + + createRKE2ClusterPage.nameNsDescription().name().set('abc'); + + createRKE2ClusterPage.clusterConfigurationTabs().clickTabWithSelector('#rke2-calico'); + + createRKE2ClusterPage.resourceDetail().createEditView().saveButtonPo().expectToBeEnabled(); + + createRKE2ClusterPage.calicoAddonConfig().yamlEditor().input().set('badvalue: -'); + createRKE2ClusterPage.resourceDetail().createEditView().saveButtonPo().expectToBeDisabled(); + + createRKE2ClusterPage.calicoAddonConfig().yamlEditor().input().set('goodvalue: yay'); + createRKE2ClusterPage.resourceDetail().createEditView().saveButtonPo().expectToBeEnabled(); + }); + it('can view cluster YAML editor', () => { clusterList.goTo(); clusterList.list().actionMenu(rke2CustomName).getMenuItem('Edit YAML').click(); diff --git a/pkg/eks/components/CruEKS.vue b/pkg/eks/components/CruEKS.vue index 41bb5f5ae14..ca08cb9e538 100644 --- a/pkg/eks/components/CruEKS.vue +++ b/pkg/eks/components/CruEKS.vue @@ -120,8 +120,6 @@ export default defineComponent({ if (this.value.id) { const liveNormanCluster = await this.value.findNormanCluster(); - debugger; - this.normanCluster = await store.dispatch(`rancher/clone`, { resource: liveNormanCluster }); // ensure any fields editable through this UI that have been altered in aws are shown here - see syncUpstreamConfig jsdoc for details if (!this.isNewOrUnprovisioned) { diff --git a/shell/components/CodeMirror.vue b/shell/components/CodeMirror.vue index f7c8cb91ffb..e773ecaf79f 100644 --- a/shell/components/CodeMirror.vue +++ b/shell/components/CodeMirror.vue @@ -66,6 +66,7 @@ export default { foldGutter: true, styleSelectedText: true, showCursorWhenSelecting: true, + autocorrect: false, }; if (this.asTextArea) { diff --git a/shell/edit/provisioning.cattle.io.cluster/tabs/AddOnConfig.vue b/shell/edit/provisioning.cattle.io.cluster/tabs/AddOnConfig.vue index 7bd01d50052..671036ee0f2 100644 --- a/shell/edit/provisioning.cattle.io.cluster/tabs/AddOnConfig.vue +++ b/shell/edit/provisioning.cattle.io.cluster/tabs/AddOnConfig.vue @@ -97,6 +97,7 @@ export default { Date: Thu, 9 Jan 2025 10:25:42 -0800 Subject: [PATCH 3/3] account for codemirror being used with no options --- shell/components/CodeMirror.vue | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/shell/components/CodeMirror.vue b/shell/components/CodeMirror.vue index e773ecaf79f..d125007f204 100644 --- a/shell/components/CodeMirror.vue +++ b/shell/components/CodeMirror.vue @@ -79,7 +79,7 @@ export default { Object.assign(out, this.options); // parent components control lint with a boolean; if linting is enabled, we need to override that boolean with a custom error handler to wire lint errors into dashboard validation - if (this.options.lint) { + if (this.options?.lint) { out.lint = { onUpdateLinting: this.handleLintErrors }; }