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 profile resolution when children of nested controls selected #233

Open
wants to merge 2 commits into
base: develop
Choose a base branch
from

Conversation

aj-stein-nist
Copy link
Collaborator

@aj-stein-nist aj-stein-nist commented Feb 1, 2024

Committer Notes

Closes #232.

All Submissions:

  • Have you followed the guidelines in our Contributing document?
  • Have you checked to ensure there aren't other open Pull Requests for the same update/change?
  • Have you squashed any non-relevant commits and commit messages? [instructions]
  • Do all automated CI/CD checks pass?

Changes to Core Features:

  • Have you added an explanation of what your changes do and why you'd like us to include them?
  • Have you written new tests for your core changes, as applicable?
  • Have you included examples of how to use your new feature(s)?
  • Have you updated all website and readme documentation affected by the changes you made?

@aj-stein-nist aj-stein-nist self-assigned this Feb 1, 2024
@aj-stein-nist aj-stein-nist changed the base branch from main to develop February 1, 2024 15:01
This may be a business logic error or perhaps at one time there was/is
a valid reason, but as it stands the BasicIndexer would detect
unselected controls for resolution of the profile but only drop them
when their parent was selected for the default strategy of flattening.
For simple trees where c1 is the root control and c1.1 is the child, so
c1 -> c1.1, c1 was not being correctly removed.

NOTE: Logging indicates a double index of c1.1 even with this change
so that still needs to be fixed most likely to resolve this issue.
@aj-stein-nist aj-stein-nist force-pushed the 232-fix-profile-resolution branch from 19e4ca4 to 37fcc27 Compare February 1, 2024 15:10
@aj-stein-nist
Copy link
Collaborator Author

@david-waltermire I am going to ask for a preliminary review but the fix seems pretty simple so I am worried I am misunderstand the scope of the issue. Additionally, when running the regression test, I am notice in the gov.nist.secauto.oscal.lib.profile.resolver.ProfileResolver.resolveProfile() function I get the following warning because of c1.1 control being a dupe at the handleMerge() stage because of duplicate index records.

10:39:58.383 [main] DEBUG gov.nist.secauto.oscal.lib.profile.resolver.ProfileResolver.resolveImport(ProfileResolver.java:322) - resolving profile import 'issue106-catalog.xml'
10:39:58.540 [main] ERROR gov.nist.secauto.metaschema.model.common.constraint.LoggingConstraintValidationHandler.logConstraint(LoggingConstraintValidationHandler.java:121) - ERROR: (/catalog/control[1]) Expect constraint 'prop[@name='status']/@value=('withdrawn','Withdrawn') or part[@name='statement']' did not match the data at path '/catalog/control[1]'
10:39:58.543 [main] ERROR gov.nist.secauto.metaschema.model.common.constraint.LoggingConstraintValidationHandler.logConstraint(LoggingConstraintValidationHandler.java:121) - ERROR: (/catalog/control[1]/control[1]) Expect constraint 'prop[@name='status']/@value=('withdrawn','Withdrawn') or part[@name='statement']' did not match the data at path '/catalog/control[1]/control[1]'
10:43:53.065 [main] DEBUG gov.nist.secauto.oscal.lib.profile.resolver.selection.DefaultResult.promoteControl(DefaultResult.java:103) - promoting control 'c1.1'
10:45:05.843 [main] DEBUG gov.nist.secauto.oscal.lib.profile.resolver.selection.DefaultResult.removeControl(DefaultResult.java:199) - Requesting removal of control 'c1'.
10:53:58.002 [main] DEBUG gov.nist.secauto.oscal.lib.profile.resolver.support.BasicIndexer.removeItem(BasicIndexer.java:246) - Removing CONTROL 'c1' from index.
10:55:28.634 [main] DEBUG gov.nist.secauto.oscal.lib.profile.resolver.ProfileResolver.structureFlat(ProfileResolver.java:462) - applying flat structuring directive
10:57:38.239 [main] DEBUG gov.nist.secauto.oscal.lib.profile.resolver.selection.DefaultResult.promoteControl(DefaultResult.java:103) - promoting control 'c1.1'
10:59:44.536 [main] WARN  gov.nist.secauto.oscal.lib.profile.resolver.support.BasicIndexer.addItem(BasicIndexer.java:218) - Duplicate control found with identifier c1.1 in index.

I cannot seem to track that down and I am not sure given the warning type on the log record if it is serious, but since the document should be reindex at this point, I am left scratching my head. Let me know.

@aj-stein-nist aj-stein-nist marked this pull request as ready for review February 1, 2024 16:07
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.

NPE when resolving profile selecting catalog children controls without parent
1 participant