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: adds initial implementation for the fedramp-transform command #27

Closed
wants to merge 6 commits into from

Conversation

jpower432
Copy link
Member

@jpower432 jpower432 commented Feb 12, 2024

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • My code follows the code style of this project.
  • My change requires a change to the documentation. (compliance-trestle PR)
  • I have updated the documentation accordingly. . (compliance-trestle PR)
  • I have added tests to cover my changes.
  • All new and existing tests passed.
  • All commits are signed-off.

Summary

Blocked by #30

Partially addresses #20

This creates the fedramp-transform command and the initial classes.
This only populates the control origination values.

The description, implementation status, and parameter values will be on follow-on PRs.

Key links:

Signed-off-by: Jennifer Power <barnabei.jennifer@gmail.com>
Initial population is control origination in the
control summary table

Signed-off-by: Jennifer Power <barnabei.jennifer@gmail.com>
Signed-off-by: Jennifer Power <barnabei.jennifer@gmail.com>
CUST_CONFIGURED = 'customer-configured'
CUST_PROVIDED = 'customer-provided'
INHERITED = 'inherited'

Copy link
Collaborator

Choose a reason for hiding this comment

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

Should these short names be in a const file? Also are these defined by FedRAMP or us?

Copy link
Member Author

@jpower432 jpower432 Feb 15, 2024

Choose a reason for hiding this comment

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

These are defined by FedRAMP here. I went back and forth over whether I should define them here or parse the xml file. I ended up defining them since the values seem to be very stable, but I can also see how that would not scale as easily as we add more FedRAMP specific values. Interested in your thoughts since fedramp resources are already being copied over.

Copy link
Member Author

Choose a reason for hiding this comment

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

Also wanted to add that the control origination values in the xml do not cover the hybrid or shared values that are check boxes on the template, but the FedRAMP SSP template shows examples with multiple control origination values. So the logic on handling multiple control origination values and how they translate to hybrid and shared is defined here.

Signed-off-by: Jennifer Power <barnabei.jennifer@gmail.com>
Signed-off-by: Jennifer Power <barnabei.jennifer@gmail.com>
@jpower432 jpower432 marked this pull request as draft February 19, 2024 16:54
@jpower432
Copy link
Member Author

jpower432 commented Feb 19, 2024

@vikas-agarwal76 Putting this back into draft to address feedback I received around which check boxes are marked when properties are present (i.e. supporting multiple boxes being checked instead of a single box for each control)

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

@vikas-agarwal76 I addressed your comment about putting the short-labels in the const.py. I also added support for multiple boxes to be checked at a time to better align with real word use cases.

@jpower432
Copy link
Member Author

Closing in favor of #39

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.

2 participants