-
Notifications
You must be signed in to change notification settings - Fork 432
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
Update policy.json #859
Update policy.json #859
Conversation
fix-issue(#852)
Hey @ShubhamPalriwala, You can test the policy now. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey @professorabhay, I took a quick glance and do not remember seeing a ListBuckets action in AWS? Can you confirm that please? I think it was something like ListAllMyBuckets?
And have you verified that this covers all the resources we fetch?
Hey @ShubhamPalriwala, I take a look to the original |
|
Hey @professorabhay, yes, the docs should mostly cover all the resources we support, additionally just take a look here https://github.com/tailwarden/komiser/tree/develop/providers/aws and see if there is anything missing in the docs that is present here! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the PR @professorabhay but there are a couple of permissions missing such as:
cloudfront:ListTagsForResource
As mentioned by @ShubhamPalriwala I recommend you go through the supported services here https://github.com/tailwarden/komiser/tree/develop/providers/aws and add the missing permissions to the JSON file.
Hey @mlabouardy, Thanks for review. |
@professorabhay, let me know if you need any help here |
@ShubhamPalriwala, No need for now. |
hey @ShubhamPalriwala, I make changes. |
Hey @professorabhay, I just tried to validate it and faced the following errors: To reproduce it,
PS: The above does not require any AWS credits so should be doable as well easily! Let me know if you need any help with it |
Any update on this @professorabhay ? |
I will make changes by the end of this week |
Hey @professorabhay can we plan this for the next release ie sometime this week? |
Hai @ShubhamPalriwala, Kindly take a look |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey @professorabhay, apologies for the delay in review! I can still see some errors:
Was it working for you in your AWS IAM account?
Hey @professorabhay I still see error in the policy Is there anything we can help you get this PR merged? |
Hey @AvineshTripathi, I'll be great that you can help me. I gave it a try while learning but it's way more difficult to solve. |
Here is the one I generated without error
cc @mlabouardy @ShubhamPalriwala for review |
thanks Avinesh, I noticed that some permissions are duplicated for example: could you merge them? :) |
Any reason why we are using multiplee statement inplace of single, is there a best practice we should follow? Also Should I close the PR and create a new one with all the resources(only supported one's as currently there are many *)? @mlabouardy |
not sure I got it :) Are you referring to "s3:Get*" and "s3:List*" operations? |
@professorabhay if it is Okay for you can I close thiis PR and create a new one to get changes fast and also I can add you as co-author if you want |
Sure @AvineshTripathi ! |
fix-issue(#852)