martysweet / cfn-lint

A CloudFormation JSON and YAML Validator
MIT License
163 stars 38 forks source link

Support for SAM transform? #56

Open tomasaschan opened 6 years ago

tomasaschan commented 6 years ago

Is it possible to start supporting the Serverless Application Model transform in the linter?

Currently, I get a bunch of errors like this on my template, which makes the tool quite useless:

Resource: Resources > GetStateFunction Message: Resource GetStateFunction has an invalid Type of AWS::Serverless::Function. Documentation: https://docs.aws.amazon.com/AWSCloudFormation/latest/UserGuide/resources-section-structure.html

martysweet commented 6 years ago

@tlycken Do you know whether SAM has a specification file similar to CloudFormations http://docs.aws.amazon.com/AWSCloudFormation/latest/UserGuide/cfn-resource-specification.html?

From a quick search I couldn't find one which makes implementing this tricky...

tomasaschan commented 6 years ago

AFAIK, the contents of the repo I linked are the closest thing to a formal spec there is; specifically this file.

I don't know if there's any process setup for change/adding new versions or if Amazon just do whenever it pleases them (i.e. whenever they publish a new implementation on AWS...) but that document is hopefully enough to implement linting.

hoshsadiq commented 6 years ago

There is a json schema file if that helps?

martysweet commented 6 years ago

@akdor1154 @RazzM13 Any ideas for this?

I feel it could get very complicated very quickly, but this is an important feature to keep this tool relevant with the rising SAM popularity. Designs/discussion for implementation welcome. ex. Detect if using SAM, then extend schema by xyz.

RazzM13 commented 6 years ago

Hi @martysweet, as part of PR #142 I'm looking into implementing a mechanism for extending the current resource schema, which might be a step into SAM support as well or perhaps at least to pave the way for a user to specify an extension file for AWS template schemas...

RazzM13 commented 6 years ago

Hi @martysweet @akdor1154, I was contemplating about SAM support and it seems to me that the first inconvenience would be that the SAM schema is currently based on json schema format and the AWS resource specification is not. On the other hand, json schema offers already made tools for validation such as ajv, which would provide a very nice comfort zone by alleviating a lot of potential concerns such as circular depedencies, and is gaining a lot of traction due to projects such as Swagger, probably becoming the defacto way of defining structured data.

Regardless of whichever way this featured will proceed I guess it stands to reason that a common format must be attained and therefore, AFAICT, we are left with choosing to either convert the AWS resource specification to json schema or SAM schema to the AWS resource specification format.

martysweet commented 6 years ago

Good points.

I would sway towards "SAM schema to the AWS resource specification", as we already have all the functionality to process the hundreds of resources which are currently available in normal CFN.

I have only used SAM once or twice so haven't had that much experience with it but AFAIK there isn't any wild new syntax/commands, simply some AWS::Serverless::xyz resource specification and property types, which can easily be ported over with the information in that specification file.

Using such a json schema, while can help prove your template is "valid", doesn't do much in the way of checking references, getatt validity and other sanity checks which this project provides. However, I see how it could help for the initial checking process before we do deeper inspection. I think this route would only be viable if it delivers significantly more value, in terms of better error messages to the user.

RazzM13 commented 6 years ago

Hi @martysweet, I most definitely agree with you and I wasn't suggesting we actually do go down the json schema path as that would imply a terrible amount of work and reiteration of the multitude of excellent features that cfn-lint currently implements, just the intrinsics and user-friendly error messages are enough to give me the creeps, it was just a not so well expressed thought :)

In a more serious vein, I believe that one of the primary concerns in porting the SAM schema to the AWS resource specification format would be maintenance in the eventuality that AWS decides to update the schema. Therefore, I would suggest in perhaps bulding an adapter that would allow us to access the json schema the same way we do the AWS resource specification thus, we can augment the resource specification at runtime, perhaps using the extendSpecification function from PR #142 or something similar.

Last but not least, apart from the new resources, are there other new elements reflected in the SAM template anatomy?

martysweet commented 6 years ago

That's a point. Transforming at runtime/enabling parsing if the Transform: AWS::Serverless-2016-10-31 is present would be ideal, and would reduce the burden of updating the specification file at release time.

I can't see any examples of any major changes in SAM compared to plain CFN, apart from new resources and property types. @tlycken or @hoshsadiq do you know of any?

tomasaschan commented 6 years ago

Nope, none that I know of.

hoshsadiq commented 6 years ago

I have not seen anything besides new resources and properties either, however, I have not used SAM full feature set to give you a full answer.

RazzM13 commented 6 years ago

Short update: I've almost got the SAM translator working, stay tuned for a PR :) PS: even though it probably isn't obvious from an end-user's perspective but a lot of properties found in SAM resources can accept different value types therefore I've went for a parameterized type approach, more details about it in the upcoming PR.

RazzM13 commented 6 years ago

Hello everyone, sorry for the small delay :) After a lot of torment, I've just managed to create PR #167 that provides basic SAM support for validating templates, it can currently validate the sample SAM templates and hopefully is the bulk of the needed functionality for SAM linting, though with SAM you never know :)

Hi @martysweet could you or perhaps @akdor1154 have a look at it and let me know if this is a suitable / reasonable solution, albeit it is in dire need of testing and refinement?

samcodrington commented 6 years ago

Any update on this? Will PR #167 be added soon?

RazzM13 commented 6 years ago

Hi @samcodrington and everyone, PR #167 is not ready yet, unfortunately I've been really struggling with finding the time to get back to work on it, I'll try to make an extra effort this weekend and hopefully get it to a better state, in the mean time if you have a better approach to the issue or would like to pitch into the existing work then please do so as it's most certainly appreciated, though perhaps a heads up would be in order so that we don't duplicate the effort.

estebancrw commented 5 years ago

Since #167 is merged, should this be marked as closed or is something pending? cc @samcodrington