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

Check existence and values of specific tag keys #23

Closed
f0o opened this issue Jun 23, 2020 · 20 comments
Closed

Check existence and values of specific tag keys #23

f0o opened this issue Jun 23, 2020 · 20 comments
Labels
bug Something isn't working release-0.5.2
Projects

Comments

@f0o
Copy link

f0o commented Jun 23, 2020

Hi,

Is there away to enforce specific tags to be set, ideally with specific values?

AWS::EC2::EIP Tags == [{"Key":"Foo","Value":"abc"},{"Key":"Bar","Value":"def"}]

would fail if the resource has additional Tags

@nathanataws
Copy link
Contributor

Do you have an example template to run against this rule?

As written, it would look at the Tags property of the AWS::EC2::EIP type and expect it to exactly match that list of maps on the right-side of the rule. If there were an additional value inside that right-side list, it would fail to exactly match and the check should fail.

Here's what I got from a test that sounds similar to your question:

"[CheckRequirements] failed because [HttpFloodProtectionLogParserActivated] is [[\"HttpFloodProtectionLogParserActivated\",\"yes\",\"no\",\"maybe\"]] and the permitted value is [[\"HttpFloodProtectionLogParserActivated\",\"yes\",\"no\"]]"
Number of failures: 1

@nathanataws nathanataws added the question Further information is requested label Jun 23, 2020
@f0o
Copy link
Author

f0o commented Jun 24, 2020

Generally we want that all tagable resources have 2 specific tags, one that denotes the owner and one that denotes the environment type.

However the resources may also have any number of additional tags that we do not track or care about and it should not fail cfn-guard if those additional tags are defined.

The only hard requirement is the existence the Owner and Environment tags.

@nathanataws
Copy link
Contributor

That sounds like a good candidate for wildcards. Try something like:

AWS::EC2::EIP Tags.* == {"Key":"Foo","Value":"abc"}
AWS::EC2::EIP Tags.* == {"Key":"Bar","Value":"def"}

What these rules say is "At least item in the list under Tags must contain this value". Taken together they guarantee that each instance of EIP Tags needs to have these somewhere in its list.

Note the lack of whitespace, though. Because the parsers eliminate them when they deserialize, your right-side value needs to be {"Key":"Foo","Value":"abc"} instead of {"Key": "Foo", "Value": "abc"}. When in doubt, run a template through cfn-guard-rulegen and you'll get a view of how the parsers view the values. And you can always increase the verbosity logging level to make the processing more clear. In particular, -vvv will give you everything that happens from start to finish.

@f0o
Copy link
Author

f0o commented Jun 25, 2020

Hi,

That didnt work as expected, now it complains twice for every other tag that's not specifically listed like:

"[Alb] failed because [Tags.0] is [{\"Key\":\"Name\",\"Value\":\"${EnvironmentType}-${ClusterName}-ecs-alb\"}] and the permitted value is [{\"Key\":\"EnvironmentType\",\"Value\":\"EnvironmentType\"}]"
"[Alb] failed because [Tags.0] is [{\"Key\":\"Name\",\"Value\":\"${EnvironmentType}-${ClusterName}-ecs-alb\"}] and the permitted value is [{\"Key\":\"OwnerContact\",\"Value\":\"OwnerContact\"}]"
"[Alb] failed because [Tags.1] is [{\"Key\":\"EnvironmentType\",\"Value\":\"EnvironmentType\"}] and the permitted value is [{\"Key\":\"OwnerContact\",\"Value\":\"OwnerContact\"}]"
"[Alb] failed because [Tags.2] is [{\"Key\":\"ClusterName\",\"Value\":\"ClusterName\"}] and the permitted value is [{\"Key\":\"EnvironmentType\",\"Value\":\"EnvironmentType\"}]"
"[Alb] failed because [Tags.2] is [{\"Key\":\"ClusterName\",\"Value\":\"ClusterName\"}] and the permitted value is [{\"Key\":\"OwnerContact\",\"Value\":\"OwnerContact\"}]"
"[Alb] failed because [Tags.3] is [{\"Key\":\"Scope\",\"Value\":\"ecs\"}] and the permitted value is [{\"Key\":\"EnvironmentType\",\"Value\":\"EnvironmentType\"}]"
"[Alb] failed because [Tags.3] is [{\"Key\":\"Scope\",\"Value\":\"ecs\"}] and the permitted value is [{\"Key\":\"OwnerContact\",\"Value\":\"OwnerContact\"}]"

The ruleset is:

AWS::ElasticLoadBalancingV2::LoadBalancer Tags.* == {"Key":"EnvironmentType","Value":"EnvironmentType"}
AWS::ElasticLoadBalancingV2::LoadBalancer Tags.* == {"Key":"OwnerContact","Value":"OwnerContact"}

The template is missing OwnerContact Tag

//Edit:

It seems that I can only enforce it using positional Tags such as Tags.0 == {"Key":"EnvironmentType","Value":"EnvironmentType"} and Tags.1 == {"Key":"OwnerContact","Value":"OwnerContact"}

@nathanataws
Copy link
Contributor

Hmm. Sorry to hear that's not working. The extra messages are a result of wildcard == being treated as a set of |OR|'s. So if it doesn't hit any of allowed values in the Tags field, the result from both failed sides of the |OR| are output.

I think I can write a simple rule to address this use case but before I do, can I get an example template (nothing sensitive in it) to try it on my side?

@f0o
Copy link
Author

f0o commented Jun 29, 2020

This would be an example:

  ClusterSg:
    Type: AWS::EC2::SecurityGroup
    Properties:
      VpcId:
        Fn::ImportValue: !Sub ${EnvironmentType}-net-vpc-VpcId
      GroupDescription: !Sub ${EnvironmentType}-sg-${ClusterName}
      SecurityGroupEgress:
        - CidrIp: "0.0.0.0/0"
          IpProtocol : "-1"
          Description: "Allow all outgoing traffic"
      SecurityGroupIngress:
        - SourceSecurityGroupId: !Ref AlbSg
          IpProtocol : "-1"
          Description: !Sub "Allow incoming traffic from ${ClusterName} ALB"
      Tags:
        - Key: EnvironmentType
          Value: !Ref EnvironmentType
        - Key: OwnerContact
          Value: !Ref OwnerContact
        - Key: Name
          Value: !Sub ${EnvironmentType}-${ClusterName}-sg
        - Key: ClusterName
          Value: !Ref ECSCluster
        - Key: Scope
          Value: ecs

The only tags we want to enforce setting is EnvironmentType, OwnerContact and Name - Order doesnt matter, right now since I do regex matching to solve it the order is fixed like this in all templates but ideally the devs should be allowed to put them in any order.

I appreciate the help :)

@nathanataws nathanataws added this to Coming Soon in Roadmap Jul 1, 2020
@nathanataws nathanataws added bug Something isn't working release-0.5.2 and removed question Further information is requested labels Jul 1, 2020
@nathanataws
Copy link
Contributor

You caught a bug! Good eye. The issue was that the regex was too restrictive and mandated a value after the * (which is incorrect behavior). I added tests for correct pass & fail behavior for stuff like Tags.* and merged a fix into the branch release-0.5.2 in PR #34.

Can you build from the release-0.5.2 branch and see if this particular issue is resolved with a ruleset like:

AWS::EC2::SecurityGroup Tags.* == {"Key":"EnvironmentType","Value":"EnvironmentType"}
AWS::EC2::SecurityGroup Tags.* == {"Key":"OwnerContact","Value":"OwnerContact"}

It should mandate that each of those tags must be present at least once.

You'll still get the multiple errors when they're not present because the * == wildcard operation is an OR across all the possible matches. But that is an accurate outcome. In other words, with the rules above and the tags that match commented out in your template:

Resources:
    ClusterSg:
      Type: AWS::EC2::SecurityGroup
      Properties:
        VpcId:
          Fn::ImportValue: !Sub ${EnvironmentType}-net-vpc-VpcId
        GroupDescription: !Sub ${EnvironmentType}-sg-${ClusterName}
        SecurityGroupEgress:
          - CidrIp: "0.0.0.0/0"
            IpProtocol : "-1"
            Description: "Allow all outgoing traffic"
        SecurityGroupIngress:
          - SourceSecurityGroupId: !Ref AlbSg
            IpProtocol : "-1"
            Description: !Sub "Allow incoming traffic from ${ClusterName} ALB"
        Tags:
          #- Key: EnvironmentType
          #  Value: !Ref EnvironmentType
          #- Key: OwnerContact
          #  Value: !Ref OwnerContact
          - Key: Name
            Value: !Sub ${EnvironmentType}-${ClusterName}-sg
          - Key: ClusterName
            Value: !Ref ECSCluster
          - Key: Scope
            Value: ecs

You should get the following result:

⋊> ~/p/c/cfn-guard on release-0.5.2 ◦ cargo run -- -t ~/scratch_template.yaml -r ~/scratch.ruleset                                                                                                                                                                      10:46:42
    Finished dev [unoptimized + debuginfo] target(s) in 0.03s
     Running `target/debug/cfn-guard -t /Users/nathanmc/scratch_template.yaml -r /Users/nathanmc/scratch.ruleset`
[ClusterSg] failed because [Tags.0] is [{"Key":"Name","Value":"${EnvironmentType}-${ClusterName}-sg"}] and the permitted value is [{"Key":"EnvironmentType","Value":"EnvironmentType"}]
[ClusterSg] failed because [Tags.0] is [{"Key":"Name","Value":"${EnvironmentType}-${ClusterName}-sg"}] and the permitted value is [{"Key":"OwnerContact","Value":"OwnerContact"}]
[ClusterSg] failed because [Tags.1] is [{"Key":"ClusterName","Value":"ECSCluster"}] and the permitted value is [{"Key":"EnvironmentType","Value":"EnvironmentType"}]
[ClusterSg] failed because [Tags.1] is [{"Key":"ClusterName","Value":"ECSCluster"}] and the permitted value is [{"Key":"OwnerContact","Value":"OwnerContact"}]
[ClusterSg] failed because [Tags.2] is [{"Key":"Scope","Value":"ecs"}] and the permitted value is [{"Key":"EnvironmentType","Value":"EnvironmentType"}]
[ClusterSg] failed because [Tags.2] is [{"Key":"Scope","Value":"ecs"}] and the permitted value is [{"Key":"OwnerContact","Value":"OwnerContact"}]
Number of failures: 6

But with the tags in the template uncommented, it should pass.

@f0o
Copy link
Author

f0o commented Jul 2, 2020

Haha nice, this will slim down my rules a lot. I'll build 0.5.2 and test it :)

@nathanataws
Copy link
Contributor

Awesome. I'll close this for now. Please do re-open it if you're still jammed-up.

@f0o
Copy link
Author

f0o commented Jul 3, 2020

Seems like we need to reopen this because some recent commit of release-0.5.2 caused regression and broke it again :/

//EDIT:
#35 This worked
#36 Broke it again

Did a format change happen?

@nathanataws
Copy link
Contributor

👀 Taking a look. This should be covered by test cases. If not, that's a bug in and of itself.

@nathanataws
Copy link
Contributor

I just tested from the head of release-0.5.2 using the template:

Resources:
  ClusterSg:
    Type: AWS::EC2::SecurityGroup
    Properties:
      VpcId:
        Fn::ImportValue: !Sub ${EnvironmentType}-net-vpc-VpcId
      GroupDescription: !Sub ${EnvironmentType}-sg-${ClusterName}
      SecurityGroupEgress:
        - CidrIp: "0.0.0.0/0"
          IpProtocol : "-1"
          Description: "Allow all outgoing traffic"
      SecurityGroupIngress:
        - SourceSecurityGroupId: !Ref AlbSg
          IpProtocol : "-1"
          Description: !Sub "Allow incoming traffic from ${ClusterName} ALB"
      Tags:
        - Key: EnvironmentType
          Value: !Ref EnvironmentType
        - Key: OwnerContact
          Value: !Ref OwnerContact
        - Key: Name
          Value: !Sub ${EnvironmentType}-${ClusterName}-sg
        - Key: ClusterName
          Value: !Ref ECSCluster
        - Key: Scope
          Value: ecs

And the ruleset:

AWS::EC2::SecurityGroup Tags.* == {"Key":"EnvironmentType","Value":"EnvironmentType"}
AWS::EC2::SecurityGroup Tags.* == {"Key":"OwnerContact","Value":"OwnerContact"}

It failed with the tags commented out:

⋊> ~/p/cloudformation-guard on release-0.5.2 ◦ cfn-guard -t ~/scratch-template.yaml -r ~/scratch.ruleset                                                                                                                              12:58:19
[ClusterSg] failed because [Tags.0] is [{"Key":"Name","Value":"${EnvironmentType}-${ClusterName}-sg"}] and the permitted value is [{"Key":"EnvironmentType","Value":"EnvironmentType"}]
[ClusterSg] failed because [Tags.0] is [{"Key":"Name","Value":"${EnvironmentType}-${ClusterName}-sg"}] and the permitted value is [{"Key":"OwnerContact","Value":"OwnerContact"}]
[ClusterSg] failed because [Tags.1] is [{"Key":"ClusterName","Value":"ECSCluster"}] and the permitted value is [{"Key":"EnvironmentType","Value":"EnvironmentType"}]
[ClusterSg] failed because [Tags.1] is [{"Key":"ClusterName","Value":"ECSCluster"}] and the permitted value is [{"Key":"OwnerContact","Value":"OwnerContact"}]
[ClusterSg] failed because [Tags.2] is [{"Key":"Scope","Value":"ecs"}] and the permitted value is [{"Key":"EnvironmentType","Value":"EnvironmentType"}]
[ClusterSg] failed because [Tags.2] is [{"Key":"Scope","Value":"ecs"}] and the permitted value is [{"Key":"OwnerContact","Value":"OwnerContact"}]
Number of failures: 6

And passed when the tags were uncommented:

⋊> ~/p/cloudformation-guard on release-0.5.2 ◦ cfn-guard -t ~/scratch-template.yaml -r ~/scratch.ruleset                                                                                                                              12:58:38
⋊> ~/p/cloudformation-guard on release-0.5.2 ◦

What's the error you got?

@f0o
Copy link
Author

f0o commented Jul 6, 2020

Actually looking at the results again it seems to be matching different Tags objects that it just didnt match before...

Something like:

    Type: "AWS::MediaLive::Channel"
    Properties:
      Tags:
        OwnerContact: !Ref OwnerContact
        EnvironmentType: !Ref EnvironmentType

Had to heavily redact this one sadly.

But this will be matched against:

[MediaLiveChannel] failed because [EnvironmentType] is not in [{"Key":"EnvironmentType","Value":"EnvironmentType"},{"Key":"EnvironmentType","Value":{"Ref":"EnvironmentType"}},{"Key":"EnvironmentType","PropagateAtLaunch":true,"Value":"EnvironmentType"},{"Key":"EnvironmentType","PropagateAtLaunch":true,"Value":{"Ref":"EnvironmentType"}}] for [Tags.EnvironmentType]

So I just added 'EnvironmentType' as literal into the array and it started matching again.

@nathanataws
Copy link
Contributor

Interesting! Can you give me the rule? I think some of the unexpected behavior might be tied to the new JSON list vs regular list.

@nathanataws
Copy link
Contributor

I'll just assume it's a list type rule and test with that, but I'd love to have the exact rule to make sure I'm not making bad assumptions.

@nathanataws
Copy link
Contributor

nathanataws commented Jul 6, 2020

Based on what you were saying about adding the literal making it work, I think I might know what you're running into:

let tag_list = [{"Key":"EnvironmentType","Value":"EnvironmentType"},{"Key":"EnvironmentType","Value":{"Ref":"EnvironmentType"}},{"Key":"EnvironmentType","PropagateAtLaunch":true,"Value":"EnvironmentType"},{"Key":"EnvironmentType","PropagateAtLaunch":true,"Value":{"Ref":"EnvironmentType"}}]

AWS::EC2::SecurityGroup Tags.* IN %tag_list

Works because when it's looking at tags, each item in the tag list represented by the * themselves resolve to the full item (eg, Key: "key", Value: "value").

However if the rule is written like:

AWS::EC2::SecurityGroup Tags.*.Key IN %tag_list

It'll fail because "Key" would resolve to "key", not Key: "key":

⋊> ~/p/cloudformation-guard on release-0.5.2 ◦ cfn-guard -t ~/scratch-template.yaml -r ~/scratch.ruleset                                                                                                                              13:39:54
[ClusterSg] failed because [ClusterName] is not in [{"Key":"EnvironmentType","Value":"EnvironmentType"},{"Key":"EnvironmentType","Value":{"Ref":"EnvironmentType"}},{"Key":"EnvironmentType","PropagateAtLaunch":true,"Value":"EnvironmentType"},{"Key":"EnvironmentType","PropagateAtLaunch":true,"Value":{"Ref":"EnvironmentType"}}] for [Tags.3.Key]
[ClusterSg] failed because [EnvironmentType] is not in [{"Key":"EnvironmentType","Value":"EnvironmentType"},{"Key":"EnvironmentType","Value":{"Ref":"EnvironmentType"}},{"Key":"EnvironmentType","PropagateAtLaunch":true,"Value":"EnvironmentType"},{"Key":"EnvironmentType","PropagateAtLaunch":true,"Value":{"Ref":"EnvironmentType"}}] for [Tags.0.Key]
[ClusterSg] failed because [Name] is not in [{"Key":"EnvironmentType","Value":"EnvironmentType"},{"Key":"EnvironmentType","Value":{"Ref":"EnvironmentType"}},{"Key":"EnvironmentType","PropagateAtLaunch":true,"Value":"EnvironmentType"},{"Key":"EnvironmentType","PropagateAtLaunch":true,"Value":{"Ref":"EnvironmentType"}}] for [Tags.2.Key]
[ClusterSg] failed because [OwnerContact] is not in [{"Key":"EnvironmentType","Value":"EnvironmentType"},{"Key":"EnvironmentType","Value":{"Ref":"EnvironmentType"}},{"Key":"EnvironmentType","PropagateAtLaunch":true,"Value":"EnvironmentType"},{"Key":"EnvironmentType","PropagateAtLaunch":true,"Value":{"Ref":"EnvironmentType"}}] for [Tags.1.Key]
[ClusterSg] failed because [Scope] is not in [{"Key":"EnvironmentType","Value":"EnvironmentType"},{"Key":"EnvironmentType","Value":{"Ref":"EnvironmentType"}},{"Key":"EnvironmentType","PropagateAtLaunch":true,"Value":"EnvironmentType"},{"Key":"EnvironmentType","PropagateAtLaunch":true,"Value":{"Ref":"EnvironmentType"}}] for [Tags.4.Key]
Number of failures: 5

If you added a literal item to the list that would match to "key", eg:

let tag_list = ["EnvironmentType", {"Key":"EnvironmentType","Value":"EnvironmentType"}]

It'll pass because a "Key"'s actual value will match to EnvironmentType.

@f0o
Copy link
Author

f0o commented Jul 6, 2020

The ruleset hasn't changed, I still just use .. Tags.* IN %var - I just had to add the EnvironmentType as string into the %var array and it fixed itself :)

I assume it wasnt matching the Tags of that resource previously so all in all it got better :D

@nathanataws
Copy link
Contributor

Okay. If you're cool with how it works, that's a good thing. :-)

On my end, things are working as expected and I have lots of tests around both the list types and the wildcards, so hopefully, I have all of them nailed down. If not, please do reopen and we'll keep unpacking this.

@nathanataws nathanataws moved this from Coming Soon to Shipped in Roadmap Jul 7, 2020
priyap286 pushed a commit that referenced this issue May 17, 2021
@KarthikVenkatraman
Copy link

KarthikVenkatraman commented Nov 5, 2021

Hi @f0o
were you able to accomplish what you wanted (check for mandatory tags on your resources in addition to other tags it may have) ? If so can you please share the cfn-guard rule snippet please ? I have been struggling to get this working and came across your post.. Will be really helpful if you can please share the working rule.

@f0o
Copy link
Author

f0o commented Nov 7, 2021

@KarthikVenkatraman have a look at #152 for v1 rulesets - otherwise the ruleset of v0 as posted above in this thread still work fine

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working release-0.5.2
Projects
Roadmap
  
Shipped
Development

No branches or pull requests

3 participants