diff --git a/dashboard/package.json b/dashboard/package.json index e122e51e0e0..b52ec442554 100644 --- a/dashboard/package.json +++ b/dashboard/package.json @@ -22,6 +22,7 @@ "axios": "^0.19.0", "connected-react-router": "^4.5.0", "enzyme-adapter-react-16": "^1.1.1", + "fast-json-patch": "^3.0.0-1", "fstream": "^1.0.12", "js-yaml": "^3.13.1", "json-schema": "^0.2.5", diff --git a/dashboard/src/components/DeploymentFormBody/DeploymentFormBody.test.tsx b/dashboard/src/components/DeploymentFormBody/DeploymentFormBody.test.tsx index 864f17aa1c3..0b79bb3d723 100644 --- a/dashboard/src/components/DeploymentFormBody/DeploymentFormBody.test.tsx +++ b/dashboard/src/components/DeploymentFormBody/DeploymentFormBody.test.tsx @@ -29,7 +29,7 @@ const defaultProps = { valuesModified: false, setValues: jest.fn(), setValuesModified: jest.fn(), - originalValues: undefined, + deployedValues: undefined, } as IDeploymentFormBodyProps; const versions = [{ id: "foo", attributes: { version: "1.2.3" } }] as IChartVersion[]; @@ -118,9 +118,9 @@ describe("when there are changes in the selected version", () => { describe("when the user has not modified any value", () => { it("selects the original values if the version doesn't change", () => { const setValues = jest.fn(); - const originalValues = "foo: notBar"; + const deployedValues = "foo: notBar"; const wrapper = shallow( - , + , ); wrapper.setProps({ selected: { @@ -139,13 +139,13 @@ describe("when there are changes in the selected version", () => { const localState: IDeploymentFormBodyState = wrapper.instance() .state as IDeploymentFormBodyState; expect(localState.basicFormParameters).toEqual(basicFormParameters); - expect(setValues).toHaveBeenCalledWith("foo: notBar"); + expect(setValues).toHaveBeenCalledWith("foo: notBar\n"); }); it("uses the chart default values when original values are not defined", () => { const setValues = jest.fn(); const wrapper = shallow( - , + , ); wrapper.setProps({ selected: { @@ -171,13 +171,13 @@ describe("when there are changes in the selected version", () => { describe("when the user has modified the values", () => { it("will ignore original or default values", () => { const setValues = jest.fn(); - const originalValues = "foo: ignored-value"; + const deployedValues = "foo: ignored-value"; const modifiedValues = "foo: notBar"; const wrapper = shallow( , @@ -382,3 +382,106 @@ it("restores the default chart values when clicking on the button", () => { expect(setValues).toHaveBeenCalledWith("foo: value"); }); + +[ + { + description: "should merge modifications from the values and the new version defaults", + defaultValues: "foo: bar\n", + deployedValues: "foo: bar\nmy: var\n", + newDefaultValues: "notFoo: bar", + result: "notFoo: bar\nmy: var\n", + }, + { + description: "should modify the default values", + defaultValues: "foo: bar\n", + deployedValues: "foo: BAR\nmy: var\n", + newDefaultValues: "foo: bar", + result: "foo: BAR\nmy: var\n", + }, + { + description: "should delete an element in the defaults", + defaultValues: "foo: bar\n", + deployedValues: "my: var\n", + newDefaultValues: "foo: bar\n", + result: "my: var\n", + }, + { + description: "should add an element in an array", + defaultValues: `foo: + - foo1: + bar1: value1 +`, + deployedValues: `foo: + - foo1: + bar1: value1 + - foo2: + bar2: value2 +`, + newDefaultValues: `foo: + - foo1: + bar1: value1 +`, + result: `foo: + - foo1: + bar1: value1 + - foo2: + bar2: value2 +`, + }, + { + description: "should delete an element in an array", + defaultValues: `foo: + - foo1: + bar1: value1 + - foo2: + bar2: value2 +`, + deployedValues: `foo: + - foo1: + bar1: value1 +`, + newDefaultValues: `foo: + - foo1: + bar1: value1 + - foo2: + bar2: value2 +`, + result: `foo: + - foo1: + bar1: value1 +`, + }, +].forEach(t => { + it(t.description, () => { + const selected = { + ...defaultProps.selected, + versions: [chartVersion], + version: chartVersion, + values: t.defaultValues, + schema: initialSchema, + }; + const newSelected = { + ...defaultProps.selected, + versions: [chartVersion], + version: chartVersion, + values: t.newDefaultValues, + schema: initialSchema, + }; + const setValues = jest.fn(); + const wrapper = shallow( + , + ); + // Store the modifications + wrapper.setProps({ selected }); + expect(setValues).toHaveBeenCalledWith(t.deployedValues); + + // Apply new version + wrapper.setProps({ selected: newSelected }); + expect(setValues).toHaveBeenCalledWith(t.result); + }); +}); diff --git a/dashboard/src/components/DeploymentFormBody/DeploymentFormBody.tsx b/dashboard/src/components/DeploymentFormBody/DeploymentFormBody.tsx index 3a9f6571802..6dd8e05f04e 100644 --- a/dashboard/src/components/DeploymentFormBody/DeploymentFormBody.tsx +++ b/dashboard/src/components/DeploymentFormBody/DeploymentFormBody.tsx @@ -1,8 +1,10 @@ import { RouterAction } from "connected-react-router"; +import * as jsonpatch from "fast-json-patch"; import * as React from "react"; import { Tab, TabList, TabPanel, Tabs } from "react-tabs"; +import * as YAML from "yaml"; -import { retrieveBasicFormParams, setValue } from "../../shared/schema"; +import { deleteValue, retrieveBasicFormParams, setValue } from "../../shared/schema"; import { IBasicFormParam, IChartState } from "../../shared/types"; import { getValueFromEvent } from "../../shared/utils"; import ConfirmDialog from "../ConfirmDialog"; @@ -18,7 +20,7 @@ import "./Tabs.css"; export interface IDeploymentFormBodyProps { chartID: string; chartVersion: string; - originalValues?: string; + deployedValues?: string; namespace: string; releaseName?: string; selected: IChartState["selected"]; @@ -35,6 +37,7 @@ export interface IDeploymentFormBodyProps { export interface IDeploymentFormBodyState { basicFormParameters: IBasicFormParam[]; restoreDefaultValuesModalIsOpen: boolean; + modifications?: jsonpatch.Operation[]; } class DeploymentFormBody extends React.Component< @@ -64,18 +67,37 @@ class DeploymentFormBody extends React.Component< if (nextProps.selected !== this.props.selected) { // The values or the schema has changed let values = ""; + // Get the original modification to the values if exists + let modifications = this.state.modifications; + // TODO(andresmgot): Right now, are taking advantage of the fact that when first + // loaded this component (in the upgrade scenario) the "selected" version is the + // currently deployed version. We should change that to be the latest version available + // so the current approach won't be possible. We should also try to avoid to modify + // the behavior of this component depending on the scenario (install/upgrade). + if (nextProps.selected.values && this.props.deployedValues && !modifications) { + const defaultValuesObj = YAML.parse(nextProps.selected.values); + const deployedValuesObj = YAML.parse(this.props.deployedValues); + modifications = jsonpatch.compare(defaultValuesObj, deployedValuesObj); + this.setState({ modifications }); + } + if (!this.props.valuesModified) { - // If the version is the current one, reuse original params - // (this only applies to the upgrade form that has originalValues defined) - if ( - nextProps.selected.version && - nextProps.selected.version.attributes.version === this.props.chartVersion && - this.props.originalValues - ) { - values = this.props.originalValues || ""; - } else { - // In other case, use the default values for the selected version - values = nextProps.selected.values || ""; + // In other case, use the default values for the selected version + values = nextProps.selected.values || ""; + // And we add any possible change made to the original version + if (modifications) { + modifications.forEach(modification => { + // Transform the JSON Path to the format expected by setValue + // /a/b/c => a.b.c + const path = modification.path.replace(/^\//, "").replace(/\//g, "."); + if (modification.op === "remove") { + values = deleteValue(values, path); + } else { + // Transform the modification as a ReplaceOperation to read its value + const value = (modification as jsonpatch.ReplaceOperation).value; + values = setValue(values, path, value); + } + }); } this.props.setValues(values); } else { diff --git a/dashboard/src/components/UpgradeForm/UpgradeForm.tsx b/dashboard/src/components/UpgradeForm/UpgradeForm.tsx index 550d236ecd5..2edfc463fc9 100644 --- a/dashboard/src/components/UpgradeForm/UpgradeForm.tsx +++ b/dashboard/src/components/UpgradeForm/UpgradeForm.tsx @@ -60,7 +60,7 @@ class UpgradeForm extends React.Component { @@ -315,6 +315,14 @@ describe("setValue", () => { result: "foo: bar\nthis: { new: { value: 1 } }\n", error: false, }, + { + description: "Adding a value in an empty doc", + values: "", + path: "foo", + newValue: "bar", + result: "foo: bar\n", + error: false, + }, ].forEach(t => { it(t.description, () => { let res: any; @@ -328,6 +336,44 @@ describe("setValue", () => { }); }); +describe("deleteValue", () => { + [ + { + description: "should delete a value", + values: "foo: bar\nbar: foo\n", + path: "bar", + result: "foo: bar\n", + }, + { + description: "should delete a value from an array", + values: `foo: + - bar + - foobar +`, + path: "foo.0", + result: `foo: + - foobar +`, + }, + { + description: "should leave the document emtpy", + values: "foo: bar", + path: "foo", + result: "\n", + }, + { + description: "noop when trying to delete a missing property", + values: "foo: bar\nbar: foo\n", + path: "var", + result: "foo: bar\nbar: foo\n", + }, + ].forEach(t => { + it(t.description, () => { + expect(deleteValue(t.values, t.path)).toEqual(t.result); + }); + }); +}); + describe("validate", () => { [ { diff --git a/dashboard/src/shared/schema.ts b/dashboard/src/shared/schema.ts index 01f9d327042..c7d34df5a28 100644 --- a/dashboard/src/shared/schema.ts +++ b/dashboard/src/shared/schema.ts @@ -3,7 +3,7 @@ // that are used in this package import * as AJV from "ajv"; import * as jsonSchema from "json-schema"; -import { set } from "lodash"; +import { isEmpty, set } from "lodash"; import * as YAML from "yaml"; import { IBasicFormParam } from "./types"; @@ -73,9 +73,11 @@ function getDefinedPath(allElementsButTheLast: string[], doc: YAML.ast.Document) return currentPath; } -// setValue modifies the current values (text) based on a path -export function setValue(values: string, path: string, newValue: any) { - const doc = YAML.parseDocument(values); +function parsePathAndValue(doc: YAML.ast.Document, path: string, value?: any) { + if (isEmpty(doc.contents)) { + // If the doc is empty we have an special case + return { value: set({}, path, value), splittedPath: [] }; + } let splittedPath = path.split("."); // If the path is not defined (the parent nodes are undefined) // We need to change the path and the value to set to avoid accessing @@ -88,13 +90,29 @@ export function setValue(values: string, path: string, newValue: any) { if (parentNode === undefined) { const definedPath = getDefinedPath(allElementsButTheLast, doc); const remainingPath = splittedPath.slice(definedPath.length + 1); - newValue = set({}, remainingPath.join("."), newValue); + value = set({}, remainingPath.join("."), value); splittedPath = splittedPath.slice(0, definedPath.length + 1); } - (doc as any).setIn(splittedPath, newValue); + return { splittedPath, value }; +} + +// setValue modifies the current values (text) based on a path +export function setValue(values: string, path: string, newValue: any) { + const doc = YAML.parseDocument(values); + const { splittedPath, value } = parsePathAndValue(doc, path, newValue); + (doc as any).setIn(splittedPath, value); return doc.toString(); } +export function deleteValue(values: string, path: string) { + const doc = YAML.parseDocument(values); + const { splittedPath } = parsePathAndValue(doc, path); + (doc as any).deleteIn(splittedPath); + // If the document is empty after the deletion instead of returning {} + // we return an empty line "\n" + return doc.contents && !isEmpty((doc.contents as any).items) ? doc.toString() : "\n"; +} + // getValue returns the current value of an object based on YAML text and its path export function getValue(values: string, path: string, defaultValue?: any) { const doc = YAML.parseDocument(values); diff --git a/dashboard/yarn.lock b/dashboard/yarn.lock index ba00582b197..170fdd1320c 100644 --- a/dashboard/yarn.lock +++ b/dashboard/yarn.lock @@ -3905,6 +3905,11 @@ fast-glob@^2.2.6: merge2 "^1.2.3" micromatch "^3.1.10" +fast-json-patch@^3.0.0-1: + version "3.0.0-1" + resolved "https://registry.yarnpkg.com/fast-json-patch/-/fast-json-patch-3.0.0-1.tgz#4c68f2e7acfbab6d29d1719c44be51899c93dabb" + integrity sha512-6pdFb07cknxvPzCeLsFHStEy+MysPJPgZQ9LbQ/2O67unQF93SNqfdSqnPPl71YMHX+AD8gbl7iuoGFzHEdDuw== + fast-json-stable-stringify@^2.0.0: version "2.0.0" resolved "https://registry.yarnpkg.com/fast-json-stable-stringify/-/fast-json-stable-stringify-2.0.0.tgz#d5142c0caee6b1189f87d3a76111064f86c8bbf2"