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

NoneType during payload extraction from add_athena_partitions #274

Closed
5 of 6 tasks
hoilchoi opened this issue Nov 15, 2024 · 5 comments
Closed
5 of 6 tasks

NoneType during payload extraction from add_athena_partitions #274

hoilchoi opened this issue Nov 15, 2024 · 5 comments
Assignees
Labels

Comments

@hoilchoi
Copy link

hoilchoi commented Nov 15, 2024

Describe the bug
This block caused deployment failure for me, due to NoneType value in payload:

sanitized_payload = json.dumps({k: sanitize_string(v) for k, v in payload.items()})

Failure is during CloudFormation's eventual consistency check.

Error from CFN:

Received response status [FAILED] from custom resource.  Message returned:  'NoneType' object has no attribute 'items'

added a quick log message to check that Payload from Response:

... 'StatusCode': 200, 'ExecutedVersion': '$LATEST', 'Payload': <botocore.response.StreamingBody object at 0x7fbba4faf0d0>}",

I also confirmed that the StreamingBody Payload was in fact, null by logging payload after json load:

payload = json.loads(response.get('Payload').read().decode('utf-8'))
self.log.info(f"[add_athena_partitions] payload: {payload}")
    "level": "INFO",
    "location": "add_athena_partitions:630",
    "message": "[add_athena_partitions] payload: None",

I could not find what the parsed payload used for, so I just tried deploy with this quick change:

if payload:
    sanitized_payload = json.dumps({k: sanitize_string(v) for k, v in payload.items()})
else:
    sanitized_payload = None

and was able to deploy just fine.

But I'm still not sure if payload was supposed to be used for something, so didn't want to leave at that.

Is payload expected from Lambda response, and critical that we extract it?

To Reproduce
For me, I just deployed with these params modified:

  • ActivateScannersProbesProtectionParam: yes - Amazon Athena log parser
  • ActivateAWSManagedRulesParam: yes
  • ActivateAWSManagedSQLParam: yes
  • AppAccessLogBucket: [bucket_name]`
  • RequestThreshold: 1000
  • HTTPFloodAthenaQueryGroupByParam: Country

Expected behavior
Deploy successfully without code change

Please complete the following information about the solution:

  • Version: 4.0.5
  • Region: us-west-2
  • Was the solution modified from the version published on this repository? no
  • If the answer to the previous question was yes, are the changes available on GitHub?
  • Have you checked your service quotas for the services this solution uses? no
  • Were there any errors in the CloudWatch Logs? no (there were info logs that were helpful tho)
@hoilchoi hoilchoi added the bug label Nov 15, 2024
@dadmukta dadmukta self-assigned this Nov 15, 2024
@bios6
Copy link
Member

bios6 commented Nov 15, 2024

Hi @hoilchoi ,

Thanks for raising this issue. We will take a look at this issue but are glad that the workaround you tried got you unblocked. The main purpose of this sanitized payload seems to be so that we don't log any sensitive/critical information. We will take a look at this.

@mosestam97
Copy link

@bios6 is there any update on this?

@bios6
Copy link
Member

bios6 commented Dec 12, 2024

Hi,

According to the boto3 documentation, lambda_client.invoke should ideally always return a payload : https://boto3.amazonaws.com/v1/documentation/api/latest/reference/services/lambda/client/invoke.html . Prior to v4.0.5 we were just always logging the full response and wanted to sanitize it for v4.0.5. Based on what we saw from @hoilchoi , it does look like the payload can be empty and we plan to fix this bug in the next minor release. The parsed payload isn’t used for anything, it’s just being logged. If payload ... else works fine because we don’t need to log anything if we don’t receive a payload.

Thanks!

@mosestam97
Copy link

Hi @bios6

Thanks for the info, do we have an ETA for when the next minor release?
Also following for the fix, how can we set the payload to empty for now? i did a search on all the template but not able to find the setting, if you can provide more instruction will be great

thanks in advance

@bios6
Copy link
Member

bios6 commented Dec 17, 2024

Hi @hoilchoi and @mosestam97 ,

We just released v4.0.6 that should fix and address this issue. Closing this issue now. Thank you!

@bios6 bios6 closed this as completed Dec 17, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

4 participants