Validation of AWS tags

Question

I have to validate Tags with Rules for AWS resource.

Input contains list of dictionaries. [{"keyName": "KeyValue", "valueName": "ValueValue"}, {"keyName": "KeyValue", "valueName": "ValueValue"}, {"keyName": "KeyValue", "valueName": "ValueValue"}, ...]

There are following rules for validation:

1. Tag key must be non empty.
2. No Duplicate tags
3. No tag have key from reserved keys list
4. 30 tags allowed only.
5. Insert Reserved tags in Replication Tags.
6. Key Length should be less than 127.
7. aws: prefix in tag names should not be allowed
8. Value Length should be less than 255.

Code:

class ReplicationSettings:
    def __init__(self):
        pass

    def replication_tag_validation(self, replication_tags):
        """ Validate Replication Tags"""
        # Tag key must be non empty.
        # No Duplicate tags
        # No tag have key from reserved keys list
        # 30 tags allowed only.
        # Insert Reserved tags in Replication Tags.

        reserved_tags = ["C01_USAGE"]
        unique_tags = []
        new_replication_tags = []

        try:
            for tag in replication_tags:
                key_str = tag["key"].strip()
                value_str = tag["value"].strip()
                if not key_str:
                    msg = f"Tag key should not empty."
                    print(msg)
                    return (False, {"message": msg})

                if key_str in reserved_tags:
                    msg = f"Tag with key {key_str} is not allowed."
                    print(msg)
                    return (False, {"message": msg})

                if key_str in unique_tags:
                    msg = f"Tag key {key_str} should be unique."
                    print(msg)
                    return (False, {"message": msg})

                (status, message) = self.validate_key(key_str)
                if status is False:
                    return (False, message)

                (status, message) = self.validate_value(value_str)
                if status is False:
                    return (False, message)

                new_replication_tags.append(
                    {"key": key_str, "value": value_str})
                unique_tags.append(key_str)

            if len(new_replication_tags) > 29:
                msg = "Allow maximum of 30 tags in replication settings."
                print(msg)
                return (False, {"message": msg})

            new_replication_tags.insert(0, {"key": "C01_USAGE",
                                            "value": "INTERNAL"})
            return (True, {"replicationTags": new_replication_tags})

        except Exception as err:
            print("Unknown error while validating replication tags: {}"
                  "".format(err))
            return (500, {"message": "Unable to validate Replication tags"})

    def validate_key(self, key_str):
        """ Validate key string. """
        # Length should be less than 127.
        # aws: prefix in tag names should not be allowed.

        if len(key_str) > 127:
            msg = f"Length of Tag key f{key_str} must be less than " \
                  f"127 characters"
            return False, {"message": msg}

        if key_str.lower().startswith("aws"):
            msg = f"Tag key f{key_str} should not start with 'aws:' prefix"
            return False, {"message": msg}

        return True, {"message": "successful"}

    def validate_value(self, value_str):
        """ Validate value string. """
        # Length should be less than 255.
        # aws: prefix in tag values should not be allowed.

        if len(value_str) > 255:
            msg = f"Length of Tag value f{value_str} must be less than" \
                  f" 255 characters"
            return False, {"message": msg}

        if value_str.lower().startswith("aws"):
            msg = f"Tag value f{value_str} should not start with 'aws:' prefix"
            return False, {"message": msg}

        return True, {"message": "successful"}



input_tags = [
    {"key": "Key01", "value": "value01"},
    {"key": "Key02", "value": "value01"},
    {"key": "Key03", "value": "value01"},
    {"key": "Key04", "value": "value01"},
    {"key": "Key05", "value": "value01"},
    {"key": "Key06", "value": "value01"},
    {"key": "Key07", "value": "value01"},
    {"key": "Key08", "value": "value01"}
]

obj = ReplicationSettings()
(status, message) = obj.replication_tag_validation(input_tags)

Above code is working. but it is not Pythonic code means not using p?ython features. Can you help me to write in more Pythonic way?

Thanks in advance.

Answer 1

general

The code looks good and readable, however I have some remarks.

(I could not run the original code because of the use of 'f-string', a feature since Python 3.6 and I am still at 3.5)

class not needed

In this case a class is not needed, a simple function would satisfy. If you want to hide the helper functions validate_key and validate_value you could make them inner functions of replication_tag_validation.

too many parentheses

You enclose return values with parentheses, for instance:

return (False, {"message": msg}

This is not neccesary, without the parentheses it also returns a tuple:

return False, {"message": msg}

The same yields for the assigment of the results of a function call

status, message = self.validate_key(key_str)

different return types

The first item in the returning tuple is always a boolean except for one case where it is an integer (500, probably an error code?). This might cause confusion.

large try... except block

There is a large try... except block in replication_tag_validation that catches everything and might give confusing error messages. From the code I assume that input parameter of that function must be a list of dictionaries where each dictionary contains two keys 'key' and 'value' and the values should be strings. You could give more specific error messages if the input does not comply with this.

docstring

The comment lines after the docstring of replication_tag_validation should be included in the docstring because they explain what the function does.

printing and returning error messages

The replication_tag_validation function at several places prints a message
and returns exacly the same message:

print(msg)
return (False, {"message": msg})

The printing (or not) of these error messages should be left to the caller of this function.

almost duplicate functions

The functions validate_value and validate_key are almost the same and could be combined to one.

---

summing these remarks up in changed code:

def validate_replication_tags(replication_tags):
    """ 
    Returns validated replication tags or raises an error.
    - 'replication_tags' should be a list of 'tag's.
    - a 'tag' should be dictionary with keys 'key' and 'value'
    - the 'key' values should be a string with length between 0 and 127
    - the 'value' values should be a string with length between 0 and 255
    - the 'key' values should be unique
    - the 'key' and 'value' values must not start with 'aws'
    - a mandatory 'tag' with 'key' value "C01_USAGE" will be added
    """

    # helper method to validate keys and values    
    def validate_string(val, s_type, s_max_length):
        if type(val) != str:
            raise ValueError("Non string " + s_type + " found:" + str(val))
        str_val = val.strip()
        if not len(str_val): 
            raise ValueError("Empty " + s_type + " found.")
        if len(str_val) >= s_max_length:
            raise ValueError("Too long " + s_type + " found: " + str_val)
        if str_val.lower().startswith("aws"):
            raise ValueError(s_type + " starting with aws found: " + str_val)
        return str_val


    if type(replication_tags) != list:
        raise ValueError("replication_tags should be a list")

    reserved_tags = ["C01_USAGE"]
    unique_tags = []
    new_replication_tags = []

    for tag in replication_tags:

        if not (type(tag) == dict and len(tag) == 2 and
                'key' in tag and 'value' in tag):
            raise ValueError("tags should be dicts with keys 'key' and 'value'")

        key = validate_string(tag['key'], 'key', 127)
        value = validate_string(tag['value'], 'value', 255)

        if key in reserved_tags:
            raise ValueError("tag encountered with reserved key: " + key)

        if key in unique_tags:
            raise ValueError("tag encountered with duplicate key: " + key)

        unique_tags.append(key)          
        new_replication_tags.append({"key": key, "value": value})

        if len(new_replication_tags) > 29:
            raise ValueError("More than 30 tags in replication settings.")

    # add reserved tag
    new_replication_tags.insert(0, {"key": "C01_USAGE", "value": "INTERNAL"})
    return new_replication_tags


input_tags = [
    {"key": "Key01", "value": "value01"},
    {"key": "Key02", "value": "value01"},
    {"key": "Key03", "value": "value01"},
    {"key": "Key04", "value": "value01"},
    {"key": "Key05", "value": "value01"},
    {"key": "Key06", "value": "value01"},
    {"key": "Key07", "value": "value01"},
    {"key": "Key08", "value": "value01"}
]


# 'normal' usage
try:
    validated_tags = validate_replication_tags(input_tags)
except ValueError as err:
    # do stuff in case of an error:
    validated_tags = None
    print(err)

# to make it compatible with the original version
try:
    validated_tags = validate_replication_tags(input_tags)
    status = True, 
    message = {"replicationTags": validated_tags}
except ValueError as err:
    status = False, 
    message = {"message": str(err)}