openclimatefix / ocf-infrastructure

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

created a usergroup with full access #496

Closed aryanbhosale closed 4 months ago

aryanbhosale commented 5 months ago

Pull Request

Description

Solves #488

Checklist:

peterdudfield commented 5 months ago

And could you create this user group in the india/development/main.tf?

aryanbhosale commented 5 months ago

And could you create this user group in the india/development/main.tf?

okay let me try

aryanbhosale commented 5 months ago

And could you create this user group in the india/development/main.tf?

from what i understand, i need to create a new module say developer_groups and add a source to the user_groups directory and the local region, and I need to attach aws_iam_policy_attachment for each group separately, is that right?

aryanbhosale commented 5 months ago

And could you create this user group in the india/development/main.tf?

Hi @peterdudfield , i've tried to add the changes in my latest pr, could you please review and let me know if that was what was expected? Thank you

aryanbhosale commented 5 months ago

Hi @peterdudfield ,I'd made the changes in the India/development directory, could you please review my latest commit on it and let me know if that's the way to do it? Thank you

aryanbhosale commented 4 months ago

Hi @peterdudfield ,I'd made the changes in the India/development directory, could you please review my latest commit on it and let me know if that's the way to do it? Thank you

hi @peterdudfield is this ok?

peterdudfield commented 4 months ago

@devsjc do you mind reviewing?

devsjc commented 4 months ago

Hi @peterdudfield, is there an issue this corresponds to? Can't review whether it is fit for purpose if I don't know what the purpose is! 😅 Is the intention to create a user group that has access to all of our AWS services? If so, I assume it's going to replace the existing ml-engineers user group, I guess in favour of having one defined in terraform?

aryanbhosale commented 4 months ago

Hi @peterdudfield, is there an issue this corresponds to? Can't review whether it is fit for purpose if I don't know what the purpose is! 😅 Is the intention to create a user group that has access to all of our AWS services? If so, I assume it's going to replace the existing ml-engineers user group, I guess in favour of having one defined in terraform?

Hi @devsjc , really sorry for the bad PR description, I basically tried to create a new usergroup for ap-south 1 as asked in #488 :

Create a usergroup that has full access to the following services in ap-south-1:

ECS S3 Secrets manager Cloudwatch Elastic beanstalk RDS EC2 The name could be {region}-developer

It would be great to create this as a general terraform component, then we can import this into the different regions

We can then manually add users to this group

peterdudfield commented 4 months ago

Yea @devsjc, just wanted to have the user group moved over to terraform. I was making it manually for India project and it seemed a good step to move it to terrafrom. Less likely to make errors then

devsjc commented 4 months ago

Makes sense, thanks for the clarification @peterdudfield and @aryanbhosale!

peterdudfield commented 4 months ago

@devsjc we might have to trigger a run on terraform, using a different branch on this repo, just to check terraform works

aryanbhosale commented 4 months ago

Looks good, thanks! I have created a branch aryanbhosale/user-group - if you could update the merge target from openclimatefix:main to openclimatefix:aryanbhosale/user-group we can merge it and run the checks.

Thank you @devsjc ! I have updated it to match with the main branch. I think we can run the checks now

aryanbhosale commented 4 months ago

Looks good, thanks! I have created a branch aryanbhosale/user-group - if you could update the merge target from openclimatefix:main to openclimatefix:aryanbhosale/user-group we can merge it and run the checks.

image Does this mean that we can now merge?

devsjc commented 4 months ago

That branch I created for the moment is just a copy of main, and so the successful checks you're seeing there are from the last merge into main, not from the changes being requested here - the checks won't run on external requests. Hence why I was wondering whether we can go from openclimatefix:main <- aryanbhosale:main to openclimatefix:aryanbhosale/user-group <- aryanbhosale:main in this pull request's targets. But perhaps you can't update the target after the pull request has been created?

aryanbhosale commented 4 months ago

That branch I created for the moment is just a copy of main, and so the successful checks you're seeing there are from the last merge into main, not from the changes being requested here - the checks won't run on external requests. Hence why I was wondering whether we can go from openclimatefix:main <- aryanbhosale:main to openclimatefix:aryanbhosale/user-group <- aryanbhosale:main in this pull request's targets. But perhaps you can't update the target after the pull request has been created?

Oh okay, understood. So am i supposed to open a PR from aryanbhosale:main --> openclimatefix:aryanbhosale/user-group Is that right?

devsjc commented 4 months ago

Yes please, if this one can't be edited after the fact!

aryanbhosale commented 4 months ago

Hi @devsjc , ive changed the merge target here #514

devsjc commented 4 months ago

Merged in via the other branch. Thanks for your contribution @aryanbhosale!

aryanbhosale commented 4 months ago

Merged in via the other branch. Thanks for your contribution @aryanbhosale!

That's great! Thank you :)