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

fix: give optional array a default value #101

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

jkruse14
Copy link

If the excludeDBInstanceClassList property is not given a value, the hook will fail. This array is marked as optional in the python code. This update assigns a default value to the array.

Copy link
Contributor

@mrinaudo-aws mrinaudo-aws left a comment

Choose a reason for hiding this comment

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

Hi @jkruse14, and thank you for your time in looking into this sample hook!

The models.py is a file that is automatically generated: making changes to it will result in such changes being removed the next time the user or maintainer for a hook runs cfn generate. Thus, proposed change(s) should not be applied to this file, because such changes will not be persisted.

If I use the following type configuration in my ~/.cfn-cli/typeConfiguration.json file:

{
  "CloudFormationConfiguration": {
    "HookConfiguration": {
      "TargetStacks": "ALL",
      "FailureMode": "FAIL",
      "Properties": {
        "excludeDBInstanceClassList": [
        ]
      }
    }
  }
}

(whereas I pass an empty list to excludeDBInstanceClassList), and I do not make additional code changes to the sample hook (I also leave the models.py in its current state without proposed changes), when I package up the hook with cfn submit --dry-run and then I run contract tests locally with cfn test, the hook passes contract tests (7 passed, 3 skipped, 14 deselected). I also used the same settings when I tested using the CloudFormation console, whereas the hook in that case failed as I expected because the db instance described in the template was missing the hook's desired encryption settings and the exclusion list was an empty list.

Is this what you meant when you mentioned the use case of that the property not being given a value, or did you wish to encompass a use case that is different than the one I tried?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants