Skip to content
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

Add ability to manage resource policy for AWS Secrets Manager secrets #843

Merged
merged 10 commits into from
Jan 25, 2022

Conversation

ykrysko
Copy link
Contributor

@ykrysko ykrysko commented Dec 21, 2021

SUMMARY

AWS Secrets Manager secrets support attaching resource policy. The benefit is huge when necessary to access secrets from other AWS accounts. This pull request adds ability to manage (add new/remove or modify existing) secrets resource policy.

ISSUE TYPE
  • Feature Pull Request
COMPONENT NAME

module: aws_secret

ADDITIONAL INFORMATION

@ykrysko
Copy link
Contributor Author

ykrysko commented Dec 21, 2021

Added required IAM permissions to the aws-terminator repo, need to trigger retest

@ykrysko
Copy link
Contributor Author

ykrysko commented Dec 22, 2021

Submitted pull request to add missing IAM permissions to fix failing tests mattclay/aws-terminator#183

Added more tests
@ykrysko ykrysko changed the title Added ability to manage resource policy for AWS Secrets Manager secrets Add ability to manage resource policy for AWS Secrets Manager secrets Dec 22, 2021
Relaxed secret resource policy permissions for tests to work from the same account
@jillr
Copy link
Collaborator

jillr commented Jan 4, 2022

recheck

@marknet15
Copy link
Contributor

Hey @ykrysko I think you've just got the 1 last sanity check failure issue:

ERROR: Found 1 pep8 issue(s) which need to be resolved:
ERROR: plugins/modules/aws_secret.py:58:114: W291: trailing whitespace

Removed trailing whitespace
@ykrysko
Copy link
Contributor Author

ykrysko commented Jan 14, 2022

@marknet15 , thanks! Fixed.

@marknet15
Copy link
Contributor

@ykrysko just noticed, but you also need to add a changelog fragment
https://github.com/ansible-collections/community.aws/tree/main/changelogs/fragments

@ykrysko
Copy link
Contributor Author

ykrysko commented Jan 15, 2022

@ykrysko just noticed, but you also need to add a changelog fragment https://github.com/ansible-collections/community.aws/tree/main/changelogs/fragments

Thanks, @marknet15 . Added.

version_added bump
Copy link
Contributor

@alinabuzachis alinabuzachis left a comment

Choose a reason for hiding this comment

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

@ykrysko That looks great! Just a few nits!

plugins/modules/aws_secret.py Outdated Show resolved Hide resolved
plugins/modules/aws_secret.py Outdated Show resolved Hide resolved
plugins/modules/aws_secret.py Outdated Show resolved Hide resolved
Added fixes per the review
@marknet15
Copy link
Contributor

recheck

@markuman markuman added the mergeit Merge the PR (SoftwareFactory) label Jan 25, 2022
@softwarefactory-project-zuul softwarefactory-project-zuul bot merged commit 9110162 into ansible-collections:main Jan 25, 2022
abikouo pushed a commit to abikouo/community.aws that referenced this pull request Oct 24, 2023
*_info - improve RETURN block of docs

Depends-On: ansible-collections#856
SUMMARY
Fixes ansible-collections#843
Can things like checking for a period after description be added to our sanity checks?
ISSUE TYPE

Docs Pull Request

COMPONENT NAME

aws_ax_info
aws_caller_info
aws_s3
cloudformation_info
ec2_eni_info
ec2_group
ec2_group_info
ec2_instance_info
ec2_key
ec2_metadata_facts
ec2_snapshot_info
ec2_spot_instance
ec2_spot_instance_info
ec2_tag
ec2_tag_info
ec2_vpc_dhcp_option_info
ec2_vpc_endpoint_info
ec2_vpc_endpoint_service_info
ec2_vpc_igw_info
ec2_vpc_nat_gateway
ec2_vpc_nat_gateway_info
ec2_vpc_net_info
ec2_vpc_route_table_info
elb_classic_lb

Reviewed-by: Jill R <None>
Reviewed-by: Mark Chappell <None>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
mergeit Merge the PR (SoftwareFactory)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants