Bugfix/issue 416 #546
Bugfix/issue 416 #546
Conversation
Some specs had to be rewritten and this will probably be not compatible with specs written in the old style.
There was a problem hiding this comment.
I agree with the introduction of -1 and 0.0.0.0/0 notation.
However, I would like you to at least consider adding the features in a way that maintains backward compatibility.
I would like to keep the behavior except for obvious bugs ( be_opened(nil) ), and I would like to avoid erasing test cases to show it as much as possible.
| 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') } |
There was a problem hiding this comment.
I'm concerned about the loss of the all test.
I agree with the inclusion of -1, but would like to see all supported for backward compatibility.
There was a problem hiding this comment.
I think we can safely create an alias for it. Let me try and get back to you.
| 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) } |
There was a problem hiding this comment.
I think that there are times when you just want to test that the port is open. So I'd like to see support for this notation as well.
There was a problem hiding this comment.
I'm not sure, I would need to test from both console and SDK, but my guess is that is not acceptable to create a security group with such rule, without specifying protocol and CIDR. You can, of course, specify anywhere and all protocols, but at least this is rigidly defined.
Even if it would work, it would be a security flaw in my opinion: there is a big different about asserting that a port is opened for a specific IP or the whole internet (0.0.0.0).
There was a problem hiding this comment.
but my guess is that is not acceptable to create a security group with such rule, without specifying protocol and CIDR.
Yes.
Even if it would work, it would be a security flaw in my opinion: there is a big different about asserting that a port is opened for a specific IP or the whole internet (0.0.0.0).
Yes. This test does not specify the IP and protocol, so it will be to make sure that port 22 is open with some protocol on some IP.
Another way to say it is the negation of should_not be_opened(22).
Those deleted test cases are just wrong: they were assuming an output from a different security group name. It's like testing security group B, but using output from the request of security group A details. |
Also included other bug fixes and improvements.
Tests are also more restrictive now:
targetorfor) unless0.0.0.0/0is acceptable;