Refactor resources - save SBOM on disk#45
Conversation
Signed-off-by: rcohen <rcohen@armosec.io>
Signed-off-by: rcohen <rcohen@armosec.io>
|
You have successfully added a new CodeQL configuration |
|
Summary:
|
dwertent
left a comment
dwertent
left a comment
There was a problem hiding this comment.
You made changes in 6 different files but did not change any unit tests.
This means the code is missing many unit tests.
Since the proposed changes involve re-filtering the SBOM', and as the PR indicates that we are "refactoring", I expect to see unit tests.
| func init() { | ||
| wd, err := os.Getwd() | ||
| if err != nil { | ||
| logger.L().Ctx(context.GetBackgroundContext()).Error("failed to get working directory", helpers.Error(err)) |
There was a problem hiding this comment.
I suggest this function should return an error in case of an error.
| } | ||
| spdxDataDirPath = fmt.Sprintf("%s/%s", wd, directorySBOM) | ||
| err = os.MkdirAll(spdxDataDirPath, os.ModeDir|os.ModePerm) | ||
| if err != nil { |
There was a problem hiding this comment.
What will happen in such a case? Do we want the nodeAgent to panic?
Here you just print an error without doing anything.
If the program will continue running, then it is not an error.
| instanceID instanceidhandler.IInstanceID | ||
| } | ||
|
|
||
| func init() { |
| } | ||
| } | ||
|
|
||
| func (sbom *SBOMData) saveSBOM(spdxData *spdxv1beta1.SBOMSPDXv2p3) error { |
| } | ||
|
|
||
| func CreateSBOMDataSPDXVersionV040() *SBOMData { | ||
| func CreateSBOMDataSPDXVersionV040(instanceID instanceidhandler.IInstanceID) SBOMFormat { |
| return nil | ||
| } | ||
|
|
||
| func (sbom *SBOMData) getSBOMData() (*spdxv1beta1.SBOMSPDXv2p3, error) { |
| } | ||
| } | ||
|
|
||
| func (sc *SBOMData) CleanResources() { |
| func (sc *SBOMData) CleanResources() { | ||
| err := os.Remove(sc.spdxDataPath) | ||
| if err != nil { | ||
| logger.L().Warning("fail to remove file", helpers.String("file name", sc.spdxDataPath), helpers.Error(err)) |
There was a problem hiding this comment.
Should this be a warning? When will this operation fail and why?
| func (sbom *SBOMData) FilterSBOM(sbomFileRelevantMap map[string]bool) error { | ||
| sbom.newRelevantData = false | ||
|
|
||
| spdxData, err := sbom.getSBOMData() |
There was a problem hiding this comment.
I'm confused, you named the function SBOMData, then you name the local variable spdxData.
Is the FilterSBOM a converter from one format to another?
| return &spdxData, nil | ||
| } | ||
|
|
||
| func (sbom *SBOMData) FilterSBOM(sbomFileRelevantMap map[string]bool) error { |
There was a problem hiding this comment.
This is a really important function.
Add unitests for it.
Signed-off-by: rcohen <rcohen@armosec.io>
Signed-off-by: rcohen <rcohen@armosec.io>
|
Summary:
|
3 participants
Overview