Added a default `limit = 1` value to `aws_cloudwatch_log_group.rb`

so1omon563 commented 2 years ago

Description

The resource is using the describe_log_groups api call, which is actually doing a prefix lookup. This means that it can easily return multiple log groups that have the same prefix.

For example, if I have a log group called /my-environment/my-app that I want to check, but I also have a log group called /my-environment/my-app-testing, it is impossible to check the /my-environment/my-app log group because it returns multiple groups.

This change includes adding a defaulted limit: 1 value to the call. That will return only the first log group match, which in examples as shown above, will be the exact match.

It allows for an override if desired by simply adding a limit: value option to the resource.

I believe that since this is a singular resource, that this new behavior is more in line with expected results.

Issues Resolved

Resolves issue https://github.com/inspec/inspec-aws/issues/885

Check List

Please fill box or appropriate ([x]) or mark N/A.

[ N/A ] New functionality includes integration tests/controls
[ N/A] New Terraform resources
[x] Documentation provided or updated for resources
[ ] All Integration Tests pass
[ ] All Unit Tests pass
[ ] rake lint passes
[x] All commits have been signed-off for the Developer Certificate of Origin. See https://github.com/chef/chef/blob/master/CONTRIBUTING.md#developer-certification-of-origin-dco

netlify[bot] commented 2 years ago

Deploy Preview for inspec-aws ready!

Name	Link
Latest commit	a1b06dbccc6a2ebe9009701b6492e3e5d5fea04e
Latest deploy log	https://app.netlify.com/sites/inspec-aws/deploys/6321825344f5990008fab4d1
Deploy Preview	https://deploy-preview-928--inspec-aws.netlify.app
Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

so1omon563 commented 2 years ago

Unsure if unit / integration tests need to be modified. I believe the existing tests for the resource will pass.

so1omon563 commented 2 years ago

Any updates on this request?

soumyo13 commented 2 years ago

In a particular region, we cant create two log_groups with the same name.

I tried to replicate the issue. I created a cloud-log-group in region ‘us-east-2’ and created another cloud-log-group with the same name in the region 'us-west-2'.

Then I tried running the same in both the two regions.

Test Checked:

aws_cloud_watch_log_group_name = 'test1'

control 'aws-cloudwatch-log-group-1.0' do
  impact 1.0
  title 'Ensure AWS Cloudwatch Log Group has the correct properties.'

  describe aws_cloudwatch_log_group(log_group_name:  aws_cloud_watch_log_group_name) do
    it { should exist }
    its('retention_in_days') { should eq nil }
    its('kms_key_id') { should eq nil }
    its('tags') { should be_empty }
  end
end

I used the below command to execute the test in different regions:

This test I have executed for region 'us-east-2'

inspec exec . -t aws://us-east-2

And here is the result:

This test I have executed for region 'us-west-2'

inspec exec . -t aws://us-west-2

And here is the result:

so1omon563 commented 2 years ago

The issue isn't two log groups with the same name. It's two log groups that start with the same prefix. This information is in my description.

To replicate the issue, try creating a log group called test, and a log group called test1, both in the same region. Then run your check against only the test log group.

It will fail, because there are now two log groups with a prefix of test, and the check only works with one result, since it's a singular resource.

My submitted fix will prevent this from happening, as it would default to only looking at the first match.

Here is sample output of the output from running a basic should exist test against a test log group exactly as I have described above.

  RuntimeError:
    Found multiple CloudWatch Log Groups. The following matched: {:log_group_name=>"test", :creation_time=>1663082860460, :retention_in_days=>14, :metric_filter_count=>0, :arn=>"arn:aws:logs:us-east-1:508551232565:log-group:test:*", :stored_bytes=>0, :kms_key_id=>nil}, {:log_group_name=>"test1", :creation_time=>1663082968352, :retention_in_days=>14, :metric_filter_count=>0, :arn=>"arn:aws:logs:us-east-1:508551232565:log-group:test1:*", :stored_bytes=>0, :kms_key_id=>nil}.  Try to restrict your resource criteria.
  # ./vendor/bundle/ruby/2.7.0/gems/inspec-core-5.17.4/lib/inspec/rule.rb:61:in `block (2 levels) in initialize'
  # ./vendor/bundle/ruby/2.7.0/gems/inspec-core-5.17.4/lib/inspec/runner_rspec.rb:97:in `run'
  # ./vendor/bundle/ruby/2.7.0/gems/inspec-core-5.17.4/lib/inspec/runner.rb:183:in `run_tests'
  # ./vendor/bundle/ruby/2.7.0/gems/inspec-core-5.17.4/lib/inspec/runner.rb:154:in `run'
  # ./vendor/bundle/ruby/2.7.0/gems/inspec-core-5.17.4/lib/inspec/cli.rb:313:in `exec'
  # ./vendor/bundle/ruby/2.7.0/gems/thor-1.2.1/lib/thor/command.rb:27:in `run'
  # ./vendor/bundle/ruby/2.7.0/gems/thor-1.2.1/lib/thor/invocation.rb:127:in `invoke_command'
  # ./vendor/bundle/ruby/2.7.0/gems/thor-1.2.1/lib/thor.rb:392:in `dispatch'
  # ./vendor/bundle/ruby/2.7.0/gems/thor-1.2.1/lib/thor/base.rb:485:in `start'
  # ./vendor/bundle/ruby/2.7.0/gems/inspec-core-5.17.4/lib/inspec/base_cli.rb:35:in `start'
./vendor/bundle/ruby/2.7.0/gems/inspec-core-5.17.4/lib/inspec/rule.rb:61:in `block (2 levels) in initialize'
./vendor/bundle/ruby/2.7.0/gems/inspec-core-5.17.4/lib/inspec/runner_rspec.rb:97:in `run'
./vendor/bundle/ruby/2.7.0/gems/inspec-core-5.17.4/lib/inspec/runner.rb:183:in `run_tests'
./vendor/bundle/ruby/2.7.0/gems/inspec-core-5.17.4/lib/inspec/runner.rb:154:in `run'
./vendor/bundle/ruby/2.7.0/gems/inspec-core-5.17.4/lib/inspec/cli.rb:313:in `exec'
./vendor/bundle/ruby/2.7.0/gems/thor-1.2.1/lib/thor/command.rb:27:in `run'
./vendor/bundle/ruby/2.7.0/gems/thor-1.2.1/lib/thor/invocation.rb:127:in `invoke_command'
./vendor/bundle/ruby/2.7.0/gems/thor-1.2.1/lib/thor.rb:392:in `dispatch'
./vendor/bundle/ruby/2.7.0/gems/thor-1.2.1/lib/thor/base.rb:485:in `start'
./vendor/bundle/ruby/2.7.0/gems/inspec-core-5.17.4/lib/inspec/base_cli.rb:35:in `start'

soumyo13 commented 2 years ago

The issue isn't two log groups with the same name. It's two log groups that start with the same prefix. This information is in my description.

To replicate the issue, try creating a log group called test, and a log group called test1, both in the same region. Then run your check against only the test log group.

It will fail, because there are now two log groups with a prefix of test, and the check only works with one result, since it's a singular resource.

My submitted fix will prevent this from happening, as it would default to only looking at the first match.

Here is sample output of the output from running a basic should exist test against a test log group exactly as I have described above.

  RuntimeError:
    Found multiple CloudWatch Log Groups. The following matched: {:log_group_name=>"test", :creation_time=>1663082860460, :retention_in_days=>14, :metric_filter_count=>0, :arn=>"arn:aws:logs:us-east-1:508551232565:log-group:test:*", :stored_bytes=>0, :kms_key_id=>nil}, {:log_group_name=>"test1", :creation_time=>1663082968352, :retention_in_days=>14, :metric_filter_count=>0, :arn=>"arn:aws:logs:us-east-1:508551232565:log-group:test1:*", :stored_bytes=>0, :kms_key_id=>nil}.  Try to restrict your resource criteria.
  # ./vendor/bundle/ruby/2.7.0/gems/inspec-core-5.17.4/lib/inspec/rule.rb:61:in `block (2 levels) in initialize'
  # ./vendor/bundle/ruby/2.7.0/gems/inspec-core-5.17.4/lib/inspec/runner_rspec.rb:97:in `run'
  # ./vendor/bundle/ruby/2.7.0/gems/inspec-core-5.17.4/lib/inspec/runner.rb:183:in `run_tests'
  # ./vendor/bundle/ruby/2.7.0/gems/inspec-core-5.17.4/lib/inspec/runner.rb:154:in `run'
  # ./vendor/bundle/ruby/2.7.0/gems/inspec-core-5.17.4/lib/inspec/cli.rb:313:in `exec'
  # ./vendor/bundle/ruby/2.7.0/gems/thor-1.2.1/lib/thor/command.rb:27:in `run'
  # ./vendor/bundle/ruby/2.7.0/gems/thor-1.2.1/lib/thor/invocation.rb:127:in `invoke_command'
  # ./vendor/bundle/ruby/2.7.0/gems/thor-1.2.1/lib/thor.rb:392:in `dispatch'
  # ./vendor/bundle/ruby/2.7.0/gems/thor-1.2.1/lib/thor/base.rb:485:in `start'
  # ./vendor/bundle/ruby/2.7.0/gems/inspec-core-5.17.4/lib/inspec/base_cli.rb:35:in `start'
./vendor/bundle/ruby/2.7.0/gems/inspec-core-5.17.4/lib/inspec/rule.rb:61:in `block (2 levels) in initialize'
./vendor/bundle/ruby/2.7.0/gems/inspec-core-5.17.4/lib/inspec/runner_rspec.rb:97:in `run'
./vendor/bundle/ruby/2.7.0/gems/inspec-core-5.17.4/lib/inspec/runner.rb:183:in `run_tests'
./vendor/bundle/ruby/2.7.0/gems/inspec-core-5.17.4/lib/inspec/runner.rb:154:in `run'
./vendor/bundle/ruby/2.7.0/gems/inspec-core-5.17.4/lib/inspec/cli.rb:313:in `exec'
./vendor/bundle/ruby/2.7.0/gems/thor-1.2.1/lib/thor/command.rb:27:in `run'
./vendor/bundle/ruby/2.7.0/gems/thor-1.2.1/lib/thor/invocation.rb:127:in `invoke_command'
./vendor/bundle/ruby/2.7.0/gems/thor-1.2.1/lib/thor.rb:392:in `dispatch'
./vendor/bundle/ruby/2.7.0/gems/thor-1.2.1/lib/thor/base.rb:485:in `start'
./vendor/bundle/ruby/2.7.0/gems/inspec-core-5.17.4/lib/inspec/base_cli.rb:35:in `start'

Thanks got the problem.

inspec / inspec-aws