Adding the Opensearch service as a shared module
Details
Details
- Reviewers
varun - Commits
- rCOMM49f169b272b0: [terraform] [1/n] adding Opensearch service
Tested on staging. Properly deployed and interactive
Diff Detail
Diff Detail
- Repository
- rCOMM Comm
- Lint
No Lint Coverage - Unit
No Test Coverage
Event Timeline
Comment Actions
This diff overlaps with D9757 - are you going to abandon that one?
| services/terraform/remote/aws_iam.tf | ||
|---|---|---|
| 199–211 ↗ | (On Diff #33479) | My comment https://phab.comm.dev/D9757?id=32929#inline-61182 hasn't been addressed. If you're not planning doing it here, can you create a follow-up task as a subtask of ENG-4540? |
Comment Actions
It's generally best to update existing diffs instead of putting up new ones. If you do want to abandon an existing diff, you should use the abandon option from the menu at the bottom (at this point you should do this)
Comment Actions
looks fine to me but setting @bartek as blocking since he's more well-versed in terraform
Comment Actions
You addressed my feedback from this diff in D9878 which is currently a draft. That one looks good to me 👍
But changes from that diff overlap with this diff (you created opensearch.tf in both diffs) and that makes me confused which one I should review. I guess you again had some trouble when rebasing
Requesting changes now but feel free to click "Request Review" when you're ready 😉
Comment Actions
@wyilio, it's critical that you figure out how to handle diff stacks – right now it looks like you have a pattern of repeatedly opening conflicting diffs.
I'm not exactly sure as to your workflow. I'd suggest that in your next 1:1 with another engineer (eg. @varun, @bartek, or me), you talk through your workflow and get some feedback.
Some high-level advice: if you have multiple diffs touching the same code, it's critical that those diffs correspond to commits within the same local branch.
Comment Actions
I was having trouble getting the HTTP requests to work with the opensearch domain when I included opensearch_domain_access under managed_policy_arns. After placing opensearch_domain_access in its own policy, there didn't seem to be any more 403 Forbidden errors when accessing the opensearch domain with the search index lambda.
In this diff, I lay out the policy for future usage. In [4/n], I set the principals identifier to the lambda arn. Hopefully this addresses the earlier comment on limiting access scope.
This comment was removed by will.