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

Add interfaces for workspace, other yamls. #212

Closed
wants to merge 27 commits into from
Closed

Conversation

jcscottiii
Copy link
Member

@jcscottiii jcscottiii commented Aug 1, 2016

To enable inter-operability between different versions of different things like standards and certifications (opencontrol.yaml and component.yaml already has it) and easy mocking, this patch introduces the use of more interfaces. Also, this patch introduces workspace interface which enables for external plugins to easily test by just using the interface to mock instead of worrying about fixtures.

Major Additions:

  • Added example plugin, plugin tests with mocks and README.
  • Added interfaces for Certifications, Control, Standard, Workspace to the common package
  • Moved interfaces Component, Satisfies, and Section to the common package
  • Added importable mocks for Component, Satisfies, Certifications, Control, Section, Standard, Workspace
  • Moved Standard to it's own package for future versioning and loading by versions.
    • A lot of plumbing is still needed here like adding a special UnmarshalYAML but the whole point of moving it was to move all yaml files into their own package, out of the base lib package.
  • Moved Certification to it's own package for future versioning and loading by versions.
    • A lot of plumbing is still needed here like adding a special UnmarshalYAML but the whole point of moving it was to move all yaml files into their own package, out of the base lib package

Other changes:

  • Removed the use of callbacks
    • This will help with mocking
  • Made the maps Standards and Components and their methods private.
    • This makes it more clear for people who import the lib package, which things they can use. If something is private, they won't see it.
  • Fixed a lot of golint problems
    • When I moved files around, the old golint problems became new again so I had to fix them.

Actual logic changes:

  • For versions 2.0.0, and 3.0.0 of component, GetImplementationStatuses returned an empty array. It should return the implementation status if it's there.
    • Can probably be moved out of this PR but it's a small change compared to the rest.

For future PR:

  • Add verifications to common package.
    • This will allow for workspace to also be added to common without creating a circular dependency chain.

Excerpt from lib/README.md:

Lib Package

This package (and the sub packages) are the only packages that should be imported by external projects. This lib package is very helpful for those writing plugins to extend the functionality of Masonry.

Workspace

Workspace is the starting point for all the information available to plugins. For those using Go plugins, using the interface methods are all you need. It represents all the information collected into the opencontrols folder after running the get command.

YAML files

The yaml files are represented by:

  • Components
  • Certifications
  • Standards

Result Data

Justifications is a data structure that is not represented in yaml but rather a post-processed map of data to help quickly getting component data for a particular control name - standard name combination.

Plugin Developer Guide

Developers should not have to worry about loading real data / workspaces for their unit tests (they should for integration tests).

There is an example of developing your Go plugin and tests in exampleplugin/example.go and exampleplugin/example_test.go respectively.

@codecov-io
Copy link

codecov-io commented Aug 2, 2016

Current coverage is 88.40% (diff: 82.83%)

Merging #212 into master will decrease coverage by 4.16%

@@             master       #212   diff @@
==========================================
  Files            26         31     +5   
  Lines           888        923    +35   
  Methods           0          0          
  Messages          0          0          
  Branches          0          0          
==========================================
- Hits            822        816     -6   
- Misses           52         91    +39   
- Partials         14         16     +2   

Powered by Codecov. Last update a0746d3...11c8c70

@jcscottiii
Copy link
Member Author

@afeld yeppp.. I did it again. this is a mega elephant patch. A lot of it is moving files around though. Details above. This PR was one of those that started off one way then turned 90 degrees to something else so it was hard to break it up. Now that it's done, I can do that now. Let me know and I'll start to break this up later. Not a huge priority now so I'll get to it later probably.

@jcscottiii jcscottiii changed the title Add interfaces for workspace, other yamls. [WIP] Add interfaces for workspace, other yamls. Aug 3, 2016
import (
"testing"
"github.com/opencontrol/compliance-masonry/lib/mocks"
commonMocks"github.com/opencontrol/compliance-masonry/lib/common/mocks"
Copy link
Member

Choose a reason for hiding this comment

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

Needs a space (right?)

Copy link
Member Author

Choose a reason for hiding this comment

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

whoops! will fix

@afeld
Copy link
Member

afeld commented Aug 11, 2016

A few pieces of general feedback:

  • Wow, yeah, this was massive...let's not do that again 😬
  • Love the example plugin and its documentation! How about we split its addition out to its own pull request?
  • Not sure that I understand the common package...maybe it could have a more descriptive name?

Sorry for the slow turnaround on the review, by the way!

For versions 2.0.0, and 3.0.0 of component, GetImplementationStatuses returned an empty array. It should return the implementation status if it's there. Can probably be moved out of this PR

If it's not hard, that would be good!

@jcscottiii
Copy link
Member Author

jcscottiii commented Aug 11, 2016

Not sure that I understand the common package...maybe it could have a more descriptive name?

Yeah, I agree. There are some things like Component and Standard there so it can read as common.Component and the Workspace is supposed to be able to return these common Components and common Standards (regardless of which version it is; 3.0.0 vs. 3.1.0) but there were some existing things like Entry that don't belong in the same package. Will look into moving out some of that out and add documentation for that.

@afeld
Copy link
Member

afeld commented Aug 19, 2016

@jcscottiii Are you planning to rebase this one (sooner or later) or should we just close it?

@afeld
Copy link
Member

afeld commented Oct 3, 2016

Bump!

@jcscottiii
Copy link
Member Author

Let's close this. The other recent PRs have been just parts of this one.
The only PRs yet to be made are 1) generate the mocks and 2) example plugin.

On Mon, Oct 3, 2016, 5:50 PM Aidan Feldman [email protected] wrote:

Bump!


You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#212 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/AHbZgrvrYvbFio05ytQd3m24ibrjwk5Kks5qwXiLgaJpZM4JZmxL
.

@afeld afeld closed this Oct 4, 2016
@afeld afeld deleted the workspace-pt2 branch October 4, 2016 03:03
@jcscottiii
Copy link
Member Author

Split out to #213 #214 #215 #216 #217 #226 #227 #228

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.

3 participants