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 Service Account Impersonation + Secret Manager Filter #321

Open
wants to merge 12 commits into
base: master
Choose a base branch
from

Conversation

upodroid
Copy link
Contributor

@upodroid upodroid commented Nov 2, 2020

SUMMARY
  • Added Service Account Impersonation
  • Tweaked KMS Filter to default to ADC Auth.
  • Added Secret Manager Filter
ISSUE TYPE
  • Feature Pull Request
COMPONENT NAME
ADDITIONAL INFORMATION

@rambleraptor ^

modular-magician and others added 12 commits August 6, 2020 07:23
…nsible-collections#273)

* add source_image to google_compute_image

* add source_snapshot to google_compute_image

* PR comment changes

Signed-off-by: Modular Magician <[email protected]>
* Add new field filter to pubsub.

Fixes: hashicorp/terraform-provider-google#6727

* Fixed filter name, it was improperly set.

* add filter key to pubsub subscription unit test

* spaces not tabs!

* hardcode filter value in test

* revert remove escaped quotes

Co-authored-by: Tim O'Connell <[email protected]>
Signed-off-by: Modular Magician <[email protected]>

Co-authored-by: Tim O'Connell <[email protected]>
…llections#278)

Add enableMessageOrdering to Pub/Sub Subscription

Signed-off-by: Modular Magician <[email protected]>
* use {product}.googleapis.com endpoints

* use actual correct urls

* fix zone data source test

* fix network peering tests

* possibly fix deleting default network

Signed-off-by: Modular Magician <[email protected]>
…le-collections#280)

* [provider-yaml] - Removed instances where input and output are both true

* modified to only supply output. Following pattern from bigquerydatatransfer

Co-authored-by: Scott Suarez <[email protected]>
Signed-off-by: Modular Magician <[email protected]>

Co-authored-by: Scott Suarez <[email protected]>
* retrypolicy attribute added

* test case updated

Signed-off-by: Modular Magician <[email protected]>
@aoktox
Copy link

aoktox commented Oct 5, 2022

The impersonation part would be very helpful for us. Any chance of getting it merged?

@aoktox
Copy link

aoktox commented Oct 5, 2022

cc @toumorokoshi

Copy link
Collaborator

@toumorokoshi toumorokoshi left a comment

Choose a reason for hiding this comment

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

Thanks for the commit! I'm reviewing this a bit out-of-order since I hear it's valuable.

I left a couple comments, but I think there should be a test of this new feature (in the tests/integrations/targets directory, or perhaps a unit test) to get this merged in.

Obviously this commit is years old, so if someone wants to send an updated PR with the changes, I'm happy to take a look. Otherwise I'll process this as we have more tests and see if I can fix it up.

@@ -0,0 +1,60 @@
# (c) 2019, Eric Anderson <[email protected]>
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think this copyright is incorrect? seems like copy-pasted code from a separate file.

It should reflect the author of the code.

'service_account_file': kwargs.get('service_account_file', None),
'service_account_email': kwargs.get('service_account_email', None),
}
if not params['scopes']:
params['scopes'] = ['https://www.googleapis.com/auth/cloudkms']
params['scopes'] = ['https://www.googleapis.com/auth/cloud-platform']
Copy link
Collaborator

Choose a reason for hiding this comment

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

this is technically a backwards-incompatible change (the default value of a field changing).

Is there a specific reason this needs to be done in this change?

@toumorokoshi
Copy link
Collaborator

@aoktox thanks for the ping! Hopefully we get all these cleaned up, but calling out the important ones is helpful for now.

I took a quick look but I'm worried we don't have enough testing to merge it in as-is. If you want to try updating it that would be great, otherwise it'll be in my queue along with the other 30 PRs, and likely an AI after I get all the tests passing in the CI.

@toumorokoshi toumorokoshi self-assigned this Oct 9, 2022
@toumorokoshi toumorokoshi added the enhancement New feature or request label Nov 12, 2022
@DaveSchile-Zonar
Copy link

What's the deal with this one? This would be very useful for my team. SA impersonation is an widely used feature of GCP automation. Seems like ansible should be leveraging its use already.

@ktchernov
Copy link

I've created a version of this with just the impersonation changes, plus documentation updates: #627

@toumorokoshi toumorokoshi removed their assignment Sep 15, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants