Skip to content

Conversation

@kevin-bates
Copy link
Member

@kevin-bates kevin-bates commented May 11, 2022

This pull request adds an additional GitHub action named "Validate Image Environment" which runs for each supported version of Python and validates the requirements-elyra.txt and requirements-elyra-py37.txt (in this PR) files that consists of the package dependencies used in the generic component containers can be successfully installed relative to the corresponding version of Python.
This also modifies the validate-runtime-image target to validate the elyra sample runtime images against these requirements

Resolves: #2719

Developer's Certificate of Origin 1.1

   By making a contribution to this project, I certify that:

   (a) The contribution was created in whole or in part by me and I
       have the right to submit it under the Apache License 2.0; or

   (b) The contribution is based upon previous work that, to the best
       of my knowledge, is covered under an appropriate open source
       license and I have the right under that license to submit that
       work with modifications, whether created in whole or in part
       by me, under the same open source license (unless I am
       permitted to submit under a different license), as indicated
       in the file; or

   (c) The contribution was provided directly to me by some other
       person who certified (a), (b) or (c) and I have not modified
       it.

   (d) I understand and agree that this project and the contribution
       are public and that a record of the contribution (including all
       personal information I submit with it, including my sign-off) is
       maintained indefinitely and may be redistributed consistent with
       this project or the open source license(s) involved.

@elyra-bot
Copy link

elyra-bot bot commented May 11, 2022

Thanks for making a pull request to Elyra!

To try out this branch on binder, follow this link: Binder

@kevin-bates kevin-bates marked this pull request as draft May 11, 2022 15:58
@ptitzler ptitzler added the kind:CI Impacts continuous integration (build, test, etc.) label May 11, 2022
@akchinSTC
Copy link
Member

Ive added second version of the requirements file for ipython <8 (e.g. py37 support)for generic components, which is essentially the same as the original with after reverting the updates to it. This would mean vulnerabilities that we attempted to close in #2491 -> #2487 would exist for this version of the file.

The original requirements file has been updated with bumps for nbclient for the original issue, and a bump for pyzmq due to failures in testing with python 3.10 env and the conda runtime image due to missing or incompatible native compilation libraries (gcc).

Follow up PR will include operator changes pull in both requirements files, bootstrapper would change to detect which version is running and pull in the appropriate version of the requirements file.

@akchinSTC akchinSTC marked this pull request as ready for review May 12, 2022 17:55
@kevin-bates
Copy link
Member Author

I figured only the versions with known issues with 3.7 would be adjusted, like the version of ipython and any related dependencies stemming from that.

This would mean vulnerabilities that we attempted to close in #2491 -> #2487 would exist for this version of the file.

This seems like an issue.

If we wanted to bring things up to date relative to 3.7, I ran make PYTHON_VERSION=3.7 elyra-image-env after (temporarily) removing the ==<version> portion from the etc/generic/requirements-elyra-py37.txt entries so that the most current versions applicable to 3.7 are installed. I also removed ipython-genutils completely, knowing that work has been done to remove it and, if still referenced via a transitive dependency, it would be present in the output. Then ran pip freeze and got the following. (Note that ipython-genutils is not included and believe it could be removed from both files.)

pip freeze
ansiwrap==0.8.4
appnope==0.1.3
attrs==21.4.0
backcall==0.2.0
beautifulsoup4==4.11.1
bleach==5.0.0
certifi==2021.10.8
charset-normalizer==2.0.12
click==8.1.3
debugpy==1.6.0
decorator==5.1.1
defusedxml==0.7.1
entrypoints==0.4
fastjsonschema==2.15.3
idna==3.3
importlib-metadata==4.11.3
importlib-resources==5.7.1
ipykernel==6.13.0
ipython==7.33.0
jedi==0.18.1
Jinja2==3.1.2
jsonschema==4.5.1
jupyter-client==7.3.1
jupyter-core==4.10.0
jupyterlab-pygments==0.2.2
MarkupSafe==2.1.1
matplotlib-inline==0.1.3
minio==7.1.8
mistune==0.8.4
nbclient==0.6.3
nbconvert==6.5.0
nbformat==5.4.0
nest-asyncio==1.5.5
packaging==21.3
pandocfilters==1.5.0
papermill==2.3.4
parso==0.8.3
pexpect==4.8.0
pickleshare==0.7.5
prompt-toolkit==3.0.29
psutil==5.9.0
ptyprocess==0.7.0
Pygments==2.12.0
pyparsing==3.0.9
pyrsistent==0.18.1
python-dateutil==2.8.2
PyYAML==6.0
pyzmq==22.3.0
requests==2.27.1
six==1.16.0
soupsieve==2.3.2.post1
tenacity==8.0.1
textwrap3==0.9.2
tinycss2==1.1.1
tornado==6.1
tqdm==4.64.0
traitlets==5.2.0
typing_extensions==4.2.0
urllib3==1.26.9
wcwidth==0.2.5
webencodings==0.5.1
zipp==3.8.0

Is what is listed address the vulnerabilities comment?

We could also be more direct and only update the problematic packages. The above, however, shows what 3.7 could support.

@akchinSTC
Copy link
Member

Is what is listed address the vulnerabilities comment?

Yes looks like those versions would resolve the vulnerabilities. Updated the requirements file to match.

Pro-active...should we also do the same for the 3.8+ version and bump the associated requirements?

@kevin-bates
Copy link
Member Author

Pro-active...should we also do the same for the 3.8+ version and bump the associated requirements?

This seems reasonable provided we can re-test things. It would be nice to avoid introducing per-version requirements for 3.8+, so perhaps using the versions found against 3.8 will be sufficient (and until we learn otherwise).

Also, in visiting each of the capped packages in setup.py, it looks like we'd be violating the caps for jsonschema, pyyaml, and typing-extensions. These caps are due to KFP and I'm not sure if those same caps should be in play for the image requirements, but wanted to point that out. Should these also apply to the image? Since we're not using the KFP API within the container (at least that I'm aware), I'm not sure the caps are necessary.

@akchinSTC
Copy link
Member

Since we're not using the KFP API within the container (at least that I'm aware), I'm not sure the caps are necessary.

Pretty sure they dont apply. We dont use any kfp libraries to my knowledge in the generic component execution

@akchinSTC akchinSTC changed the title Validate image environment Validate runtime image environments in CI testing May 13, 2022
@akchinSTC akchinSTC force-pushed the validate-image-env branch from abca4bd to 7264a8f Compare May 13, 2022 16:45
Co-authored-by: Patrick Titzler <[email protected]>
Copy link
Member

@ptitzler ptitzler left a comment

Choose a reason for hiding this comment

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

LGTM

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

kind:CI Impacts continuous integration (build, test, etc.)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Elyra pipelines failing with version conflicts even with system owned images

3 participants