Skip to content
This repository has been archived by the owner on Jul 10, 2019. It is now read-only.

[checker] Checker bin, JSON config file, new Check interface #121

Open
wants to merge 6 commits into
base: master
Choose a base branch
from

Conversation

kawmarco
Copy link

@kawmarco kawmarco commented May 10, 2019

  • Added checker command line entry point (checker/main.go) which accepts a JSON configuration file as input (--config-file), containing a list of Checks
  • Adapted Check structs to be more friendly to JSON configuration and JSON report output, thinking of the following workflow
    • Developers define CheckFun(CheckArgs) functions, which contain the actual logic of the check, and "register" their functions using registerCheckFun on init() (so that they are registered in a global map and we can then find the check functions by name at runtime)
    • A JSON-friendly Check struct defines how CheckFuns should be called, including CheckArgs, possible remediations and metadata. The config file is a checklist (a list of Check)
    • Calling Check.Run() returns a JSON-friendly CheckResult, containing the result of the Check and the result of the configured remediations

TODO:

  • Document pkg/checker/checker.go
  • Proper error/exit value on invalid/insufficient arguments (maybe just support one positional argument)?
  • Ensure checker binary is in the ramdisk

@insomniacslk
Copy link
Collaborator

hey @kawmarco , thanks for the contribution! Could you please re-submit with DCO (just amend the commit with -s), and address the linter warnings?
You may want to run golangci-lint run and fix any other warning before re-submitting (it will be caught by the CI anyway)

@kawmarco kawmarco force-pushed the checker branch 6 times, most recently from 0cea717 to cd6541a Compare May 10, 2019 15:13
@codecov-io
Copy link

codecov-io commented May 10, 2019

Codecov Report

Merging #121 into master will decrease coverage by 3.18%.
The diff coverage is 45.45%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #121      +/-   ##
==========================================
- Coverage   50.17%   46.99%   -3.19%     
==========================================
  Files          14       21       +7     
  Lines         562      798     +236     
==========================================
+ Hits          282      375      +93     
- Misses        252      391     +139     
- Partials       28       32       +4
Impacted Files Coverage Δ
pkg/checker/commandexecutor.go 10% <10%> (ø)
pkg/checker/interfaces.go 9.41% <12.5%> (ø)
pkg/checker/checkerrepo.go 70.58% <70.58%> (ø)
cmds/checker/main.go 70.96% <70.96%> (ø)
pkg/checker/checker.go 88.88% <88.88%> (ø)
pkg/checker/dmesg.go 0% <0%> (ø)
pkg/checker/colours.go 57.14% <0%> (ø)
... and 2 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update b5f8835...619c414. Read the comment docs.

@kawmarco kawmarco force-pushed the checker branch 4 times, most recently from 3d2ca90 to 34d9ed4 Compare May 13, 2019 16:21
@kawmarco kawmarco changed the title [WIP][checker] Checker bin, JSON config file, new Check interface [checker] Checker bin, JSON config file, new Check interface May 13, 2019
Copy link
Collaborator

@insomniacslk insomniacslk left a comment

Choose a reason for hiding this comment

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

I've done only a partial review, publishing it to unblock you.

Also note - several files have changed permissions with this commit, they have now an executable bit set where they should not

README.md Outdated Show resolved Hide resolved
checker/main.go Outdated Show resolved Hide resolved
checker/main.go Outdated Show resolved Hide resolved
checker/main.go Outdated Show resolved Hide resolved
checker/main.go Outdated Show resolved Hide resolved
checker/main.go Outdated Show resolved Hide resolved
checker/main.go Outdated Show resolved Hide resolved
checker/main.go Outdated Show resolved Hide resolved
checker/main.go Outdated Show resolved Hide resolved
pkg/checker/checker.go Outdated Show resolved Hide resolved
kawmarco pushed a commit to kawmarco/systemboot that referenced this pull request May 29, 2019
kawmarco pushed a commit to kawmarco/systemboot that referenced this pull request May 29, 2019
kawmarco pushed a commit to kawmarco/systemboot that referenced this pull request May 29, 2019
kawmarco pushed a commit to kawmarco/systemboot that referenced this pull request May 29, 2019
@tfg13
Copy link
Collaborator

tfg13 commented Jun 7, 2019

Discussed offline, we will reopen this one against uroot after the move is done

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

Successfully merging this pull request may close these issues.

None yet

4 participants