Skip to content
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.

Sign up for GitHub

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

Jump to bottom

aix-explainer: update versions to fix CVEs #2364

Merged
merged 10 commits into from
Aug 6, 2022
Merged

aix-explainer: update versions to fix CVEs #2364

merged 10 commits into from
Aug 6, 2022

Conversation

MessKon
Copy link
Contributor

@MessKon MessKon commented Aug 1, 2022 •
edited
Loading

What this PR does / why we need it:
Fixing critical CVEs in latest kserve/alx-explainer version
Comparing this to latest stable and v0.9.0 kserve/aix-explainer images:
image

image

image

Which issue(s) this PR fixes (optional, in fixes #<issue number>(, fixes #<issue_number>, ...) format, will close the issue(s) when PR gets merged):
Not sure there's an open issue for this

Type of changes

  • Bug fix (non-breaking change which fixes an issue)

Feature/Issue validation/testing:

kserve/alx-explainer image builds ok with no issues

  • Logs

Special notes for your reviewer:

  1. This PR only changes image versions

Release note:

fixing vulnerabilities (CVEs) in kserve/aix-explainer

Tomcli
Tomcli reviewed Aug 1, 2022
View reviewed changes
python/aixexplainer/aixserver/model.py Outdated
@@ -17,7 +17,7 @@
import logging
import kserve
import numpy as np
from aix360.algorithms.lime import LimeImageExplainer
from aix360.algorithms.lime.lime_wrapper import LimeImageExplainer
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you don't need .lime_wrapper since lime init.py exposed LimeImageExplainer as a public package.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thank you Tommy. I just went for this as the error on the e2e test was hinting that the aix360.algorithms.lime module cannot be found. I'll revert the change

MessKon
MessKon commented Aug 2, 2022
View reviewed changes
python/aixexplainer/setup.py
@@ -33,7 +33,7 @@
install_requires=[
"kserve>=0.7.0",
"argparse >= 1.4.0",
"aix360 >= 0.1.0",
"aix360 >= 0.2.0",
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

needed so that we make sure we avoid version 0.1.0 which does not include the lime module used here.

MessKon
MessKon commented Aug 2, 2022
View reviewed changes
python/aixexplainer.Dockerfile

COPY third_party third_party

COPY kserve kserve
RUN pip install --no-cache-dir --upgrade pip && pip install --no-cache-dir -e ./kserve

RUN apt update && apt install -y build-essential
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

python:3.7-slim does not include gcc which is used fro installing few pip dependencies in aixexplainer.
we install build-essential and remove it after, as no longer needed

MessKon
MessKon commented Aug 2, 2022
View reviewed changes
python/aixexplainer.Dockerfile
@@ -1,12 +1,14 @@
FROM python:3.7
FROM python:3.7-slim
Copy link
Contributor Author

@MessKon MessKon Aug 2, 2022 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ideally we'd want to use python:3.8-slim which:
a. includes only 5 critical CVEs (according to grype)
b. includes gcc and so there's no need for apt get and apt remove in this file.

however, python:3.8 supports tensorflow >= 2.2 whilst aix360 versions 0.2.0 and 0.2.1 demand tensorflow 1.14.
in fact the extra critical CVEs reported in python:3.7-slim all come from tensorflow.
image

For now, there's no other option for fixing CVEs, unless aix360 update their dependency

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let me open an issue to the aix360 community to update the tensorflow version.

Copy link
Contributor Author

@MessKon MessKon Aug 2, 2022 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thank you Tommy!
dependabot seems to have already raised a PR for it.

@Tomcli
Copy link
Member

Tomcli commented Aug 2, 2022

/lgtm

@Tomcli Tomcli mentioned this pull request Aug 2, 2022
Open
@yuzisun
Copy link
Member

yuzisun commented Aug 6, 2022

/approve

@kserve-oss-bot
Copy link
Collaborator

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: MessKon, yuzisun

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@kserve-oss-bot kserve-oss-bot merged commit 637c441 into kserve:master Aug 6, 2022
yuzisun pushed a commit to yuzisun/kserve that referenced this pull request Aug 7, 2022
@MessKon @yuzisun
aix-explainer: update versions to fix CVEs (kserve#2364)
eb9ccc6 
* aix-explainer: update versions to fix CVEs

Signed-off-by: MessKon <[email protected]>

* fix flake8 lint errors

Signed-off-by: MessKon <[email protected]>

* update aix360 module reference

Signed-off-by: MessKon <[email protected]>

* revert aix360 module reference

Signed-off-by: MessKon <[email protected]>

* allow python version 3.6

Signed-off-by: MessKon <[email protected]>

* allow python version >3.4

Signed-off-by: MessKon <[email protected]>

* use python:3.8-slim-bullseye

Signed-off-by: MessKon <[email protected]>

* use python:3.8-slim and revert cvxpy change

Signed-off-by: MessKon <[email protected]>

* use python:3.7-slim and update aix360 requirement

Signed-off-by: MessKon <[email protected]>
Signed-off-by: Dan Sun <[email protected]>
@MessKon MessKon deleted the kmessis/aix-explainer_fix_crit_CVEs branch August 8, 2022 07:00
alexagriffith pushed a commit to alexagriffith/kserve that referenced this pull request Sep 19, 2022
@MessKon @alexagriffith
aix-explainer: update versions to fix CVEs (kserve#2364)
1b8fba4 
* aix-explainer: update versions to fix CVEs

Signed-off-by: MessKon <[email protected]>

* fix flake8 lint errors

Signed-off-by: MessKon <[email protected]>

* update aix360 module reference

Signed-off-by: MessKon <[email protected]>

* revert aix360 module reference

Signed-off-by: MessKon <[email protected]>

* allow python version 3.6

Signed-off-by: MessKon <[email protected]>

* allow python version >3.4

Signed-off-by: MessKon <[email protected]>

* use python:3.8-slim-bullseye

Signed-off-by: MessKon <[email protected]>

* use python:3.8-slim and revert cvxpy change

Signed-off-by: MessKon <[email protected]>

* use python:3.7-slim and update aix360 requirement

Signed-off-by: MessKon <[email protected]>
Signed-off-by: alexagriffith <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Tomcli Tomcli Tomcli left review comments

@Iamlovingit Iamlovingit Awaiting requested review from Iamlovingit

Assignees

@Tomcli Tomcli

Labels
approved lgtm
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@MessKon @Tomcli @yuzisun @kserve-oss-bot