Skip to content

ROX-13722: Move NodeInventory into Node #3757

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.

Sign up for GitHub

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

Closed
vikin91 wants to merge 2 commits into from
Closed

ROX-13722: Move NodeInventory into Node #3757

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
3ea8b25
Proto: Move NodeInventory into Node
vikin91 Nov 3, 2022
96bb013
Regenerate graphql resolvers
vikin91 Jan 17, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
359 changes: 359 additions & 0 deletions central/graphql/resolvers/generated.go
View file
Open in desktop

Large diffs are not rendered by default.

8 changes: 5 additions & 3 deletions central/node/datastore/dackbox/datastore/datastore_sac_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -262,9 +262,11 @@ func (s *nodeDatastoreSACSuite) TestGetNode() {
s.True(found)
s.NotNil(fetchedNode)

// Priority can have updated value, and we want to ignore it.
fetchedNode.Priority = s.testNodes[nodeID].Priority
s.Equal(*s.testNodes[nodeID], *fetchedNode)
if fetchedNode != nil {
Copy link
Copy Markdown
Contributor Author

@vikin91 vikin91 Dec 7, 2022

Choose a reason for hiding this comment

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

This prevents potential panic if fetchedNode is nil.

Copy link
Copy Markdown
Contributor

@rhybrillou rhybrillou Dec 23, 2022

Choose a reason for hiding this comment

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

Good catch, thanks.
I suppose an alternative is to have s.Require().NotNil(fetchedNode) two lines above, but I prefer this variant.

Copy link
Copy Markdown
Contributor

@Maddosaurus Maddosaurus Jan 9, 2023

Choose a reason for hiding this comment

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

For me to understand: s.NotNil() will not stop execution of the test when the subject is nil, correct?

Copy link
Copy Markdown
Contributor Author

@vikin91 vikin91 Jan 9, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

Correct, NotNil will do the assertion, print a message and then continue. The continuation in this case would lead to a panic.
It is not so bad as this is only a test code that would panic, but in this case it was an clear bug.

// Priority can have updated value, and we want to ignore it.
fetchedNode.Priority = s.testNodes[nodeID].Priority
s.Equal(*s.testNodes[nodeID], *fetchedNode)
}
} else {
s.False(found)
s.Nil(fetchedNode)
Expand Down
4 changes: 0 additions & 4 deletions central/sensor/service/pipeline/all/factory.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@ import (
"github.com/stackrox/rox/central/sensor/service/pipeline/networkflowupdate"
"github.com/stackrox/rox/central/sensor/service/pipeline/networkpolicies"
"github.com/stackrox/rox/central/sensor/service/pipeline/nodes"
"github.com/stackrox/rox/central/sensor/service/pipeline/nodescansv2"
"github.com/stackrox/rox/central/sensor/service/pipeline/podevents"
"github.com/stackrox/rox/central/sensor/service/pipeline/processindicators"
"github.com/stackrox/rox/central/sensor/service/pipeline/reprocessing"
Expand Down Expand Up @@ -65,9 +64,6 @@ func (s *factoryImpl) PipelineForCluster(ctx context.Context, clusterID string)
alerts.GetPipeline(),
auditlogstateupdate.GetPipeline(),
}
if features.RHCOSNodeScanning.Enabled() {
pipelines = append(pipelines, nodescansv2.GetPipeline())
}
if features.ComplianceOperatorCheckResults.Enabled() {
pipelines = append(pipelines,
complianceoperatorresults.GetPipeline(),
Expand Down
57 changes: 0 additions & 57 deletions central/sensor/service/pipeline/nodescansv2/pipeline.go
View file
Open in desktop

This file was deleted.

3 changes: 3 additions & 0 deletions compliance/collection/nodeinventorizer/nodeinventory.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -112,6 +112,9 @@ func convertAndDedupRHELComponents(rc *database.RHELv2Components) []*storage.Nod
}

func convertExecutables(exe []*scannerV1.Executable) []*storage.NodeInventory_Components_RHELComponent_Executable {
Copy link
Copy Markdown
Contributor Author

@vikin91 vikin91 Jan 12, 2023

Choose a reason for hiding this comment

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

Make sure to use getters here to avoid nils

if exe == nil {
return nil
}
arr := make([]*storage.NodeInventory_Components_RHELComponent_Executable, len(exe))
for i, executable := range exe {
arr[i] = &storage.NodeInventory_Components_RHELComponent_Executable{
Expand Down
117 changes: 116 additions & 1 deletion generated/api/v1/node_service.swagger.json
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading