Skip to content

Conversation

nozmore
Copy link
Collaborator

@nozmore nozmore commented Sep 29, 2021

…ased annotations to control class, updated threatlib, enabled json output and updated existing tests.

…ased annotations to control class, updated threatlib, enabled json output and updated existing tests.
@nozmore nozmore requested a review from izar October 13, 2021 02:03
@nozmore
Copy link
Collaborator Author

nozmore commented Oct 13, 2021

I still need some comments on this one. Its still draft but is functional, I think the only thing left is to remove the commented out control annotations on the various Element subclasses.

I had previously started some discussion on Slack but didn't get a thumbs up or down.

I wanted to have some separation between controls. I think there are still room for documentation so this could allow us to focus on documenting element-based annotations ("what the thing is") then focus on the controls. Having them here we can document them once rather than some annotations are on multiple Element subclasses.

I was debating if:

  1. we should have a single Controls class or have multiple subclasses.
    I think the single class is good enough for now and we could easily have a follow on PR to separate them out to lock things down or bring clarity. I could see differences between Dataflow vs Server/Process/Lambda. The only place I would see different controls is for a future PR I plan to submit for Data. I want to define Data once then have a DataUsage object on an Element. As an example one Data control would be isValidated instead of validatesInput on an Element. With this change I want to be able to produce a diagram to see how a specific piece of data flows thru the system and where controls are in place, possibly have ties to Classification requirements.

  2. are all the attributes pulled out actually meant to be controls, there were some I didn't know what the original intent was and they were not used in the threatlib.

  3. likewise is there any controls I missed.

@ghost
Copy link

ghost commented Oct 30, 2021

CodeSee Review Map:

Review these changes using an interactive CodeSee Map

Review in an interactive map

View more CodeSee Maps

Legend

CodeSee Map Legend

@nozmore nozmore marked this pull request as ready for review October 30, 2021 04:23
@izar izar merged commit 755318b into master Oct 31, 2021
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