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

handlers.py: Exception caught sequence item 0: expected str instance, dict found #39

Open
pauly4it opened this issue Aug 2, 2022 · 2 comments

Comments

@pauly4it
Copy link
Member

pauly4it commented Aug 2, 2022

After setting up the CloudFormation hook and creating a stack with an S3 bucket with public access, the following was logged to CloudWatch for the hook:

Exception caught sequence item 0: expected str instance, dict found
Traceback (most recent call last):
  File "/var/task/cloudformation_cli_python_lib/hook.py", line 273, in __call__
    raise error
  File "/var/task/cloudformation_cli_python_lib/hook.py", line 262, in __call__
    caller_sess, request, invocation_point, callback, type_configuration
  File "/var/task/cloudformation_cli_python_lib/hook.py", line 100, in _invoke_handler
    return handler(session, request, callback_context, type_configuration)
  File "/var/task/styra_opa_hook/handlers.py", line 127, in pre_handler
    return opa_query(request, session, type_configuration, action)
  File "/var/task/styra_opa_hook/handlers.py", line 97, in opa_query
    message = " | ".join(body["violations"])
TypeError: sequence item 0: expected str instance, dict found

For reference, the OPA agent returned the following:

{"allow":false,"violations":[{"allowed":false,"message":"public access not blocked for bucket testing"}]}
@anderseknert
Copy link
Member

Yeah, violations is currently assumed to be a set of strings only. While we could extend this to support more response formats, I'm not sure what the added value in this case would be? 🙂 That allowed is false is already implied by the presence of violations, so also adding that to each violation entry does not seems a little redundant.

One thing that might be useful though is if we had a case for adding other types of metadata to a violation, like severity or something like that. But whatever we'd go with, we'd need to document properly what works and what doesn't.

@anderseknert
Copy link
Member

Regardless of that — we could definitely print something more informational than a stacktrace here. Just something like the below would be an improvement:

Unrecognized format in OPA response.
Expected: {"allow": <boolean>, "violations": <array<string>>}
Got: {"allow": <boolean>, "violations": <array<object>>}

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