Skip to content
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.

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

bugfix: ensure node name is correctly assigned when running ssm scan #3110

Merged
merged 2 commits into from
Mar 17, 2020

Conversation

vjeffrey
Copy link

🔩 Description: What code changed, and why?

I noticed there was a bug with ssm jobs during some acceptance testing -- all nodes showed up nameless after an ssm scan.

Figured out we had some faulty logic in resolver.go -- we were looking for a key value to determine the name and completely ignoring the name of the instance as we had discovered it.

This code change fixes that!

⛓️ Related Resources

#2640

👍 Definition of Done

Cloud scans of ec2 result in nodes with names!

👟 How to Build and Test the Change

rebuild compliance
run an ssm scan of the nodes (via aws or azure)

  • this requires running the automate instance in aws, or having azure creds

✅ Checklist

Copy link
Contributor

@stevendanna stevendanna left a comment

Choose a reason for hiding this comment

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

Thanks for looking into this! This looks reasonable to me, but it has been a while since I've looked at how some of these pieces fit together.

I've left a minor comment that is non blocking.

@@ -485,17 +485,28 @@ func (r *Resolver) handleAzureVmNodes(ctx context.Context, m *manager.NodeManage
return r.handleManagerNodes(ctx, m, nodeCollections, job)
}

func (r *Resolver) handleInstanceCredentials(ctx context.Context, instanceCreds []*manager.CredentialsByTags, node *manager.ManagerNode) ([]*inspec.Secrets, nodeInfo) {
var nodeInfo nodeInfo
func handleNodeInfo(node *manager.ManagerNode) nodeInfo {
Copy link
Contributor

Choose a reason for hiding this comment

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

I know we use handle in many of the other function names, but I'm wondering if nodeInfoFromManagerNode would be a more clear name?

Signed-off-by: Victoria Jeffrey <[email protected]>
@vjeffrey vjeffrey merged commit 440f656 into master Mar 17, 2020
@chef-expeditor chef-expeditor bot deleted the vj/fix-ssm-node-name branch March 17, 2020 22:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants