-
Notifications
You must be signed in to change notification settings - Fork 130
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 Secret Manager Lookup plugins #357
base: master
Are you sure you want to change the base?
Conversation
This is awesome, but it doesnt look like PRs for this repo get merged :/ |
Yes, it looks like this repo doesn't have a maintainer. |
not sure why the readme for this repo doesn't say that this repository is only the generated output of https://github.com/GoogleCloudPlatform/magic-modules |
Yes, but if you try to raise Ansible specific stuff (non generated like this) there they tell you to raise it here instead 🤷 |
Right, as in GoogleCloudPlatform/magic-modules#3933, apparently we're to use this repo for bugs, but submit PRs to the upstream repo. Yet most have no idea about magic-modules and waste time looking for bugs in the Python here, and/or submitting PRs that will never be looked at. Even worse, look here: hashicorp/terraform-provider-google#9588 via GoogleCloudPlatform/magic-modules#5069, to wit:
So basically @levonet was right that this collection is basically abandonware and we're supposed to use Terraform for provisioning and/or GCP-native automation. It's not very nice to not even notify people about this and have them waste their time filing bugs/PRs and looking through auto-generated python code which isn't even updated anymore. I guess they don't care because it's a small number of users. The situation has been like this for multiple years with this collection... wonder if Ansible core team even knows about the defunct status. There's an interesting clue here:
not sure what to make of that |
@levonet You should make a merge request on the community google ? |
Fix #460 |
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 contribution! A couple minor changes, but otherwise LGTM.
Is there anything else that needs to be done besides put the lookup in the plugins directory?
@@ -0,0 +1,102 @@ | |||
from __future__ import (absolute_import, division, print_function) |
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.
I think this file needs a copyright / license notice too?
|
||
def client(self, secretmanager): | ||
if self.access_token is not None: | ||
credentials=google.oauth2.credentials.Credentials(self.access_token) |
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.
should scope be added?
if self.secret_id is None: | ||
raise AnsibleError("{0} lookup plugin required option: secret or resource id".format(self.plugin_name)) | ||
|
||
self.name = f"projects/{self.project_id}/secrets/{self.secret_id}/versions/{self.version_id}" |
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.
could this be switched away from f-strings? we don't use them today, and I think that syntax-wise this repository is still python2 compatible.
Not that we can always have it be py2 compatible, but I'd like to avoid py3 syntax to make things py2-usable as much as possible for now.
any chance it's getting merged? |
is this a dup of #578? |
SUMMARY
I need to get the secret content and the resource id from GCP Secret Manager.
Lookup plugins are used because this format allows embedding Secret Manager queries in templates and files with variables (external configuration contour).
ISSUE TYPE
COMPONENT NAME
ADDITIONAL INFORMATION
Examples: