add manager type to nodemanager process node flow #3630

vjeffrey · 2020-05-11T00:30:21Z

🔩 Description: What code changed, and why?

this change adds the manager type to the nodemanager process node flow, meaning

we can assign any node which is coming through ingest service the manager "chef" since we know that node is managed by chef
any node that is automate or aws-ec2 or azure-api etc will get the type of the processnode message, meaning that we dont have to look up the type of the manager based on the id and save an extra db call

i also added a format tags bit. we have a troublesome tag example in the chef-load codebase used for testing, and it seems we fail to list nodes when this troublesome tag exists in the db, because we choke on the json (the tag value in question is Dot.Comma,Big;\"Trouble)

⛓️ Related Resources

#3515

👍 Definition of Done

manager type is on process node message
nodes managed by chef are labelled as such

👟 How to Build and Test the Change

rebuild compliance, ingest, nodemanager
load some nodes (chef_load_nodes, load_compliance_reports, chef_load_compliance_nodes)
see the manager type for the nodes is correct!

✅ Checklist

I have read the CONTRIBUTING document.
Tests added/updated?
[n/a] Docs added/updated?
All commits have been signed-off for the Developer Certification of Origin.

Aha! Link: https://chef.aha.io/features/SH-1835

lancewf · 2020-05-11T17:51:04Z

components/ingest-service/pipeline/publisher/node_manager_publisher.go

@@ -98,6 +98,7 @@ func gatherInfoForNode(msg message.ChefRun) (*manager.NodeMetadata, error) {
 			EndTime: timestamp,
 			Status:  status,
 		},
+		ManagerType: "chef",


To have the managed type of "Chef" does that include Effortless/Chef Solo Runs?

right now this code is just putting "chef" on anything coming through the ingest service -- so ya those would also get "chef" as the manager

lancewf · 2020-05-11T18:49:04Z

I have to think about this more but this test is ensuring we are only filtering on nodes with an empty Manager. Where before an empty manager meant it was a client runs node.

automate/components/nodemanager-service/tests/project_filtering_test.go

Line 380 in f8ef8ff

if node.Manager == "" {

lancewf · 2020-05-11T18:52:42Z

Here is the code where we only filter nodes that have an empty manager.

automate/components/nodemanager-service/pgdb/filtering.go

Line 279 in 099a38e

condition = condition + " OR n.manager <> ''"

lancewf · 2020-05-11T19:32:51Z

I just confirmed locally that this does break the project filtering on the nodemanager APIs for client run nodes. I wonder why the cypress tests did not fail 🤷

vjeffrey · 2020-05-11T19:44:22Z

oof im glad you thought of testing that!

we can extend the code to say empty of 'chef' manager..but :( that the test didn't fail

vjeffrey · 2020-05-11T22:30:32Z

i also need to update the ui call for scan job nodes, bc right now it is excluding nodes with empty string manager, which means all the chef nodes would show up there without further changes

Signed-off-by: Victoria Jeffrey <[email protected]>

vjeffrey · 2020-05-17T21:13:10Z

components/nodemanager-service/server/nodes/server.go

@@ -316,7 +316,7 @@ func (srv *Server) validateNodeUpdate(ctx context.Context, in *nodes.Node) error
 	switch node.Manager {
 	case "aws-api", "azure-api", "gcp-api":
 		return &errorutils.InvalidError{Msg: fmt.Sprintf("invalid option. unable to update %s node", in.Manager)}
-	case "":
+	case "", "chef":


this ensures we cannot update the name or tags of an ingested node

vjeffrey · 2020-05-17T21:13:46Z

components/nodemanager-service/pgdb/nodes.go

@@ -690,7 +690,7 @@ func validateNodeFilters(filters []*common.Filter) error {
 func isValidNodeManagerType(item string) bool {
 	switch item {
 	// empty string covers legacy nodes (nodes created before we started associating all manual nodes with "automate" as a manager and nodes brought over from a1)
-	case "aws-ec2", "aws-api", "azure-api", "azure-vm", "automate", "":
+	case "aws-ec2", "aws-api", "azure-api", "azure-vm", "automate", "gcp-api", "chef":


should've added gcp-api a long time ago!

vjeffrey · 2020-05-17T21:14:18Z

components/nodemanager-service/pgdb/node-managers-process-node.go

@@ -127,20 +127,12 @@ func (db *DB) ProcessIncomingNode(node *manager.NodeMetadata) error {
 		return errors.Wrap(err, "ProcessIncomingNode unable to marshal projects data")
 	}

-	var mgrType string
-	if node.ManagerId != "" {


now that the type is included in the report, we dont have to look up the type in the db

vjeffrey · 2020-05-17T21:14:50Z

components/compliance-service/inspec-agent/runner/runner.go

@@ -521,6 +521,7 @@ func (t *InspecJobTask) reportIt(ctx context.Context, job *types.InspecJob, cont
 	report.SourceRegion = job.TargetConfig.TargetBaseConfig.Region
 	report.SourceAccountId = job.SourceAccountID
 	report.AutomateManagerId = job.ManagerID
+	report.AutomateManagerType = job.ManagerType


this is where we stick the manager type on the report

vjeffrey · 2020-05-17T21:15:46Z

components/compliance-service/ingest/pipeline/publisher/node_manager_publisher.go

@@ -90,6 +91,10 @@ func gatherInfoForNode(in message.Compliance) (*manager.NodeMetadata, error) {
 		tags = append(tags, &common.Kv{Key: "environment", Value: in.Report.GetEnvironment()})
 	}

+	mgrType := in.Report.GetAutomateManagerType()
+	if in.Report.GetSourceFqdn() != "" {


get the type on the report, but if the sourcefqdn is not empty string then we know it's managed by chef

vjeffrey · 2020-05-20T02:43:50Z

closing this til i can get back to it

vjeffrey requested a review from a team May 11, 2020 15:34

vjeffrey marked this pull request as ready for review May 11, 2020 17:33

lancewf reviewed May 11, 2020

View reviewed changes

Victoria Jeffrey added 10 commits May 17, 2020 13:52

ingest infra reports before inspec reports

584810c

Signed-off-by: Victoria Jeffrey <[email protected]>

add manager type to interservice proto

50b8109

Signed-off-by: Victoria Jeffrey <[email protected]>

ensure resolver includes manager type field

3e88876

Signed-off-by: Victoria Jeffrey <[email protected]>

put manager type on message to nodemanager

70bfb67

Signed-off-by: Victoria Jeffrey <[email protected]>

nodemanager updates

10b0ec5

Signed-off-by: Victoria Jeffrey <[email protected]>

do not add tag if it has doublequote in it

b230559

Signed-off-by: Victoria Jeffrey <[email protected]>

test fix

ace2208

Signed-off-by: Victoria Jeffrey <[email protected]>

do not allow update of name or tags on chef node

10478b7

Signed-off-by: Victoria Jeffrey <[email protected]>

expand allowed filter by mgr types

3bb2da2

Signed-off-by: Victoria Jeffrey <[email protected]>

fix ui

a1b6ebe

Signed-off-by: Victoria Jeffrey <[email protected]>

vjeffrey force-pushed the vj/mgr-fix branch from 96212c7 to a1b6ebe Compare May 17, 2020 21:08

vjeffrey changed the base branch from master to vj/nodes-proj-filt May 17, 2020 21:11

vjeffrey commented May 17, 2020

View reviewed changes

vjeffrey closed this May 20, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

add manager type to nodemanager process node flow #3630

add manager type to nodemanager process node flow #3630

vjeffrey commented May 11, 2020 •

edited

Loading

lancewf May 11, 2020

vjeffrey May 11, 2020

lancewf commented May 11, 2020

lancewf commented May 11, 2020

lancewf commented May 11, 2020 •

edited

Loading

vjeffrey commented May 11, 2020 •

edited

Loading

vjeffrey commented May 11, 2020

vjeffrey May 17, 2020 •

edited

Loading

vjeffrey May 17, 2020

vjeffrey May 17, 2020

vjeffrey May 17, 2020

vjeffrey May 17, 2020

vjeffrey commented May 20, 2020

add manager type to nodemanager process node flow #3630

add manager type to nodemanager process node flow #3630

Conversation

vjeffrey commented May 11, 2020 • edited Loading

🔩 Description: What code changed, and why?

⛓️ Related Resources

👍 Definition of Done

👟 How to Build and Test the Change

✅ Checklist

lancewf May 11, 2020

Choose a reason for hiding this comment

vjeffrey May 11, 2020

Choose a reason for hiding this comment

lancewf commented May 11, 2020

lancewf commented May 11, 2020

lancewf commented May 11, 2020 • edited Loading

vjeffrey commented May 11, 2020 • edited Loading

vjeffrey commented May 11, 2020

vjeffrey May 17, 2020 • edited Loading

Choose a reason for hiding this comment

vjeffrey May 17, 2020

Choose a reason for hiding this comment

vjeffrey May 17, 2020

Choose a reason for hiding this comment

vjeffrey May 17, 2020

Choose a reason for hiding this comment

vjeffrey May 17, 2020

Choose a reason for hiding this comment

vjeffrey commented May 20, 2020

vjeffrey commented May 11, 2020 •

edited

Loading

lancewf commented May 11, 2020 •

edited

Loading

vjeffrey commented May 11, 2020 •

edited

Loading

vjeffrey May 17, 2020 •

edited

Loading