Skip to content
This repository was archived by the owner on May 6, 2020. It is now read-only.

os: Add a default shell to mini-os#37

Closed
jcvenegas wants to merge 1 commit into
clearcontainers:masterfrom
jcvenegas:default-shell
Closed

os: Add a default shell to mini-os#37
jcvenegas wants to merge 1 commit into
clearcontainers:masterfrom
jcvenegas:default-shell

Conversation

@jcvenegas
Copy link
Copy Markdown
Contributor

Need to provide a shell for vm debuging.

Fixes: #36

Signed-off-by: Jose Carlos Venegas Munoz jose.carlos.venegas.munoz@intel.com

Comment thread scripts/osbuilder.sh
DNF="$DNF --config=$DNF_CONF -y --installroot=${ROOTFS_DIR} --noplugins"
$DNF install systemd hyperstart cc-oci-runtime-extras coreutils-bin \
systemd-bootchart iptables-bin clear-containers-agent ${EXTRA_PKGS}
systemd-bootchart iptables-bin clear-containers-agent bash-bin ${EXTRA_PKGS}
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

does the agent need bash ?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

hey @devimc, it does not. It a request to allow to debug the vm.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

my vote that if this is only for debug the developer build their own image (with this tool) with bash included, and we do not need to ship with bash for security risks.

Documentation will fix that IMHO

Copy link
Copy Markdown

@mcastelino mcastelino Oct 6, 2017

Choose a reason for hiding this comment

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

@devimc @gorozco1 @jodh-intel

How is the security risk different between us including as shell and the user of the container who normally already has access to a shell as part of the container workload. If there is a material difference between the two then it makes a difference. If not we are just sacrificing usability.

So the question is, how is this shell different from the shell available in the workload. Both are root.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

@mcastelino well, if we provide bash this is the vm rootfs, the shell that is provided with the workload is in another layer, limited within the container - imho - correct me if I am wrong.

Need to provide a shell for vm debuging.

Fixes: clearcontainers#36

Signed-off-by: Jose Carlos Venegas Munoz <jose.carlos.venegas.munoz@intel.com>
@jodh-intel
Copy link
Copy Markdown

Since this tool creates custom images for development/debugging, I think giving the ability to add a shell is reasonable in most scenarios. However, this might not be required for some users and does bloat the image beyond the current minimal config.

As such, why not add an option / env var to also have a shell included? Alternatively, we could just document in make help that if you want a shell, set EXTRA_PKGS="bash-bin"?

@jodh-intel
Copy link
Copy Markdown

Any update on this?

@mcastelino
Copy link
Copy Markdown

@jcvenegas @jodh-intel @sameo @grahamwhaley can we close on this.
IMO we should either include a shell in the default distribution. Or we should include two images (a debug and a release image). I am ok with either approach. But asking the user to build the image just to debug is not the right decision.

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants