Skip to content

Conversation

bmbferreira
Copy link
Contributor

what

Adds a new parameter to be able to pass a json string with a custom access policy to set for the elasticsearch.

why

In my opinion, the access policies are too "opinionated" on this module, especially when it is on "vpc mode". I think it should be more flexible and allow to customize it however we want and not having it based on the iam_role_arn variable.

references

N/A

P.S.: This is already the 3rd time I'm trying to have this new parameter accepted. Please take a look at it this time 🙇 thank you.
Previous tries:

@bmbferreira
Copy link
Contributor Author

Hi @osterman ! Sorry to ping you and thank you for all the work on this and other modules 🙇 Do you think that this change on the module makes sense? I've been trying to have it merged for a long time now 😅 Thank you and all the best!

@goruha
Copy link
Member

goruha commented Jul 30, 2025

@bmbferreira Hello.
Sorry for the long response. If this PR is still relevant to you, I'm ready to merge it.
But I can not resolve conflicts without your assistance.

CleanShot 2025-07-30 at 12 44 39@2x

@goruha goruha self-assigned this Jul 30, 2025
Copy link

mergify bot commented Jul 30, 2025

💥 This pull request now has conflicts. Could you fix it @bmbferreira? 🙏

@goruha
Copy link
Member

goruha commented Aug 14, 2025

@bmbferreira Thanks for the contribution.
I pulled your changes to #210 and will get it merged today

@goruha goruha closed this Aug 14, 2025
@mergify mergify bot removed conflict This PR has conflicts triage Needs triage labels Aug 14, 2025
goruha added a commit that referenced this pull request Aug 14, 2025
… try) (#210)

* parameterize access policies json for more flexibility

* Auto Format

* Pull changes from bmbferreira/terraform-aws-elasticsearch/tree/master

---------

Co-authored-by: Bruno Ferreira <[email protected]>
Co-authored-by: cloudpossebot <[email protected]>
@bmbferreira
Copy link
Contributor Author

@bmbferreira Thanks for the contribution. I pulled your changes to #210 and will get it merged today

Sorry for the delay replying to you. Thanks a lot @goruha for finally merging these changes. 🙇‍♂️

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.

3 participants