Skip to content

Conversation

nheijmans
Copy link

Issue #, if available:
#1136

Description of changes:
Added the policy for Athena to the policy_template.json file

Description of how you validated changes:
Added the suggested code to the local file and checked with a workgroup if access was granted and queries could be executed.

Checklist:

  • Write/update tests
  • make pr passes
  • Update documentation
  • Verify transformed template deploys and application functions as expected
  • Add/update example to examples/2016-10-31

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

@codecov-io
Copy link

codecov-io commented Sep 11, 2019

Codecov Report

Merging #1137 into develop will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff            @@
##           develop    #1137   +/-   ##
========================================
  Coverage    94.33%   94.33%           
========================================
  Files           72       72           
  Lines         3725     3725           
  Branches       733      733           
========================================
  Hits          3514     3514           
  Misses         107      107           
  Partials       104      104

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update d5e05d0...4e75b52. Read the comment docs.

@praneetap praneetap changed the title Develop feat: AthenaQueryPolicy template Sep 24, 2019
Copy link

@ShreyaGangishetty ShreyaGangishetty Sep 24, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@nheijmans looking at this AWS docs there are more permissions required for accessing a specified athena workgroup - https://docs.aws.amazon.com/athena/latest/ug/example-policies-workgroup.html
Could you add those as well?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@praneetap Yep I'll add them as well!

@ShreyaGangishetty
Copy link

@nheijmans Thank you for contributing this feature!!
Can you please add unit tests for this policy template here?

@nheijmans
Copy link
Author

@nheijmans Thank you for contributing this feature!!
Can you please add unit tests for this policy template here?

Added in PR #1158!

@ShreyaGangishetty
Copy link

@nheijmans I added test cases for this and also resolved the merge conflicts. This PR looks good to me

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: indentation

@praneetap
Copy link
Contributor

@nheijmans The tests need to be updated as well, the build is failing currently. Once you fix that it should be ready to go :)

@nheijmans
Copy link
Author

@nheijmans The tests need to be updated as well, the build is failing currently. Once you fix that it should be ready to go :)

How can I do that? Can't find this in GitHub :(

Sorry for replying late, was on leave...

@keetonian keetonian assigned keetonian and unassigned praneetap Dec 6, 2019
nheijmans and others added 4 commits December 9, 2019 14:52
Added a template where a Athena workgroup can be queried and the results retrieved. By default the workgroup "primary" will be used but with the parameter a user can change it to the desired workgroup
Added the template policy for Athena to execute queries on a workgroup. Workgroup can be named with the parameter
Added the additional access needed as per referenced document https://docs.aws.amazon.com/athena/latest/ug/example-policies-workgroup.html#example1-full-access-all-wkgs excluding the deletion part which I think is a bit much for SAM
@keetonian
Copy link
Contributor

Rebased and fixed the tests 👍

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.

5 participants