-
Notifications
You must be signed in to change notification settings - Fork 680
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
New Resource aws_config_recorder #2635
Conversation
dromazmj
commented
Feb 14, 2018
- Adds functionality to be able to test an AWS Configuration Recorder
Signed-off-by: Matthew Dromazos <dromazmj@dukes.jmu.edu>
Signed-off-by: Matthew Dromazos <dromazmj@dukes.jmu.edu>
Signed-off-by: Matthew Dromazos <dromazmj@dukes.jmu.edu>
Signed-off-by: Matthew Dromazos <dromazmj@dukes.jmu.edu>
…inspec into md/aws_config_recorder
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Very good! A few matchers to rename and doc edits, and some changes in exception handling. Your tests are very thorough - great work, this will be very useful!
@@ -0,0 +1,68 @@ | |||
--- |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Docs files should now end in .md.erb
title: About the aws_config_recorder Resource | ||
--- | ||
|
||
# aws_config_recorder |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Escape underscores in unquoted doc strings
|
||
# aws_config_recorder | ||
|
||
Use the `aws_config_recorder` InSpec audit resource to test properties of your aws config service. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Capitalize: "AWS Config service". That helps make it clear we're talking about a specific offering from AWS.
|
||
# aws_config_recorder | ||
|
||
Use the `aws_config_recorder` InSpec audit resource to test properties of your aws config service. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Might also be good to describe what a Config Recorder is. What does it represent in the Config Service? What does it interact with?
|
||
### role_arn | ||
|
||
Provides the IAM role arn associated with the configuration recorder. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A few words about what the role is used for might be good.
return unless @exists | ||
backend = BackendFactory.create(inspec_runner) | ||
@resp = backend.describe_configuration_recorder_status(@query) | ||
@status = @resp.configuration_recorders_status.first.to_h |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm always leery at accepting the first result from a possibly unsorted array. How do we know it's the right one?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The first one will always work because you are specifying the recording name. So then it will always return an array of one element
backend = BackendFactory.create(inspec_runner) | ||
@query = { configuration_recorder_names: [@recorder_name] } | ||
|
||
@resp = backend.describe_configuration_recorders(@query) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
wrap in catch_aws_errors
|
||
def describe_configuration_recorders(query) | ||
aws_service_client.describe_configuration_recorders(query) | ||
rescue Aws::ConfigService::Errors::NoSuchConfigurationRecorderException |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Don't catch exceptions here - catch them in fetch_from_api. It's part of the contract of the AWS API that it will throw that exception. If you catch it here, that means we're not testing for exception handling elsewhere.
fixtures[fixture_name] = attribute( | ||
fixture_name, | ||
default: "default.#{fixture_name}", | ||
description: 'See ../build/iam.tf', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
copypasta - config.tf
'empty' => {} | ||
} | ||
return recorders[query[:configuration_recorder_names][0]] unless recorders[query[:configuration_recorder_names][0]].nil? | ||
recorders['empty'] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You should raise Aws::ConfigService::Errors::NoSuchConfigurationRecorderException
here
* Wraps calls to api in catch_aws_errors method * Changes the names of two matchers Signed-off-by: Matthew Dromazos <dromazmj@dukes.jmu.edu>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks good to me! Thanks @dromazmj !