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

feat: add profile-inherit command #1392

Merged
merged 12 commits into from
Jun 16, 2023

Conversation

jpower432
Copy link
Collaborator

@jpower432 jpower432 commented May 25, 2023

Types of changes

  • Hot fix (emergency fix and release)
  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Documentation (change which affects the documentation site)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Release (develop -> main)

Quality assurance (all should be covered).

  • My code follows the code style of this project.
  • Documentation for my change is up to date?
  • My PR meets testing requirements.
  • All new and existing tests passed.
  • All commits are signed-off.

Summary

This PR adds a new subcommand for author called profile-inherit. This initializes and seeds a profile with information from a leveraged SSP and parent parent. The with-ids field is populate with a control delta from the profile and inherited controls.

Closes #1388

Key links:

Before you merge

  • Ensure it is a 'squash commit' if not a release.
  • Ensure CI is currently passing
  • Check sonar. If you are working for a fork a maintainer will reach out, if required.

Signed-off-by: Jennifer Power <barnabei.jennifer@gmail.com>
Adds profile-seed as author subcommand
Adds profile-seed unit test
Adds SSP testdata

Closes oscal-compass#1388

Signed-off-by: Jennifer Power <barnabei.jennifer@gmail.com>
Signed-off-by: Jennifer Power <barnabei.jennifer@gmail.com>
Adds additional test case to check for ids output
when all controls are filtered out

Signed-off-by: Jennifer Power <barnabei.jennifer@gmail.com>
Signed-off-by: Jennifer Power <barnabei.jennifer@gmail.com>
…ed command

Signed-off-by: Jennifer Power <barnabei.jennifer@gmail.com>
@jpower432 jpower432 changed the title feat: add profile-seed command feat: add profile-crm command May 30, 2023
Signed-off-by: Jennifer Power <barnabei.jennifer@gmail.com>
Signed-off-by: Jennifer Power <barnabei.jennifer@gmail.com>
@jpower432 jpower432 changed the title feat: add profile-crm command feat: add profile-inherit command May 31, 2023
@jpower432
Copy link
Collaborator Author

cc @afflom

@jpower432 jpower432 marked this pull request as ready for review May 31, 2023 23:14
assert len(result_prof.imports[0].exclude_controls[0].with_ids) == 1
assert result_prof.imports[0].exclude_controls[0].with_ids[0] == excluded

# Test with a profile that has more controls that the ssp
Copy link
Collaborator

Choose a reason for hiding this comment

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

than* typo

assert len(result_prof.imports[0].exclude_controls[0].with_ids) == 1
assert result_prof.imports[0].exclude_controls[0].with_ids[0] == excluded

# Test with a profile that has less controls that the ssp
Copy link
Collaborator

Choose a reason for hiding this comment

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

than* typo


# Create dictionary containing all by-components by control for faster searching
for implemented_requirement in leveraged_ssp.control_implementation.implemented_requirements:
components: List[ssp.ByComponent] = []
Copy link
Collaborator

Choose a reason for hiding this comment

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

Pls change variable name to by_components to be more meaningful. I agree that in the end are components, but for readability purposes I´d prefer to call this variable as it is

if control_id not in components_by_id:
continue

by_comps = components_by_id[control_id]
Copy link
Collaborator

Choose a reason for hiding this comment

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

where´s the by_comps declaration? Am I missing it somewhere?


include_with_ids: Set[prof.withId] = catalog_control_ids - exclude_with_ids

include_controls = [prof.SelectControlById(with_ids=sorted(include_with_ids))]
Copy link
Collaborator

@AleJo2995 AleJo2995 Jun 15, 2023

Choose a reason for hiding this comment

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

where´s include_controls declaration? As we try to keep strongly typed variables, please declare variables with its type when possible :)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Thanks @AleJo2995! Ended up removing the intermediary variable and just set the value on the import. Please let me know if this adequately addresses your feedback.

include_with_ids: Set[prof.withId] = catalog_control_ids - exclude_with_ids

include_controls = [prof.SelectControlById(with_ids=sorted(include_with_ids))]
exclude_controls = [prof.SelectControlById(with_ids=sorted(exclude_with_ids))]
Copy link
Collaborator

@AleJo2995 AleJo2995 Jun 15, 2023

Choose a reason for hiding this comment

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

where´s exclude_controls declaration? As we try to keep strongly typed variables, please declare variables with its type when possible :)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Same.

@AleJo2995
Copy link
Collaborator

AleJo2995 commented Jun 15, 2023

@jpower432 I´ve added some feedback regarding style than functionality. I have tested functionality and all seems correct to me. I´ll make a second pass today to it and provide more feedback if needed. If not, we´re all set :)

Signed-off-by: Jennifer Power <barnabei.jennifer@gmail.com>
Copy link
Collaborator

@AleJo2995 AleJo2995 left a comment

Choose a reason for hiding this comment

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

LGTM

@AleJo2995
Copy link
Collaborator

@jpower432 you can go ahead and squash and merge whenever you want :)

@jpower432 jpower432 merged commit 3bd53ff into oscal-compass:develop Jun 16, 2023
16 checks passed
@AleJo2995 AleJo2995 mentioned this pull request Jun 27, 2023
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.

Support profile initialization based on leveraged SSP and parent profile inputs
2 participants