-
Notifications
You must be signed in to change notification settings - Fork 21
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
feat(cve): add compare cves feature for 2 images #420
base: main
Are you sure you want to change the base?
Changes from all commits
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 |
---|---|---|
|
@@ -104,6 +104,19 @@ | |
}, | ||
allVulnerabilitiesForRepo: (name) => | ||
`/v2/_zot/ext/search?query={CVEListForImage(image: "${name}"){Tag Page {TotalCount ItemCount} CVEList {Id Title Description Severity Reference PackageList {Name InstalledVersion FixedVersion}}}}`, | ||
cveDiffForImages: (minuend = {}, subtrahend = {}, { pageNumber = 1, pageSize = 3 }) => { | ||
let imageInput = (img) => { | ||
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. if the value for the variable doesn't change, please use 'const' for declaration 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. maybe we will want to reuse this imageInput function in the future for other endpoints. We should declare it outside 'cveDiffForImages' |
||
let digest = img.digest ? `, digest: ${img.digest}` : ''; | ||
let platform = img.platform ? `, platform: {os: ${img.platform.os}, arch: ${img.platform.arch}}` : ''; | ||
return `{repo: "${img.repo}", tag: "${img.tag}"${digest}${platform}}`; | ||
}; | ||
let query = `/v2/_zot/ext/search?query={CVEDiffListForImages(minuend: ${imageInput( | ||
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. no need for the 'query' variable, we can just return the interpolated string directly |
||
minuend | ||
)}, subtrahend: ${imageInput(subtrahend)}, requestedPage: {limit:${pageSize} offset:${ | ||
(pageNumber - 1) * pageSize | ||
}}) {Minuend Subtrahend CVEList{Id Title Description Severity Reference PackageList {Name InstalledVersion FixedVersion}} Summary {Count UnknownCount LowCount MediumCount HighCount CriticalCount} Page {TotalCount ItemCount}}}`; | ||
return query; | ||
}, | ||
imageListWithCVEFixed: (cveId, repoName, { pageNumber = 1, pageSize = 3 }, filter = {}) => { | ||
let filterParam = ''; | ||
if (filter.Os || filter.Arch) { | ||
|
@@ -150,6 +163,11 @@ | |
const paginationParam = `requestedPage: {limit:${pageSize} offset:${(pageNumber - 1) * pageSize} sortBy:RELEVANCE}`; | ||
return `/v2/_zot/ext/search?query={GlobalSearch(${searchParam}, ${paginationParam}) {Images {RepoName Tag}}}`; | ||
}, | ||
imageSuggestionsWithPlatforms: ({ searchQuery = '""', pageNumber = 1, pageSize = 15 }) => { | ||
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. Is there a point for this endpoint? Can we not achieve this already with the 'globalSearch'? |
||
const searchParam = searchQuery !== '' ? `query:"${searchQuery}"` : `query:""`; | ||
const paginationParam = `requestedPage: {limit:${pageSize} offset:${(pageNumber - 1) * pageSize} sortBy:RELEVANCE}`; | ||
return `/v2/_zot/ext/search?query={GlobalSearch(${searchParam}, ${paginationParam}) {Images {RepoName Tag Manifests {Platform {Os Arch}}}}}`; | ||
}, | ||
referrers: ({ repo, digest, type = '' }) => | ||
`/v2/_zot/ext/search?query={Referrers(repo: "${repo}" digest: "${digest}" type: "${type}"){MediaType ArtifactType Size Digest Annotations{Key Value}}}`, | ||
bookmarkToggle: (repo) => `/v2/_zot/ext/userprefs?repo=${repo}&action=toggleBookmark`, | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,154 @@ | ||
import React, { useState, useMemo } from 'react'; | ||
|
||
// components | ||
import { Button, Modal, Typography, Stack, Box } from '@mui/material'; | ||
import makeStyles from '@mui/styles/makeStyles'; | ||
|
||
import { api, endpoints } from 'api'; | ||
import { host } from 'host'; | ||
import ImageSelector from 'components/Tag/Tabs/ImageSelector'; | ||
import VulnerabilitiyCard from 'components/Shared/VulnerabilityCard'; | ||
import { isEmpty } from 'lodash'; | ||
import { mapCVEInfo } from 'utilities/objectModels'; | ||
|
||
const useStyles = makeStyles(() => ({ | ||
imagesInputBox: { | ||
padding: '0.5rem', | ||
marginBottom: '0.5rem' | ||
}, | ||
input: { | ||
color: '#464141', | ||
'&::placeholder': { | ||
opacity: '1' | ||
} | ||
}, | ||
searchInputBase: { | ||
width: '90%', | ||
paddingLeft: '1rem', | ||
border: '1px solid black', | ||
height: 40, | ||
color: 'rgba(0, 0, 0, 0.6)' | ||
}, | ||
compareImagesPopup: { | ||
position: 'absolute', | ||
top: '50%', | ||
left: '50%', | ||
transform: 'translate(-50%, -50%)', | ||
backgroundColor: 'white', | ||
border: '2px solid #000', | ||
padding: '1rem' | ||
}, | ||
compareButton: { | ||
paddingLeft: '0.5rem' | ||
} | ||
})); | ||
|
||
function CompareImages({ name, tag, platform }) { | ||
const classes = useStyles(); | ||
const [open, setOpen] = useState(false); | ||
const [minuend, setMinuend] = useState(''); | ||
const [subtrahend, setSubtrahend] = useState(''); | ||
const [cveData, setCVEData] = useState([]); | ||
const handleOpen = () => setOpen(true); | ||
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. can just inline the handleOpen and handleClose functions if we don't want to add additional logic |
||
const handleClose = () => setOpen(false); | ||
const handleMinuendInput = (value) => { | ||
setMinuend(value); | ||
}; | ||
const handleSubtrahendInput = (value) => { | ||
setSubtrahend(value); | ||
}; | ||
const abortController = useMemo(() => new AbortController(), []); | ||
|
||
const imageCVECompare = (minuend, subtrahend) => { | ||
api | ||
.get( | ||
`${host()}${endpoints.cveDiffForImages(minuend, subtrahend, { pageNumber: 1, pageSize: 9 })}`, | ||
abortController.signal | ||
) | ||
.then((diffResponse) => { | ||
const cveListData = mapCVEInfo(diffResponse.data.data.CVEDiffListForImages.CVEList); | ||
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. we should have some checks here before passing data to mapCVEInfo, make sure the response actually contains data |
||
setCVEData(cveListData); | ||
}) | ||
.catch((e) => { | ||
console.error(e); | ||
}); | ||
}; | ||
|
||
const getImageRefComponents = (image) => { | ||
if (image.includes('@')) { | ||
let components = image.split('@'); | ||
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. prefer const over let |
||
return [components[0], '', components[1]]; | ||
} else if (image.includes(':')) { | ||
let components = image.split(':'); | ||
return [components[0], components[1], '']; | ||
} | ||
|
||
return [image, '', '']; | ||
}; | ||
|
||
const handleCompare = () => { | ||
let [minuendRepo, minuendTag] = getImageRefComponents(minuend); | ||
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. I don't think this call will achieve what we want. You are only taking the first 2 values returned by the getImageRefComponents call, so in the return-case on line 80, the components[1] value will be lost |
||
let [subtrahendRepo, subtrahendTag] = getImageRefComponents(subtrahend); | ||
imageCVECompare({ repo: minuendRepo, tag: minuendTag }, { repo: subtrahendRepo, tag: subtrahendTag }); | ||
}; | ||
|
||
const renderCVEs = () => { | ||
return !isEmpty(cveData) ? ( | ||
cveData.map((cve, index) => { | ||
return <VulnerabilitiyCard key={index} cve={cve} name={name} platform={platform} expand={true} />; | ||
}) | ||
) : ( | ||
<></> | ||
); | ||
}; | ||
|
||
return ( | ||
<div> | ||
<Button variant="outlined" onClick={handleOpen}> | ||
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. if this component is a modal popup, then control over when the modal is shown should be given to the parent component, meaning: take out the button that opens it, that should be in the parent component |
||
Compare | ||
</Button> | ||
<Modal | ||
open={open} | ||
onClose={handleClose} | ||
aria-labelledby="modal-modal-title" | ||
aria-describedby="modal-modal-description" | ||
> | ||
<Stack direction="column" className={classes.compareImagesPopup}> | ||
<Typography variant="h6" component="h2"> | ||
Compare the vulnerabilities of 2 images | ||
</Typography> | ||
<Stack direction="row" spacing={2} className={classes.imagesInputBox}> | ||
<ImageSelector | ||
setSearchCurrentValue={handleMinuendInput} | ||
name={name} | ||
tag={tag} | ||
// digest={digest} | ||
// platform={platform} | ||
/> | ||
<ImageSelector setSearchCurrentValue={handleSubtrahendInput} name={name} tag={tag} /> | ||
</Stack> | ||
<Box className={classes.compareButton}> | ||
<Button variant="outlined" onClick={handleCompare}> | ||
Compare | ||
</Button> | ||
</Box> | ||
<Box | ||
sx={{ | ||
mb: 2, | ||
display: 'flex', | ||
flexDirection: 'column', | ||
height: 300, | ||
overflow: 'hidden', | ||
overflowY: 'scroll' | ||
// justifyContent="flex-end" # DO NOT USE THIS WITH 'scroll' | ||
}} | ||
> | ||
{renderCVEs()} | ||
</Box> | ||
</Stack> | ||
</Modal> | ||
</div> | ||
); | ||
} | ||
|
||
export default CompareImages; |
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.
'minuend' and 'subtrahend' don't sound very suggestive of what the parameter represents. Is there a better name we can use perhaps?
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 struggled finding a better name, minuend means "from which you subtract" and subtrahend "how much you subtract". These are the most concise terms I know but I agree that they're quite mathematical and unfriendly
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.
Add a comment in the JS code explaining these.