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

Add plural resources #18

Merged
merged 6 commits into from
Jun 14, 2018
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
80 changes: 80 additions & 0 deletions docs/resources/google_compute_firewalls.md.erb
Original file line number Diff line number Diff line change
@@ -0,0 +1,80 @@
---
title: About the google_compute_firewalls Resource
platform: gcp
---

# google\_compute\_firewalls

Use the `google_compute_firewalls` InSpec audit resource to test properties of all GCP compute firewalls for a project.

<br>

## Syntax

A `google_compute_firewalls` resource block collects GCP firewalls by project then tests that group.

describe google_compute_firewalls(project: 'chef-inspec-gcp') do
it { should exist }
end

Use this InSpec resource to enumerate IDs then test in-depth using `google_compute_firewall`.

google_compute_firewalls(project: 'chef-inspec-gcp').firewall_names.each do |firewall_name|
describe google_compute_firewall(project: 'chef-inspec-gcp', name: firewall_name) do
it { should exist }
its('kind') { should eq "compute#firewall" }
end
end

<br>

## Examples

The following examples show how to use this InSpec audit resource.

### Test that there are no more than a specified number of firewalls available for the project

describe google_compute_firewalls(project: 'chef-inspec-gcp') do
its('entries.count') { should be <= 100}
Copy link
Contributor

Choose a reason for hiding this comment

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

You can just use its('count') ... here.

Copy link
Author

Choose a reason for hiding this comment

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

sure, will update

end

### Test the exact number of firewalls in the project

describe google_compute_firewalls(project: 'chef-inspec-gcp') do
its('firewall_ids.count') { should cmp 8 }
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not clear on the difference between this example and the previous one. If the resource matches 5 firewalls, are you not expected to have 5 firewall IDs?

Copy link
Author

Choose a reason for hiding this comment

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

agreed, will remove

end

### Test that an expected firewall is available for the project

describe google_compute_firewalls(project: 'chef-inspec-gcp') do
its('firewall_names') { should include "my-app-firewall-rule" }
end

### Test that a particular named rule does not exist

describe google_compute_firewalls(project: 'chef-inspec-gcp') do
its('firewall_names') { should_not include "default-allow-ssh" }
end

### Test there are no firewalls for the "INGRESS" direction
Copy link
Contributor

Choose a reason for hiding this comment

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

Hrm, this is usually expressed like this:

describe google_compute_firewalls(project: 'chef-inspec-gcp').where(firewall_direction: 'INGRESS') do
  it { should_not exist }
end

Copy link
Author

Choose a reason for hiding this comment

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

thanks, will update the example


describe google_compute_firewalls(project: 'chef-inspec-gcp') do
its('firewall_directions') { should_not include "INGRESS" }
end

<br>

## Filter Criteria

This resource currently does not support any filter criteria; it will always fetch all available firewalls.
Copy link
Contributor

Choose a reason for hiding this comment

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

Actually, you have firewall_id, firewall_name, and firewall_direction. You can use any of them with where, as a block or as a method.

Please read https://github.com/inspec/inspec/blob/master/docs/dev/filtertable-usage.md

Copy link
Author

Choose a reason for hiding this comment

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

thanks, likely another copy/paste oversight, will update accordingly


## Properties
Copy link
Contributor

Choose a reason for hiding this comment

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

Let's go ahead and ship this, but typically we include one example per property. Users links straight to the property, then copy and paste the example.


* `firewall_id`, `firewall_name`, `firewall_directions`
Copy link
Contributor

Choose a reason for hiding this comment

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

The properties are all the plural forms - so firewall_ids, firewall_names. Typical would be to explicitly document each, describing what they are (likely arrays of strings, but what do the Strings mean?)

Copy link
Author

Choose a reason for hiding this comment

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

sure


<br>


## GCP Permissions

Ensure the [Compute Engine API](https://console.cloud.google.com/apis/library/compute.googleapis.com/) is enabled for the project where the resource is located.
8 changes: 7 additions & 1 deletion docs/resources/google_compute_instance.md.erb
Original file line number Diff line number Diff line change
Expand Up @@ -75,11 +75,17 @@ The following examples show how to use this InSpec audit resource.
its('first_network_interface_type'){ should eq "one_to_one_nat" }
end

### Test that a particular compute instance label key is present

describe google_compute_instance(project: 'chef-inspec-gcp', zone: 'us-east1-b', name: 'inspec-test-vm') do
Copy link
Contributor

Choose a reason for hiding this comment

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

This seems to be an unrelated change to the PR

Copy link
Author

Choose a reason for hiding this comment

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

was related to the upcoming blog post example that uses the plural resources

its('labels_keys') { should include 'my_favourite_label' }
end

<br>

## Properties

* `cpu_platform`, `creation_timestamp`, `deletion_protection`, `disks`, `id`, `kind`, `label_fingerprint`, `machine_type`, `metadata`, `name`, `network_interfaces`, `scheduling`, `start_restricted`, `status`, `tags`, `zone`
* `cpu_platform`, `creation_timestamp`, `deletion_protection`, `disks`, `id`, `kind`, `label_fingerprint`, `machine_type`, `metadata`, `name`, `network_interfaces`, `scheduling`, `start_restricted`, `status`, `tags`, `zone`, `labels_keys`, `labels_values`
Copy link
Contributor

Choose a reason for hiding this comment

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

We should talk about a tagging / label API. Azure does it with dynamic methods; AWS doesn't really do it at all yet. We should do it in a similar way, and I think a custom matcher might make more sense than a pair of properties.

Copy link
Author

Choose a reason for hiding this comment

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

that would be great - let's start with what's there and iterate

Copy link
Contributor

Choose a reason for hiding this comment

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

I'd be reluctant to add anything you want to remove later. It's very hard to deprecate once thing are in the wild. Since this isn't in core, let's ship it, but keep in mind if we have doubts about something, better to not expose it.

Copy link
Contributor

Choose a reason for hiding this comment

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


<br>

Expand Down
56 changes: 56 additions & 0 deletions docs/resources/google_compute_zone.md.erb
Original file line number Diff line number Diff line change
@@ -0,0 +1,56 @@
---
title: About the google_compute_zone Resource
platform: gcp
---

# google\_compute\_zone

Use the `google_compute_zone` InSpec audit resource to test properties of a single GCP compute zone.

<br>

## Syntax

A `google_compute_zone` resource block declares the tests for a single GCP zone by project and name.

describe google_compute_zone(project: 'chef-inspec-gcp', zone: 'us-east1-b') do
it { should exist }
its('name') { should match 'us-east1-b' }
end

<br>

## Examples

The following examples show how to use this InSpec audit resource.

### Test that a GCP compute zone does not exist

Choose a reason for hiding this comment

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

Suggest flipping logic to test that a specified zone does exist.

Copy link
Author

Choose a reason for hiding this comment

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

sure can do - thanks


describe google_compute_zone(project: 'chef-inspec-gcp', zone: 'us-east1-b') do
it { should_not exist }
end

### Test that a GCP compute zone is in the expected state

describe google_compute_zone(project: 'chef-inspec-gcp', zone: 'us-east1-b') do
Copy link
Contributor

Choose a reason for hiding this comment

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

This seems like a great chance for a bit of UX sugar. If you defined a method def up? ..., you'd have a be_up matcher, and you can say

describe google_compute_zone(...) do
  it { should be_up }
end

Copy link
Author

Choose a reason for hiding this comment

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

will add this - much more readable

its('status') { should eq 'UP' }
end

### Test that a GCP compute zone has an expected CPU platform

describe google_compute_zone(project: 'chef-inspec-gcp', zone: 'us-east1-b') do
its('available_cpu_platforms') { should include "Intel Skylake" }
end

<br>

## Properties

* `available_cpu_platforms`, `creation_timestamp`, `description`, `id`, `kind`, `name`, `region`, `status`, `region_name`

<br>


## GCP Permissions

Ensure the [Compute Engine API](https://console.cloud.google.com/apis/library/compute.googleapis.com/) is enabled for the project where the resource is located.
75 changes: 75 additions & 0 deletions docs/resources/google_compute_zones.md.erb
Original file line number Diff line number Diff line change
@@ -0,0 +1,75 @@
---
title: About the google_compute_zones Resource
platform: gcp
---

# google\_compute\_zones

Use the `google_compute_zones` InSpec audit resource to test properties of all GCP compute zones for a project in a particular zone.
Copy link
Contributor

Choose a reason for hiding this comment

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

...all or a filtered group of ...

Copy link
Author

Choose a reason for hiding this comment

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

will add


<br>

## Syntax

A `google_compute_zones` resource block collects GCP zones by project then tests that group.

describe google_compute_zones(project: 'chef-inspec-gcp') do
it { should exist }
end

Use this InSpec resource to enumerate IDs then test in-depth using `google_compute_zone`.

google_compute_zones(project: 'chef-inspec-gcp').zone_names.each do |zone_name|
describe google_compute_zone(project: 'chef-inspec-gcp', zone: zone_name) do
it { should exist }
its('kind') { should eq "compute#zone" }
its('status') { should eq 'UP' }
end
end

<br>

## Examples

The following examples show how to use this InSpec audit resource.

### Test that there are no more than a specified number of zones available for the project

describe google_compute_zones(project: 'chef-inspec-gcp') do
Copy link
Contributor

Choose a reason for hiding this comment

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

So, similar comments as on firewalls. Use count instead of entries.count. You do actually have filter criteria. Your properties are plural.

Copy link
Author

Choose a reason for hiding this comment

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

agreed

its('entries.count') { should be <= 100}
end

### Test the exact number of zones in the project

describe google_compute_zones(project: 'chef-inspec-gcp') do
its('zone_ids.count') { should cmp 9 }
end

### Test that an expected zone is available for the project

describe google_compute_zones(project: 'chef-inspec-gcp') do
its('zone_names') { should include "us-east1-b" }
end

### Test whether any zones are in status "DOWN"

describe google_compute_zones(project: 'chef-inspec-gcp') do
its('zone_statuses') { should_not include "DOWN" }
end

<br>

## Filter Criteria

This resource currently does not support any filter criteria; it will always fetch all available zones.

## Properties

* `zone_id`, `zone_name`, `zone_statuses`

<br>


## GCP Permissions

Ensure the [Compute Engine API](https://console.cloud.google.com/apis/library/compute.googleapis.com/) is enabled for the project where the resource is located.
6 changes: 5 additions & 1 deletion libraries/google_compute_firewall.rb
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ class GoogleComputeFirewall < GcpResourceBase
desc 'Verifies settings for a compute firewall rule'

example "
describe google_compute_firewall(project: 'chef-inspec-gcp', location: 'us-west2', name: 'gcp-inspec-test') do
describe google_compute_firewall(project: 'chef-inspec-gcp', name: 'gcp-inspec-test') do
it { should exist }
its('name') { should eq 'inspec-test' }
its('status') { should eq 'in_use' }
Expand Down Expand Up @@ -51,6 +51,10 @@ def ports_protocol_allowed(port_list, protocol = 'tcp', index = 0)
end
end

def exists?
[email protected]?
end

def to_s
"Firewall Rule #{@display_name}"
end
Expand Down
50 changes: 50 additions & 0 deletions libraries/google_compute_firewalls.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
# frozen_string_literal: true

require 'gcp_backend'

module Inspec::Resources
class GoogleComputeFirewalls < GcpResourceBase
name 'google_compute_firewalls'
desc 'Verifies settings for GCP compute firewalls in bulk'

example "
describe google_compute_firewalls(project: 'chef-inspec-gcp') do
it { should exist }
...
end
"

def initialize(opts = {})
# Call the parent class constructor
super(opts)
@display_name = opts[:name]
@project = opts[:project]
end

# FilterTable setup
filter_table_config = FilterTable.create
filter_table_config.add_accessor(:where)
filter_table_config.add_accessor(:entries)
filter_table_config.add(:exist?) { |filter_table| !filter_table.entries.empty? }
filter_table_config.add(:count) { |filter_table| filter_table.entries.count }
Copy link
Contributor

Choose a reason for hiding this comment

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

With inspec/inspec#3104 we don't need to add where, entries, exist? or count properties to filter tables, they are now automatic.

Copy link
Author

Choose a reason for hiding this comment

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

Happy to do this but worth noting all profiles using inspec-gcp will now have to update the minimum InSpec version because of inspec/inspec#3066

filter_table_config.add(:firewall_ids, field: :firewall_id)
filter_table_config.add(:firewall_names, field: :firewall_name)
filter_table_config.add(:firewall_directions, field: :firewall_direction)
Copy link
Contributor

Choose a reason for hiding this comment

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

This is good as-is, but you might consider adding style: :simple - having this de-duplicated might be useful (or at least having individual entries for each rule isn't very useful).

filter_table_config.add(:colors, field: :color, type: :simple)
Copy link
Contributor

Choose a reason for hiding this comment

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

Colors appears to be copypasta. Check your other plurals as well.

Copy link
Author

Choose a reason for hiding this comment

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

yup, removed

filter_table_config.connect(self, :fetch_data)

def fetch_data
firewall_rows = []
catch_gcp_errors do
@firewalls = @gcp.gcp_compute_client.list_firewalls(@project)
Copy link
Contributor

Choose a reason for hiding this comment

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

Do you need to paginate?

Choose a reason for hiding this comment

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

The API supports pagination [https://cloud.google.com/compute/docs/reference/rest/v1/firewalls/list] but the default is 500 so it's probably fine to skip that for now.

Copy link
Author

@skpaterson skpaterson Jun 12, 2018

Choose a reason for hiding this comment

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

thanks - just saw all the comments, will add pagination as it shouldn't take too much

end
return [] if [email protected]
@firewalls.items.map do |firewall|
firewall_rows+=[{ firewall_id: firewall.id,
firewall_name: firewall.name,
firewall_direction: firewall.direction }]
end
@table = firewall_rows
end
end
end
12 changes: 12 additions & 0 deletions libraries/google_compute_instance.rb
Original file line number Diff line number Diff line change
Expand Up @@ -98,6 +98,18 @@ def machine_size
machine_type.split('/').last
end

# helper for returning label keys to perform checks
def labels_keys
return [] if !defined?(labels)
labels.item.keys
end

# helper for returning label values to perform checks
def labels_values
return [] if !defined?(labels)
labels.item.values
end

def exists?
[email protected]?
end
Expand Down
1 change: 1 addition & 0 deletions libraries/google_compute_instances.rb
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ def fetch_data
catch_gcp_errors do
@instances = @gcp.gcp_compute_client.list_instances(@project, @zone)
end
return [] if [email protected]
@instances.items.map do |instance|
instance_rows+=[{ instance_id: instance.id,
instance_name: instance.name }]
Expand Down
41 changes: 41 additions & 0 deletions libraries/google_compute_zone.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
# frozen_string_literal: true

require 'gcp_backend'

module Inspec::Resources
class GoogleComputeZone < GcpResourceBase
name 'google_compute_zone'
desc 'Verifies settings for a zone'

example "
describe google_compute_zone(project: 'chef-inspec-gcp', zone: 'us-east1-b') do
it { should exist }
its('name') { should match 'us-east1-b' }
end
"

def initialize(opts = {})
# Call the parent class constructor
super(opts)
@display_name = opts[:name]
catch_gcp_errors do
@zone = @gcp.gcp_compute_client.get_zone(opts[:project], opts[:name])
create_resource_methods(@zone)
end
end

# helper method for retrieving a region name
def region_name
return false if !defined?(region)
region.split('/').last
end

def exists?
[email protected]?
end

def to_s
"Zone #{@display_name}"
end
end
end
Loading