Open bact opened 6 days ago
@jspeed-meyers @goneall This is still a work in progress, the actual functionality of checking the compliance of CISA Framing Software Component Transparency, Third Edition is not ready but I would like to have your comments about the refactoring, particularly about class/subclassing, naming of parameters and things, and any potential break of backward compatibility.
May be it would be nice as well if I can try if current code pass the CI checks on GitHub. Will put this to draft afterwards.
Thank you.
Thanks @bact
Overall structure LGTM - but my expertise is not Python, so take my feedback with a grain of salt.
I just approved the workflows - so they are running now.
I haven't had a chance to review in detail - I'll try to look in more detail over the next week or so
Thank you Gary. Not need to rush, I still working on this. I will turn this to draft.
This looks great to me. Wow! I like it. I don't have anything to add at the moment. If the current tests are passing, then I'm a happy reviewer.
@bact: I begin paternity leave later today. If you want a review sometime today, please say something. I notice the build isn't passing, at the time of me writing this message, so I thought it might be best for me to hold off on a review.
@jspeed-meyers I found a bug in my TagValue test data and I think it may take some time for me to figure that out. Trying to work on this as much as possible this afternoon but please don't worry about this. This can be waited.
Please enjoy your leave :)
@jspeed-meyers I think I have found the bug in my test data. The boolean values in Tag/Value format are case sensitive and have to be exactly "true" or "false" (and not "True", "False", etc). So that make the test of concluded licenses failed at an irrelevant point. Fixed now. All tests are passed.
If you have time today and feel like to review please kindly go ahead. If not, we still have a lot of time after you come back.
Happy new year.
Thank you. Yes, it is a very large change and addition. Maybe it's better to take more time to review.
I guess we can advertise that the addition of FSCTv3 is experimental, not guaranteed its completeness yet, and open for wider review.
So this PR main contribution should be about restructuring the codebase to allow more compliance checkers in the future. And the priority is to keep the behavior of the NTIA minimum checker.
Improvements can be made further to refine the completeness of FSCTv3 checker. And at some point we can advertise that it is supported.
I'm not particular sure about the use of the word "comply" (as used in the option "--comply"), "compliance", "conformance", "standard", "document", "specification", etc. that I may put inside the printing message/code comment, and also the README (#226), whether they have a specific/different meaning or preference within this project.
Please flag any misuse or inconsistency. Thank you.
I'm not particular sure about the use of the word "comply" (as used in the option "--comply"), "compliance", "conformance", "standard", "document", "specification", etc. that I may put inside the printing message/code comment, and also the README (#226), whether they have a specific/different meaning or preference within this project.
Please flag any misuse or inconsistency. Thank you.
I'm not the best to comment on the best term - I've been in meetings where participants expressed strong opinions, but I'm personally OK comply and compliance. In SPDX, we settled on "conformance" for the profile compliance term. Based on this one datapoint, conformance may be a less controversial choice. I think that is an OMG term if I recall correctly. I don't know if @kestewart has any feedback from the framing document work in terms of a preferred term.
Objective
To add the compliance check of the Common SBOM Baseline Attributes, as outlined in "Framing Software Component Transparency, Third Edition" https://www.cisa.gov/resources-tools/resources/framing-software-component-transparency-2024
To resolve #214
What this PR do?
SbomChecker
class:NTIAChecker
BaseChecker
SbomChecker
now acts as a "dispatcher" or a factory that returns a subclass ofBaseChecker
based on the given "compliance" argument during instantiation.NTIAChecker
to maintain backward compatibilitySbomChecker
is expected to behave the same.BaseChecker
contains common methods likeparse_file
print_components_missing_info
andoutput_json
. These methods' details are specific to compliance standards and have to be implemented in subclasses.NTIAChecker
, a subclass ofBaseChecker
FSCT3Checker
, a subclass ofBaseChecker
, contains functionalities specific to Framing Software Component Transparency, Third Edition check--comply
parameter:main.py
to add a new parameter for choosing a compliance standard to check. It has two choices "fsct3-min" and "ntia". Default value is "ntia" to maintain backward compatibility.sbomcheck
command for CLI:sbomcheck
to theproject.scripts
section inpyproject.toml
sbomcheck
to the machinebin/
directory and allows the user to callsbomcheck
from command line.prog="ntia-checker",
is removed frommain.py
. This allows theArgumentParser
to determine the actual program name.Please see the design document https://docs.google.com/document/d/1pueRxlxoM9n1eG9g6AihjLvybEBTd77m22mRYBQltpg/edit?usp=sharing for reference.
Test