Make names for AWS Config service objects optional (#2928)

* Update tests and docs to assume one recorder per region
* Config recorder supports singleton fetch
* Docs and tests for singleton mode delivery_channel
* Implementation for singleton delivery channel, and some other code cleanup
* Implement some feedback, and fix a bug in traversing the struct in looking for empty results

Signed-off-by: Clinton Wolfe <clintoncwolfe@gmail.com>

docs/resources/aws_config_delivery_channel.md.erb

@@ -4,16 +4,20 @@ title: About the aws_config_delivery_channel Resource
 
 # aws_config_delivery_channel
 
-The AWS Config service can monitor and record changes to your AWS resource configurations.  A Delivery Channel can record the changes
+The AWS Config service can monitor and record changes to your AWS resource configurations. A Delivery Channel can record the changes
 to an S3 Bucket, an SNS or both.
 
 Use the `aws_config_delivery_channel` InSpec audit resource to examine how the AWS Config service delivers those change notifications.
 
+As of April 2018, each AWS region may have only one Delivery Channel.
+
 <br>
 
-## Syntax
+## Resource Parameters
 
-An `aws_config_delivery_channel` resource block declares the tests for a single AWS Config delivery channel.
+An `aws_config_delivery_channel` resource block declares the tests for a single AWS Config Delivery Channel.
+
+You may specify the Delivery Channel name:
 
     describe aws_config_delivery_channel('my_channel') do
       it { should exist }
@@ -23,45 +27,35 @@ An `aws_config_delivery_channel` resource block declares the tests for a single
       it { should exist }
     end
 
+However, since you may only have one Delivery Channel per region, and InSpec connections are per-region, you may also omit the `channel_name` to obtain the one Delivery Channel (if any) that exists:
+
+    describe aws_config_delivery_channel do
+      it { should exist }
+    end
+
 <br>
 
 ## Examples
 
 The following examples show how to use this InSpec audit resource.
 
-### Test how frequent the channel writes configuration changes to the s3 bucket.
+### Test how frequently the channel writes configuration changes to the s3 bucket.
 
     describe aws_config_delivery_channel(channel_name: 'my-recorder') do
       its(delivery_frequency_in_hours) { should be > 3 }
     end
     
 ## Properties
-    
-### s3_bucket_name
 
-Provides the name of the s3 bucket that the channel sends configuration changes to.  This is an optional value since a Delivery Channel can also talk to an SNS.
+### channel\_name
 
-    describe aws_config_delivery_channel(channel_name: 'my_channel')
-      its('s3_bucket_name') { should eq 'my_bucket' }
+Returns the name of the Delivery Channel.
+
+    describe aws_config_delivery_channel do
+      its('channel_name') { should cmp 'my-channel' }
     end
-    
-### s3_key_prefix
 
-Provides the s3 object key prefix (or "path") under which configuration data will be recorded.
-
-    describe aws_config_delivery_channel(channel_name: 'my_channel')
-      its('s3_key_prefix') { should eq 'log/' }
-    end
-    
-### sns_topic_arn
-
-Provides the ARN of the SNS topic for which the channel sends notifications about configuration changes.
-
-    describe aws_config_delivery_channel(channel_name: 'my_channel')
-      its('sns_topic_arn') { should eq 'arn:aws:sns:us-east-1:721741954427:sns_topic' }
-    end
-    
-### delivery_frequency_in_hours
+### delivery\_frequency\_in\_hours
 
 Provides how often the AWS Config sends configuration changes to the s3 bucket in the delivery channel.
 
@@ -69,11 +63,34 @@ Provides how often the AWS Config sends configuration changes to the s3 bucket i
       its('delivery_frequency_in_hours') { should eq 24 }
       its('delivery_frequency_in_hours') { should be > 24 }
     end
+
+### s3\_bucket\_name
+
+Provides the name of the s3 bucket that the channel sends configuration changes to. This is an optional value since a Delivery Channel can also talk to an SNS.
+
+    describe aws_config_delivery_channel(channel_name: 'my_channel')
+      its('s3_bucket_name') { should eq 'my_bucket' }
+    end
     
+### s3\_key\_prefix
+
+Provides the s3 object key prefix (or "path") under which configuration data will be recorded.
+
+    describe aws_config_delivery_channel(channel_name: 'my_channel')
+      its('s3_key_prefix') { should eq 'log/' }
+    end
+    
+### sns\_topic\_arn
+
+Provides the ARN of the SNS topic for which the channel sends notifications about configuration changes.
+
+    describe aws_config_delivery_channel(channel_name: 'my_channel')
+      its('sns_topic_arn') { should eq 'arn:aws:sns:us-east-1:721741954427:sns_topic' }
+    end
     
 <br>
 
 ## Matchers
 
-This resource provides no matchers, aside from the standard exists matcher.
+This resource provides no matchers, aside from the standard `exist` matcher.
 

docs/resources/aws_config_recorder.md.erb

@@ -8,12 +8,16 @@ Use the `aws_config_recorder` InSpec audit resource to test properties of your A
 
 The AWS Config service can monitor and record changes to your AWS resource configurations.  The Aws Config Recorder is used to detect changes in resource configurations and capture these changes as configuration items.
 
+As of April 2018, you are only permitted one configuration recorder per region.
+
 <br>
 
-## Syntax
+## Resource Parameters
 
 An `aws_config_recorder` resource block declares the tests for a single AWS configuration recorder.
 
+You may specify a recorder by name:
+
     describe aws_config_recorder('my_recorder') do
       it { should exist }
     end
@@ -22,6 +26,12 @@ An `aws_config_recorder` resource block declares the tests for a single AWS conf
       it { should exist }
     end
 
+However, since you may only have one recorder per region, and InSpec connections are per-region, you may also omit the `recorder_name` to obtain the one recorder (if any) that exists:
+
+    describe aws_config_recorder do
+      it { should exist }
+    end
+
 <br>
 
 ## Examples

lib/resources/aws/aws_config_delivery_channel.rb

@@ -28,29 +28,23 @@ class AwsConfigDeliveryChannel < Inspec.resource(1)
       allowed_scalar_type: String,
     )
 
-    # Make sure channel_name is given as param
-    if validated_params[:channel_name].nil?
-      raise ArgumentError, 'You must provide a channel_name to aws_config_delivery_channel'
-    end
-
     validated_params
   end
 
   def fetch_from_api
     backend = BackendFactory.create(inspec_runner)
-    query = { delivery_channel_names: [@channel_name] }
-    catch_aws_errors do
-      @resp = backend.describe_delivery_channels(query)
-    end
-    @exists = !@resp.empty?
-    return unless @exists
+    query = @channel_name ? { delivery_channel_names: [@channel_name] } : {}
+    response = backend.describe_delivery_channels(query)
 
-    @channel = @resp.delivery_channels.first.to_h
-    @channel_name = @channel[:name]
-    @s3_bucket_name = @channel[:s3_bucket_name]
-    @s3_key_prefix = @channel[:s3_key_prefix]
-    @sns_topic_arn = @channel[:sns_topic_arn]
-    @delivery_frequency_in_hours = @channel[:config_snapshot_delivery_properties][:delivery_frequency] unless @channel[:config_snapshot_delivery_properties].nil?
+    @exists = !response.delivery_channels.empty?
+    return unless exists?
+
+    channel = response.delivery_channels.first.to_h
+    @channel_name = channel[:name]
+    @s3_bucket_name = channel[:s3_bucket_name]
+    @s3_key_prefix = channel[:s3_key_prefix]
+    @sns_topic_arn = channel[:sns_topic_arn]
+    @delivery_frequency_in_hours = channel.dig(:config_snapshot_delivery_properties, :delivery_frequency)
     frequencies = {
       'One_Hour' => 1,
       'TwentyFour_Hours' => 24,
@@ -59,6 +53,8 @@ class AwsConfigDeliveryChannel < Inspec.resource(1)
       'Twelve_Hours' => 12,
     }
     @delivery_frequency_in_hours = frequencies[@delivery_frequency_in_hours]
+  rescue Aws::ConfigService::Errors::NoSuchDeliveryChannelException
+    @exists = false
   end
 
   class Backend
@@ -66,10 +62,8 @@ class AwsConfigDeliveryChannel < Inspec.resource(1)
       BackendFactory.set_default_backend(self)
       self.aws_client_class = Aws::ConfigService::Client
 
-      def describe_delivery_channels(query)
+      def describe_delivery_channels(query = {})
         aws_service_client.describe_delivery_channels(query)
-      rescue Aws::ConfigService::Errors::NoSuchDeliveryChannelException
-        return {}
       end
     end
   end

lib/resources/aws/aws_config_recorder.rb

@@ -12,7 +12,7 @@ class AwsConfigurationRecorder < Inspec.resource(1)
   supports platform: 'aws'
 
   include AwsSingularResourceMixin
-  attr_reader :role_arn, :resource_types, :recorder_name, :resp
+  attr_reader :role_arn, :resource_types, :recorder_name
 
   def to_s
     "Configuration_Recorder: #{@recorder_name}"
@@ -27,11 +27,11 @@ class AwsConfigurationRecorder < Inspec.resource(1)
   end
 
   def status
-    return unless @exists
+    return {} unless @exists
     backend = BackendFactory.create(inspec_runner)
     catch_aws_errors do
-      @resp = backend.describe_configuration_recorder_status(@query)
-      @status = @resp.configuration_recorders_status.first.to_h
+      response = backend.describe_configuration_recorder_status(configuration_recorder_names: [@recorder_name])
+      @status = response.configuration_recorders_status.first.to_h
     end
   end
 
@@ -50,35 +50,30 @@ class AwsConfigurationRecorder < Inspec.resource(1)
       allowed_scalar_type: String,
     )
 
-    # Must give it a recorder_name
-    if validated_params[:recorder_name].nil?
-      raise ArgumentError, 'You must provide recorder_name to aws_config_recorder'
-    end
-
     validated_params
   end
 
   def fetch_from_api
     backend = BackendFactory.create(inspec_runner)
-    @query = { configuration_recorder_names: [@recorder_name] }
+    query = @recorder_name ? { configuration_recorder_names: [@recorder_name] } : {}
+    response = backend.describe_configuration_recorders(query)
 
-    catch_aws_errors do
-      begin
-        @resp = backend.describe_configuration_recorders(@query)
-      rescue Aws::ConfigService::Errors::NoSuchConfigurationRecorderException
-        @exists = false
-        return
-      end
-      @exists = !@resp.empty?
-      return unless @exists
+    @exists = !response.configuration_recorders.empty?
+    return unless exists?
 
-      @recorder = @resp.configuration_recorders.first.to_h
-      @recorder_name = @recorder[:name]
-      @role_arn = @recorder[:role_arn]
-      @recording_all_resource_types = @recorder[:recording_group][:all_supported]
-      @recording_all_global_types = @recorder[:recording_group][:include_global_resource_types]
-      @resource_types = @recorder[:recording_group][:resource_types]
+    if response.configuration_recorders.count > 1
+      raise ArgumentError, 'Internal error: unexpectedly received multiple AWS Config Recorder objects from API; expected to be singleton per-region.  Please file a bug report at https://github.com/chef/inspec/issues .'
     end
+
+    recorder = response.configuration_recorders.first.to_h
+    @recorder_name = recorder[:name]
+    @role_arn = recorder[:role_arn]
+    @recording_all_resource_types = recorder[:recording_group][:all_supported]
+    @recording_all_global_types = recorder[:recording_group][:include_global_resource_types]
+    @resource_types = recorder[:recording_group][:resource_types]
+  rescue Aws::ConfigService::Errors::NoSuchConfigurationRecorderException
+    @exists = false
+    return
   end
 
   class Backend

test/integration/aws/default/build/config.tf

@@ -47,13 +47,13 @@ resource "aws_config_delivery_channel" "delivery_channel_01" {
   s3_bucket_name = "${aws_s3_bucket.bucket_for_delivery_channel.bucket}"
   depends_on     = ["aws_config_configuration_recorder.config_recorder"]
   sns_topic_arn  = "${aws_sns_topic.sns_topic_for_delivery_channel.arn}"
+
   snapshot_delivery_properties = {
     delivery_frequency = "TwentyFour_Hours"
   }
 }
 
-
-output "delivery_channel_01" {
+output "delivery_channel_01_name" {
   value = "${aws_config_delivery_channel.delivery_channel_01.id}"
 }
 
@@ -97,7 +97,7 @@ resource "aws_s3_bucket" "bucket_for_delivery_channel" {
   force_destroy = true
 }
 
-output "s3_bucket_for_delivery_channel" {
+output "s3_bucket_for_delivery_channel_name" {
   value = "${aws_s3_bucket.bucket_for_delivery_channel.id}"
 }
 

test/integration/aws/default/verify/controls/aws_config_delivery_channel.rb

@@ -1,8 +1,8 @@
 fixtures = {}
 [
-  'delivery_channel_01',
+  'delivery_channel_01_name',
   'config_recorder_for_delivery_channel_role_arn',
-  's3_bucket_for_delivery_channel',
+  's3_bucket_for_delivery_channel_name',
   'delivery_channel_01_bucket_prefix',
   'sns_topic_for_delivery_channel_arn'
 ].each do |fixture_name|
@@ -20,17 +20,23 @@ end
 #-------------------  Recall / Miss -------------------#
 
 control "aws_config_delivery_channel recall" do
+
+  # Test default singleton return
+  describe aws_config_delivery_channel do
+    it { should exist }
+  end
+
   # Test scalar param
-  describe aws_config_delivery_channel(fixtures['delivery_channel_01']) do
+  describe aws_config_delivery_channel(fixtures['delivery_channel_01_name']) do
     it { should exist }
   end
   
   # Test hash parameter
-  describe aws_config_delivery_channel(channel_name: fixtures['delivery_channel_01']) do
+  describe aws_config_delivery_channel(channel_name: fixtures['delivery_channel_01_name']) do
     it { should exist }
   end
   
-  # Test recorder that doesnt exist
+  # Test recorder that doesn't exist
   describe aws_config_delivery_channel(channel_name: 'NonExistentChannel') do
     it { should_not exist }
   end
@@ -38,11 +44,16 @@ end
 
 #-------------------  Properties -------------------#
 control "aws_config_delivery_channel properties" do
-  describe aws_config_delivery_channel(fixtures['delivery_channel_01']) do
-    its('s3_bucket_name') { should eq fixtures['s3_bucket_for_delivery_channel'] }
+  describe aws_config_delivery_channel(fixtures['delivery_channel_01_name']) do
+    its('channel_name') { should eq fixtures['delivery_channel_01_name'] }
+    its('s3_bucket_name') { should eq fixtures['s3_bucket_for_delivery_channel_name'] }
     its('s3_key_prefix') { should eq nil }
     its('sns_topic_arn') { should eq fixtures['sns_topic_for_delivery_channel_arn'] }
     its('delivery_frequency_in_hours') { should eq 24 }
     its('delivery_frequency_in_hours') { should be > 3 }
   end
+
+  describe aws_config_delivery_channel do
+    its('channel_name') { should eq fixtures['delivery_channel_01_name'] }
+  end  
 end

test/integration/aws/default/verify/controls/aws_config_recorder.rb

@@ -17,6 +17,12 @@ end
 #-------------------  Recall / Miss -------------------#
 
 control "aws_config_recorder recall" do
+
+  # Get the singleton if you don't pass a name
+  describe aws_config_recorder do
+    it { should exist }
+  end
+  
   # Test scalar param
   describe aws_config_recorder(fixtures['config_recorder_name']) do
     it { should exist }
@@ -35,6 +41,10 @@ end
 
 #-------------------  Properties -------------------#
 control "aws_config_recorder properties" do
+  describe aws_config_recorder do
+    its('recorder_name') { should eq fixtures['config_recorder_name'] }
+  end
+
   describe aws_config_recorder(fixtures['config_recorder_name']) do
     its('recorder_name') { should eq fixtures['config_recorder_name'] }
     its('role_arn') { should eq fixtures['role_for_config_recorder_arn'] }

test/unit/resources/aws_config_delivery_channel_test.rb

@@ -0,0 +1,128 @@
+# encoding: utf-8
+require 'helper'
+
+# MDCSB = MockDeliveryChannelSingleBackend
+# Abbreviation not used outside this file
+
+#=============================================================================#
+#                            Constructor Tests
+#=============================================================================#
+class AwsConfigDeliveryChannelConstructorTest < Minitest::Test
+  def setup
+    AwsConfigDeliveryChannel::BackendFactory.select(AwsMDCSB::Basic)
+  end
+
+  def test_constructor_when_no_params_provided
+    AwsConfigDeliveryChannel.new
+  end
+
+
+  def test_constructor_expected_well_formed_args_scalar
+    AwsConfigDeliveryChannel.new('default')
+  end
+
+  def test_constructor_expected_well_formed_args_hash
+    AwsConfigDeliveryChannel.new(channel_name: 'default')
+  end
+  
+  def test_constructor_reject_unknown_resource_params
+    assert_raises(ArgumentError) { AwsConfigDeliveryChannel.new(bla: 'blabla') }
+  end
+end
+
+#=============================================================================#
+#                               Recall
+#=============================================================================#
+
+class AwsConfigDeliveryChannelRecallTest < Minitest::Test
+  def setup
+    AwsConfigDeliveryChannel::BackendFactory.select(AwsMDCSB::Basic)
+  end
+
+  def test_search_hit_by_default
+    assert AwsConfigDeliveryChannel.new.exists?
+  end
+
+  def test_search_hit_via_scalar
+    assert AwsConfigDeliveryChannel.new('default').exists?
+  end
+
+  def test_search_hit_via_hash
+    assert AwsConfigDeliveryChannel.new(channel_name: 'default').exists?
+  end
+
+  def test_search_miss_is_not_an_exception
+    refute AwsConfigDeliveryChannel.new(channel_name: 'NonExistentDeliveryChannel').exists?
+  end
+end
+
+#=============================================================================#
+#                               properties
+#=============================================================================#
+
+class AwsConfigDeliveryChannelPropertiesTest < Minitest::Test
+  def setup
+    AwsConfigDeliveryChannel::BackendFactory.select(AwsMDCSB::Basic)
+  end
+
+  def test_property_channel_name
+    assert_equal('default', AwsConfigDeliveryChannel.new('default').channel_name)
+    assert_equal('default', AwsConfigDeliveryChannel.new.channel_name)
+    assert_equal('NonExistentDeliveryChannel',AwsConfigDeliveryChannel.new('NonExistentDeliveryChannel').channel_name)    
+  end
+
+  def test_property_delivery_frequency_in_hours
+    assert_equal(3, AwsConfigDeliveryChannel.new('default').delivery_frequency_in_hours)
+    assert_nil(AwsConfigDeliveryChannel.new('NonExistentDeliveryChannel').delivery_frequency_in_hours)
+  end
+
+  def test_property_s3_bucket_name
+    assert_equal('my-bucket', AwsConfigDeliveryChannel.new('default').s3_bucket_name)
+    assert_nil(AwsConfigDeliveryChannel.new('NonExistentDeliveryChannel').s3_bucket_name)
+  end
+
+  def test_property_s3_key_prefix
+    assert_equal('config-logs/', AwsConfigDeliveryChannel.new('default').s3_key_prefix)
+    assert_nil(AwsConfigDeliveryChannel.new('NonExistentDeliveryChannel').s3_key_prefix)
+  end
+  
+  def test_property_sns_topic_arn
+    assert_equal('arn:aws:sns:::my-topic-name', AwsConfigDeliveryChannel.new('default').sns_topic_arn)
+    assert_nil(AwsConfigDeliveryChannel.new('NonExistentDeliveryChannel').sns_topic_arn)
+  end
+end
+
+#=============================================================================#
+#                               Test Matchers
+#=============================================================================#
+
+# None
+
+#=============================================================================#
+#                               Test Fixtures
+#=============================================================================#
+
+module AwsMDCSB
+  class Basic < AwsBackendBase
+    def describe_delivery_channels(query = {})
+      fixtures = {
+        'default' => Aws::ConfigService::Types::DescribeDeliveryChannelsResponse.new(
+          :delivery_channels => [
+            {
+              name: "default",
+              s3_bucket_name: 'my-bucket',
+              s3_key_prefix: 'config-logs/',
+              sns_topic_arn: 'arn:aws:sns:::my-topic-name',
+              config_snapshot_delivery_properties: {
+                 delivery_frequency: 'Three_Hours'
+              },
+            },
+          ]
+        ),
+      }
+      return fixtures['default'] if query.empty?
+      return fixtures[query[:delivery_channel_names][0]] unless fixtures[query[:delivery_channel_names][0]].nil?
+      raise Aws::ConfigService::Errors::NoSuchDeliveryChannelException.new(nil, nil)
+    end
+  end
+end

test/unit/resources/aws_config_recorder_test.rb

@@ -11,7 +11,12 @@ class AwsConfigurationRecorderConstructorTest < Minitest::Test
   def setup
     AwsConfigurationRecorder::BackendFactory.select(AwsMCRSB::Basic)
   end
-  
+
+  def test_constructor_when_no_params_provided
+    AwsConfigurationRecorder.new
+  end
+
+
   def test_constructor_expected_well_formed_args_scalar
     AwsConfigurationRecorder.new('default')
   end
@@ -20,10 +25,6 @@ class AwsConfigurationRecorderConstructorTest < Minitest::Test
     AwsConfigurationRecorder.new(recorder_name: 'default')
   end
   
-  def test_constructor_reject_no_params
-    assert_raises(ArgumentError) { AwsConfigurationRecorder.new }
-  end
-
   def test_constructor_reject_unknown_resource_params
     assert_raises(ArgumentError) { AwsConfigurationRecorder.new(bla: 'blabla') }
   end
@@ -37,7 +38,11 @@ class AwsConfigurationRecorderRecallTest < Minitest::Test
   def setup
     AwsConfigurationRecorder::BackendFactory.select(AwsMCRSB::Basic)
   end
-  
+
+  def test_search_hit_by_default
+    assert AwsConfigurationRecorder.new.exists?
+  end
+
   def test_search_hit_via_scalar
     assert AwsConfigurationRecorder.new('default').exists?
   end
@@ -62,6 +67,7 @@ class AwsConfigurationRecorderPropertiesTest < Minitest::Test
 
   def test_property_recorder_name
     assert_equal('default', AwsConfigurationRecorder.new(recorder_name: 'default').recorder_name)
+    assert_equal('default', AwsConfigurationRecorder.new.recorder_name)
   end
 
   def test_property_role_arn
@@ -70,7 +76,7 @@ class AwsConfigurationRecorderPropertiesTest < Minitest::Test
   end
   
   def test_property_resource_types
-    assert_equal(['AWS::EC2::CustomerGateway', 'AWS::EC2::EIP'], AwsConfigurationRecorder.new(recorder_name: 'Recorder_2').resource_types)
+    assert_equal(['AWS::EC2::CustomerGateway', 'AWS::EC2::EIP'], AwsConfigurationRecorder.new(recorder_name: 'default').resource_types)
     assert_nil(AwsConfigurationRecorder.new(recorder_name: 'NonExistentRecorder').resource_types)
   end
 end
@@ -95,14 +101,13 @@ class AwsConfigurationRecorderPropertiesTest < Minitest::Test
   end
 end
 
-
 #=============================================================================#
 #                               Test Fixtures
 #=============================================================================#
 
 module AwsMCRSB
   class Basic < AwsBackendBase
-    def describe_configuration_recorders(query)
+    def describe_configuration_recorders(query = {})
       recorders = {
         'default' => OpenStruct.new({
           :configuration_recorders => [
@@ -111,28 +116,21 @@ module AwsMCRSB
             :recording_group => OpenStruct.new({
               all_supported: true,
               include_global_resource_types: true,
-              resource_types: []
+              resource_types: ['AWS::EC2::CustomerGateway', 'AWS::EC2::EIP'],
             }),
           ]
         }),
-        'Recorder_2' => OpenStruct.new({
-          :configuration_recorders => [
-            name: "Recorder_2",
-            role_arn: "arn:aws:iam::721741954427:role/Recorder_1",
-            :recording_group => OpenStruct.new({
-              all_supported: false,
-              include_global_resource_types: false,
-              resource_types: ['AWS::EC2::CustomerGateway', 'AWS::EC2::EIP']
-            }),
-          ]
-        }),
-        'empty' => {}
       }
-      return recorders[query[:configuration_recorder_names][0]] unless recorders[query[:configuration_recorder_names][0]].nil?
-      recorders['empty']
+      if query.empty?
+        return recorders['default']
+      elsif recorders.key?(query[:configuration_recorder_names][0])
+        return recorders[query[:configuration_recorder_names][0]]
+      else
+        raise Aws::ConfigService::Errors::NoSuchConfigurationRecorderException.new(nil, nil)
+      end
     end
     
-    def describe_configuration_recorder_status(query)
+    def describe_configuration_recorder_status(query = {})
       recorders = {
         'default' => OpenStruct.new({
           :configuration_recorders_status => [