Skip to content

Commit

Permalink
Fix rke2 addon validation (#13017)
Browse files Browse the repository at this point in the history
* add codemirror error reporting

wire in codemirror validation to rke2 addon config

update addon config validation status when list of available addons changes

* added e2e test for addon config validation;

* account for codemirror being used with no options
  • Loading branch information
mantis-toboggan-md authored Jan 9, 2025
1 parent 4085c86 commit 23c0cdb
Show file tree
Hide file tree
Showing 8 changed files with 99 additions and 6 deletions.
12 changes: 12 additions & 0 deletions cypress/e2e/po/components/addon-config.po.ts
Original file line number Diff line number Diff line change
@@ -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"]'));
}
}
4 changes: 4 additions & 0 deletions cypress/e2e/po/components/create-edit-view.po.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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'));
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -44,4 +45,8 @@ export default class ClusterManagerCreateRke2CustomPagePo extends ClusterManager
network(): NetworkTabPo {
return new NetworkTabPo();
}

calicoAddonConfig(): AddonConfigPo {
return new AddonConfigPo();
}
}
25 changes: 25 additions & 0 deletions cypress/e2e/tests/pages/manager/cluster-manager.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down
29 changes: 28 additions & 1 deletion shell/components/CodeMirror.vue
Original file line number Diff line number Diff line change
Expand Up @@ -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: {
/**
Expand Down Expand Up @@ -39,6 +39,7 @@ export default {
codeMirrorRef: null,
loaded: false,
removeKeyMapBox: false,
hasLintErrors: false,
};
},
Expand All @@ -65,6 +66,7 @@ export default {
foldGutter: true,
styleSelectedText: true,
showCursorWhenSelecting: true,
autocorrect: false,
};
if (this.asTextArea) {
Expand All @@ -76,6 +78,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;
},
Expand Down Expand Up @@ -104,7 +111,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();
Expand All @@ -118,6 +143,8 @@ export default {
},
onReady(codeMirrorRef) {
this.$emit('validationChanged', true);
this.$nextTick(() => {
codeMirrorRef.refresh();
this.codeMirrorRef = codeMirrorRef;
Expand Down
3 changes: 2 additions & 1 deletion shell/components/YamlEditor.vue
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -236,6 +236,7 @@ export default {
@onInput="onInput"
@onReady="onReady"
@onChanges="onChanges"
@validationChanged="$emit('validationChanged', $event)"
/>
<FileDiff
v-else
Expand Down
15 changes: 13 additions & 2 deletions shell/edit/provisioning.cattle.io.cluster/rke2.vue
Original file line number Diff line number Diff line change
Expand Up @@ -248,6 +248,7 @@ export default {
busy: false,
machinePoolValidation: {}, // map of validation states for each machine pool
machinePoolErrors: {},
addonConfigValidation: {}, // validation state of each addon config (boolean of whether codemirror's yaml lint passed)
allNamespaces: [],
extensionTabs: getApplicableExtensionEnhancements(this, ExtensionPoint.TAB, TabLocation.CLUSTER_CREATE_RKE2, this.$route, this),
labelForAddon
Expand Down Expand Up @@ -797,7 +798,9 @@ export default {
// and in all of the validation statuses for each machine pool
Object.values(this.machinePoolValidation).forEach((v) => (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) {
Expand Down Expand Up @@ -1565,6 +1568,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];
Expand Down Expand Up @@ -2133,7 +2138,11 @@ export default {
}
}
}
}
},
addonConfigValidationChanged(configName, isValid) {
this.addonConfigValidation[configName] = isValid;
},
}
};
</script>
Expand Down Expand Up @@ -2430,6 +2439,7 @@ export default {
:label="labelForAddon($store, v.name, false)"
:weight="9"
:showHeader="false"
:error="addonConfigValidation[v.name]===false"
@active="showAddons(v.name)"
>
<AddOnConfig
Expand All @@ -2444,6 +2454,7 @@ export default {
@update:value="$emit('input', $event)"
@update-questions="syncChartValues"
@update-values="updateValues"
@validationChanged="e => addonConfigValidationChanged(v.name, e)"
/>
</Tab>
Expand Down
12 changes: 10 additions & 2 deletions shell/edit/provisioning.cattle.io.cluster/tabs/AddOnConfig.vue
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -60,7 +60,13 @@ export default {
isEdit() {
return this.mode === _EDIT;
}
}
},
methods: {
handleValidationChanged(e) {
this.$emit('validationChanged', e);
}
},
};
</script>

Expand Down Expand Up @@ -91,12 +97,14 @@ export default {
<YamlEditor
v-else
ref="yaml-values"
data-testid="addon-yaml-editor"
:value="initYamlEditor(addonVersion.name)"
:scrolling="true"
:as-object="true"
:editor-mode="mode === 'view' ? 'VIEW_CODE' : 'EDIT_CODE'"
:hide-preview-buttons="true"
@update:value="$emit('update-values', addonVersion.name, $event)"
@validationChanged="handleValidationChanged"
/>
<div class="spacer" />
</div>
Expand Down

0 comments on commit 23c0cdb

Please sign in to comment.