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.

Sign up for GitHub

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

Jump to bottom

added Fn::Transport to Resources.resources on validation schema #8337

Merged
merged 2 commits into from Oct 6, 2020
Merged

added Fn::Transport to Resources.resources on validation schema #8337

Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
7b36650
added Fn::Transport to Resources.resources on validation schema
rzaldana Oct 2, 2020
5ea6464
removed awsCfTransform definition
rzaldana Oct 5, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
18 changes: 18 additions & 0 deletions lib/plugins/aws/provider/awsProvider.js
View file
Open in desktop
Expand Up @@ -282,6 +282,21 @@ class AwsProvider {
required: ['Fn::Sub'],
additionalProperties: false,
},
awsCfTransform: {
type: 'object',
properties: {
'Fn::Transform': {
type: 'object',
properties: {
Name: { type: 'string' },
Parameters: { type: 'object' },
},
required: ['Name'],
additionalProperties: false,
},
},
required: ['Fn::Transform'],
},
awsIamPolicyAction: { type: 'array', items: { type: 'string' } },
awsIamPolicyPrincipal: {
anyOf: [
Expand Down Expand Up @@ -796,6 +811,9 @@ class AwsProvider {
// See also https://docs.aws.amazon.com/AWSCloudFormation/latest/UserGuide/resources-section-structure.html
Resources: {
type: 'object',
properties: {
$ref: '#/definitions/awsCfTransform/properties',
Copy link
Contributor

Choose a reason for hiding this comment

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

Have you tested that it works as intended?

If I see correctly it translates to:

Resources: {
  type: 'object', 
  properties: {
    type: 'object',
    properties: {...}
  },
  ...
}

And that looks invalid (note that AJV unfortunately does not notify on unrecognized properties in Schema, it simply ignores them).

I think we don't have to introduce awsCfTransform definition (no reusabilty use case at this point), so we can fix it simply by putting definition inline

Copy link
Contributor Author

@rzaldana rzaldana Oct 5, 2020 •
edited

Choose a reason for hiding this comment

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

It did work in my tests. In this case, because the JSON Pointer is to awscfTransform/properties and not just awscfTransform, it would translate to:

Resources: {
  type: 'object', 
  properties: {
    type: 'object',
    properties:
      'Fn::Transform': {...}
  },
  ...
}

However, if there is no reusability use case I'll just put the properties inline.

Copy link
Contributor

Choose a reason for hiding this comment

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

Indeed, sorry, didn't read carefully that per path right object is pointed.

Anyway we usually do not generalize until there's a use case for it. Thanks for update!

},
patternProperties: {
'^[a-zA-Z0-9]{1,255}$': {
type: 'object',
Expand Down