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

new step @then(u'{something:ANY} must be enabled') #123

Closed
vrbcntrl opened this issue Jul 9, 2019 · 8 comments
Closed

new step @then(u'{something:ANY} must be enabled') #123

vrbcntrl opened this issue Jul 9, 2019 · 8 comments
Assignees
Labels
enhancement waiting for confirmation Workaround/Fix applied, waiting for confirmation

Comments

@vrbcntrl
Copy link
Contributor

vrbcntrl commented Jul 9, 2019

Hi @eerkunt

We came across a pretty common use case and found that the corresponding step is not available, so I have created the new step by referencing the encryption enabled step, hope it will be useful.. :)
Let me know if the step needs any improvements...

BDD:

Scenario: AWS EC2 Instance monitoring not enabled
      Given I have aws_instance defined
      Then monitoring must be enabled

Step:

@then(u'{something:ANY} must be enabled')
def something_is_enabled(_step_obj,something):
    for resource in _step_obj.context.stash:
        if type(resource) is dict:
            property_value = seek_key_in_dict(resource.get('values', {}), something)
            #print('property_value: {}'.format(property_value))

            if len(property_value):
                property_value = property_value[0]

                if type(property_value) is dict:
                    property_value = property_value[something]

            if not property_value:
                raise Failure('Resource {} does not have {} enabled ({}={}).'.format(resource['address'],something,
                                                                                             something,
                                                                                             property_value))

    return True
@eerkunt
Copy link
Member

eerkunt commented Jul 10, 2019

Hi @vrbcntrl,

Thanks for providing improvement to the tool :)

Few things that will cause problems ;

  1. if something is encryption, then it will conflict with the step we have, so we need to modify encryption must be enabled step to process something while if something is enabled when it should check encryption_property.
  2. Just like encryption_property, this means we need to create more dictionaries based on resource types, that will be checked during the step run.
  3. seek_key_in_dict might cause a problem on some cases due to resource mounting, where it may find something key for a mounted resource.
  4. There are no tests written for this step :)
  5. This is not a PR, but an Issue. Better to create this as a PR, reviewing from here is not really what we should done.

Thanks again!

@vrbcntrl
Copy link
Contributor Author

Yes, understood and agree with you.

Please let me know when you get a chance to incorporate this changes in to the encryption enabled steps.

I am also wondering if you could add/modify the encryption enabled step to accommodate both encryption_at_rest and encryption_in_flight use cases for a same resource.
Ex: AWS Elastic Cache Replication group resource has 2 encryption properties
1) at_rest_encryption_enabled and 2) transit_encryption_enabled

so to check if these 2 encryption properties are enabled, I had to write

Given I have AWS Elastic Cache Replication Group defined
Then it must contain <encryption>
Then its value must match the "<value>" regex

Example:
| encryption                               | value   |
| at_rest_encryption_enabled               | true     |
| transit_encryption_enabled                | true    |

However, I am wondering some thing like below would make it more easy

Given I have AWS Elastic Cache Replication Group defined
Then encryption_at_rest must be enabled
And encryption_in_flight must be enabled

to support this we might need to update the encryption enabled step some thing like this

@then(u'encryption_at_rest is enabled')
@then(u'encryption_in_flight is enabled')
@then(u'encryption_in_flight must be enabled')
@then(u'encryption_at_rest must be enabled')

any thoughts??

@vrbcntrl
Copy link
Contributor Author

so, I cam up with this generic step in place of encryption enabled step, please let me know if it make any sense to you :)

@then(u'{something:ANY} is be enabled')
@then(u'{something:ANY} must be enabled')
def property_is_enabled(_step_obj, something):
    prop = None
    for resource in _step_obj.context.stash:
        if type(resource) is dict:
            print('something : {}'.format(something))
            if something == 'encryption_at_rest':
                prop = encryption_at_rest_property.get(resource['type'], None)
            elif something == 'encryption_in_flight':
                prop = encryption_in_flight_property.get(resource['type'], None)
            if not prop:
                #raise TerraformComplianceNotImplemented('property {} for {} ''is not implemented yet.'.format(resource['type'],prop))
                prop = something

           
            property_value = seek_key_in_dict(resource.get('values', {}), prop)
            if len(property_value):
                property_value = property_value[0]

                if type(property_value) is dict:
                    print('property_value type is dict')
                    property_value = property_value[prop]

            if not property_value:
                raise Failure('Resource {} does not have {} property enabled ({}={}).'.format(resource['address'],
                                                                                             prop,
                                                                                             prop,
                                                                                             property_value))


    return True

I have tested the above step with my below BDD features

Scenario Outline: AWS Elastic Cache Replication Group Encryption
    Given I have AWS Elastic Cache Replication Group defined
    Then <encryption> must be enabled
	
	
	Examples:
	| encryption           |
	| encryption_at_rest   |
	| encryption_in_flight |
   
 Scenario Outline: AWS Elastic Cache Replication Group Enable properties
    Given I have AWS Elastic Cache Replication Group defined
    Then <key> must be enabled
	
	
	Examples:
	| key                        |
	| monitoring                 |
	| auto_minor_version_upgrade |
	| automatic_failover_enabled |

Test results are shown below:

Scenario Outline: AWS Elastic Cache Replication Group Encryption
      Given I have AWS Elastic Cache Replication Group defined
      Then <encryption> must be enabled

  Examples:
      | encryption           |
      | encryption_at_rest   |
        Failure: Resource aws_elasticache_replication_group.example does not have at_rest_encryption_enabled property enabled (at_rest_encryption_enabled=False).
      | encryption_in_flight |
        Failure: Resource aws_elasticache_replication_group.example does not have transit_encryption_enabled property enabled (transit_encryption_enabled=False).

  Scenario Outline: AWS Elastic Cache Replication Group Enable properties
      Given I have AWS Elastic Cache Replication Group defined
      Then <key> must be enabled

  Examples:
      | key                        |
      | monitoring                 |
        Failure: Resource aws_elasticache_replication_group.example does not have monitoring property enabled (monitoring=[]).
      | auto_minor_version_upgrade |
      | automatic_failover_enabled |
        Failure: Resource aws_elasticache_replication_group.example does not have automatic_failover_enabled property enabled (automatic_failover_enabled=False).

@eerkunt
Copy link
Member

eerkunt commented Jul 12, 2019

Hard to say anything since this is not a Pull Request.

All code changes must come via Pull Request, not via Issues :)

I think I understood what we are aiming for, will have a look

@vrbcntrl
Copy link
Contributor Author

thank you!
BTW - I have created the PR, could you pls check when you get a chance?

@eerkunt
Copy link
Member

eerkunt commented Jul 16, 2019

#125

@eerkunt
Copy link
Member

eerkunt commented Jul 16, 2019

Could you please have a test with 1.0.25 when you have time ?

Thanks for the PR again! 🎉

@eerkunt eerkunt added the waiting for confirmation Workaround/Fix applied, waiting for confirmation label Jul 16, 2019
@vrbcntrl
Copy link
Contributor Author

Tested the changes with 1.0.25 and it works fine, however I had to fix the encryption_at_rest and encryption_in_flight.feature files , please see attached.
encryption_at_rest.feature.txt
encryption_in_flight.feature.txt

Thanks!

@eerkunt eerkunt closed this as completed Jul 21, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement waiting for confirmation Workaround/Fix applied, waiting for confirmation
Projects
None yet
Development

No branches or pull requests

2 participants