-
Notifications
You must be signed in to change notification settings - Fork 597
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
cfn-lint v1 does not warn about missing mapping keys (nested FindInMap) #3451
Comments
+1 to this I want to figure this out but also trying to scale it for larger usage. The logic isn't great in v0 but for sure provides the error you have above in the scenario you provided. I'm going to use the docs description of map name, top level key, and second level key. The way the v0 logic works is it requires a string for the map name and second level key and if the top level key is a function then we can validate the second level key exists for all top level keys. Let me ask back a few questions around assumptions I keep thinking about. Can second level keys be different between top level keys? What should the error, if any, be here? Should we say A doesn't exist when Parameters:
Parameter:
Type: String
Mappings:
Map:
One:
A: "foo"
Two:
B: "bar"
Resources:
Bucket:
Type: AWS::S3::Bucket
Properties:
BucketName: !FindInMap [Map, !Ref Parameter, A] what if I change the parameter to Parameters:
Parameter:
Type: String
AllowedValues: ["One"] Do we want to expand this to when we are using functions for the map name and second level key? |
I will take a stab at these answers but obviously I am biased towards my use-case and the specific scenario I am trying to catch. I may not have thought about this enough and others may have different expectations. I think a general approach would be:
There might be other second level keys but if a specific second level key exists under one top level key but not under another that could raise an error if there is a bounded set of function outputs and at least one would require the missing key, and a warning if the function outputs are unbounded.
Yes, as a warning.
Yes, because the is no constraint on the values
Yes
This should output no warnings or errors, but if you change it to
In an ideal world, yes. |
I've done something like this before: Mappings:
Constants:
default:
ToolWorkspaceId: 12345
OtherToolDomain: api.tool.com So I could use If you have a lot of constants, you could change this to be
I however do not think you'd use that pattern with a I personally think it's fine to warn / err if the First Level Key is set by a !Ref and there is a missing Second Level Key |
what do you mean? I have extensive use of these 3 patterns: Something: !FindInMap
- Map1
- !FindInMap
- Map2
- !FindInMap
- Map3
- !Ref AWS::Region
- !Ref Param
- !Ref AWS::AccountId
- SomeString Something: !FindInMap
- Map1
- !FindInMap
- Map2
- !Ref AWS::Region
- !Ref Param
- SomeString Something: !FindInMap
- Map
- !Ref Param
- SomeString |
I don't think you'd use Mappings for constants, with different 2nd level keys, while also dynamically building the FindInMap E.g, this seems unlikely: Mappings:
Constants:
ToolOne:
ApiKey: 12345
ToolTwo:
Domain: tool.tld together with |
The original issue in this template is now caught. We have disabled the ability to check a mapping with a pseudo parameter. This is because we need to figure out what the account ID is or could be. Mappings:
Account:
111111111111:
BucketName: string
222222222222:
BucketName: string
There would have to be some understanding of logic. Mappings:
Account:
111111111111:
BucketName: string
222222222222:
SourceBucketName: string
NotValidAccountId:
BucketName: string Whats the valid errors or warnings here... |
CloudFormation Lint Version
1.4.2 & git (30760e2)
What operating system are you using?
Mac
Describe the bug
Very similar to #3385. The basic case is fixed but keys missing that involve nested
FindInMap
are still not detected.cfn-lint
0.87.9 can detect:cfn-lint
1.4.2 / git 30760e2 accept the example template without warnings or errorsExpected behavior
Reproduction template
The text was updated successfully, but these errors were encountered: