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

No PhysicalResourceId is generated when FAILED by init_failure() #67

Open
AladdinKnight opened this issue Aug 30, 2022 · 4 comments
Open

Comments

@AladdinKnight
Copy link

When init_failure() function is used to send a failure to Cloudformation, it doesn't generate PhysicalResourceId. Hence, Cloudformation cannot provide a meaningful error message for the failed resource. Instead, it simply says- Invalid PhysicalResourceId.

In Cloudwatch log it shows-

[DEBUG] 2022-08-30T02:26:04.505Z b9753bbc-aa02-4cdd-9ee8-d94cf5d3f54e {"Status": "FAILED", "PhysicalResourceId": "", "StackId": "arn:aws:cloudformation:us-west-2:123456789012:stack/stack-name/guid", "RequestId": "123", "LogicalResourceId": "MyTestResource", "Reason": "An error occurred (InvalidParameter) when calling the RouteTableId operation: Must provide a RouteTableId", "Data": {}, "NoEcho": false}

My code snippet:

helper = CfnResource()

def lambda_handler(event, context):
    if some_condition is True:
        helper.init_failure(Exception("Test error"))

    helper(event, context)

I tried adding helper.PhysicalResourceId = "SomeID" before the helper() function. But it doesn't make any difference.

I guess this line (https://github.com/aws-cloudformation/custom-resource-helper/blob/main/crhelper/resource_helper.py#L121) is resetting it before it sends the response in this line (https://github.com/aws-cloudformation/custom-resource-helper/blob/main/crhelper/resource_helper.py#L135)

@AladdinKnight
Copy link
Author

By the way, I also tried putting the init_failure() function at the global level like the example in the README.md. But same result

try:
    raise Exception("Test error")
except Exception as e:
    helper.init_failure(e)

def handler(event, context):
    helper(event, context)

@iainelder
Copy link

I tried adding helper.PhysicalResourceId = "SomeID" before the helper() function. But it doesn't make any difference.

What you are suggesting has worked for me in the past. I show an example here: #7 (comment). Does it help?

I don't know about the helper.init_failure function. I wouldn't call it if there was a failure. I would just raise an exception.

I also wouldn't call the bare helper function from inside the Lambda handler. Instead I would decorate the handler with one of the helper.create, helper.update, or helper.delete functions.

Try that and you might get the behavior your expect.

Example copied from linked comment:

@helper.create
def create(event, context):
    logger.info("Got Create")
    
    helper.Data.update({"test": "testdata"})
    
    if not helper.Data.get("test"):
        helper.PhysicalResourceId = "MyResourceId"
        raise ValueError("this error will show in the cloudformation events log and console.")
        
    return "MyResourceId"

@AladdinKnight
Copy link
Author

Thank you for responding. I already have decorated create() update() and delete() functions. My full code is example is as below.

What you are suggesting has worked for me in the past. I show an example here: #7 (comment). Does it help?

I think I saw that before. It worked for you because you added helper.PhysicalResourceId = "MyResourceId" inside your create() function. In my case, I tried that in the lambda_handler (main) function, before calling helper(event, context). I am pretty sure calling the helper() [which we must] initialises the value, and I lose my defined PhysicalResourceId.

In general scenario, we won't have any codes (other than calling the helper() ) in the lambda_handler(). All our logics should be inside create/update/delete() functions. And that will work normally. In fact, just raising an Exception in those function will ensure to send a FAIL signal to Cloudformation.

However, these don't work if we do it in the lambda_handler().

My use case is: I need to validate something in the lambda_handler() before it calls any of create/update/delete() functions. And if the validation fails, signal FAIL to cloudformation.

Apparently that's not possible with current code. It must hit one of create/update/delete() in order for it to generate PhysicalResourceId or reuse one if provided.

In fact, if you take the Example in https://github.com/aws-cloudformation/custom-resource-helper/blob/main/README.md and raise an Exception in the "try" block (Line 10) instead of "pass" You'll have this exact issue.

@AladdinKnight
Copy link
Author

Full example code:

from __future__ import print_function
from crhelper import CfnResource
import logging

logger = logging.getLogger(__name__)

helper = CfnResource(json_logging=False, log_level='DEBUG', boto_level='CRITICAL', sleep_on_delete=120, ssl_verify=None)

def lambda_handler(event, context):
    if some_condition is True:
        helper.init_failure(Exception("Test error"))

    # helper.PhysicalResourceId = "MyResourceId"    <----- This has no impact
    helper(event, context)

@helper.create
def create(event, context):
    logger.info("Got Create")

@helper.update
def update(event, context):
    logger.info("Got Update")

@helper.delete
def delete(event, context):
    logger.info("Got Delete")

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

No branches or pull requests

2 participants