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

fix: add check for empty label to fix failure for statement with no label property #1507

Merged
merged 4 commits into from
Feb 8, 2024

Conversation

jpower432
Copy link
Collaborator

@jpower432 jpower432 commented Feb 5, 2024

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

Updates Catalog Interface get_statement_part_id_map to check if the label is empty. If the label is empty and label_as_key is True, the statement is not added to the map.

This may be only a partial fix because any statement that does not have a label property would be skipped over here.

Test added to verify the fix

Alternatives

Some alternatives fixes I tried.

  • If a label does not existing for a statement part, set it to the part id. This broke several unit tests.
  • Changing the the convention that an empty label means adding it to the implemented requirement, but this also broke several unit tests.

Fixes #1502

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.

jpower432 and others added 4 commits February 5, 2024 09:44
Signed-off-by: Jennifer Power <barnabei.jennifer@gmail.com>
Signed-off-by: Jennifer Power <barnabei.jennifer@gmail.com>
Signed-off-by: Lou DeGenaro <lou.degenaro@gmail.com>
Signed-off-by: Lou DeGenaro <lou.degenaro@gmail.com>
@jpower432 jpower432 marked this pull request as ready for review February 6, 2024 15:19
Copy link
Collaborator

@vikas-agarwal76 vikas-agarwal76 left a comment

Choose a reason for hiding this comment

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

LGTM

@jpower432 jpower432 merged commit 55ed462 into develop Feb 8, 2024
16 checks passed
@jpower432 jpower432 deleted the bug/fedramp_profile_ssp_error branch February 8, 2024 14:29
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.

Internal failure when using ssp-assemble with the FedRAMP Rev 5 High profile
3 participants