search

openclimatefix / ocf-infrastructure

Infrastructure code for OCF's cloud environments
3 stars 6 forks source link

give analysis dashbaord access to satellite bucket #479

Closed peterdudfield closed 3 months ago

peterdudfield commented 5 months ago

Detailed Description

Would be good if the analysis dashbaord has access to the satellite bucket

Context

Possible Implementation

aryanbhosale commented 5 months ago

Hi @peterdudfield , I have tried to solve this in #482 Could you please review it and let me know if I have tried along the correct lines? Thank you

peterdudfield commented 5 months ago

Kinda, I was thinking, that current the be_app takes one bucket_read_policy_arn, and then attached it to the IAM role. I was hopping that we could change this to take a list of them

Current its here - https://github.com/openclimatefix/ocf-infrastructure/blob/main/terraform/modules/services/eb_app/variables.tf#L86

Would need to change this to a list and then adjust where the policy is attached.

Then on when we call this module, we can pass in more than one bucket_read_policy_arn

Also, i dont think the eb.tf needs to be touched

aryanbhosale commented 5 months ago

Kinda, I was thinking, that current the be_app takes one bucket_read_policy_arn, and then attached it to the IAM role. I was hopping that we could change this to take a list of them

Current its here - https://github.com/openclimatefix/ocf-infrastructure/blob/main/terraform/modules/services/eb_app/variables.tf#L86

Would need to change this to a list and then adjust where the policy is attached.

Then on when we call this module, we can pass in more than one bucket_read_policy_arn

Also, i dont think the eb.tf needs to be touched

Okay got it, so if i'm not wrong, I need to make changes to variables.tf file and the iam.tf file right?

aryanbhosale commented 5 months ago

Kinda, I was thinking, that current the be_app takes one bucket_read_policy_arn, and then attached it to the IAM role. I was hopping that we could change this to take a list of them

Current its here - https://github.com/openclimatefix/ocf-infrastructure/blob/main/terraform/modules/services/eb_app/variables.tf#L86

Would need to change this to a list and then adjust where the policy is attached.

Then on when we call this module, we can pass in more than one bucket_read_policy_arn

Also, i dont think the eb.tf needs to be touched

Hi @peterdudfield I have made the changes, could you please review and let me know if it's right? Also, should I be changing the india-api and analysis_dashboard modules in main.tf to incorporate the new policies?

peterdudfield commented 5 months ago

Yea it would be great if you could also update where eb_app is used, perhaps in here and here

aryanbhosale commented 5 months ago

Yea it would be great if you could also update where eb_app is used, perhaps in here and here

Hi @peterdudfield , I've added the new changes in #482 , is that what was expected?

aryanbhosale commented 5 months ago

Yea it would be great if you could also update where eb_app is used, perhaps in here and here

Hi @peterdudfield , I've added the new changes in #482 , is that what was expected?

Hi @peterdudfield , do I need to make any more modifications or is anything missing in my code?