This repository has been archived by the owner on Jan 4, 2022. It is now read-only.
Add error handling to utility functions, remove assumptions about host #301
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…t path. Most error checking was missing from pkg/bootstrap/node.go. Added to all utility functions. In addition, pkg/bootstrap/node.go made assumptions about host-path for binaries (machinectl/qemu-img/mount etc). Remove this. If the binaries are not on the PATH, fail. This is the reason for Unix PATH, to indicate where to find executables, hard-coding fallbacks can create a security issue or astonishment. The code had a hard-coded assumption that the machine had selinux available (e.g. it assumped selinux is enforcing/not-enforcing) where there is a 3rd-option (not available). If selinux is not available (e.g. Ubuntu-derived hosts), then return false from isSELinuxEnforcing().
dongsupark
reviewed
Aug 22, 2018
| log.Println("Warning: overlayfs not found, docker would not run.") | ||
| log.Println("loading overlay module... ") | ||
| // This is an incorrect assumption. It depends on the docker | ||
| // storage driver |
There was a problem hiding this comment.
In this function, code looks effectively similar as before.
But I'm not sure I understand this comment.
Are you saying that we should check for docker storage driver in the future?
In that case, we should create another github issue, or add TODO to the comment.
There was a problem hiding this comment.
Thanks for adding the issue.
dongsupark
reviewed
Aug 22, 2018
dongsupark
approved these changes
Aug 22, 2018
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Most error checking was missing from pkg/bootstrap/node.go.
Added to all utility functions.
In addition, pkg/bootstrap/node.go made assumptions about
host-path for binaries (machinectl/qemu-img/mount etc). Remove this.
If the binaries are not on the PATH, fail. This is the reason
for Unix PATH, to indicate where to find executables, hard-coding
fallbacks can create a security issue or astonishment.
The code had a hard-coded assumption that the machine had
selinux available (e.g. it assumed selinux is enforcing/not-enforcing)
where there is a 3rd-option (not available). If selinux is not
available (e.g. Ubuntu-derived hosts), then return false
from isSELinuxEnforcing().