From 5b3012e9598e3240cc7adf2ec909c2ac7379f761 Mon Sep 17 00:00:00 2001 From: Alceu Rodrigues de Freitas Junior Date: Sat, 7 Aug 2021 23:45:45 -0300 Subject: [PATCH 01/10] feat: required exceptions for SG --- lib/awspec/error.rb | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/lib/awspec/error.rb b/lib/awspec/error.rb index a05e850c0..67d698da2 100644 --- a/lib/awspec/error.rb +++ b/lib/awspec/error.rb @@ -29,4 +29,18 @@ def initialize(resource_type, id) # Let the user know the configuration they provided is not known. class UnknownConfiguration < StandardError end + + class MissingPortSpecification < StandardError + def initialize(default_protocol) + msg = "Port must be specificed unless protocol is #{default_protocol}" + super msg + end + end + + class InvalidPortRange < StandardError + def initialize(port, from_port, to_port) + msg = "Found a port range from #{from_port} to #{to_port}, but cannot compare it with '#{port}'" + super msg + end + end end From 14b9e9ac82e2f0b079d5dcd0b6815bfd1d3cd660 Mon Sep 17 00:00:00 2001 From: Alceu Rodrigues de Freitas Junior Date: Sat, 7 Aug 2021 23:48:06 -0300 Subject: [PATCH 02/10] fix: properly handling invalid arguments Some specs had to be rewritten and this will probably be not compatible with specs written in the old style. --- lib/awspec/type/security_group.rb | 40 ++++++++++++++---- spec/type/security_group_spec.rb | 69 +++++++++++++++++++++++-------- 2 files changed, 84 insertions(+), 25 deletions(-) diff --git a/lib/awspec/type/security_group.rb b/lib/awspec/type/security_group.rb index 12be625a5..61a57eb5a 100644 --- a/lib/awspec/type/security_group.rb +++ b/lib/awspec/type/security_group.rb @@ -4,6 +4,9 @@ class SecurityGroup < ResourceBase aws_resource Aws::EC2::SecurityGroup tags_allowed + DEFAULT_PROTOCOL = '-1' + DEFAULT_ROUTE = '0.0.0.0/0' + def resource_via_client @resource_via_client ||= find_security_group(@display_name) end @@ -22,10 +25,17 @@ def opened_only?(port = nil, protocol = nil, cidr = nil) outbound_opened_only?(port, protocol, cidr) end - def inbound_opened?(port = nil, protocol = nil, cidr = nil) + def inbound_opened?(port, protocol, cidr = DEFAULT_ROUTE) + # https://stackoverflow.com/questions/39021545/how-to-specify-all-ports-in-security-group-cloudformation + raise Awspec::MissingPortSpecification, DEFAULT_PROTOCOL if port.nil? && protocol != DEFAULT_PROTOCOL + # a security group will accept everything from itself + return true if cidr == id + resource_via_client.ip_permissions.find do |permission| - cidr_opened?(permission, cidr) && protocol_opened?(permission, protocol) && port_opened?(permission, port) + return true if cidr_opened?(permission, cidr) && protocol_opened?(permission, protocol) && port_opened?(permission, port) end + + false end def inbound_opened_only?(port = nil, protocol = nil, cidr = nil) @@ -40,6 +50,12 @@ def inbound_opened_only?(port = nil, protocol = nil, cidr = nil) end def outbound_opened?(port = nil, protocol = nil, cidr = nil) + # https://stackoverflow.com/questions/39021545/how-to-specify-all-ports-in-security-group-cloudformation + raise Awspec::MissingPortSpecification, DEFAULT_PROTOCOL if port.nil? && protocol != DEFAULT_PROTOCOL + + # a security group will accept everything from itself + return true if cidr == id + resource_via_client.ip_permissions_egress.find do |permission| cidr_opened?(permission, cidr) && protocol_opened?(permission, protocol) && port_opened?(permission, port) end @@ -103,11 +119,13 @@ def outbound_rule_count private def cidr_opened?(permission, cidr) - return true unless cidr + return false if cidr.nil? + ret = permission.prefix_list_ids.select do |prefix_list_id| prefix_list_id.prefix_list_id == cidr end return true if ret.count > 0 + ret = permission.ip_ranges.select do |ip_range| # if the cidr is an IP address then do a true CIDR match if cidr =~ /^\d+\.\d+\.\d+\.\d+/ @@ -118,6 +136,7 @@ def cidr_opened?(permission, cidr) end end return true if ret.count > 0 + ret = permission.user_id_group_pairs.select do |sg| # Compare the sg group_name if the remote group is in another account. # find_security_group call doesn't return info on a remote security group. @@ -136,25 +155,32 @@ def cidr_opened?(permission, cidr) end def protocol_opened?(permission, protocol) - return true unless protocol return false if protocol == 'all' && permission.ip_protocol != '-1' return true if permission.ip_protocol == '-1' permission.ip_protocol == protocol end def port_opened?(permission, port) - return true unless port return true unless permission.from_port return true unless permission.to_port port_between?(port, permission.from_port, permission.to_port) end def port_between?(port, from_port, to_port) - if port.is_a?(String) && port.include?('-') + if port.is_a?(String) && port.include?('-') && port =~ /^\d+-\d+$/ f, t = port.split('-') from_port == f.to_i && to_port == t.to_i else - port.between?(from_port, to_port) + if port.is_a?(String) + if port =~ /^\d+$/ + converted = port.to_i + converted.between?(from_port, to_port) + else + raise Awspec::InvalidPortRange.new(port, from_port, to_port) + end + else + port.between?(from_port, to_port) + end end end diff --git a/spec/type/security_group_spec.rb b/spec/type/security_group_spec.rb index 4587d9509..3b073220a 100644 --- a/spec/type/security_group_spec.rb +++ b/spec/type/security_group_spec.rb @@ -1,25 +1,58 @@ require 'spec_helper' +require 'awspec/error' + Awspec::Stub.load 'security_group' describe security_group('sg-1a2b3cd4') do it { should exist } - its(:inbound) { should be_opened(80) } + it do + # only way found to force and test the exception + subject.inbound + expect { subject.opened?(nil) }.to raise_error(Awspec::MissingPortSpecification, /protocol\sis\s-1$/) + end + its(:inbound) { should_not be_opened(80) } + its(:inbound) { should_not be_opened(80).protocol(-1) } + its(:inbound) { should be_opened(80).protocol(-1).for('sg-3a4b5cd6') } its(:inbound) { should be_opened(80).protocol('tcp').for('123.45.68.89/32') } its(:inbound) { should be_opened(80).protocol('tcp').for('123.45.67.0/25') } its(:inbound) { should be_opened(80).protocol('tcp').for('123.45.67.1/32') } its(:inbound) { should_not be_opened(80).protocol('tcp').for('123.45.0.0/16') } - its(:inbound) { should be_opened(22) } + + its(:inbound) { should be_opened(22).protocol('tcp').for('sg-1a2b3cd4') } + its(:inbound) { should be_opened(22).protocol('udp').for('sg-1a2b3cd4') } + its(:inbound) { should be_opened(22).protocol('-1').for('sg-1a2b3cd4') } + its(:inbound) { should be_opened(22).protocol('icmp').for('sg-1a2b3cd4') } + its(:inbound) { should be_opened(22).protocol('tcp').for('sg-5a6b7cd8') } its(:inbound) { should be_opened('50000-50009').protocol('tcp').for('123.45.67.89/32') } its(:inbound) { should_not be_opened('50010-50019').protocol('tcp').for('123.45.67.89/32') } - its(:outbound) { should be_opened(50_000) } + + its(:outbound) { should be_opened(50_000).protocol('tcp').for('sg-1a2b3cd4') } + its(:outbound) { should be_opened(50_000).protocol('udp').for('sg-1a2b3cd4') } + its(:outbound) { should be_opened(50_000).protocol('icmp').for('sg-1a2b3cd4') } + its(:outbound) { should be_opened(50_000).protocol('-1').for('sg-1a2b3cd4') } + its(:outbound) { should be_opened(8080).protocol('tcp').for('sg-9a8b7c6d') } its(:outbound) { should be_opened(8080).protocol('tcp').for('group-in-other-aws-account-with-vpc-peering') } its(:inbound) { should be_opened_only(60_000).protocol('tcp').for('100.45.67.12/32') } its(:inbound) { should be_opened_only(70_000).protocol('tcp').for(['100.45.67.89/32', '100.45.67.12/32']) } its(:outbound) { should be_opened_only(50_000).protocol('tcp').for('100.45.67.12/32') } - its(:inbound) { should be_opened.protocol('all').for('sg-3a4b5cd6') } - its(:outbound) { should be_opened.protocol('tcp').for('pl-a5321fa3') } + its(:inbound) { should be_opened.protocol('-1').for('sg-3a4b5cd6') } + + it do + # only way found to force and test the exception + subject.outbound + expect { subject.opened?(nil, 'tcp', 'pl-a5321fa3') }.to raise_error(Awspec::MissingPortSpecification, /protocol\sis\s-1$/) + end + its(:outbound) { should be_opened('443').protocol('tcp').for('pl-a5321fa3') } + its(:outbound) { should be_opened(443).protocol('tcp').for('pl-a5321fa3') } + its(:outbound) { should be_opened('443-443').protocol('tcp').for('pl-a5321fa3') } + it do + # only way found to force and test the exception + subject.outbound + expect { subject.opened?('yada-443-yada', 'tcp', 'pl-a5321fa3') }.to raise_error(Awspec::InvalidPortRange) + end + its(:inbound_permissions_count) { should eq 7 } its(:ip_permissions_count) { should eq 7 } its(:outbound_permissions_count) { should eq 3 } @@ -93,16 +126,16 @@ it { should have_outbound_rule({ ip_protocol: 'tcp', from_port: 443, to_port: 443, prefix_list_id: 'pl-a5321fa3' }) } end -describe security_group('my-security-group-name') do - it { should exist } - its(:outbound) { should be_opened(50_000) } - its(:inbound) { should be_opened(80) } - it { should belong_to_vpc('my-vpc') } - it { should have_tag('env').value('dev') } -end - -describe security_group('my-security-tag-name') do - its(:outbound) { should be_opened(50_000) } - its(:inbound) { should be_opened(80) } - it { should belong_to_vpc('my-vpc') } -end +# describe security_group('my-security-group-name') do +# it { should exist } +# its(:outbound) { should be_opened(50_000) } +# its(:inbound) { should be_opened(80) } +# it { should belong_to_vpc('my-vpc') } +# it { should have_tag('env').value('dev') } +# end +# +# describe security_group('my-security-tag-name') do +# its(:outbound) { should be_opened(50_000) } +# its(:inbound) { should be_opened(80) } +# it { should belong_to_vpc('my-vpc') } +# end From 5272f58b381e887b0e4215017e9afb9702f2178b Mon Sep 17 00:00:00 2001 From: Alceu Rodrigues de Freitas Junior Date: Sun, 22 Aug 2021 20:59:21 -0300 Subject: [PATCH 03/10] fix: using specific be_opened declaration --- spec/type/security_group_spec.rb | 19 ++++++++++++------- 1 file changed, 12 insertions(+), 7 deletions(-) diff --git a/spec/type/security_group_spec.rb b/spec/type/security_group_spec.rb index 3b073220a..c4df9586b 100644 --- a/spec/type/security_group_spec.rb +++ b/spec/type/security_group_spec.rb @@ -126,13 +126,18 @@ it { should have_outbound_rule({ ip_protocol: 'tcp', from_port: 443, to_port: 443, prefix_list_id: 'pl-a5321fa3' }) } end -# describe security_group('my-security-group-name') do -# it { should exist } -# its(:outbound) { should be_opened(50_000) } -# its(:inbound) { should be_opened(80) } -# it { should belong_to_vpc('my-vpc') } -# it { should have_tag('env').value('dev') } -# end +describe security_group('my-security-group-name') do + it { should exist } + its(:outbound) { should_not be_opened(50_000) } + its(:outbound) { should_not be_opened(50_000).protocol('tcp') } + its(:outbound) { should be_opened(50_000).protocol('tcp').target('100.45.67.12/32') } + its(:inbound) { should_not be_opened(80) } + its(:inbound) { should_not be_opened(80).protocol('tcp') } + its(:inbound) { should be_opened(80).protocol('tcp').for('123.45.67.0/24') } + its(:inbound) { should be_opened(80).protocol('tcp').for('123.45.68.89/32') } + it { should belong_to_vpc('my-vpc') } + it { should have_tag('env').value('dev') } +end # # describe security_group('my-security-tag-name') do # its(:outbound) { should be_opened(50_000) } From 6be52d74389780bc062d29dfa5ddf52b2d470395 Mon Sep 17 00:00:00 2001 From: Alceu Rodrigues de Freitas Junior Date: Mon, 23 Aug 2021 01:14:57 -0300 Subject: [PATCH 04/10] feat: custom description for be_opened --- lib/awspec/matcher/be_opened.rb | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/lib/awspec/matcher/be_opened.rb b/lib/awspec/matcher/be_opened.rb index 7f5182d1c..f93dc1606 100644 --- a/lib/awspec/matcher/be_opened.rb +++ b/lib/awspec/matcher/be_opened.rb @@ -1,8 +1,17 @@ +DEFAULT_PROTOCOL = '-1' +DEFAULT_ROUTE = '0.0.0.0/0' + RSpec::Matchers.define :be_opened do |port| match do |sg| sg.opened?(port, @protocol, @cidr) end + description do + @protocol = DEFAULT_PROTOCOL if @protocol.nil? + @cidr = DEFAULT_ROUTE if @cidr.nil? + "to be opened on port #{port}, protocol \"#{@protocol}\", to/from #{@cidr}" + end + chain :protocol do |protocol| @protocol = protocol end From efb6bd6b40cae9f2bdc20109830d858f82964ffc Mon Sep 17 00:00:00 2001 From: "alceu.freitas" Date: Mon, 23 Aug 2021 14:04:54 -0300 Subject: [PATCH 05/10] doc: defaults section included --- doc/_resource_types/security_group.md | 19 ++++++++++++++- doc/resource_types.md | 35 +++++++++++++-------------- 2 files changed, 35 insertions(+), 19 deletions(-) diff --git a/doc/_resource_types/security_group.md b/doc/_resource_types/security_group.md index c3a74f6c8..a3803c443 100644 --- a/doc/_resource_types/security_group.md +++ b/doc/_resource_types/security_group.md @@ -19,13 +19,30 @@ end ```ruby describe security_group('my-security-group-name') do its(:outbound) { should be_opened } + its(:outbound) { should be_opened(443).protocol('tcp').target('203.0.113.1/32') } its(:inbound) { should be_opened(80) } its(:inbound) { should be_opened(80).protocol('tcp').for('203.0.113.1/32') } its(:inbound) { should be_opened(22).protocol('tcp').for('sg-5a6b7cd8') } end ``` -### advanced +#### Defaults for `:inbound` and `:outbound` + +Beware for defaults when not specifying protocol (`protocol()`) and/or the CIDR +(`for()` or `target()`). + +If no protocol is provided, it is assumed to be `-1`. +If no CIDR is provided, it is assumed to be `0.0.0.0/0`. + +Of course, if your current security group being tested does not include those +defaults your spec will fail to match with `should be_opened`. + +In most cases, you will want to at least provide the port and protocol. + +If no port number is provided, the `MissingPortSpecification` exception will be +raised. + +### Advanced `security_group` can use `Aws::EC2::SecurityGroup` resource (see http://docs.aws.amazon.com/sdkforruby/api/Aws/EC2/SecurityGroup.html). diff --git a/doc/resource_types.md b/doc/resource_types.md index df9c41a01..12f9c2085 100644 --- a/doc/resource_types.md +++ b/doc/resource_types.md @@ -360,7 +360,7 @@ describe autoscaling_group('my-auto-scaling-group') do end ``` -### its(:auto_scaling_group_name), its(:auto_scaling_group_arn), its(:launch_configuration_name), its(:launch_template), its(:mixed_instances_policy), its(:min_size), its(:max_size), its(:desired_capacity), its(:predicted_capacity), its(:default_cooldown), its(:availability_zones), its(:load_balancer_names), its(:target_group_arns), its(:health_check_type), its(:health_check_grace_period), its(:created_time), its(:placement_group), its(:vpc_zone_identifier), its(:enabled_metrics), its(:status), its(:termination_policies), its(:new_instances_protected_from_scale_in), its(:service_linked_role_arn), its(:max_instance_lifetime), its(:capacity_rebalance), its(:warm_pool_configuration), its(:warm_pool_size), its(:context) +### its(:auto_scaling_group_name), its(:auto_scaling_group_arn), its(:launch_configuration_name), its(:launch_template), its(:mixed_instances_policy), its(:min_size), its(:max_size), its(:desired_capacity), its(:predicted_capacity), its(:default_cooldown), its(:availability_zones), its(:load_balancer_names), its(:target_group_arns), its(:health_check_type), its(:health_check_grace_period), its(:created_time), its(:placement_group), its(:vpc_zone_identifier), its(:enabled_metrics), its(:status), its(:termination_policies), its(:new_instances_protected_from_scale_in), its(:service_linked_role_arn), its(:max_instance_lifetime), its(:capacity_rebalance), its(:warm_pool_configuration), its(:warm_pool_size) ## batch_compute_environment BatchComputeEnvironment resource type. @@ -845,7 +845,7 @@ describe directconnect_virtual_interface('my-directconnect-virtual-interface') d end ``` -### its(:owner_account), its(:virtual_interface_id), its(:location), its(:connection_id), its(:virtual_interface_type), its(:virtual_interface_name), its(:vlan), its(:asn), its(:amazon_side_asn), its(:auth_key), its(:amazon_address), its(:customer_address), its(:address_family), its(:virtual_interface_state), its(:customer_router_config), its(:mtu), its(:jumbo_frame_capable), its(:virtual_gateway_id), its(:direct_connect_gateway_id), its(:route_filter_prefixes), its(:bgp_peers), its(:region), its(:aws_device_v2), its(:aws_logical_device_id), its(:tags) +### its(:owner_account), its(:virtual_interface_id), its(:location), its(:connection_id), its(:virtual_interface_type), its(:virtual_interface_name), its(:vlan), its(:asn), its(:amazon_side_asn), its(:auth_key), its(:amazon_address), its(:customer_address), its(:address_family), its(:virtual_interface_state), its(:customer_router_config), its(:mtu), its(:jumbo_frame_capable), its(:virtual_gateway_id), its(:direct_connect_gateway_id), its(:route_filter_prefixes), its(:bgp_peers), its(:region), its(:aws_device_v2), its(:tags) ## dynamodb_table DynamodbTable resource type. @@ -2626,7 +2626,7 @@ describe network_interface('eni-12ab3cde') do end ``` -### its(:association), its(:availability_zone), its(:description), its(:interface_type), its(:ipv_6_addresses), its(:mac_address), its(:network_interface_id), its(:outpost_arn), its(:owner_id), its(:private_dns_name), its(:private_ip_address), its(:ipv_4_prefixes), its(:ipv_6_prefixes), its(:requester_id), its(:requester_managed), its(:source_dest_check), its(:status), its(:subnet_id), its(:vpc_id) +### its(:association), its(:availability_zone), its(:description), its(:interface_type), its(:ipv_6_addresses), its(:mac_address), its(:network_interface_id), its(:outpost_arn), its(:owner_id), its(:private_dns_name), its(:private_ip_address), its(:requester_id), its(:requester_managed), its(:source_dest_check), its(:status), its(:subnet_id), its(:vpc_id) ## nlb NLB resource type. @@ -2854,7 +2854,7 @@ end ``` -### its(:vpc_id), its(:db_instance_identifier), its(:db_instance_class), its(:engine), its(:db_instance_status), its(:automatic_restart_time), its(:master_username), its(:db_name), its(:endpoint), its(:allocated_storage), its(:instance_create_time), its(:preferred_backup_window), its(:backup_retention_period), its(:db_security_groups), its(:availability_zone), its(:preferred_maintenance_window), its(:pending_modified_values), its(:latest_restorable_time), its(:multi_az), its(:engine_version), its(:auto_minor_version_upgrade), its(:read_replica_source_db_instance_identifier), its(:read_replica_db_instance_identifiers), its(:read_replica_db_cluster_identifiers), its(:replica_mode), its(:license_model), its(:iops), its(:character_set_name), its(:nchar_character_set_name), its(:secondary_availability_zone), its(:publicly_accessible), its(:status_infos), its(:storage_type), its(:tde_credential_arn), its(:db_instance_port), its(:db_cluster_identifier), its(:storage_encrypted), its(:kms_key_id), its(:dbi_resource_id), its(:ca_certificate_identifier), its(:domain_memberships), its(:copy_tags_to_snapshot), its(:monitoring_interval), its(:enhanced_monitoring_resource_arn), its(:monitoring_role_arn), its(:promotion_tier), its(:db_instance_arn), its(:timezone), its(:iam_database_authentication_enabled), its(:performance_insights_enabled), its(:performance_insights_kms_key_id), its(:performance_insights_retention_period), its(:enabled_cloudwatch_logs_exports), its(:processor_features), its(:deletion_protection), its(:associated_roles), its(:listener_endpoint), its(:max_allocated_storage), its(:tag_list), its(:db_instance_automated_backups_replications), its(:customer_owned_ip_enabled), its(:aws_backup_recovery_point_arn), its(:activity_stream_status), its(:activity_stream_kms_key_id), its(:activity_stream_kinesis_stream_name), its(:activity_stream_mode), its(:activity_stream_engine_native_audit_fields_included) +### its(:vpc_id), its(:db_instance_identifier), its(:db_instance_class), its(:engine), its(:db_instance_status), its(:master_username), its(:db_name), its(:endpoint), its(:allocated_storage), its(:instance_create_time), its(:preferred_backup_window), its(:backup_retention_period), its(:db_security_groups), its(:availability_zone), its(:preferred_maintenance_window), its(:pending_modified_values), its(:latest_restorable_time), its(:multi_az), its(:engine_version), its(:auto_minor_version_upgrade), its(:read_replica_source_db_instance_identifier), its(:read_replica_db_instance_identifiers), its(:read_replica_db_cluster_identifiers), its(:replica_mode), its(:license_model), its(:iops), its(:character_set_name), its(:nchar_character_set_name), its(:secondary_availability_zone), its(:publicly_accessible), its(:status_infos), its(:storage_type), its(:tde_credential_arn), its(:db_instance_port), its(:db_cluster_identifier), its(:storage_encrypted), its(:kms_key_id), its(:dbi_resource_id), its(:ca_certificate_identifier), its(:domain_memberships), its(:copy_tags_to_snapshot), its(:monitoring_interval), its(:enhanced_monitoring_resource_arn), its(:monitoring_role_arn), its(:promotion_tier), its(:db_instance_arn), its(:timezone), its(:iam_database_authentication_enabled), its(:performance_insights_enabled), its(:performance_insights_kms_key_id), its(:performance_insights_retention_period), its(:enabled_cloudwatch_logs_exports), its(:processor_features), its(:deletion_protection), its(:associated_roles), its(:listener_endpoint), its(:max_allocated_storage), its(:tag_list), its(:db_instance_automated_backups_replications), its(:customer_owned_ip_enabled), its(:aws_backup_recovery_point_arn), its(:activity_stream_status), its(:activity_stream_kms_key_id), its(:activity_stream_kinesis_stream_name), its(:activity_stream_mode), its(:activity_stream_engine_native_audit_fields_included) ### :unlock: Advanced use `rds` can use `Aws::RDS::DBInstance` resource (see http://docs.aws.amazon.com/sdkforruby/api/Aws/RDS/DBInstance.html). @@ -3349,32 +3349,31 @@ end ```ruby describe security_group('my-security-group-name') do its(:outbound) { should be_opened } + its(:outbound) { should be_opened(443).protocol('tcp').target('203.0.113.1/32') } its(:inbound) { should be_opened(80) } its(:inbound) { should be_opened(80).protocol('tcp').for('203.0.113.1/32') } its(:inbound) { should be_opened(22).protocol('tcp').for('sg-5a6b7cd8') } end ``` +#### Defaults for `:inbound` and `:outbound` -### its(:inbound_rule_count), its(:outbound_rule_count), its(:inbound_permissions_count), its(:outbound_permissions_count), its(:description), its(:group_name), its(:owner_id), its(:group_id), its(:vpc_id) -### :unlock: Advanced use +Beware for defaults when not specifying protocol (`protocol()`) and/or the CIDR +(`for()` or `target()`). -`security_group` can use `Aws::EC2::SecurityGroup` resource (see http://docs.aws.amazon.com/sdkforruby/api/Aws/EC2/SecurityGroup.html). +If no protocol is provided, it is assumed to be `-1`. +If no CIDR is provided, it is assumed to be `0.0.0.0/0`. -```ruby -describe security_group('my-security-group-name') do - its('group_name') { should eq 'my-security-group-name' } -end -``` +Of course, if your current security group being tested does not include those +defaults your spec will fail to match with `should be_opened`. -or +In most cases, you will want to at least provide the port and protocol. + +If no port number is provided, the `MissingPortSpecification` exception will be +raised. -```ruby -describe security_group('my-security-group-name') do - its('resource.group_name') { should eq 'my-security-group-name' } -end -``` +### its(:inbound_rule_count), its(:outbound_rule_count), its(:inbound_permissions_count), its(:outbound_permissions_count), its(:description), its(:group_name), its(:owner_id), its(:group_id), its(:vpc_id) ## ses_identity SesIdentity resource type. From 58221ff04892319b1e7c770fa458a302cd63149b Mon Sep 17 00:00:00 2001 From: "alceu.freitas" Date: Mon, 23 Aug 2021 14:15:56 -0300 Subject: [PATCH 06/10] feat: extended tests to support DEFAULT_ROUTE --- lib/awspec/stub/security_group.rb | 10 ++++++++++ lib/awspec/type/security_group.rb | 2 +- spec/type/security_group_spec.rb | 14 +++++--------- 3 files changed, 16 insertions(+), 10 deletions(-) diff --git a/lib/awspec/stub/security_group.rb b/lib/awspec/stub/security_group.rb index 49f60dd18..076d0a5b9 100644 --- a/lib/awspec/stub/security_group.rb +++ b/lib/awspec/stub/security_group.rb @@ -112,6 +112,16 @@ } ] }, + { + from_port: 443, + to_port: 443, + ip_protocol: 'tcp', + ip_ranges: [ + { + cidr_ip: '0.0.0.0/0' + } + ] + }, { from_port: 8080, to_port: 8080, diff --git a/lib/awspec/type/security_group.rb b/lib/awspec/type/security_group.rb index 61a57eb5a..a1da3bc1c 100644 --- a/lib/awspec/type/security_group.rb +++ b/lib/awspec/type/security_group.rb @@ -119,7 +119,7 @@ def outbound_rule_count private def cidr_opened?(permission, cidr) - return false if cidr.nil? + cidr = DEFAULT_ROUTE if cidr.nil? ret = permission.prefix_list_ids.select do |prefix_list_id| prefix_list_id.prefix_list_id == cidr diff --git a/spec/type/security_group_spec.rb b/spec/type/security_group_spec.rb index c4df9586b..a1de80dd3 100644 --- a/spec/type/security_group_spec.rb +++ b/spec/type/security_group_spec.rb @@ -55,10 +55,10 @@ its(:inbound_permissions_count) { should eq 7 } its(:ip_permissions_count) { should eq 7 } - its(:outbound_permissions_count) { should eq 3 } - its(:ip_permissions_egress_count) { should eq 3 } + its(:outbound_permissions_count) { should eq 4 } + its(:ip_permissions_egress_count) { should eq 4 } its(:inbound_rule_count) { should eq 8 } - its(:outbound_rule_count) { should eq 2 } + its(:outbound_rule_count) { should eq 3 } it { should belong_to_vpc('vpc-ab123cde') } it { should belong_to_vpc('my-vpc') } @@ -131,6 +131,8 @@ its(:outbound) { should_not be_opened(50_000) } its(:outbound) { should_not be_opened(50_000).protocol('tcp') } its(:outbound) { should be_opened(50_000).protocol('tcp').target('100.45.67.12/32') } + its(:outbound) { should be_opened(443).protocol('tcp').target('0.0.0.0/0') } + its(:outbound) { should be_opened(443).protocol('tcp') } its(:inbound) { should_not be_opened(80) } its(:inbound) { should_not be_opened(80).protocol('tcp') } its(:inbound) { should be_opened(80).protocol('tcp').for('123.45.67.0/24') } @@ -138,9 +140,3 @@ it { should belong_to_vpc('my-vpc') } it { should have_tag('env').value('dev') } end -# -# describe security_group('my-security-tag-name') do -# its(:outbound) { should be_opened(50_000) } -# its(:inbound) { should be_opened(80) } -# it { should belong_to_vpc('my-vpc') } -# end From 89f79383475f1ad6380f823ce9c3a3e1ac5fc20a Mon Sep 17 00:00:00 2001 From: "alceu.freitas" Date: Mon, 23 Aug 2021 14:43:49 -0300 Subject: [PATCH 07/10] fix: rubocop offense --- lib/awspec/type/security_group.rb | 3 ++- spec/type/security_group_spec.rb | 3 ++- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/lib/awspec/type/security_group.rb b/lib/awspec/type/security_group.rb index a1da3bc1c..6bdff28de 100644 --- a/lib/awspec/type/security_group.rb +++ b/lib/awspec/type/security_group.rb @@ -32,7 +32,8 @@ def inbound_opened?(port, protocol, cidr = DEFAULT_ROUTE) return true if cidr == id resource_via_client.ip_permissions.find do |permission| - return true if cidr_opened?(permission, cidr) && protocol_opened?(permission, protocol) && port_opened?(permission, port) + return true if cidr_opened?(permission, cidr) && protocol_opened?(permission, protocol) && port_opened?( + permission, port) end false diff --git a/spec/type/security_group_spec.rb b/spec/type/security_group_spec.rb index a1de80dd3..7734bf963 100644 --- a/spec/type/security_group_spec.rb +++ b/spec/type/security_group_spec.rb @@ -42,7 +42,8 @@ it do # only way found to force and test the exception subject.outbound - expect { subject.opened?(nil, 'tcp', 'pl-a5321fa3') }.to raise_error(Awspec::MissingPortSpecification, /protocol\sis\s-1$/) + expect { subject.opened?(nil, 'tcp', 'pl-a5321fa3') }.to raise_error( + Awspec::MissingPortSpecification, /protocol\sis\s-1$/) end its(:outbound) { should be_opened('443').protocol('tcp').for('pl-a5321fa3') } its(:outbound) { should be_opened(443).protocol('tcp').for('pl-a5321fa3') } From 34766f92c0909b6fa0146db98d31452d2018f0b8 Mon Sep 17 00:00:00 2001 From: "alceu.freitas" Date: Mon, 23 Aug 2021 14:43:58 -0300 Subject: [PATCH 08/10] fix: doc validation --- doc/resource_types.md | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/doc/resource_types.md b/doc/resource_types.md index 12f9c2085..1fed369fa 100644 --- a/doc/resource_types.md +++ b/doc/resource_types.md @@ -360,7 +360,7 @@ describe autoscaling_group('my-auto-scaling-group') do end ``` -### its(:auto_scaling_group_name), its(:auto_scaling_group_arn), its(:launch_configuration_name), its(:launch_template), its(:mixed_instances_policy), its(:min_size), its(:max_size), its(:desired_capacity), its(:predicted_capacity), its(:default_cooldown), its(:availability_zones), its(:load_balancer_names), its(:target_group_arns), its(:health_check_type), its(:health_check_grace_period), its(:created_time), its(:placement_group), its(:vpc_zone_identifier), its(:enabled_metrics), its(:status), its(:termination_policies), its(:new_instances_protected_from_scale_in), its(:service_linked_role_arn), its(:max_instance_lifetime), its(:capacity_rebalance), its(:warm_pool_configuration), its(:warm_pool_size) +### its(:auto_scaling_group_name), its(:auto_scaling_group_arn), its(:launch_configuration_name), its(:launch_template), its(:mixed_instances_policy), its(:min_size), its(:max_size), its(:desired_capacity), its(:predicted_capacity), its(:default_cooldown), its(:availability_zones), its(:load_balancer_names), its(:target_group_arns), its(:health_check_type), its(:health_check_grace_period), its(:created_time), its(:placement_group), its(:vpc_zone_identifier), its(:enabled_metrics), its(:status), its(:termination_policies), its(:new_instances_protected_from_scale_in), its(:service_linked_role_arn), its(:max_instance_lifetime), its(:capacity_rebalance), its(:warm_pool_configuration), its(:warm_pool_size), its(:context) ## batch_compute_environment BatchComputeEnvironment resource type. @@ -845,7 +845,7 @@ describe directconnect_virtual_interface('my-directconnect-virtual-interface') d end ``` -### its(:owner_account), its(:virtual_interface_id), its(:location), its(:connection_id), its(:virtual_interface_type), its(:virtual_interface_name), its(:vlan), its(:asn), its(:amazon_side_asn), its(:auth_key), its(:amazon_address), its(:customer_address), its(:address_family), its(:virtual_interface_state), its(:customer_router_config), its(:mtu), its(:jumbo_frame_capable), its(:virtual_gateway_id), its(:direct_connect_gateway_id), its(:route_filter_prefixes), its(:bgp_peers), its(:region), its(:aws_device_v2), its(:tags) +### its(:owner_account), its(:virtual_interface_id), its(:location), its(:connection_id), its(:virtual_interface_type), its(:virtual_interface_name), its(:vlan), its(:asn), its(:amazon_side_asn), its(:auth_key), its(:amazon_address), its(:customer_address), its(:address_family), its(:virtual_interface_state), its(:customer_router_config), its(:mtu), its(:jumbo_frame_capable), its(:virtual_gateway_id), its(:direct_connect_gateway_id), its(:route_filter_prefixes), its(:bgp_peers), its(:region), its(:aws_device_v2), its(:aws_logical_device_id), its(:tags) ## dynamodb_table DynamodbTable resource type. @@ -2626,7 +2626,7 @@ describe network_interface('eni-12ab3cde') do end ``` -### its(:association), its(:availability_zone), its(:description), its(:interface_type), its(:ipv_6_addresses), its(:mac_address), its(:network_interface_id), its(:outpost_arn), its(:owner_id), its(:private_dns_name), its(:private_ip_address), its(:requester_id), its(:requester_managed), its(:source_dest_check), its(:status), its(:subnet_id), its(:vpc_id) +### its(:association), its(:availability_zone), its(:description), its(:interface_type), its(:ipv_6_addresses), its(:mac_address), its(:network_interface_id), its(:outpost_arn), its(:owner_id), its(:private_dns_name), its(:private_ip_address), its(:ipv_4_prefixes), its(:ipv_6_prefixes), its(:requester_id), its(:requester_managed), its(:source_dest_check), its(:status), its(:subnet_id), its(:vpc_id) ## nlb NLB resource type. @@ -2854,7 +2854,7 @@ end ``` -### its(:vpc_id), its(:db_instance_identifier), its(:db_instance_class), its(:engine), its(:db_instance_status), its(:master_username), its(:db_name), its(:endpoint), its(:allocated_storage), its(:instance_create_time), its(:preferred_backup_window), its(:backup_retention_period), its(:db_security_groups), its(:availability_zone), its(:preferred_maintenance_window), its(:pending_modified_values), its(:latest_restorable_time), its(:multi_az), its(:engine_version), its(:auto_minor_version_upgrade), its(:read_replica_source_db_instance_identifier), its(:read_replica_db_instance_identifiers), its(:read_replica_db_cluster_identifiers), its(:replica_mode), its(:license_model), its(:iops), its(:character_set_name), its(:nchar_character_set_name), its(:secondary_availability_zone), its(:publicly_accessible), its(:status_infos), its(:storage_type), its(:tde_credential_arn), its(:db_instance_port), its(:db_cluster_identifier), its(:storage_encrypted), its(:kms_key_id), its(:dbi_resource_id), its(:ca_certificate_identifier), its(:domain_memberships), its(:copy_tags_to_snapshot), its(:monitoring_interval), its(:enhanced_monitoring_resource_arn), its(:monitoring_role_arn), its(:promotion_tier), its(:db_instance_arn), its(:timezone), its(:iam_database_authentication_enabled), its(:performance_insights_enabled), its(:performance_insights_kms_key_id), its(:performance_insights_retention_period), its(:enabled_cloudwatch_logs_exports), its(:processor_features), its(:deletion_protection), its(:associated_roles), its(:listener_endpoint), its(:max_allocated_storage), its(:tag_list), its(:db_instance_automated_backups_replications), its(:customer_owned_ip_enabled), its(:aws_backup_recovery_point_arn), its(:activity_stream_status), its(:activity_stream_kms_key_id), its(:activity_stream_kinesis_stream_name), its(:activity_stream_mode), its(:activity_stream_engine_native_audit_fields_included) +### its(:vpc_id), its(:db_instance_identifier), its(:db_instance_class), its(:engine), its(:db_instance_status), its(:automatic_restart_time), its(:master_username), its(:db_name), its(:endpoint), its(:allocated_storage), its(:instance_create_time), its(:preferred_backup_window), its(:backup_retention_period), its(:db_security_groups), its(:availability_zone), its(:preferred_maintenance_window), its(:pending_modified_values), its(:latest_restorable_time), its(:multi_az), its(:engine_version), its(:auto_minor_version_upgrade), its(:read_replica_source_db_instance_identifier), its(:read_replica_db_instance_identifiers), its(:read_replica_db_cluster_identifiers), its(:replica_mode), its(:license_model), its(:iops), its(:character_set_name), its(:nchar_character_set_name), its(:secondary_availability_zone), its(:publicly_accessible), its(:status_infos), its(:storage_type), its(:tde_credential_arn), its(:db_instance_port), its(:db_cluster_identifier), its(:storage_encrypted), its(:kms_key_id), its(:dbi_resource_id), its(:ca_certificate_identifier), its(:domain_memberships), its(:copy_tags_to_snapshot), its(:monitoring_interval), its(:enhanced_monitoring_resource_arn), its(:monitoring_role_arn), its(:promotion_tier), its(:db_instance_arn), its(:timezone), its(:iam_database_authentication_enabled), its(:performance_insights_enabled), its(:performance_insights_kms_key_id), its(:performance_insights_retention_period), its(:enabled_cloudwatch_logs_exports), its(:processor_features), its(:deletion_protection), its(:associated_roles), its(:listener_endpoint), its(:max_allocated_storage), its(:tag_list), its(:db_instance_automated_backups_replications), its(:customer_owned_ip_enabled), its(:aws_backup_recovery_point_arn), its(:activity_stream_status), its(:activity_stream_kms_key_id), its(:activity_stream_kinesis_stream_name), its(:activity_stream_mode), its(:activity_stream_engine_native_audit_fields_included) ### :unlock: Advanced use `rds` can use `Aws::RDS::DBInstance` resource (see http://docs.aws.amazon.com/sdkforruby/api/Aws/RDS/DBInstance.html). From 48552dd90deaaec63b06855b125ec30f472abb70 Mon Sep 17 00:00:00 2001 From: "alceu.freitas" Date: Mon, 23 Aug 2021 16:03:57 -0300 Subject: [PATCH 09/10] fix: another rubocop offense --- lib/awspec/type/security_group.rb | 15 +++++---------- 1 file changed, 5 insertions(+), 10 deletions(-) diff --git a/lib/awspec/type/security_group.rb b/lib/awspec/type/security_group.rb index 6bdff28de..6e28fd462 100644 --- a/lib/awspec/type/security_group.rb +++ b/lib/awspec/type/security_group.rb @@ -171,17 +171,12 @@ def port_between?(port, from_port, to_port) if port.is_a?(String) && port.include?('-') && port =~ /^\d+-\d+$/ f, t = port.split('-') from_port == f.to_i && to_port == t.to_i + elsif port.is_a?(String) + raise Awspec::InvalidPortRange.new(port, from_port, to_port) unless port =~ /^\d+$/ + converted = port.to_i + converted.between?(from_port, to_port) else - if port.is_a?(String) - if port =~ /^\d+$/ - converted = port.to_i - converted.between?(from_port, to_port) - else - raise Awspec::InvalidPortRange.new(port, from_port, to_port) - end - else - port.between?(from_port, to_port) - end + port.between?(from_port, to_port) end end From fe67270473a7996daa545735c16f472aad63a60d Mon Sep 17 00:00:00 2001 From: "alceu.freitas" Date: Mon, 23 Aug 2021 16:04:23 -0300 Subject: [PATCH 10/10] fix: introduced bug due increased data on stub --- spec/generator/spec/security_group_spec.rb | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/spec/generator/spec/security_group_spec.rb b/spec/generator/spec/security_group_spec.rb index 5a26ec49d..81395b0dd 100644 --- a/spec/generator/spec/security_group_spec.rb +++ b/spec/generator/spec/security_group_spec.rb @@ -20,11 +20,12 @@ its(:inbound) { should be_opened('50000-50009').protocol('tcp').for('123.45.67.89/32') } its(:inbound) { should be_opened.protocol('all').for('sg-3a4b5cd6') } its(:outbound) { should be_opened(50000).protocol('tcp').for('100.45.67.12/32') } + its(:outbound) { should be_opened(443).protocol('tcp').for('0.0.0.0/0') } its(:outbound) { should be_opened(8080).protocol('tcp').for('group-in-other-aws-account-with-vpc-peering') } its(:inbound_rule_count) { should eq 8 } - its(:outbound_rule_count) { should eq 2 } + its(:outbound_rule_count) { should eq 3 } its(:inbound_permissions_count) { should eq 7 } - its(:outbound_permissions_count) { should eq 3 } + its(:outbound_permissions_count) { should eq 4 } it { should belong_to_vpc('my-vpc') } end EOF