Skip to content

Commit

Permalink
fix: run package bug (#1539)
Browse files Browse the repository at this point in the history
## Description:
Fixing these bugs:
* Mishandling of the internal representation of JSON objects in the
package manager form fields
* Default data type for fields is now set to JSON
* Invalid json fields were not failing the validity checker on the
field. Fixed as well.

## Is this change user facing?
YES

## References (if applicable):
Closes #1501
Closes #1479
  • Loading branch information
adschwartz authored and leoporoli committed Oct 12, 2023
1 parent 5754867 commit c0335ee
Show file tree
Hide file tree
Showing 8 changed files with 71 additions and 36 deletions.
2 changes: 1 addition & 1 deletion engine/frontend/src/App.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ import Home from './component/Home';
import { ChakraProvider } from '@chakra-ui/react'

const App = () => {
console.log("Enclave Manager version: 2023-10-05-01")
console.log("Enclave Manager version: 2023-10-12-01")
return (
<ChakraProvider>
<div className="h-screen w-screen bg-[#171923]">
Expand Down
10 changes: 3 additions & 7 deletions engine/frontend/src/component/KeyValueTable.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,13 +6,9 @@ const KeyValueTable = ({
dataCallback,
defaultState = {}
}) => {

// parse the inputs, even if it's serialized json
let parsed_json
try{
let parsed_json = {}
if (defaultState !== undefined && defaultState !== "") {
parsed_json = JSON.parse(defaultState)
} catch {
parsed_json = JSON.parse(JSON.stringify(defaultState))
}

const [value, setValue] = useState(parsed_json)
Expand Down Expand Up @@ -68,7 +64,7 @@ const KeyValueTable = ({
<InputLeftAddon children='Value'/>
<Input
type="text"
value={value || " "} // value will be undefined for new rows
value={value || ""} // value will be undefined for new rows
onChange={e => updateValue(e.target.value)}
size="md"
variant='filled'
Expand Down
79 changes: 59 additions & 20 deletions engine/frontend/src/component/PackageCatalogForm.js
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ const renderArgs = (args, handleChange, formData, errorData, packageName) => {
return
}

let dataType = "STRING";
let dataType = "JSON";
try {
switch (getType(arg)) {
case "INTEGER":
Expand Down Expand Up @@ -183,24 +183,38 @@ const renderSingleArg = (fieldName, type, errorData, formData, index, handleChan

const checkValidUndefinedType = (data) => {
try {
const val = yaml.load(data)
yaml.load(data)
} catch (ex) {
return false;
}
return true;
}

const checkValidJsonType = (data) => {
if (data === undefined || data === "undefined" || data.length === 0) {
return false
}

try {
JSON.parse(JSON.stringify(data))
return true;
} catch (ex) {
return false
const checkValidJsonType = (rawData, required) => {
// data is normally going to be a string serialized json object as it comes from the CodeEditor, so we try and process it as such
if (typeof rawData === "string") {
try {
const parsed = JSON.parse(rawData)
if (required && Object.keys(parsed).length < 1) {
return false
}
return true;
} catch (ex) {
console.error("Data is not serialized json", rawData)
return false
}
} else if (typeof rawData === "object") {
// if it's already an object then we only check that it's non-empty (if it's required)
try {
if (required && Object.keys(rawData).length < 1) {
return false
}
return true;
} catch (ex) {
return false
}
}
console.error(`Data is unknown type ${typeof rawData}`, rawData)
}

const checkValidStringType = (data) => {
Expand Down Expand Up @@ -342,7 +356,7 @@ const PackageCatalogForm = ({createEnclave, mode}) => {
let processedData = data // assumption is it's not json
try {
// serialize if it's json object
if( typeof processedData === 'object') {
if (typeof processedData === 'object') {
processedData = JSON.stringify(processedData)
}
} catch {
Expand Down Expand Up @@ -448,14 +462,15 @@ const PackageCatalogForm = ({createEnclave, mode}) => {
let subType = getFirstSubType(arg)
valid = checkValidListType(formData[key], subType)
} else if (type === "DICT") {
valid = checkValidJsonType(formData[key])
valid = checkValidJsonType(formData[key], required)
} else if (type === "JSON") {
valid = checkValidJsonType(formData[key])
// required = false, always because we have a later check that ensures the object is not null
valid = checkValidJsonType(formData[key], false)
} else {
valid = checkValidUndefinedType(formData[key])
}

let typeToPrint = type
let typeToPrint
if (type === undefined) {
typeToPrint = "JSON"
} else if (type === "BOOL") {
Expand Down Expand Up @@ -502,8 +517,8 @@ const PackageCatalogForm = ({createEnclave, mode}) => {
val = parseInt(value)
args[argName] = val
} else if (type === "BOOL") {
val = value.toUpperCase()
args[argName] = (val === "TRUE") ? true : false
val = value.trim().toUpperCase()
args[argName] = val === "TRUE"
} else if (type === "FLOAT") {
val = parseFloat(value)
args[argName] = val
Expand All @@ -513,13 +528,37 @@ const PackageCatalogForm = ({createEnclave, mode}) => {
args[argName] = val
} else if (type === "STRING") {
args[argName] = value
} else if (type === "DICT") {
if (typeof value === "string") {
try {
val = JSON.parse(value)
// Check that the object is non-empty before adding it
if (Object.keys(val).length > 0) {
args[argName] = val
}
} catch {
console.error(`Expected data to be serialized json, but parsing failed. arg=${argName}, value=${value}`)
}
} else if (typeof value === "object") {
// Check that the object is non-empty before adding it
if (Object.keys(value).length > 0) {
args[argName] = value
}
} else {
console.error(`Data field '${argName}' was not a valid object but was type ${typeof value}. Contained value: '${value}'`)
}
} else {
val = JSON.parse(value)
args[argName] = val
try {
val = JSON.parse(value)
args[argName] = val
} catch (ex) {
console.error(`Data field '${argName}' was not a valid object but was type ${typeof value}. Contained value: '${value}'`)
}
}
}
})

console.log("raw args", args)
const stringifiedArgs = JSON.stringify(args)
const runKurtosisPackageArgs = {
packageId: thisKurtosisPackage.name,
Expand Down
6 changes: 3 additions & 3 deletions engine/server/webapp/asset-manifest.json
Original file line number Diff line number Diff line change
@@ -1,13 +1,13 @@
{
"files": {
"main.css": "./static/css/main.d56b8d0f.css",
"main.js": "./static/js/main.42d08ad3.js",
"main.js": "./static/js/main.8687326d.js",
"index.html": "./index.html",
"main.d56b8d0f.css.map": "./static/css/main.d56b8d0f.css.map",
"main.42d08ad3.js.map": "./static/js/main.42d08ad3.js.map"
"main.8687326d.js.map": "./static/js/main.8687326d.js.map"
},
"entrypoints": [
"static/css/main.d56b8d0f.css",
"static/js/main.42d08ad3.js"
"static/js/main.8687326d.js"
]
}
2 changes: 1 addition & 1 deletion engine/server/webapp/index.html
Original file line number Diff line number Diff line change
@@ -1 +1 @@
<!doctype html><html lang="en"><head><meta charset="utf-8"/><meta name="viewport" content="width=device-width,initial-scale=1"/><meta name="theme-color" content="#000000"/><title>Kurtosis Enclave Manager</title><script defer="defer" src="./static/js/main.42d08ad3.js"></script><link href="./static/css/main.d56b8d0f.css" rel="stylesheet"></head><body><noscript>You need to enable JavaScript to run this app.</noscript><div id="root"></div></body></html>
<!doctype html><html lang="en"><head><meta charset="utf-8"/><meta name="viewport" content="width=device-width,initial-scale=1"/><meta name="theme-color" content="#000000"/><title>Kurtosis Enclave Manager</title><script defer="defer" src="./static/js/main.8687326d.js"></script><link href="./static/css/main.d56b8d0f.css" rel="stylesheet"></head><body><noscript>You need to enable JavaScript to run this app.</noscript><div id="root"></div></body></html>

Large diffs are not rendered by default.

Large diffs are not rendered by default.

0 comments on commit c0335ee

Please sign in to comment.