From 9ae711a64fe5936bd061893c34bf74ebf67d61e2 Mon Sep 17 00:00:00 2001 From: Rohan Vazarkar Date: Tue, 5 Mar 2024 11:41:37 -0500 Subject: [PATCH] Fix ingest decoder bug (#462) * fix: allocate struct properly in decoders to prevent re-use of underlying map * test: add tests for property assertions --- .../api/v2/file_uploads_integration_test.go | 3 ++ cmd/api/src/api/v2/integration/ingest.go | 10 ++++++ cmd/api/src/daemons/datapipe/decoders.go | 6 ++-- .../test/fixtures/fixtures/expected_ingest.go | 31 +++++++++++++++++++ 4 files changed, 47 insertions(+), 3 deletions(-) diff --git a/cmd/api/src/api/v2/file_uploads_integration_test.go b/cmd/api/src/api/v2/file_uploads_integration_test.go index 7c75fe44b..a51edf9fe 100644 --- a/cmd/api/src/api/v2/file_uploads_integration_test.go +++ b/cmd/api/src/api/v2/file_uploads_integration_test.go @@ -179,6 +179,7 @@ func Test_FileUploadWorkFlowVersion6(t *testing.T) { //Assert that we created stuff we expected testCtx.AssertIngest(fixtures.IngestAssertions) testCtx.AssertIngest(fixtures.IngestAssertionsv6) + testCtx.AssertIngest(fixtures.PropertyAssertions) } func Test_FileUploadVersion6AllOptionADCS(t *testing.T) { @@ -219,6 +220,7 @@ func Test_CompressedFileUploadWorkFlowVersion5(t *testing.T) { //Assert that we created stuff we expected testCtx.AssertIngest(fixtures.IngestAssertions) + testCtx.AssertIngest(fixtures.PropertyAssertions) } func Test_CompressedFileUploadWorkFlowVersion6(t *testing.T) { @@ -239,4 +241,5 @@ func Test_CompressedFileUploadWorkFlowVersion6(t *testing.T) { //Assert that we created stuff we expected testCtx.AssertIngest(fixtures.IngestAssertions) testCtx.AssertIngest(fixtures.IngestAssertionsv6) + testCtx.AssertIngest(fixtures.PropertyAssertions) } diff --git a/cmd/api/src/api/v2/integration/ingest.go b/cmd/api/src/api/v2/integration/ingest.go index 833d77e17..651e696d0 100644 --- a/cmd/api/src/api/v2/integration/ingest.go +++ b/cmd/api/src/api/v2/integration/ingest.go @@ -148,3 +148,13 @@ func (s *Context) AssertIngest(assertion IngestAssertion) { return nil }), "Unexpected database error during reconciliation assertion") } + +func (s *Context) AssertIngestProperties(assertion IngestAssertion) { + graphDB := integration.OpenGraphDB(s.TestCtrl) + defer graphDB.Close(s.ctx) + + require.Nil(s.TestCtrl, graphDB.ReadTransaction(s.ctx, func(tx graph.Transaction) error { + assertion(s.TestCtrl, tx) + return nil + }), "Unexpected database error during reconciliation assertion") +} diff --git a/cmd/api/src/daemons/datapipe/decoders.go b/cmd/api/src/daemons/datapipe/decoders.go index 546656f27..887259148 100644 --- a/cmd/api/src/daemons/datapipe/decoders.go +++ b/cmd/api/src/daemons/datapipe/decoders.go @@ -73,11 +73,11 @@ func decodeGroupData(batch graph.Batch, reader io.ReadSeeker) error { var ( convertedData = ConvertedGroupData{} - group ein.Group count = 0 ) for decoder.More() { + var group ein.Group if err := decoder.Decode(&group); err != nil { log.Errorf("Error decoding group object: %v", err) } else { @@ -106,10 +106,10 @@ func decodeSessionData(batch graph.Batch, reader io.ReadSeeker) error { var ( convertedData = ConvertedSessionData{} - session ein.Session count = 0 ) for decoder.More() { + var session ein.Session if err := decoder.Decode(&session); err != nil { log.Errorf("Error decoding session object: %v", err) } else { @@ -138,11 +138,11 @@ func decodeAzureData(batch graph.Batch, reader io.ReadSeeker) error { var ( convertedData = ConvertedAzureData{} - data AzureBase count = 0 ) for decoder.More() { + var data AzureBase if err := decoder.Decode(&data); err != nil { log.Errorf("Error decoding azure object: %v", err) } else { diff --git a/cmd/api/src/test/fixtures/fixtures/expected_ingest.go b/cmd/api/src/test/fixtures/fixtures/expected_ingest.go index 427811009..02e4cb34f 100644 --- a/cmd/api/src/test/fixtures/fixtures/expected_ingest.go +++ b/cmd/api/src/test/fixtures/fixtures/expected_ingest.go @@ -28,6 +28,12 @@ import ( "github.com/stretchr/testify/require" ) +type PropertyAssertion struct { + ObjectID string + Property string + ExpectedValue any +} + var ( ingestRelationshipAssertionCriteria = []graph.Criteria{ //// DOMAINS @@ -252,6 +258,19 @@ var ( query.Kind(query.End(), ad.Domain), query.Equals(query.EndProperty(common.ObjectID.String()), "S-1-5-21-3130019616-2776909439-2417379446")), } + + propertyAssertionCriteria = []PropertyAssertion{ + { + ObjectID: "TESTLAB.LOCAL-S-1-5-32-550", + Property: common.Name.String(), + ExpectedValue: "PRINT OPERATORS@TESTLAB.LOCAL", + }, + { + ObjectID: "S-1-5-21-3130019616-2776909439-2417379446-521", + Property: common.Name.String(), + ExpectedValue: "READ-ONLY DOMAIN CONTROLLERS@TESTLAB.LOCAL", + }, + } ) func FormatQueryComponent(criteria graph.Criteria) string { @@ -280,3 +299,15 @@ func IngestAssertionsv6(testCtrl test.Controller, tx graph.Transaction) { require.Nilf(testCtrl, err, "Unable to find an expected relationship: %s", FormatQueryComponent(assertionCriteria)) } } + +func PropertyAssertions(testCtrl test.Controller, tx graph.Transaction) { + for _, assertionCriteria := range propertyAssertionCriteria { + node, err := tx.Nodes().Filterf(func() graph.Criteria { + return query.Equals(query.NodeProperty(common.ObjectID.String()), assertionCriteria.ObjectID) + }).First() + require.Nilf(testCtrl, err, "Unable to find expected node %s", assertionCriteria.ObjectID) + prop := node.Properties.Get(assertionCriteria.Property).Any() + require.Equal(testCtrl, assertionCriteria.ExpectedValue, prop) + require.Nilf(testCtrl, err, "Unable to find an expected relationship: %s", FormatQueryComponent(assertionCriteria)) + } +}