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

AllowedValues from botocore during --update-specs #1682

Merged
merged 8 commits into from
Sep 25, 2020
Merged

Conversation

PatMyron
Copy link
Contributor

@PatMyron PatMyron commented Sep 5, 2020

most commonly maintained concise constraint format last time I checked:

src/cfnlint/data/ExtendedSpecs/all $ git grep 'AllowedValues":' | wc -l
AllowedValues: 187 # only 42 hardcoded AllowedValues lists remaining after this PR
NumberMin/NumberMax: 46
AllowedPatternRegex: 12
StringMin/StringMax: 8
ListMin/ListMax: 4
JsonMax: 2

#50 (comment), terraform-linters/tflint#274, terraform-linters/tflint#421


CloudSpecs is autogenerated by cfn-lint --update-specs


AllowedValues were hardcoded in ExtendedSpecs/all and patched into all the Resource Specifications in CloudSpecs when cfn-lint --update-specs was run. Now they're references to botocore types in ExtendedSpecs/all and cfn-lint --update-specs pulls from botocore directly to write that part of CloudSpecs


AWS::WorkSpaces::Workspace.ComputeTypeName should pick up POWERPRO and GRAPHICSPRO

AWS::CodeBuild::Project.Environment.Type should pick up WINDOWS_SERVER_2019_CONTAINER

AWS::S3::Bucket.TopicConfiguration.Event should pick up s3:ObjectRestore:* and s3:Replication:*

EbsVolumeType should pick up io2 from here

AWS::GuardDuty::IPSet.Format should pick up ALIEN_VAULT and PROOF_POINT and FIRE_EYE

AWS::CodeBuild::Project.Environment.ComputeType should pick up BUILD_GENERAL1_2XLARGE

AWS::CloudFront::Distribution.SslSupportMethod should pick up static-ip

AWS::WAFv2::RuleGroup.Rate.AggregateKeyType should pick up FORWARDED_IP

AWS::Glue::Connection.ConnectionInput.ConnectionType picks up NETWORK

AWS::EC2::LaunchTemplate.TagSpecification.ResourceType should pick up 42 values


Of the decent sized hardcoded AllowedValues lists left, AWS::SecretsManager::SecretTargetAttachment.TargetType, AWS::DMS::Endpoint.EngineName, AWS::Glue::Trigger.Condition.State, and AWS::AmazonMQ::Broker.EngineVersion are the only ones that changed in 2020. Will watch their documentation with http://followthatpage.com/

#50

AWS::WorkSpaces::Workspace.ComputeTypeName should pick up POWERPRO and GRAPHICSPRO:
https://github.com/boto/botocore/blob/ae2d0855ac3162520e584a24f5522b633de4e2a9/botocore/data/workspaces/2015-04-08/service-2.json#L822-L823
https://docs.aws.amazon.com/AWSCloudFormation/latest/UserGuide/aws-properties-workspaces-workspace-workspaceproperties.html#cfn-workspaces-workspace-workspaceproperties-computetypename

AWS::CodeBuild::Project.Environment.Type should pick up WINDOWS_SERVER_2019_CONTAINER:
https://github.com/boto/botocore/blob/ae2d0855ac3162520e584a24f5522b633de4e2a9/botocore/data/codebuild/2016-10-06/service-2.json#L1873
https://docs.aws.amazon.com/AWSCloudFormation/latest/UserGuide/aws-properties-codebuild-project-environment.html#cfn-codebuild-project-environment-type

AWS::S3::Bucket.TopicConfiguration.Event should pick up s3:ObjectRestore:* and s3:Replication:*:
https://github.com/boto/botocore/blob/ae2d0855ac3162520e584a24f5522b633de4e2a9/botocore/data/s3/2006-03-01/service-2.json#L2887-L2890
https://docs.aws.amazon.com/AWSCloudFormation/latest/UserGuide/aws-properties-s3-bucket-notificationconfig-topicconfig.html#cfn-s3-bucket-notificationconfig-topicconfig-event
https://docs.aws.amazon.com/AmazonS3/latest/dev/NotificationHowTo.html#notification-how-to-event-types-and-destinations

EbsVolumeType should pick up io2:
https://aws.amazon.com/blogs/aws/new-ebs-volume-type-io2-more-iops-gib-higher-durability/
https://raw.githubusercontent.com/boto/botocore/master/botocore/data/ec2/2016-11-15/service-2.json
https://docs.aws.amazon.com/AWSCloudFormation/latest/UserGuide/aws-properties-ec2-blockdev-template.html#cfn-ec2-blockdev-template-volumetype
https://docs.aws.amazon.com/AWSCloudFormation/latest/UserGuide/aws-properties-ec2-ebs-volume.html#cfn-ec2-ebs-volume-volumetype

AWS::GuardDuty::IPSet.Format should pick up ALIEN_VAULT and PROOF_POINT and FIRE_EYE:
https://github.com/boto/botocore/blob/ae2d0855ac3162520e584a24f5522b633de4e2a9/botocore/data/guardduty/2017-11-28/service-2.json#L3130-L3132
https://docs.aws.amazon.com/AWSCloudFormation/latest/UserGuide/aws-resource-guardduty-ipset.html#cfn-guardduty-ipset-format

AWS::CodeBuild::Project.Environment.ComputeType should pick up BUILD_GENERAL1_2XLARGE:
https://github.com/boto/botocore/blob/16958153b1e01dcbf5d4334394a14715438b53f1/botocore/data/codebuild/2016-10-06/service-2.json#L1413
https://docs.aws.amazon.com/AWSCloudFormation/latest/UserGuide/aws-properties-codebuild-project-environment.html#cfn-codebuild-project-environment-computetype
https://docs.aws.amazon.com/codebuild/latest/userguide/build-env-ref-compute-types.html

AWS::CloudFront::Distribution.SslSupportMethod should pick up static-ip:
https://github.com/boto/botocore/blob/ae2d0855ac3162520e584a24f5522b633de4e2a9/botocore/data/cloudfront/2020-05-31/service-2.json#L6101
https://docs.aws.amazon.com/AWSCloudFormation/latest/UserGuide/aws-properties-cloudfront-distribution-viewercertificate.html#cfn-cloudfront-distribution-viewercertificate-sslsupportmethod

AWS::WAFv2::RuleGroup.Rate.AggregateKeyType should pick up FORWARDED_IP:
https://github.com/boto/botocore/blob/b9b494aac2a0dc03433a8cda711d3dfabb4e5b45/botocore/data/wafv2/2019-07-29/service-2.json#L2715
https://docs.aws.amazon.com/AWSCloudFormation/latest/UserGuide/aws-properties-wafv2-rulegroup-ratebasedstatementone.html#cfn-wafv2-rulegroup-ratebasedstatementone-aggregatekeytype
https://docs.aws.amazon.com/AWSCloudFormation/latest/UserGuide/aws-properties-wafv2-rulegroup-ratebasedstatementtwo.html#cfn-wafv2-rulegroup-ratebasedstatementtwo-aggregatekeytype
https://docs.aws.amazon.com/AWSCloudFormation/latest/UserGuide/aws-properties-wafv2-webacl-ratebasedstatementone.html#cfn-wafv2-webacl-ratebasedstatementone-aggregatekeytype
https://docs.aws.amazon.com/AWSCloudFormation/latest/UserGuide/aws-properties-wafv2-webacl-ratebasedstatementtwo.html#cfn-wafv2-webacl-ratebasedstatementtwo-aggregatekeytype

AWS::EC2::LaunchTemplate.TagSpecification.ResourceType should pick up 42 values:
https://raw.githubusercontent.com/boto/botocore/master/botocore/data/ec2/2016-11-15/service-2.json
https://docs.aws.amazon.com/AWSCloudFormation/latest/UserGuide/aws-properties-ec2-launchtemplate-tagspecification.html#cfn-ec2-launchtemplate-tagspecification-resourcetype

AWS::Glue::Connection.ConnectionInput.ConnectionType picks up NETWORK:
https://github.com/boto/botocore/blob/ae2d0855ac3162520e584a24f5522b633de4e2a9/botocore/data/glue/2017-03-31/service-2.json#L3372
https://docs.aws.amazon.com/AWSCloudFormation/latest/UserGuide/aws-properties-glue-connection-connectioninput.html#cfn-glue-connection-connectioninput-connectiontype
src/cfnlint/maintenance.py Outdated Show resolved Hide resolved
@PatMyron PatMyron marked this pull request as ready for review September 22, 2020 18:47
@PatMyron PatMyron changed the title AllowedValues from botocore AllowedValues from botocore during --update-specs Sep 22, 2020
there are 145 types from 37 services, so this should reduce requests ~4x

$ time cfn-lint --update-specs # before

real  10m3.694s
user  2m12.121s
sys 0m27.602s

$ time cfn-lint --update-specs # after

real  2m15.260s
user  0m52.673s
sys 0m9.445s
src/cfnlint/maintenance.py Outdated Show resolved Hide resolved
service = '/'.join(service_and_type.split('/')[:-1])
botocore_type = service_and_type.split('/')[-1]
if service not in botocore_cache:
botocore_cache[service] = json.loads(get_url_content('https://raw.githubusercontent.com/boto/botocore/master/botocore/data/' + service + '/service-2.json'))
Copy link
Contributor

@kddejong kddejong Sep 23, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Another option here is

Suggested change
botocore_cache[service] = json.loads(get_url_content('https://raw.githubusercontent.com/boto/botocore/master/botocore/data/' + service + '/service-2.json'))
botocore_cache[service] = json.loads(pkgutil.get_data('botocore', os.path.join('data', service, 'service-2.json')))

While we don't directly rely on botocore but it is getting loaded because of the SAM dependencies.

Copy link
Contributor Author

@PatMyron PatMyron Sep 23, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the biggest decision I've been debating
This would speed --update-specs up significantly, but would grab outdated information if botocore is outdated

$ time cfn-lint --update-specs # pulling from botocore latest on Github 
real  2m15.260s

$ time pip3 install botocore --upgrade
real	0m15.649s
$ time cfn-lint --update-specs # pulling directly from local copy of botocore
real	0m24.975s


# might be able to pull latest data faster than these individual requests though

$ time git clone https://github.com/boto/botocore/ --depth 1
real	0m7.271s

$ time curl -s -L https://codeload.github.com/boto/botocore/zip/master > botocore.zip
real	0m2.605s
$ time unzip botocore.zip
real	0m2.074s

https://stackoverflow.com/questions/56170380/downloading-files-raw-githubusercontent-com-is-immensely-slow

@miparnisari
Copy link
Contributor

have you ensured that there is 0 difference in the json files before this PR and with this PR?

@PatMyron
Copy link
Contributor Author

PatMyron commented Sep 25, 2020

have you ensured that there is 0 difference in the json files before this PR and with this PR?

the difference in CloudSpecs is that all of these AllowedValues are now sorted and all the additional AllowedValues from botocore we were missing listed in the description

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants