Skip to content

ROX-25797: Central Scanner v4 communication #12448

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

Merged
Maddosaurus merged 19 commits into from
Oct 9, 2024
Merged

ROX-25797: Central Scanner v4 communication #12448

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
19 commits
Select commit Hold shift + click to select a range
3eec6b1
Prefactor: Move scanner/mappers to pkg
Maddosaurus Aug 7, 2024
0591fc7
Squash previously tracked parent branches
Maddosaurus Aug 9, 2024
0f7af0b
Point to CC fork for fixed scanners
Maddosaurus Aug 13, 2024
1794013
Housekeeping, WIP code
Maddosaurus Aug 15, 2024
49effb4
Scanner v4 is called on NodeIndex
Maddosaurus Sep 2, 2024
5d3d70f
Fix tests
Maddosaurus Sep 26, 2024
02edb48
Clean up and remove rebase artifacts
Maddosaurus Sep 26, 2024
3aff078
Clean up unneeded code, refine logs
Maddosaurus Sep 26, 2024
05ef606
Update protolock, add a test, fix test
Maddosaurus Sep 27, 2024
7462877
Review comments, pt1
Maddosaurus Sep 30, 2024
22aaa58
Review comments, pt2
Maddosaurus Sep 30, 2024
bf489c0
Fix unit test and skip over unnecessary scanner calls
Maddosaurus Oct 1, 2024
1a5c76b
Rework loops, remove dead code
Maddosaurus Oct 1, 2024
6446bbb
Update pkg/nodes/enricher/enricher_impl.go
Maddosaurus Oct 1, 2024
c8b4712
Disable NodeInventory E2E tests on non-OpenShift environments
Maddosaurus Oct 2, 2024
f0aadf8
Move ignore annotation into test
Maddosaurus Oct 7, 2024
66a7a13
Rebase and regen proto code
Maddosaurus Oct 7, 2024
268826e
Remove skip condition from NodeInventoryTest
Maddosaurus Oct 7, 2024
0626984
Ensure enricher only skips scanners on specific circumstances
Maddosaurus Oct 8, 2024
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
40 changes: 28 additions & 12 deletions central/enrichment/integration.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,21 +42,31 @@ func isNodeIntegration(integration *storage.ImageIntegration) bool {
return false
}

// imageIntegrationToNodeIntegration converts the given image integration into a node integration.
// Currently, only StackRox Scanner is a supported node integration.
// Assumes integration.GetCategories() includes storage.ImageIntegrationCategory_NODE.
func imageIntegrationToNodeIntegration(integration *storage.ImageIntegration) (*storage.NodeIntegration, error) {
if integration.GetType() != scannerTypes.Clairify {
return nil, errors.Errorf("requires a %s config: %q", scannerTypes.Clairify, integration.GetName())
}
return &storage.NodeIntegration{
// ImageIntegrationToNodeIntegration converts the given image integration into a node integration.
// Currently, only StackRox Scanner and Scanner v4 are supported node integrations.
// Assumes integration.GetCategories() includes storage.ImageIntegrationCategory_NODE_SCANNER.
func ImageIntegrationToNodeIntegration(integration *storage.ImageIntegration) (*storage.NodeIntegration, error) {
i := &storage.NodeIntegration{
Copy link
Copy Markdown
Contributor

@RTann RTann Oct 28, 2024

Choose a reason for hiding this comment

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

we should do this after the switch. No need to allocate space for this when we may just exit early in the default

Id: integration.GetId(),
Name: integration.GetName(),
Type: integration.GetType(),
IntegrationConfig: &storage.NodeIntegration_Clairify{
}

switch integration.GetType() {
Copy link
Copy Markdown
Contributor

@RTann RTann Oct 28, 2024

Choose a reason for hiding this comment

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

It's probably (hopefully) unlikely, but I guess it's possible GetType and the type of the IntegrationConfig may not match. I'm thinking we can revert back to the check before (check for the config's type rather than this string type). We can do something like:

switch integration.GetIntegrationConfig().(type) {
case *v4.ImageIntegration_Clairify:
case *v4.ImageIntegration_ScannerV4:
}

case scannerTypes.ScannerV4:
i.IntegrationConfig = &storage.NodeIntegration_Scannerv4{
Scannerv4: integration.GetScannerV4(),
}
case scannerTypes.Clairify:
i.IntegrationConfig = &storage.NodeIntegration_Clairify{
Clairify: integration.GetClairify(),
},
}, nil
}
default:
return nil, errors.Errorf("unsupported integration type: %q.", integration.GetType())
}
log.Debugf("Created Node Integration %s / %s from Image integration", i.GetName(), i.GetType())

return i, nil
}

func imageIntegrationToOrchestratorIntegration(integration *storage.ImageIntegration) (*storage.OrchestratorIntegration, error) {
Expand All @@ -83,7 +93,8 @@ func (m *managerImpl) Upsert(integration *storage.ImageIntegration) error {
m.cveFetcher.RemoveIntegration(integration.GetId())
return nil
}
nodeIntegration, err := imageIntegrationToNodeIntegration(integration)
log.Debugf("Converting Integration to Node: %s / %s", integration.GetName(), integration.GetType())
nodeIntegration, err := ImageIntegrationToNodeIntegration(integration)
if err != nil {
return err
}
Expand All @@ -92,6 +103,11 @@ func (m *managerImpl) Upsert(integration *storage.ImageIntegration) error {
return err
}

if integration.GetType() == scannerTypes.ScannerV4 {
log.Debugf("Scanner v4 is not an orchestrator Scanner, exiting")
Copy link
Copy Markdown
Contributor

@RTann RTann Oct 28, 2024

Choose a reason for hiding this comment

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

Scanner V4

return nil
}

orchestratorIntegration, err := imageIntegrationToOrchestratorIntegration(integration)
if err != nil {
return err
Expand Down
97 changes: 97 additions & 0 deletions central/enrichment/integration_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,97 @@
package enrichment

import (
"fmt"
"testing"

"github.com/stackrox/rox/generated/storage"
"github.com/stackrox/rox/pkg/protoassert"
scannerTypes "github.com/stackrox/rox/pkg/scanners/types"
"github.com/stretchr/testify/assert"
)

func Test_ImageIntegrationToNodeIntegration(t *testing.T) {
cases := map[string]struct {
in *storage.ImageIntegration
expected *storage.NodeIntegration
expectedErrorMsg string
}{
"Valid v2": {
in: &storage.ImageIntegration{
Id: "169b0d3f-8277-4900-bbce-1127077defae",
Name: "Stackrox Scanner",
Type: scannerTypes.Clairify,
Categories: []storage.ImageIntegrationCategory{
storage.ImageIntegrationCategory_SCANNER,
storage.ImageIntegrationCategory_NODE_SCANNER,
},
IntegrationConfig: &storage.ImageIntegration_Clairify{
Clairify: &storage.ClairifyConfig{
Endpoint: "https://localhost:8080",
},
},
},
expected: &storage.NodeIntegration{
Id: "169b0d3f-8277-4900-bbce-1127077defae",
Name: "Stackrox Scanner",
Type: scannerTypes.Clairify,
IntegrationConfig: &storage.NodeIntegration_Clairify{
Clairify: &storage.ClairifyConfig{
Endpoint: "https://localhost:8080",
},
},
},
expectedErrorMsg: "",
},
"Valid v4": {
in: &storage.ImageIntegration{
Id: "a87471e6-9678-4e66-8348-91e302b6de07",
Name: "Scanner V4",
Type: scannerTypes.ScannerV4,
Categories: []storage.ImageIntegrationCategory{
storage.ImageIntegrationCategory_SCANNER,
storage.ImageIntegrationCategory_NODE_SCANNER,
},
IntegrationConfig: &storage.ImageIntegration_ScannerV4{
ScannerV4: &storage.ScannerV4Config{
IndexerEndpoint: "https://localhost:8443",
MatcherEndpoint: "https://localhost:9443",
},
},
},
expected: &storage.NodeIntegration{
Id: "a87471e6-9678-4e66-8348-91e302b6de07",
Name: "Scanner V4",
Type: scannerTypes.ScannerV4,
IntegrationConfig: &storage.NodeIntegration_Scannerv4{
Scannerv4: &storage.ScannerV4Config{
IndexerEndpoint: "https://localhost:8443",
MatcherEndpoint: "https://localhost:9443",
},
},
},
expectedErrorMsg: "",
},
"Invalid Scanner Type": {
in: &storage.ImageIntegration{
Id: "a87471e6-0000-0000-0000-91e302b6de07",
Name: "Quay",
Type: scannerTypes.Quay,
},
expectedErrorMsg: fmt.Sprintf("unsupported integration type: %q.", scannerTypes.Quay),
},
}

for name, c := range cases {
t.Run(name, func(t *testing.T) {
actual, err := ImageIntegrationToNodeIntegration(c.in)

if c.expectedErrorMsg != "" {
assert.ErrorContains(t, err, c.expectedErrorMsg)
} else {
protoassert.Equal(t, c.expected, actual)
assert.NoError(t, err)
}
})
}
}
1 change: 1 addition & 0 deletions central/enrichment/singleton.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,7 @@ func initializeManager() {

imageIntegrationStore = imageIntegrationDS.Singleton()
integrations, err := imageIntegrationStore.GetImageIntegrations(ctx, &v1.GetImageIntegrationsRequest{})
log.Debugf("Found %d configured image integrations", len(integrations))
if err != nil {
log.Errorf("unable to use previous integrations: %s", err)
return
Expand Down
24 changes: 0 additions & 24 deletions central/imageintegration/service/convert.go
View file
Open in desktop

This file was deleted.

2 changes: 1 addition & 1 deletion central/imageintegration/service/service_impl.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -271,7 +271,7 @@ func (s *serviceImpl) testImageIntegration(request *storage.ImageIntegration) er
}
}
if category == storage.ImageIntegrationCategory_NODE_SCANNER {
nodeIntegration, err := imageIntegrationToNodeIntegration(request)
nodeIntegration, err := enrichment.ImageIntegrationToNodeIntegration(request)
if err != nil {
return errors.Wrap(errox.InvalidArgs, errors.Wrap(err, "node scanner integration").Error())
}
Expand Down
6 changes: 5 additions & 1 deletion central/imageintegration/service/service_impl_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,13 +14,15 @@ import (
connMocks "github.com/stackrox/rox/central/sensor/service/connection/mocks"
v1 "github.com/stackrox/rox/generated/api/v1"
"github.com/stackrox/rox/generated/internalapi/central"
v4 "github.com/stackrox/rox/generated/internalapi/scanner/v4"
"github.com/stackrox/rox/generated/storage"
"github.com/stackrox/rox/pkg/errox"
nodeMocks "github.com/stackrox/rox/pkg/nodes/enricher/mocks"
"github.com/stackrox/rox/pkg/protoassert"
"github.com/stackrox/rox/pkg/sac"
scannerMocks "github.com/stackrox/rox/pkg/scanners/mocks"
"github.com/stackrox/rox/pkg/scanners/types"
scannerTypes "github.com/stackrox/rox/pkg/scanners/types"
"github.com/stackrox/rox/pkg/secrets"
"github.com/stretchr/testify/assert"
"go.uber.org/mock/gomock"
Expand Down Expand Up @@ -75,7 +77,7 @@ func (*fakeNodeScanner) GetNodeScan(*storage.Node) (*storage.NodeScan, error) {
panic("implement me")
}

func (*fakeNodeScanner) GetNodeInventoryScan(_ *storage.Node, _ *storage.NodeInventory) (*storage.NodeScan, error) {
func (*fakeNodeScanner) GetNodeInventoryScan(_ *storage.Node, _ *storage.NodeInventory, _ *v4.IndexReport) (*storage.NodeScan, error) {
panic("implement me")
}

Expand Down Expand Up @@ -300,13 +302,15 @@ func TestValidateNodeIntegration(t *testing.T) {
clairifyIntegrationConfig := &storage.ImageIntegration{
Id: "id",
Name: "name",
Type: scannerTypes.Clairify,
IntegrationConfig: &storage.ImageIntegration_Clairify{Clairify: clairifyConfig},
Categories: []storage.ImageIntegrationCategory{storage.ImageIntegrationCategory_SCANNER, storage.ImageIntegrationCategory_NODE_SCANNER},
SkipTestIntegration: true,
}
clairifyNodeIntegrationConfig := &storage.NodeIntegration{
Id: "id",
Name: "name",
Type: scannerTypes.Clairify,
IntegrationConfig: &storage.NodeIntegration_Clairify{Clairify: clairifyConfig},
}

Expand Down
6 changes: 6 additions & 0 deletions central/imageintegration/store/defaults.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import (
registryTypes "github.com/stackrox/rox/pkg/registries/types"
"github.com/stackrox/rox/pkg/scanners"
"github.com/stackrox/rox/pkg/scanners/clairify"
"github.com/stackrox/rox/pkg/scanners/scannerv4"
scannerTypes "github.com/stackrox/rox/pkg/scanners/types"
)

Expand Down Expand Up @@ -121,6 +122,7 @@ var DefaultScannerV4Integration = &storage.ImageIntegration{
Type: scannerTypes.ScannerV4,
Categories: []storage.ImageIntegrationCategory{
storage.ImageIntegrationCategory_SCANNER,
storage.ImageIntegrationCategory_NODE_SCANNER,
},
IntegrationConfig: &storage.ImageIntegration_ScannerV4{
ScannerV4: &storage.ScannerV4Config{
Expand Down Expand Up @@ -171,5 +173,9 @@ var (
_, creator := clairify.Creator(nil)
return creator
}),
makeDelayedIntegration(DefaultScannerV4Integration, func() scanners.Creator {
_, creator := scannerv4.Creator(nil)
return creator
}),
}
)
32 changes: 30 additions & 2 deletions central/sensor/service/pipeline/nodeindex/pipeline.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -66,9 +66,37 @@ func (p pipelineImpl) Run(ctx context.Context, clusterID string, msg *central.Ms
log.Errorf("index report from node %s has unsupported action: %q", event.GetNode().GetName(), event.GetAction())
return nil
}
log.Debugf("received node index report for node %s with %d packages from %d content sets",
event.GetId(), len(report.GetContents().Packages), len(report.GetContents().Repositories))
log.Debugf("received node index report with %d packages from %d content sets for node %s",
len(report.GetContents().Packages), len(report.GetContents().Repositories), event.GetId())
cr := report.CloneVT()

// Read the node from the database, if not found we fail.
node, found, err := p.nodeDatastore.GetNode(ctx, event.GetId())
Copy link
Copy Markdown
Contributor

@RTann RTann Oct 28, 2024

Choose a reason for hiding this comment

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

the node's ID is the same as the event ID? Is that allowed? Is it possible to set the Index Report's "HashID" to the node's ID instead?

if err != nil {
return errors.WithMessagef(err, "fetching node: %s", event.GetId())
}
if !found {
return errors.WithMessagef(err, "node does not exist: %s", event.GetId())
}

// Send the Node and Index Report to Scanner for enrichment
err = p.enricher.EnrichNodeWithInventory(node, nil, cr)
Copy link
Copy Markdown
Contributor

@RTann RTann Oct 28, 2024

Choose a reason for hiding this comment

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

Perhaps for a followup: I'm thinking a separate function. We seem to replace Inventory with Index, so I'm thinking we should have a separate Index function, too. If underneath it stays the same, that's ok, as at least the interface would look a bit cleaner

if err != nil {
return errors.WithMessagef(err, "enriching node %s with index report", event.GetId())
}
log.Infof("Successfully enriched node %s with index report.", node.GetName())

// TODO(ROX-26089): Update the whole node in the database with the new and previous information after conversion
/*
err = p.riskManager.CalculateRiskAndUpsertNode(node)
if err != nil {
log.Error(err)
return err
}
*/

return nil

}

func (p pipelineImpl) Reconcile(_ context.Context, _ string, _ *reconciliation.StoreMap) error {
Expand Down
2 changes: 1 addition & 1 deletion central/sensor/service/pipeline/nodeinventory/pipeline.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,7 @@ func (p *pipelineImpl) Run(ctx context.Context, _ string, msg *central.MsgFromSe
}

// Call Scanner to enrich the node inventory and attach the results to the node object.
err = p.enricher.EnrichNodeWithInventory(node, ninv)
err = p.enricher.EnrichNodeWithInventory(node, ninv, nil)
if err != nil {
log.Errorf("enriching node %s: %v", nodeDatastore.NodeString(node), err)
return errors.WithMessagef(err, "enrinching node %s", nodeDatastore.NodeString(node))
Expand Down
4 changes: 2 additions & 2 deletions central/sensor/service/pipeline/nodeinventory/pipeline_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@ func Test_pipelineImpl_Run(t *testing.T) {
a.injector = &recordingInjector{}
gomock.InOrder(
m.nodeDatastore.EXPECT().GetNode(gomock.Any(), gomock.Eq(node.GetId())).Times(1).Return(&node, true, nil),
m.enricher.EXPECT().EnrichNodeWithInventory(gomock.Any(), gomock.Any()).Times(1).Return(nil),
m.enricher.EXPECT().EnrichNodeWithInventory(gomock.Any(), gomock.Any(), nil).Times(1).Return(nil),
m.riskManager.EXPECT().CalculateRiskAndUpsertNode(gomock.Any()).Times(1).Return(nil),
)
},
Expand All @@ -93,7 +93,7 @@ func Test_pipelineImpl_Run(t *testing.T) {
a.injector = nil
gomock.InOrder(
m.nodeDatastore.EXPECT().GetNode(gomock.Any(), gomock.Eq(node.GetId())).Times(1).Return(&node, true, nil),
m.enricher.EXPECT().EnrichNodeWithInventory(gomock.Any(), gomock.Any()).Times(1).Return(nil),
m.enricher.EXPECT().EnrichNodeWithInventory(gomock.Any(), gomock.Any(), nil).Times(1).Return(nil),
m.riskManager.EXPECT().CalculateRiskAndUpsertNode(gomock.Any()).Times(1).Return(nil),
)
},
Expand Down
5 changes: 3 additions & 2 deletions central/sensor/service/pipeline/nodeinventory/two_pipelines_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ import (
"github.com/stackrox/rox/central/sensor/service/pipeline"
"github.com/stackrox/rox/central/sensor/service/pipeline/nodes"
"github.com/stackrox/rox/generated/internalapi/central"
v4 "github.com/stackrox/rox/generated/internalapi/scanner/v4"
"github.com/stackrox/rox/generated/storage"
"github.com/stackrox/rox/pkg/metrics"
nodeEnricher "github.com/stackrox/rox/pkg/nodes/enricher"
Expand Down Expand Up @@ -210,7 +211,7 @@ func Test_TwoPipelines_Run(t *testing.T) {
return &fakeNodeScanner{}, nil
}
}
tt.enricher = nodeEnricher.NewWithCreator(tt.mocks.cveDatastore, metrics.CentralSubsystem, creator)
tt.enricher = nodeEnricher.NewWithCreator(tt.mocks.cveDatastore, metrics.CentralSubsystem, creator, creator)
err := tt.enricher.UpsertNodeIntegration(&storage.NodeIntegration{
Id: "1",
Name: "dummy-scanner",
Expand Down Expand Up @@ -346,7 +347,7 @@ func (f *fakeNodeScanner) GetNodeScan(*storage.Node) (*storage.NodeScan, error)
return nodeScanFixtureWithKernel("v1"), nil
}

func (f *fakeNodeScanner) GetNodeInventoryScan(*storage.Node, *storage.NodeInventory) (*storage.NodeScan, error) {
func (f *fakeNodeScanner) GetNodeInventoryScan(*storage.Node, *storage.NodeInventory, *v4.IndexReport) (*storage.NodeScan, error) {
f.requestedScan = true
return nodeScanFixtureWithKernel("v2"), nil
}
Expand Down
Loading