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

catch common resource schema issues in cfn validate #729

Merged
merged 4 commits into from
Apr 26, 2021
Merged

Conversation

PatMyron
Copy link
Contributor

@PatMyron PatMyron commented Apr 13, 2021

continuing #663, #668, #675

more incorrect type-specific JSON schema keywords similar to #414


how to run new validations on all existing resource provider schemas:

Incorrect JSON schema keyword(s) {'additionalProperties'} for type: array for property: EC2InboundPermissions
Incorrect JSON schema keyword(s) {'additionalProperties'} for type: array for property: InstanceDefinitions
Incorrect JSON schema keyword(s) {'additionalProperties'} for type: array for property: LogPaths
Incorrect JSON schema keyword(s) {'additionalProperties'} for type: array for property: MetricGroups
Incorrect JSON schema keyword(s) {'additionalProperties'} for type: array for property: MonitoringInputs
Incorrect JSON schema keyword(s) {'additionalProperties'} for type: array for property: ServerProcesses
Incorrect JSON schema keyword(s) {'additionalProperties'} for type: array for property: Tags
Incorrect JSON schema keyword(s) {'additionalProperties'} for type: array for property: TaskProperties
Incorrect JSON schema keyword(s) {'additionalProperties'} for type: array for property: VpcSubnets
Incorrect JSON schema keyword(s) {'additionalProperties'} for type: integer for property: ConcurrentExecutions
Incorrect JSON schema keyword(s) {'additionalProperties'} for type: integer for property: DesiredEC2Instances
Incorrect JSON schema keyword(s) {'additionalProperties'} for type: integer for property: FromPort
Incorrect JSON schema keyword(s) {'additionalProperties'} for type: integer for property: GameSessionActivationTimeoutSeconds
Incorrect JSON schema keyword(s) {'additionalProperties'} for type: integer for property: MaxConcurrentGameSessionActivations
Incorrect JSON schema keyword(s) {'additionalProperties'} for type: integer for property: MaxSize
Incorrect JSON schema keyword(s) {'additionalProperties'} for type: integer for property: MinSize
Incorrect JSON schema keyword(s) {'additionalProperties'} for type: integer for property: NewGameSessionsPerCreator
Incorrect JSON schema keyword(s) {'additionalProperties'} for type: integer for property: PolicyPeriodInMinutes
Incorrect JSON schema keyword(s) {'additionalProperties'} for type: integer for property: ToPort
Incorrect JSON schema keyword(s) {'additionalProperties'} for type: number for property: EstimatedInstanceWarmup
Incorrect JSON schema keyword(s) {'additionalProperties'} for type: number for property: MaxSize
Incorrect JSON schema keyword(s) {'additionalProperties'} for type: number for property: MinSize
Incorrect JSON schema keyword(s) {'additionalProperties'} for type: number for property: TargetValue
Incorrect JSON schema keyword(s) {'additionalProperties'} for type: string for property: AutoScalingGroupArn
Incorrect JSON schema keyword(s) {'additionalProperties'} for type: string for property: BalancingStrategy
Incorrect JSON schema keyword(s) {'additionalProperties'} for type: string for property: BuildId
Incorrect JSON schema keyword(s) {'additionalProperties'} for type: string for property: CertificateType
Incorrect JSON schema keyword(s) {'additionalProperties'} for type: string for property: DeleteOption
Incorrect JSON schema keyword(s) {'additionalProperties'} for type: string for property: Description
Incorrect JSON schema keyword(s) {'additionalProperties'} for type: string for property: EC2InstanceType
Incorrect JSON schema keyword(s) {'additionalProperties'} for type: string for property: FleetId
Incorrect JSON schema keyword(s) {'additionalProperties'} for type: string for property: FleetType
Incorrect JSON schema keyword(s) {'additionalProperties'} for type: string for property: GameServerGroupArn
Incorrect JSON schema keyword(s) {'additionalProperties'} for type: string for property: GameServerGroupName
Incorrect JSON schema keyword(s) {'additionalProperties'} for type: string for property: GameServerProtectionPolicy
Incorrect JSON schema keyword(s) {'additionalProperties'} for type: string for property: InstanceRoleARN
Incorrect JSON schema keyword(s) {'additionalProperties'} for type: string for property: InstanceType
Incorrect JSON schema keyword(s) {'additionalProperties'} for type: string for property: IpRange
Incorrect JSON schema keyword(s) {'additionalProperties'} for type: string for property: LaunchPath
Incorrect JSON schema keyword(s) {'additionalProperties'} for type: string for property: LaunchTemplateId
Incorrect JSON schema keyword(s) {'additionalProperties'} for type: string for property: LaunchTemplateName
Incorrect JSON schema keyword(s) {'additionalProperties'} for type: string for property: Name
Incorrect JSON schema keyword(s) {'additionalProperties'} for type: string for property: NewGameSessionProtectionPolicy
Incorrect JSON schema keyword(s) {'additionalProperties'} for type: string for property: Parameters
Incorrect JSON schema keyword(s) {'additionalProperties'} for type: string for property: PeerVpcAwsAccountId
Incorrect JSON schema keyword(s) {'additionalProperties'} for type: string for property: PeerVpcId
Incorrect JSON schema keyword(s) {'additionalProperties'} for type: string for property: Protocol
Incorrect JSON schema keyword(s) {'additionalProperties'} for type: string for property: RoleArn
Incorrect JSON schema keyword(s) {'additionalProperties'} for type: string for property: ScriptId
Incorrect JSON schema keyword(s) {'additionalProperties'} for type: string for property: ServerLaunchParameters
Incorrect JSON schema keyword(s) {'additionalProperties'} for type: string for property: ServerLaunchPath
Incorrect JSON schema keyword(s) {'additionalProperties'} for type: string for property: Version
Incorrect JSON schema keyword(s) {'additionalProperties'} for type: string for property: WeightedCapacity

format seems intentionally used for other types?, so I guess I'll leave that out of the new validation, even though it looks string-specific in JSON schema proper

"uniqueItems",
"pattern",
"patternProperties",
"multipleOf",
}
try: # pylint: disable=R
for _key, schema in JsonSchemaFlattener(resource_spec).flatten_schema().items():
Copy link
Contributor

Choose a reason for hiding this comment

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

if u are not using _key could u change it to _

Copy link
Contributor

Choose a reason for hiding this comment

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

just to clarify - you are using flattened schema to avoid recursive traversal, since all the subschemas are on the same level?

@@ -176,36 +182,45 @@ def load_resource_spec(resource_spec_file): # pylint: disable=R # noqa: C901
"maximum",
"exclusiveMinimum",
"exclusiveMaximum",
"multipleOf",
Copy link
Contributor

Choose a reason for hiding this comment

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

cant leave the comment above but could u take this structure out and create a static _dict_? it is really hard to read this way.

Copy link
Contributor

Choose a reason for hiding this comment

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

I support this idea too. Also can you create type_specific_keywords from the _dict_ you create for this structure so that all keywords specified in the _dict_ will be automatically available in type_specific_keywords

Copy link
Contributor Author

@PatMyron PatMyron Apr 16, 2021

Choose a reason for hiding this comment

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

Started with dictionary, but integer and number types share JSON schema keywords, and lists/sets cannot be used as Python dictionary keys, so I switched

Switched type_specific_keywords to be automatically generated from the mapping :)

src/rpdk/core/data_loaders.py Show resolved Hide resolved
@@ -176,36 +182,45 @@ def load_resource_spec(resource_spec_file): # pylint: disable=R # noqa: C901
"maximum",
"exclusiveMinimum",
"exclusiveMaximum",
"multipleOf",
Copy link
Contributor

Choose a reason for hiding this comment

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

I support this idea too. Also can you create type_specific_keywords from the _dict_ you create for this structure so that all keywords specified in the _dict_ will be automatically available in type_specific_keywords

Comment on lines +222 to +223
type_specific_keywords - allowed_keywords
& property_keywords,
Copy link
Contributor

@priyap286 priyap286 Apr 16, 2021

Choose a reason for hiding this comment

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

Can you put this in a variable and use it above and here?

@PatMyron PatMyron requested a review from kddejong April 22, 2021 20:00
@PatMyron PatMyron merged commit a9ab27a into master Apr 26, 2021
@PatMyron PatMyron deleted the schema-issues branch April 26, 2021 16:35
eduardomourar pushed a commit to eduardomourar/cloudformation-cli that referenced this pull request Apr 30, 2021
@PatMyron PatMyron added schema processing schema Related to the provider meta schema labels May 5, 2021
@PatMyron
Copy link
Contributor Author

PatMyron commented May 26, 2021

Current running totals for existing AWS resource provider schemas:

 724 Explicitly specify value for insertionOrder for array
 314 Explicitly specify value for taggable
  92 Incorrect JSON schema keyword(s)
  57 Could not validate regular expression
  39 [Warning] Resource spec validation would fail from next major version. Provider should mark additionalProperties as false if the property is of object type and has properties or patternProperties defined in it. Please fix the warnings: 'additionalProperties' is a required property
  31 Use specific handler permissions instead of using wildcards
  25 Don't hardcode the aws partition in ARN patterns
  22 Consider not manually maintaining large constantly evolving enums like instance types, lambda runtimes, partitions, regions, availability zones, etc. that get outdated quickly
  15 CloudFormation properties don't usually start with lowercase letters
  10 readOnlyProperties cannot be specified by customers and should not overlap with writeOnlyProperties, createOnlyProperties, or required
   7 non-ASCII characters found in resource schema
   5 primaryIdentifier must be specified as either readOnly or createOnly
   0 LIST API inputs like MaxResults, MaxRecords, MaxItems, NextToken, NextMarker, NextPageToken, PageToken, and Filters are not resource properties
  37 AWS::GameLift::Fleet
  21 AWS::GameLift::GameServerGroup
  14 AWS::DynamoDB::GlobalTable
  10 AWS::CustomerProfiles::Integration
  10 AWS::AppFlow::Flow
   8 AWS::Kendra::DataSource
   8 AWS::AppFlow::ConnectorProfile
   7 AWS::CloudFront::Distribution
   6 AWS::ServiceCatalog::CloudFormationProvisionedProduct
   6 AWS::SageMaker::MonitoringSchedule
   6 AWS::FinSpace::Environment
   6 AWS::EC2::NetworkInsightsAnalysis
   6 AWS::ApiGateway::DomainName
   5 AWS::OpsWorksCM::Server
   5 AWS::IoTSiteWise::AccessPolicy
   5 AWS::AuditManager::Assessment
   4 AWS::SageMaker::ModelQualityJobDefinition
   4 AWS::SageMaker::ModelExplainabilityJobDefinition
   4 AWS::SageMaker::ModelBiasJobDefinition
   4 AWS::SageMaker::DataQualityJobDefinition
   4 AWS::S3Outposts::Bucket
   3 AWS::WAFv2::WebACL
   3 AWS::WAFv2::RuleGroup
   3 AWS::SageMaker::ModelPackageGroup
   3 AWS::SageMaker::FeatureGroup
   3 AWS::SSO::PermissionSet
   3 AWS::SSO::InstanceAccessControlAttributeConfiguration
   3 AWS::SSMIncidents::ResponsePlan
   3 AWS::SSM::Document
   3 AWS::KinesisFirehose::DeliveryStream
   3 AWS::IVS::Channel
   3 AWS::CodeGuruProfiler::ProfilingGroup
   2 AWS::SageMaker::Project
   2 AWS::SageMaker::DeviceFleet
   2 AWS::SageMaker::Device
   2 AWS::SSO::Assignment
   2 AWS::SSM::Association
   2 AWS::S3::AccessPoint
   2 AWS::QuickSight::DataSource
   2 AWS::MediaPackage::Asset
   2 AWS::IoT::SecurityProfile
   2 AWS::ImageBuilder::Component
   2 AWS::IVS::StreamKey
   2 AWS::GlobalAccelerator::EndpointGroup
   2 AWS::FMS::Policy
   2 AWS::DataSync::LocationObjectStorage
   2 AWS::CodeArtifact::Repository
   2 AWS::CodeArtifact::Domain
   2 AWS::Athena::DataCatalog
   2 AWS::AppIntegrations::EventIntegration
   1 AWS::WorkSpaces::ConnectionAlias
   1 AWS::SageMaker::UserProfile
   1 AWS::SageMaker::Domain
   1 AWS::SageMaker::App
   1 AWS::SSMContacts::ContactChannel
   1 AWS::SSMContacts::Contact
   1 AWS::SSM::ResourceDataSync
   1 AWS::Route53::HostedZone
   1 AWS::Route53::HealthCheck
   1 AWS::ResourceGroups::Group
   1 AWS::QuickSight::Template
   1 AWS::QuickSight::DataSet
   1 AWS::QuickSight::Dashboard
   1 AWS::QuickSight::Analysis
   1 AWS::NetworkFirewall::RuleGroup
   1 AWS::NetworkFirewall::LoggingConfiguration
   1 AWS::MediaPackage::Channel
   1 AWS::Macie::FindingsFilter
   1 AWS::MWAA::Environment
   1 AWS::Logs::LogGroup
   1 AWS::Lambda::EventSourceMapping
   1 AWS::Kinesis::Stream
   1 AWS::KMS::Key
   1 AWS::IoT::ProvisioningTemplate
   1 AWS::IoT::Authorizer
   1 AWS::ImageBuilder::InfrastructureConfiguration
   1 AWS::ImageBuilder::ImageRecipe
   1 AWS::ImageBuilder::ImagePipeline
   1 AWS::ImageBuilder::Image
   1 AWS::IVS::PlaybackKeyPair
   1 AWS::GlobalAccelerator::Listener
   1 AWS::GlobalAccelerator::Accelerator
   1 AWS::FraudDetector::Variable
   1 AWS::FraudDetector::EventType
   1 AWS::FraudDetector::Detector
   1 AWS::FMS::NotificationChannel
   1 AWS::Events::Archive
   1 AWS::ElasticLoadBalancingV2::Listener
   1 AWS::ECS::Cluster
   1 AWS::DataSync::LocationSMB
   1 AWS::DataSync::LocationS3
   1 AWS::DataSync::LocationNFS
   1 AWS::DataSync::LocationFSxWindows
   1 AWS::DataSync::LocationEFS
   1 AWS::Config::OrganizationConformancePack
   1 AWS::Config::ConformancePack
   1 AWS::CodeStarConnections::Connection
   1 AWS::CloudFormation::StackSet
   1 AWS::CUR::ReportDefinition
   1 AWS::CE::AnomalySubscription
   1 AWS::Backup::BackupVault
   1 AWS::Backup::BackupSelection
   1 AWS::Athena::WorkGroup
   1 AWS::AccessAnalyzer::Analyzer

kddejong pushed a commit to kddejong/cloudformation-cli that referenced this pull request Oct 24, 2022
kddejong pushed a commit to kddejong/cloudformation-cli that referenced this pull request Oct 24, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
schema processing schema Related to the provider meta schema
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants