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.

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(helm): add value to customise interactive session images (#795) #795

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

mdonadoni
Copy link
Member

Copy link

codecov bot commented Apr 16, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 18.15%. Comparing base (b4f4633) to head (96df221).

Additional details and impacted files

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #795   +/-   ##
=======================================
  Coverage   18.15%   18.15%           
=======================================
  Files          26       26           
  Lines        2478     2478           
=======================================
  Hits          450      450           
  Misses       2028     2028           

environments:
jupyter:
recommended:
- image: "docker.io/jupyter/scipy-notebook:notebook-6.4.5"
Copy link
Member

Choose a reason for hiding this comment

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

It may be perhaps good to have also an alias (or a list of aliases) for an image, because currently we would allow opening of docker.io/jupyter/scipy-notebook:notebook-6.4.5 but not the opening of jupyter/scipy-notebook:notebook-6.4.5 which is essentially the same image. Motivation: people often leave out docker.io from the fully-qualified image names.

This could be an issue during the use case of opening a session from the reana-client, such as:

$ rc open -w root6-roofit-snakemake-kubernetes -i jupyter/scipy-notebook:notebook-6.4.5
==> ERROR: Interactive session could not be opened:
Custom container image jupyter/scipy-notebook:notebook-6.4.5 is not allowed.

Since the docker.io prefix is kind of unique, due to Docker history, there may not be many more examplees calling for image aliases out there... So perhaps the best solution is not to alter the YAML configuration file with aliases after all, but rather to solve this command-line use case on the application level, e.g. try to add leading docker.io/ if the image name supplied by the user does not start with it, when trying to match the user-supplied value against the list of available images.

Copy link
Member Author

Choose a reason for hiding this comment

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

Done in reana-workflow-controller, the alias prefixes I have considered are:

  • docker.io/
  • docker.io/library/
  • library/

Copy link
Member Author

Choose a reason for hiding this comment

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

This means that ubuntu:24.04, library/ubuntu:24.04, docker.io/library/ubuntu:24.04 are all going to work well

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

interactive-sessions: implement configurable allowlist
2 participants