-
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
Validation of Properties on EC2 code modules #3472
Comments
If I create a registry document like {
"typeName": "ASRS::EC2::EC2::MODULE",
"description": "A fun little resource",
"additionalProperties": false,
"properties": {
"name": {
"type" : "string"
},
"instanceType": {
"type" : "string"
},
"imageID": {
"type" : "string"
},
"subnetID": {
"type" : "string"
},
"securityGroupIDs": {
"format": "AWS::EC2::SecurityGroup.Ids",
"insertionOrder": false,
"items": {
"format": "AWS::EC2::SecurityGroup.GroupId",
"type": "string"
},
"type": "array",
"uniqueItems": false
},
"numberOfAdditionalVolumes": {
"type" : "integer"
},
"volume2Size": {
"type" : "integer"
}
},
"primaryIdentifier": []
} I should be able to do This will output:
|
I must have been very tired trying variations of this exact thing, because this worked perfectly. I was also able to update my module schema.json (though i have yet to test the module registration process/usage): {
"typeName": "ASRS::EC2::EC2::MODULE",
"description": "Schema for Module Fragment of type ASRS::EC2::EC2::MODULE",
"properties": {
"securityGroupIDs": {
"format": "AWS::EC2::SecurityGroup.Ids",
"insertionOrder": false,
"items": {
"format": "AWS::EC2::SecurityGroup.GroupId",
"type": "string"
}
},
"Parameters": {
"type": "object",
"properties": {
"name": {
"type": "object",
"properties": {
"Type": {
"type": "string"
},
"Description": {
"type": "string"
}
},
"required": [
"Type",
"Description"
],
"description": "The name tag for the EC2 instance."
},
"imageID": {
"type": "object",
"properties": {
"Type": {
"type": "string"
},
"Description": {
"type": "string"
}
},
"required": [
"Type",
"Description"
],
"description": "Base AMI for the EC2 instance."
},
"instanceType": {
"type": "object",
"properties": {
"Type": {
"type": "string"
},
"Description": {
"type": "string"
}
},
"required": [
"Type",
"Description"
],
"description": "EC2 instance type."
},
"subnetID": {
"type": "object",
"properties": {
"Type": {
"type": "string"
},
"Description": {
"type": "string"
}
},
"required": [
"Type",
"Description"
],
"description": "The ID of the subnet to launch the instance into."
},
"osVolumeSize": {
"type": "object",
"properties": {
"Type": {
"type": "string"
},
"Description": {
"type": "string"
}
},
"required": [
"Type",
"Description"
],
"description": "Size of the volume that has the OS."
},
"securityGroupIDs": {
"type": "object",
"properties": {
"Type": {
"type": "string"
},
"Description": {
"type": "string"
}
},
"required": [
"Type",
"Description"
],
"description": "The IDs of the security groups."
},
"instanceKeyName": {
"type": "object",
"properties": {
"Type": {
"type": "string"
},
"Description": {
"type": "string"
}
},
"required": [
"Type",
"Description"
],
"description": "Key pair name"
},
"ebsOptimized": {
"type": "object",
"properties": {
"Type": {
"type": "string"
},
"Description": {
"type": "string"
}
},
"required": [
"Type",
"Description"
],
"description": "Indicates whether the instance is optimized for Amazon EBS I/O."
}
}
},
"Resources": {
"properties": {
"Instance": {
"type": "object",
"properties": {
"Type": {
"type": "string",
"const": "AWS::EC2::Instance"
},
"Properties": {
"type": "object"
}
}
}
},
"type": "object",
"additionalProperties": false
}
},
"primaryIdentifier": [
"/properties/name"
],
"additionalProperties": true
} Thanks again. |
After testing, the code module fails to deploy with the primary identifier specified, however the module still deploys with the subnetIDs property. I suppose, as you said, if I define a schema specifically for lint validation that will definitely solve the problem. I tried the following override spec to try and add the validation on top of the schema, but it doesn't seem to apply: This may be blurring the lines at this point, but I did notice your name on the AWS article talking about publishing CodeModules as well. Is there a way to either make the CodeModule happy with this primaryIdentifier in the schema or a way to get the override to work? I can't quite seem to close this last issue out. Here be Lotsa CodeThis is the module schema without primaryIdentifier including all the extra junk i cut out for sanity before but for completeness now. We ... do have some instances with almost 26 volumes. {
"typeName": "ASRS::EC2::EC2::MODULE",
"description": "Schema for Module Fragment of type ASRS::EC2::EC2::MODULE",
"properties": {
"securityGroupIDs": {
"format": "AWS::EC2::SecurityGroup.Ids",
"insertionOrder": false,
"items": {
"format": "AWS::EC2::SecurityGroup.GroupId",
"type": "string"
}
},
"Parameters": {
"type": "object",
"properties": {
"name": {
"type": "object",
"properties": {
"Type": {
"type": "string"
},
"Description": {
"type": "string"
}
},
"required": [
"Type",
"Description"
],
"description": "The name tag for the EC2 instance."
},
"imageID": {
"type": "object",
"properties": {
"Type": {
"type": "string"
},
"Description": {
"type": "string"
}
},
"required": [
"Type",
"Description"
],
"description": "Base AMI for the EC2 instance."
},
"instanceType": {
"type": "object",
"properties": {
"Type": {
"type": "string"
},
"Description": {
"type": "string"
}
},
"required": [
"Type",
"Description"
],
"description": "EC2 instance type."
},
"subnetID": {
"type": "object",
"properties": {
"Type": {
"type": "string"
},
"Description": {
"type": "string"
}
},
"required": [
"Type",
"Description"
],
"description": "The ID of the subnet to launch the instance into."
},
"osVolumeSize": {
"type": "object",
"properties": {
"Type": {
"type": "string"
},
"Description": {
"type": "string"
}
},
"required": [
"Type",
"Description"
],
"description": "Size of the volume that has the OS."
},
"securityGroupIDs": {
"type": "object",
"properties": {
"Type": {
"type": "string"
},
"Description": {
"type": "string"
}
},
"required": [
"Type",
"Description"
],
"description": "The IDs of the security groups."
},
"instanceKeyName": {
"type": "object",
"properties": {
"Type": {
"type": "string"
},
"Description": {
"type": "string"
}
},
"required": [
"Type",
"Description"
],
"description": "Key pair name"
},
"ebsOptimized": {
"type": "object",
"properties": {
"Type": {
"type": "string"
},
"Description": {
"type": "string"
}
},
"required": [
"Type",
"Description"
],
"description": "Indicates whether the instance is optimized for Amazon EBS I/O."
},
"numberOfAdditionalVolumes": {
"type": "object",
"properties": {
"Type": {
"type": "string"
},
"Description": {
"type": "string"
}
},
"required": [
"Type",
"Description"
],
"description": "Additional volumes in addition to OS volume. Max is 25."
},
"volume2Size": {
"type": "object",
"properties": {
"Type": {
"type": "string"
},
"Description": {
"type": "string"
}
},
"required": [
"Type",
"Description"
],
"description": "Size of volume 2"
},
"volume3Size": {
"type": "object",
"properties": {
"Type": {
"type": "string"
},
"Description": {
"type": "string"
}
},
"required": [
"Type",
"Description"
],
"description": "Size of volume 3"
},
"volume4Size": {
"type": "object",
"properties": {
"Type": {
"type": "string"
},
"Description": {
"type": "string"
}
},
"required": [
"Type",
"Description"
],
"description": "Size of volume 4"
},
"volume5Size": {
"type": "object",
"properties": {
"Type": {
"type": "string"
},
"Description": {
"type": "string"
}
},
"required": [
"Type",
"Description"
],
"description": "Size of volume 5"
},
"volume6Size": {
"type": "object",
"properties": {
"Type": {
"type": "string"
},
"Description": {
"type": "string"
}
},
"required": [
"Type",
"Description"
],
"description": "Size of volume 6"
},
"volume7Size": {
"type": "object",
"properties": {
"Type": {
"type": "string"
},
"Description": {
"type": "string"
}
},
"required": [
"Type",
"Description"
],
"description": "Size of volume 7"
},
"volume8Size": {
"type": "object",
"properties": {
"Type": {
"type": "string"
},
"Description": {
"type": "string"
}
},
"required": [
"Type",
"Description"
],
"description": "Size of volume 8"
},
"volume9Size": {
"type": "object",
"properties": {
"Type": {
"type": "string"
},
"Description": {
"type": "string"
}
},
"required": [
"Type",
"Description"
],
"description": "Size of volume 9"
},
"volume10Size": {
"type": "object",
"properties": {
"Type": {
"type": "string"
},
"Description": {
"type": "string"
}
},
"required": [
"Type",
"Description"
],
"description": "Size of volume 10"
},
"volume11Size": {
"type": "object",
"properties": {
"Type": {
"type": "string"
},
"Description": {
"type": "string"
}
},
"required": [
"Type",
"Description"
],
"description": "Size of volume 11"
},
"volume12Size": {
"type": "object",
"properties": {
"Type": {
"type": "string"
},
"Description": {
"type": "string"
}
},
"required": [
"Type",
"Description"
],
"description": "Size of volume 12"
},
"volume13Size": {
"type": "object",
"properties": {
"Type": {
"type": "string"
},
"Description": {
"type": "string"
}
},
"required": [
"Type",
"Description"
],
"description": "Size of volume 13"
},
"volume14Size": {
"type": "object",
"properties": {
"Type": {
"type": "string"
},
"Description": {
"type": "string"
}
},
"required": [
"Type",
"Description"
],
"description": "Size of volume 14"
},
"volume15Size": {
"type": "object",
"properties": {
"Type": {
"type": "string"
},
"Description": {
"type": "string"
}
},
"required": [
"Type",
"Description"
],
"description": "Size of volume 15"
},
"volume16Size": {
"type": "object",
"properties": {
"Type": {
"type": "string"
},
"Description": {
"type": "string"
}
},
"required": [
"Type",
"Description"
],
"description": "Size of volume 16"
},
"volume17Size": {
"type": "object",
"properties": {
"Type": {
"type": "string"
},
"Description": {
"type": "string"
}
},
"required": [
"Type",
"Description"
],
"description": "Size of volume 17"
},
"volume18Size": {
"type": "object",
"properties": {
"Type": {
"type": "string"
},
"Description": {
"type": "string"
}
},
"required": [
"Type",
"Description"
],
"description": "Size of volume 18"
},
"volume19Size": {
"type": "object",
"properties": {
"Type": {
"type": "string"
},
"Description": {
"type": "string"
}
},
"required": [
"Type",
"Description"
],
"description": "Size of volume 19"
},
"volume20Size": {
"type": "object",
"properties": {
"Type": {
"type": "string"
},
"Description": {
"type": "string"
}
},
"required": [
"Type",
"Description"
],
"description": "Size of volume 20"
},
"volume21Size": {
"type": "object",
"properties": {
"Type": {
"type": "string"
},
"Description": {
"type": "string"
}
},
"required": [
"Type",
"Description"
],
"description": "Size of volume 21"
},
"volume22Size": {
"type": "object",
"properties": {
"Type": {
"type": "string"
},
"Description": {
"type": "string"
}
},
"required": [
"Type",
"Description"
],
"description": "Size of volume 22"
},
"volume23Size": {
"type": "object",
"properties": {
"Type": {
"type": "string"
},
"Description": {
"type": "string"
}
},
"required": [
"Type",
"Description"
],
"description": "Size of volume 23"
},
"volume24Size": {
"type": "object",
"properties": {
"Type": {
"type": "string"
},
"Description": {
"type": "string"
}
},
"required": [
"Type",
"Description"
],
"description": "Size of volume 24"
},
"volume25Size": {
"type": "object",
"properties": {
"Type": {
"type": "string"
},
"Description": {
"type": "string"
}
},
"required": [
"Type",
"Description"
],
"description": "Size of volume 25"
},
"volume26Size": {
"type": "object",
"properties": {
"Type": {
"type": "string"
},
"Description": {
"type": "string"
}
},
"required": [
"Type",
"Description"
],
"description": "Size of volume 26"
}
}
},
"Resources": {
"properties": {
"Instance": {
"type": "object",
"properties": {
"Type": {
"type": "string",
"const": "AWS::EC2::Instance"
},
"Properties": {
"type": "object"
}
}
}
},
"type": "object",
"additionalProperties": false
}
},
"additionalProperties": true
} Here's the override spec that does work as the ImageId for normal EC2s validates properly but the EC2 module doesn't: {
"Patches": {
"AWS::EC2::Instance": [
{
"op": "remove",
"path": "/properties/ImageId/format",
"value": "true"
},
{
"op": "add",
"path": "/properties/ImageId/pattern",
"value": "{resolveEC2ImageID:.+}"
}
]
},
"ASRS::EC2::EC2::Module": [
{
"op": "remove",
"path": "/primaryIdentifier",
"value": "true"
},
{
"op": "add",
"path": "/primaryIdentifier",
"value": "/properties/Name"
}
]
} I tried it without the remove section as there should be no primaryIdentifier but also with it just in case. I also tried a few variations of parameters/properties, but maybe I'm missing something simple again. |
Haven't had a chance to fully test this but... /primaryIdentifier needs to be an array of pointers {
"op": "add",
"path": "/primaryIdentifier",
"value": "/properties/Name"
} should be {
"op": "add",
"path": "/primaryIdentifier",
"value": ["/properties/Name"]
} or {
"op": "replace",
"path": "/primaryIdentifier",
"value": ["/properties/Name"]
} |
The "replace" operation fails with the error that it can't replace a non-existent object which makes sense in this case. The "add" operation results in a successful lint, though it should fail due to duplicate primaryIdentifier(s). Here's my altered override to correctly identify the primary key as an array.
Test template:
I'm using the pipeline template to test though hypothetically the 2 modules should function the same. |
Looking into. Think I was able to replicate this. |
It should be "replace" but since we were trying to patch before the registry schemas are loaded you are trying to patch the default "module" schema. Flipping the order on operations will have us loading the registry schemas before doing the patching. |
Another option I forgot about {
"op": "add",
"path": "/primaryIdentifier/-",
"value": "/properties/Name"
} |
{
"Patches": {
"AWS::EC2::Instance": [
{
"op": "remove",
"path": "/properties/ImageId/format",
"value": "true"
},
{
"op": "add",
"path": "/properties/ImageId/pattern",
"value": "{resolveEC2ImageID:.+}"
}
],
"ASRS::CodePipeline::NormalStages::MODULE": [
{
"op": "add",
"path": "/primaryIdentifier/-",
"value": "/properties/PipelineName"
}
]
}
} Yields:
If I add something like: {
"Patches": {
"AWS::EC2::Instance": [
{
"op": "remove",
"path": "/properties/ImageId/format",
"value": "true"
},
{
"op": "add",
"path": "/properties/ImageId/pattern",
"value": "{resolveEC2ImageID:.+}"
}
],
"ASRS::CodePipeline::NormalStages::MODULE": [
{
"op": "add",
"path": "/primaryIdentifier",
"value": "true"
},
{
"op": "add",
"path": "/primaryIdentifier/-",
"value": "/properties/PipelineName"
}
]
}
} I get:
I tried to use replace after add and also tried encapsulating the value in [ ] to make it an array. |
Can you check under the mainline branch? I meant use "Replace" or the "-" after my PR was released. Sorry for the confusion. |
No worries! I get no errors now, but the pipeline file above continues to lint successfully. I tried replace/add with /- and a few permutations of with/without array brackets and mix/maxing replace/add and /- and those brackets. |
I'm getting errors on
and
when running will produce
|
I was also able to replicate an error with the instance patch:
|
Well, as of cfn-lint 1.6.1 I'm getting the following error with this setup:
I'm relying on .cfnlintrc and here are the files I have at the moment:
cfnlintrc
code module schema:
ec2 schema
It...kind of looks like cfn-lint is now checking to see if primaryIdentifier exists in the schema and failing if it doesn't? The code modules, as I mentioned before, can't have primaryIdentifier defined or the registration of the code module seems to fail. It also seems like this is only happening on my Windows 11 desktop and not in my serverless lint process for pipelines. I'm trying to determine if something is wrong locally with lint for this error. I had updated to 1.6.1 before running any tests but it was working fine last night before the update. Well except being able to get the errors I wanted anyway. |
Because {
"Patches": {
"AWS::EC2::Instance": [
{
"op": "remove",
"path": "/properties/ImageId/format",
"value": "true"
},
{
"op": "add",
"path": "/properties/ImageId/pattern",
"value": "{resolveEC2ImageID:.+}"
}
],
"ASRS::EC2::EC2::MODULE": [
{
"op": "add",
"path": "/primaryIdentifier",
"value": [
"/properties/name"
]
}
],
"ASRS::CodePipeline::NormalStages::MODULE": [
{
"op": "add",
"path": "/primaryIdentifier",
"value": [
"/properties/PipelineName"
]
}
]
}
}
If the schemas had |
I am concerned about the windows/pipeline differences. May have to add some integration tests for this. |
In this case the variance makes sense -- the patch file is different between the two of them. Everything else is the same though, so I had wrongfully assumed it was just a schema problem and not a combination of the two of them. I think if the patch spec was the same I would get the same error. I can probably confirm that in a few minutes as I test the change there, thanks again for your patience. |
Thank you for yours and all the help in this discussion. |
So after testing your changes worked great and it's properly linting now. Additionally with that override spec the serverless run also threw the same errors regarding the missing primaryIdentifier, so no discrepancy after all. Think we're all done here, finally! Have a great weekend. |
cfn-lint: 1.5.0
operation system: AWS serverless & Windows 11
This EC2 module seems to lint successfully no matter how I alter the schema for the code module or add patches to the override_spec. The one thing I was able to get to lint properly was the primaryIdentifier as was discussed in #3446 , thank you again!
The 2 basic changes I'm looking to apply are a regex validation of {resolveEC2ImageID:.+} to the imageID similar to what I was doing in #3460 for the regular AWS::EC2::Instance resource type and to validate the securityGroupIDs as valid IDs or dynamic references like a regular EC2 instance.
This is the whole test template; the baselineSecurityGroupID !Ref is not valid in the above template and should fail validation due to the references being invalid. asdfasdfasdfasdf should also fail validation according to the schemas/override_specs I've tried to supply. the SSM resolution and the devTeamSecurityGroup !Ref should both pass validation, again like a normal EC2 instance.
I am trying to use the Amazon provided schema as a reference for my changes, but in #3446 it was mentioned that the module validation is minimal by design. Here's the part of that schema I've been trying to insert into the code module schema or use in the override spec for patching (adding/removing the property/parameter, etc). I did update the casing from below to match the casing on the code module in both the schema and the template as shown above.
So I suppose the question is: can I achieve the type of validation I'm looking for with code modules to ensure our developers are providing valid inputs? I do plan to add other validation like the length of the name etc, but I figure if I can get these 2 properties to lint the rest should be easy to add.
The text was updated successfully, but these errors were encountered: