Tags and Keep Blank Values when parsing url. #224

Closed
wants to merge 4 commits into
from

Conversation

Projects
None yet
4 participants
@joekiller
Contributor

joekiller commented Oct 5, 2014

Core

Amazon treats blank values as empty strings, ie http://docs.aws.amazon.com/AWSEC2/latest/APIReference/ApiReference-query-CreateTags.html

  • Update BaseResponse to keep blank values of keys when parsing the url.

Tags

refactored tags to be able to apply a dictionary of tags as specified in http://docs.aws.amazon.com/AWSEC2/latest/APIReference/ApiReference-query-CreateTags.html

  • create_tags and delete tags validates resource ids
  • create_tags ensures that resources exist prior to tagging
  • fixed delete tag rendering unncessary template
  • Update describe_tags to use filters from querystring

Exceptions

Models

  • Added validate resource ids method which will throw a InvalidID exception if the resource_id form is incorrect
  • Renamed TaggedEC2Instance object to TaggedEC2Resource, updated all inheritance references.
  • Added TagBackend create_tags method
    • Updated TagBackend create_tags to throw InvalidParameterValueErrorTagNull if a CreateTag key doesn't include a value
    • Updated TagBackend create_tags to throw TagLimitExceeded
  • removed TagBackend create_tag and delete_tag functions
  • Added TagBackend delete_tags method
  • Updated SpotInstanceRequest to allow filter_name 'spot-instance-request-id'
  • Enhanced EC2 Model to allow for checking of if a resource exists based on the resource_id. This is necessary because tag creations verify that a resource exists prior to performing any action.
  • Updated create_tags method to check if len(tags) > 10 and raise TagLimitExceeded even if the resource doesn't have any tags.
  • Updated TaggedEC2Resource to pass in a proper filter dictionary to describe_tags
  • Updated Instance get_tags to pass in proper filter dictionary to describe_tags
  • Updated TagBackend to properly filter response based on filter dictionary

    EC2 Utils

  • Added EC2_RESOURCE_PREFIXES to help with parsing prefixes in resource_ids
  • Updates random_[resource] functions to use EC2_RESOURCE_PREFIXES dictionary
  • Updated tags_from_query_string to return a dictionary of tag keys and tag values.
  • Added get_prefix method to help with determining resource types from EC2 resource_ids
  • Added is_valid_resource_id method to determine if a resource_id is valid for EC2 resources
  • Renamed EC2_RESOURCE_PREFIXES dictionary to EC2_RESOURCE_TO_PREFIX
  • Added EC2_PREFIX_TO_RESOURCE dictionary which is EC2_RESOURCE_TO_PREFIX inverted.
  • Added simple_aws_filter_to_re method for converting AWS wildcard syntax keys to python regular expression matches

Test Tags

  • Renamed various tag test names to be more relevent
  • Added test to raise InvalidParameterValue when a tag key is None
  • Added test_tag_limit_exceeded
  • Added test_get_all_tags_resource_id_filter to test get_all_tags using resource-id filter
  • Added test_get_all_tags_resource_type_filter to test get_all_tags using resource-type filter
  • Added test_get_all_tags_key_filter to test get_all_tags using key filter
  • Added test_get_all_tags_value_filter to test get_all_tags using value filter

joekiller added some commits Oct 2, 2014

Added more complete tags implementation
Tags
===

refactored tags to be able to apply a dictionary of tags as specified in http://docs.aws.amazon.com/AWSEC2/latest/APIReference/ApiReference-query-CreateTags.html

  * create_tags and delete tags validates resource ids
  * create_tags ensures that resources exist prior to tagging
  * fixed delete tag rendering unncessary template
  * Update describe_tags to use filters from querystring

Exceptions
 ======

  * Added InvalidID exception used by CreateTags and DeleteTags per http://docs.aws.amazon.com/AWSEC2/latest/APIReference/api-error-codes.html#InvalidID
  * Added TagLimitExceeded per http://docs.aws.amazon.com/AWSEC2/latest/APIReference/ApiReference-query-CreateTags.html

Models
=====

  * Added validate resource ids method which will throw a InvalidID exception if the resource_id form is incorrect
  * Renamed TaggedEC2Instance object to TaggedEC2Resource, updated all inheritance references.
  * Added TagBackend create_tags method
    * Updated TagBackend create_tags to throw InvalidParameterValueErrorTagNull if a CreateTag key doesn't include a value
    * Updated TagBackend create_tags to throw TagLimitExceeded
  * removed TagBackend create_tag and delete_tag functions
  * Added TagBackend delete_tags method
    * Updated delete_tags method to allow key/value delete per http://docs.aws.amazon.com/AWSEC2/latest/APIReference/ApiReference-query-DeleteTags.html
  * Updated SpotInstanceRequest to allow filter_name 'spot-instance-request-id'
  * Enhanced EC2 Model to allow for checking of if a resource exists based on the resource_id.  This is necessary because tag creations verify that a resource exists prior to performing any action.
  * Updated create_tags method to check if len(tags) > 10 and raise TagLimitExceeded even if the resource doesn't have any tags.
  * Updated TaggedEC2Resource to pass in a proper filter dictionary to describe_tags
  * Updated Instance get_tags to pass in proper filter dictionary to describe_tags
  * Updated TagBackend to properly filter response based on filter dictionary

 EC2 Utils
 ======

  * Added EC2_RESOURCE_PREFIXES to help with parsing prefixes in resource_ids
  * Updates random_[resource] functions to use EC2_RESOURCE_PREFIXES dictionary
  * Updated tags_from_query_string to return a dictionary of tag keys and tag values.
  * Added get_prefix method to help with determining resource types from EC2 resource_ids
  * Added is_valid_resource_id method to determine if a resource_id is valid for EC2 resources
  * Renamed EC2_RESOURCE_PREFIXES dictionary to EC2_RESOURCE_TO_PREFIX
  * Added EC2_PREFIX_TO_RESOURCE dictionary which is EC2_RESOURCE_TO_PREFIX inverted.
  * Added simple_aws_filter_to_re method for converting AWS wildcard syntax keys to python regular expression matches

Test Tags
======

  * Renamed various tag test names to be more relevent
  * Added test to raise InvalidParameterValue when a tag key is None
  * Added test_tag_limit_exceeded
  * Added test_get_all_tags_resource_id_filter to test get_all_tags using resource-id filter
  * Added test_get_all_tags_resource_type_filter to test get_all_tags using resource-type filter
  * Added test_get_all_tags_key_filter to test get_all_tags using key filter
  * Added test_get_all_tags_value_filter to test get_all_tags using value filter
@coveralls

This comment has been minimized.

Show comment
Hide comment
@coveralls

coveralls Oct 5, 2014

Coverage Status

Coverage decreased (-0.4%) when pulling 8761924 on joekiller:master into 0a99aae on spulec:master.

Coverage Status

Coverage decreased (-0.4%) when pulling 8761924 on joekiller:master into 0a99aae on spulec:master.

@coveralls

This comment has been minimized.

Show comment
Hide comment
@coveralls

coveralls Oct 5, 2014

Coverage Status

Coverage decreased (-0.26%) when pulling 87894b6 on joekiller:master into 0a99aae on spulec:master.

Coverage Status

Coverage decreased (-0.26%) when pulling 87894b6 on joekiller:master into 0a99aae on spulec:master.

@thedrow

This comment has been minimized.

Show comment
Hide comment
Contributor

thedrow commented Oct 5, 2014

@thedrow

This comment has been minimized.

Show comment
Hide comment
@thedrow

thedrow Oct 5, 2014

Contributor

Looking at the code it seems that there are 5 ways to filter an entity.
I already added support for filtering entities by tags here: https://github.com/joekiller/moto/blob/cc2f66331b3d29a0dd99075fc8da46da59cd2ea6/moto/ec2/models.py#L103
We should make the code more reusable.
How do we do that correctly?

Contributor

thedrow commented Oct 5, 2014

Looking at the code it seems that there are 5 ways to filter an entity.
I already added support for filtering entities by tags here: https://github.com/joekiller/moto/blob/cc2f66331b3d29a0dd99075fc8da46da59cd2ea6/moto/ec2/models.py#L103
We should make the code more reusable.
How do we do that correctly?

@thedrow

This comment has been minimized.

Show comment
Hide comment
@thedrow

thedrow Oct 5, 2014

This part violates DRY since some of it is already implemented in EC2TaggedResource get_filter_value().

This part violates DRY since some of it is already implemented in EC2TaggedResource get_filter_value().

This comment has been minimized.

Show comment
Hide comment
@joekiller

joekiller Oct 5, 2014

Owner

I'm not positive as to which code is being repeated here. When filtering tags themselves, the filter parameter's key/value pairs are different than when you filter for an EC2 resource. Also, EC2 tags are generally implemented separately from the EC2 resources themselves.

Owner

joekiller replied Oct 5, 2014

I'm not positive as to which code is being repeated here. When filtering tags themselves, the filter parameter's key/value pairs are different than when you filter for an EC2 resource. Also, EC2 tags are generally implemented separately from the EC2 resources themselves.

@joekiller

This comment has been minimized.

Show comment
Hide comment
@joekiller

joekiller Oct 5, 2014

Contributor

Thanks for the feedback. I'm sure we can make this work. I'll try to address the issues.

Contributor

joekiller commented Oct 5, 2014

Thanks for the feedback. I'm sure we can make this work. I'll try to address the issues.

@joekiller

This comment has been minimized.

Show comment
Hide comment
@joekiller

joekiller Oct 5, 2014

Contributor

A general comment on my goal of this PR. I am trying to fully implement EC2 resource tagging backend independently of any attribute/tag filtering of any specified EC2 resource itself. In AWS EC2 tags are completely separate from EC2 resources themselves. So when a request to a connection is made to get resources with filters, the backend is really performing two operations. First the code is checking filter attributes against resource attributes and then the code is also checking filter attributes for tags which are independent of resources themselves.

As for code reuse, I agree that the TaggedEC2Resources themselves and the TagBackend could be refactored to use the generic passes_filter_dict method for example if we have the resources pass a valid dict to the function. Each object would have an independent valid filter dictionaries to work with.

The PR was to implement the EC2 tagging backend itself so I didn't refactor any filtering features of EC2 resources themselves.

I completely agree that the means by which I did implement the filtering of tags can be made more generic so that we can filter EC2 resource attributes in the same way but rather would leave that for another day/PR. Otherwise this turns into refactored all EC2 resource filtering and added a complete EC2 tagged backend implementation.

Contributor

joekiller commented Oct 5, 2014

A general comment on my goal of this PR. I am trying to fully implement EC2 resource tagging backend independently of any attribute/tag filtering of any specified EC2 resource itself. In AWS EC2 tags are completely separate from EC2 resources themselves. So when a request to a connection is made to get resources with filters, the backend is really performing two operations. First the code is checking filter attributes against resource attributes and then the code is also checking filter attributes for tags which are independent of resources themselves.

As for code reuse, I agree that the TaggedEC2Resources themselves and the TagBackend could be refactored to use the generic passes_filter_dict method for example if we have the resources pass a valid dict to the function. Each object would have an independent valid filter dictionaries to work with.

The PR was to implement the EC2 tagging backend itself so I didn't refactor any filtering features of EC2 resources themselves.

I completely agree that the means by which I did implement the filtering of tags can be made more generic so that we can filter EC2 resource attributes in the same way but rather would leave that for another day/PR. Otherwise this turns into refactored all EC2 resource filtering and added a complete EC2 tagged backend implementation.

@spulec

This comment has been minimized.

Show comment
Hide comment
@spulec

spulec Oct 6, 2014

Owner

Closed with ec5f5dc

Thanks for your work Joseph!

Owner

spulec commented Oct 6, 2014

Closed with ec5f5dc

Thanks for your work Joseph!

@spulec spulec closed this Oct 6, 2014

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment