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

Dynamic logs block #28

Merged
merged 5 commits into from Apr 11, 2021
Merged

Dynamic logs block #28

merged 5 commits into from Apr 11, 2021

Conversation

heathsnow
Copy link
Contributor

@heathsnow heathsnow commented Apr 6, 2021

what

why

  • Support engine_type = "RabbitMQ"

references

- RabbitMQ general logs cannot be enabled.
  - hashicorp/terraform-provider-aws#18067
@heathsnow heathsnow requested review from a team as code owners April 6, 2021 18:22
@heathsnow heathsnow requested a review from a team as a code owner April 6, 2021 18:22
@Gowiem
Copy link
Member

Gowiem commented Apr 6, 2021

@heathsnow since this is currently just a bug in the provider, I'm hesitant to say that we should completely remove the ability to enable general logs at this time. The upstream bug in the provider should be fixed (which is seems that the Hashi team is on top of to some degree) instead of removing the ability to utilize this flag all together.

If you want to open a PR to mention this issue in the README or as a comment on that variable then I believe we'd be happy to accept that, but for now this just looks like a bug that is upstream that could be fixed any day now in which case we'd need to revert your change.

@heathsnow
Copy link
Contributor Author

@Gowiem Perhaps if I update the logic so that if both general AND audit logs are set to false then the logs block is removed? This would fix the issue of not supporting RabbitMQ right now and still allow for general logging to be enabled in the future once the provider is fixed.

@Gowiem
Copy link
Member

Gowiem commented Apr 6, 2021

@heathsnow that sounds better and acceptable to me 👍

FYI, I'm utilizing this module for RMQ usage in a client project and while I didn't implement the usage of it myself... it looks like my client's team got around the issues of the logs block causing issues by setting both audit_log_enabled and general_log_enabled to null, which seems to allow the provider to be happy. That said, I think your the false + false sounds preferable, so let's go that route. Please be sure to put a NOTE comment at the top of that dynamic block which links to the provider issue in question.

Thanks!

@heathsnow
Copy link
Contributor Author

@Gowiem PR Feedback applied (Omit logs if both general and audit logs disabled).

@Gowiem
Copy link
Member

Gowiem commented Apr 7, 2021

/test all

Copy link
Member

@Gowiem Gowiem left a comment

Choose a reason for hiding this comment

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

👍 Hopefully this will cause a little less confusion for folks using RMQ -- Thanks!

@Gowiem
Copy link
Member

Gowiem commented Apr 7, 2021

@heathsnow sadly we're blocked by this annoying aws provider bug which is plaguing lots of projects: hashicorp/terraform-provider-aws#18550

TestExamplesComplete 2021-04-07T13:34:12Z logger.go:66: Error: error creating SSM parameter: ValidationException: Invalid request: tags and overwrite can't be used together. To create a parameter with tags, please remove overwrite flag. To update tags for an existing parameter, please use AddTagsToResource or RemoveTagsFromResource.

For now, I think we'll need to hold off on this one until hopefully that bug is fixed in v3.36 of the provider.

@Gowiem
Copy link
Member

Gowiem commented Apr 11, 2021

/test all

@Gowiem Gowiem merged commit bf29e34 into cloudposse:master Apr 11, 2021
@Gowiem
Copy link
Member

Gowiem commented Apr 11, 2021

@heathsnow Thanks for the contribution! Released as 0.13.0.

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.

Error building RabbitMQ : Audit logging is not supported for RabbitMQ brokers
3 participants