New properties in AWS::Cognito::UserPool cannot be used with Cognito event

[hoffa]

Description

See #2581 for original issue.

Steps to reproduce

git clone https://github.com/aws/serverless-application-model.git
cd serverless-application-model
python3 -m venv .venv && source .venv/bin/activate && make init

Save as template.yaml:

Transform: AWS::Serverless-2016-10-31
Resources:
  MyUserPool:
    Type: AWS::Cognito::UserPool
    Properties:
      SomeUnknownProperty: SomeUnknownValue

  MyFunction:
    Type: AWS::Serverless::Function
    Properties:
      Runtime: python3.8
      InlineCode: foo
      Handler: bar
      Events:
        CognitoEvent:
          Type: Cognito
          Properties:
            Trigger: CustomMessage
            UserPool: !Ref MyUserPool

Transform:

bin/sam-translate.py --template-file template.yaml

Which fails:

ERROR:main:Invalid Serverless Application Specification document. Number of errors found: 1. Resource with id [MyUserPool] is invalid. property SomeUnknownProperty not defined for resource of type AWS::Cognito::UserPool
ERROR:main:<map object at 0x10d0772b0>

Thoughts

from_dict() in Resource is the entry point:

serverless-application-model/samtranslator/model/eventsources/push.py

Line 1203 in f6b8794

resources.append(CognitoUserPool.from_dict(userpool_id, userpool, userpool_id))

serverless-application-model/samtranslator/model/__init__.py

Line 169 in f6b8794

def from_dict(cls, logical_id: str, resource_dict: Dict[str, Any], relative_id: Optional[str] = None, sam_plugins=None) -> "Resource": # type: ignore[no-untyped-def]

It stores the properties as object attributes, and does validation when storing:

serverless-application-model/samtranslator/model/__init__.py

Lines 316 to 317 in f6b8794

	if name in self._keywords or name in self.property_types:
	return super().__setattr__(name, value)

And retrieving them:

serverless-application-model/samtranslator/model/__init__.py

Lines 298 to 302 in f6b8794

	properties_dict = {}
	for name in self.property_types:
	value = getattr(self, name)
	if value is not None:
	properties_dict[name] = value

We could either add the unknown properties as attributes like usual, or perhaps better, store the initial resource dictionary, and in to_dict() merge, failing in case of conflict (to ensure it doesn't silently overwrite properties from customer template).

Also... does this affect any other resources? Maybe ones that use from_dict() as well?

[xazhao]

Ideally SAM shouldn't fail because of a unknown property in CFN resources. Because we also write to the resource, I feel this could go either way. Closing this issue for now since I don't find real use cases which are impacted.