Spec 34.0.0 definition files

[agimenez]

PR for #69

[agimenez]

Hi,

I think this is ready to be merged. In #69 you mentioned something about CPAN, but I'm really not interested in being a co-maintainer and take care of releases for this module. Could you please review and release when you're happy?

Thanks!

[pplu]

Hi,

I don't quite understand why you have deleted old fixes from the fix script. I think your fix to the spec should be added to the fixes that are in place. A test run should pass cleanly after a new spec is added.

[agimenez]

Yeah, I saw tests failing on master, so it's my fault to assume that failing tests were OK.

Regarding the fixes: a bunch of them have been already fixed in the JSON spec already, and those are not needed anymore. Some others now introduce validation errors, like for example:

$doc->{PropertyTypes}{'AWS::IoT::ProvisioningTemplate.Tags'}{Properties}{Tags}{ItemType}= 'Tag';

I guess that the spec has changed a lot since that fix was introduced, and now it's just invalid.

Looking at the failures from this PR, I can see that I overdid it, I apologise. I will revert the deletions that introduced those failures.

[agimenez]

Another example I found of a fix that doesn't apply anymore (and contradicts the spec):

AWS::RDS::OptionGroup.OptionConfigurations is required:

            "OptionConfigurations" : {
               "Documentation" : "http://docs.aws.amazon.com/AWSCloudFormation/latest/UserGuide/aws-resource-rds-optiongroup.html#cfn-rds-optiongroup-optionconfigurations",
               "DuplicatesAllowed" : true,
               "ItemType" : "OptionConfiguration",
               "Required" : true,
               "Type" : "List",
               "UpdateType" : "Immutable"
            },

But the fix script used to force it to false:

$doc->{ResourceTypes}{'AWS::RDS::OptionGroup'}{Properties}{OptionConfigurations}{Required} = JSON::MaybeXS::false;

The commit that introduced it is 66eaffd2044b6a4921b43183f7b6b20aaa46b24a, but it's not clear why we are accepting not specifying an attribute that CloudFormation requires. Any idea?

I keep digging on the other fixes!

[agimenez]

Sorry, I forgot to mention that with the latest changes everything works and tests pass. Could you review?

[pplu]

I think we're OK to merge and upload to CPAN

Another example I found of a fix that doesn't apply anymore (and contradicts the spec): AWS::RDS::OptionGroup.OptionConfigurations is required

I think that I force-unrequired the OptionsConfiguration because Cloudformation accepts an OptionGroup with no OptionsConfiguration.

The commit that introduced it is 66eaffd, but it's not clear why we are accepting not specifying an attribute that CloudFormation requires. Any idea?

I see that the commit message is not very clear, but if I put that test in there, it was to avoid regressions of things either in use or that I had found in the wild. I'm pretty sure that at the time I did that, it is because CloudFormation would accept that resource, even if the spec said otherwise. The spec sometimes is not accurate.

When you're ready to launch to CPAN, ping me, and I'll build and upload to CPAN.

[agimenez]

Yeah, I haven't tried it, but if the spec says it's required, I think Cfn should follow the spec, even if because of a bug (or something else) CloudFormation accepts an empty value. Maybe this doesn't apply anymore and we would be testing/accepting something that the actual service doesn't.

If you're happy with it, go ahead and publish :)