-
Notifications
You must be signed in to change notification settings - Fork 367
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
Replication Group missing cross-resource referencers #96
Conversation
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.
Looking pretty good to me! Have you done any manual testing thus far?
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.
dd6d860
to
3f74c67
Compare
… and security group name fields Signed-off-by: Muvaffak Onus <onus.muvaffak@gmail.com>
Signed-off-by: Muvaffak Onus <onus.muvaffak@gmail.com>
3f74c67
to
c97f12c
Compare
@hasheddan I've tested this manually and it resolves the correct name from the reference. Though for the VPC mode to work, we need to implement a managed resource @negz I think that's ready to merge. |
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.
@muvaf gotcha, thanks! LGTM
Thanks @muvaf, and @hasheddan for reviewing! |
…nces Replication Group missing cross-resource referencers
…nces Replication Group missing cross-resource referencers
Description of your changes
Adds security group name referencer for
CacheSecurityGroupName
and security group ID referencer forSecurityGroupIDs
fields. The reason that there are two different fields is thatCacheSecurityGroupName
is used in EC2-Classic mode andSecurityGroupIDs
is used in EC2-VPC mode.Fixes #83
Checklist
I have:
make reviewable
to ensure this PR is ready for review.app.yaml
to include any new role permissions.