Adding support for Google Secret Manager for issue 543 #578

dcostakos · 2023-06-21T23:27:30Z

SUMMARY

Add support for Google Secret Manager as both a module and lookup plugin. Fixes #543

ISSUE TYPE

Feature Pull Request

COMPONENT NAME

gcp_secret_manager

ADDITIONAL INFORMATION

Basic support for GCP secret manager with some testing

tze-dev · 2023-06-22T02:08:43Z

plugins/lookup/gcp_secret_manager.py

Thanks for implementing this - it's awesome to see this feature being worked on.

In terms of the code for this lookup, is there a reason of not leveraging ansible_collections.google.cloud.plugins.module_utils.gcp_utils as a helper for handling the authentication workflow? IMO it would simplify the code and also gain the benefit of being able to use the new OAUTH token as well - recently added by this PR - #574.

I have used it in my private Collection and tested working fine. Code snippet here - you can add the additional env handling, but the heavy lifting of the authentication workflows and API requests are taken care of by gcp_utils

try: import os import requests import json import base64 except ImportError: pass try: from ansible_collections.google.cloud.plugins.module_utils.gcp_utils import ( GcpSession, ) HAS_GOOGLE_CLOUD_COLLECTION = True except ImportError: HAS_GOOGLE_CLOUD_COLLECTION = False display = Display() class GcpMockModule(object): def __init__(self, params): self.params = params def fail_json(self, *args, **kwargs): raise AnsibleError(kwargs["msg"]) def raise_for_status(self, response): try: response.raise_for_status() except getattr(requests.exceptions, "RequestException"): self.fail_json(msg="GCP returned error: %s" % response.json()) class GcpSecretLookup: def run(self, variables=None, **kwargs): params = { "project": kwargs.get("project", None), "secret": kwargs.get("secret", None), "version": kwargs.get("version", "latest"), "auth_kind": kwargs.get("auth_kind", None), "service_account_file": kwargs.get("service_account_file", None), "service_account_email": kwargs.get("service_account_email", None), "access_token": kwargs.get("access_token", None), # added for https://github.com/ansible-collections/google.cloud/pull/574 "scopes": kwargs.get("scopes", None), } if not params["scopes"]: params["scopes"] = ["https://www.googleapis.com/auth/cloud-platform"] fake_module = GcpMockModule(params) result = self.get_secret(fake_module) return [base64.b64decode(result)] def get_secret(self, module): auth = GcpSession(module, "secretmanager") url = "https://secretmanager.googleapis.com/v1/projects/{project}/secrets/{secret}/versions/{version}:access".format( **module.params ) response = auth.get(url) return response.json()['payload']['data'] class LookupModule(LookupBase): def run(self, terms, variables=None, **kwargs): if not HAS_GOOGLE_CLOUD_COLLECTION: raise AnsibleError( "gcp_secret lookup needs a supported version of the google.cloud collection installed. Use `ansible-galaxy collection install google.cloud` to install it" ) return GcpSecretLookup().run(terms, variables=variables, **kwargs)

Thank you so much for this suggestion. I'll integrate these changes.

…llections/google.cloud/pull/578/files/76eb024c250af3b7ff8adec909bef06667833509#

tze-dev · 2023-06-22T23:14:48Z

plugins/lookup/gcp_secret_manager.py

+        params['name'] = params['key']
+
+        # support GCP_* env variables for some parameters
+        for param in ["project", "auth_kind", "service_account_file", "service_account_info", "service_account_email", "access_token"]:


As you're now using gcp_utils to handle authentication, this part is no longer needed here. This is because in gcp_utils there is already mechanism handling the fallback to env.

tze-dev · 2023-06-22T23:15:43Z

plugins/lookup/gcp_secret_manager.py

+        result = self.get_secret(fake_module)
+        return [base64.b64decode(result)]
+
+    def fallback_from_env(self, arg):


As per above - this is already being handled by gcp_utils

Once again, thank you for reviewing my code. Much appreciated.

There is indeed that functionality within the GcpModule class (in the init method for example) here . however, in your example, and the one I committed, we don't actually use that class only GcpSession.

I don't believe I can reuse the GcpModule class here in my lookup plugin. I did actually try this for grins and I got this error when attempting to call my super class constructor:
TypeError: AnsibleModule.__init__() got multiple values for argument 'argument_spec'

Not sure this is possible? What do you think?

And once again, thank you so much.

@dcostakos you're right. Sorry, I got mixed up between the module and lookup helper as I just created a PR for a new module.
Your env handling is absolutely fine.

Thanks for doing this. Hope your PR gets merged soon, and we can all start using the default GCP collection rather than maintaining separate collections.

toumorokoshi · 2023-07-07T22:45:53Z

plugins/modules/gcp_secret_manager.py

+    return merge_dicts(get_obj, access_obj)
+
+def merge_dicts(x, y):
+    z = x.copy()


note that this is only a shallow copy (nested objects will still be modified) - is that ok? on first glance it seems like it's fine, but just checking.

Nice catch, I don't see why it wouldn't be okay to do the shallow copy here but now that you've brought it up, I feel like I should do a deep copy :D.

toumorokoshi · 2023-07-07T22:46:17Z

plugins/modules/gcp_secret_manager.py

+    z.update(y)
+    return z
+
+def snake_to_camel(snake):


unused function? seems like it should be a utility in a separate module anyway.

Yes, unusued indeed. I forgot to remove this and will do so.

As a note, this module has someone limited parameter support in creating new secrets/secret versions. My thinking here was to get functionality out the door and see what people use. If people need more functionality, it could be added in the future.

Adding feature as-needed seems totally reasonable to me!

thanks for removing the dead code.

toumorokoshi

Thanks for the change! overall it looks solid.

A couple questions, if you could get around to them. I know my review is pretty belated.

If I don't hear back I'll try to take a stab at merging this in.

dcostakos · 2023-07-13T20:33:15Z

Thank you so much for reviewing this and trying the merge. You caught me at like the worst time on this, but I'm getting back to resolving the issues you brought up and the lint issues with the merge today/tomorrow.

Cheers, I'll make an updated commit soon.

toumorokoshi · 2023-07-14T15:36:14Z

Thank you so much for reviewing this and trying the merge. You caught me at like the worst time on this, but I'm getting back to resolving the issues you brought up and the lint issues with the merge today/tomorrow.

Cheers, I'll make an updated commit soon.

sounds great! and no worries - apologies about my belated review here.

Just ping me when you're ready again - I'm trying to be a little more reactive to PR reviews.

…rrors.

dcostakos · 2023-07-31T16:42:15Z

@toumorokoshi Just checking in on this. I'm sure you have a million things to do, but just wanted to check with you to see if this was still something that could potentially be merged. Thank you for your time on this.

dcostakos · 2023-11-02T15:38:58Z

Hey just checking back on this. Again, I'm sure you're very busy. I also have no idea what your release plans are for this. Curious if this will be merged?

toumorokoshi · 2023-11-07T19:49:02Z

Hi! Unfortunately I've left Google and am no longer the official repository maintainer.

That said: it looks like I still have permissions to merge. @SirGitsalot are you able to take a look? or do you want me to see if I can review and get this in?

dcostakos · 2023-12-20T17:59:13Z

it's a real bummer that this got missed for another release. Shoudl we just close this issue if it won't be worked on?

toumorokoshi · 2024-01-03T17:19:25Z

I'll ping @SirGitsalot again - as much as I'd like to help merge this in since I'm not an employee I think it's best if we leave it to the official maintainer.

dcostakos · 2024-01-03T18:36:07Z

Thanks @toumorokoshi, for helping even though you've moved on. Can you assign @SirGitsalot to be the reviewer of this instead of you?

toumorokoshi · 2024-01-04T15:57:34Z

Thanks @toumorokoshi, for helping even though you've moved on. Can you assign @SirGitsalot to be the reviewer of this instead of you?

done!

kuisathaverat · 2024-01-29T14:44:46Z

plugins/lookup/gcp_secret_manager.py

+    - retrieve secret keys in Secret Manager for use in playbooks
+    - see https://cloud.google.com/iam/docs/service-account-creds for details on creating
+      credentials for Google Cloud and the format of such credentials
+    - once a secret value is retreived, it is returned decoded.  It is up to the developer


Suggested change

- once a secret value is retreived, it is returned decoded. It is up to the developer

- once a secret value is retrieved, it is returned decoded. It is up to the developer

kuisathaverat · 2024-01-29T14:45:01Z

plugins/lookup/gcp_secret_manager.py

+            default: 'strict'
+        scopes:
+            description:
+            - Authenticaiton scopes for Google Secret Manager


Suggested change

- Authenticaiton scopes for Google Secret Manager

- Authentication scopes for Google Secret Manager

kuisathaverat · 2024-01-29T14:45:23Z

plugins/modules/gcp_secret_manager.py

+            fetch['msg'] = "Secret Destroyed, it may take time to propagate"
+            changed = True
+
+        # check to see if the values are the same, and update if neede


Suggested change

# check to see if the values are the same, and update if neede

# check to see if the values are the same, and update if needed

kuisathaverat · 2024-01-29T14:46:49Z

It works like a charm, you made my day, thank you 🙇

kuisathaverat · 2024-03-12T11:49:18Z

plugins/lookup/gcp_secret_manager.py

+            self.raise_error(module, f"unable to list versions of secret {response.status_code}")
+        version_list = response.json()
+        if "versions" in version_list:
+            return sorted(version_list['versions'], key=lambda d: d['name'])[-1]['name'].split('/')[-1]


It does not return the latest version when you have more than 10. The problem is that it sorts the paths before getting the version, so somepath/12 is smaller than somepath/9, so it always returns 9 as the latest version.

Suggested change

return sorted(version_list['versions'], key=lambda d: d['name'])[-1]['name'].split('/')[-1]

versions_numbers = []

for version in version_list['versions']:

versions_numbers.append(version['name'].split('/')[-1])

return sorted(versions_numbers, key=int)[-1]

kuisathaverat · 2024-03-12T11:52:39Z

plugins/modules/gcp_secret_manager.py

+            return None
+
+        if "versions" in version_list:
+            latest_version = sorted(version_list['versions'], key=lambda d: d['name'])[-1]['name'].split('/')[-1]


ditto

Suggested change

latest_version = sorted(version_list['versions'], key=lambda d: d['name'])[-1]['name'].split('/')[-1]

versions_numbers = []

for version in version_list['versions']:

versions_numbers.append(version['name'].split('/')[-1])

latest_version = sorted(versions_numbers, key=int)[-1]```

AlexAkulov · 2024-05-06T07:58:41Z

@SirGitsalot
Hi guys! Why hasn't this been merged yet? It was created almost a year ago. I need this feature.

dcostakos · 2024-05-07T15:29:27Z

I'm not sure if this will ever be merged. I'm the author, I know there's feedback on this, but I'm not open to making more changes to something that will never be merged. Not sure what Google's strategy on this collection is any longer. Feel free to try to manually merge my fork into a collection if you really need it.

martiniDB · 2024-07-15T16:46:43Z

Hi,
@SirGitsalot will that Secret manager going to be put into a release anytime soon by any chance?
Our team would love to use it officially.

azameeracn · 2024-08-01T14:29:45Z

Hi @SirGitsalot, repeating the request from @martiniDB. Any timeline for this module to be released?

runofthemill · 2024-09-13T23:13:53Z

I haven't tried it out yet, but it looks like it got published in https://github.com/ansible-collections/google.cloud/releases/tag/v1.4.0 and docs are on Galaxy here! Thanks @SirGitsalot!

Adding support for Google Secret Manager for issue 543

76eb024

tze-dev suggested changes Jun 22, 2023

View reviewed changes

updated lookuup plugin based on comment https://github.com/ansible-co…

953b06f

…llections/google.cloud/pull/578/files/76eb024c250af3b7ff8adec909bef06667833509#

tze-dev reviewed Jun 22, 2023

View reviewed changes

tze-dev approved these changes Jun 23, 2023

View reviewed changes

toumorokoshi reviewed Jul 7, 2023

View reviewed changes

toumorokoshi approved these changes Jul 7, 2023

View reviewed changes

dcostakos and others added 3 commits July 14, 2023 10:29

Merge branch 'ansible-collections:master' into gcp_secret_manager

375b317

updated plugsins based on feedback, fixed linting and documentation e…

3ce29db

…rrors.

updated plugsins based on feedback, fixed linting and documentation e…

40d2c9a

…rrors.

dcostakos requested a review from toumorokoshi January 2, 2024 16:22

toumorokoshi requested review from SirGitsalot and removed request for toumorokoshi January 4, 2024 15:57

kuisathaverat reviewed Jan 29, 2024

View reviewed changes

kuisathaverat reviewed Mar 12, 2024

View reviewed changes

SirGitsalot approved these changes May 22, 2024

View reviewed changes

SirGitsalot merged commit a9c0624 into ansible-collections:master May 22, 2024

This was referenced Jun 7, 2024

Add Secret Manager Lookup plugins #357

Open

feat: CCP secret manager lookup(https://github.com/ansible-collections/google.cloud/pull/357) #628

Open

Add GCP Secret manager plugin ansible/awx#15069

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Adding support for Google Secret Manager for issue 543 #578

Adding support for Google Secret Manager for issue 543 #578

dcostakos commented Jun 21, 2023

tze-dev Jun 22, 2023 •

edited

Loading

dcostakos Jun 22, 2023

tze-dev Jun 22, 2023

tze-dev Jun 22, 2023

dcostakos Jun 23, 2023

tze-dev Jun 23, 2023

toumorokoshi Jul 7, 2023

dcostakos Jul 13, 2023

toumorokoshi Jul 7, 2023 •

edited

Loading

dcostakos Jul 13, 2023

toumorokoshi Jul 14, 2023

toumorokoshi left a comment

dcostakos commented Jul 13, 2023

toumorokoshi commented Jul 14, 2023

dcostakos commented Jul 31, 2023

dcostakos commented Nov 2, 2023

toumorokoshi commented Nov 7, 2023

dcostakos commented Dec 20, 2023

toumorokoshi commented Jan 3, 2024

dcostakos commented Jan 3, 2024

toumorokoshi commented Jan 4, 2024

kuisathaverat Jan 29, 2024

kuisathaverat Jan 29, 2024

kuisathaverat Jan 29, 2024

kuisathaverat commented Jan 29, 2024

kuisathaverat Mar 12, 2024

kuisathaverat Mar 12, 2024

AlexAkulov commented May 6, 2024

dcostakos commented May 7, 2024

martiniDB commented Jul 15, 2024

azameeracn commented Aug 1, 2024

runofthemill commented Sep 13, 2024

	- once a secret value is retreived, it is returned decoded. It is up to the developer
	- once a secret value is retrieved, it is returned decoded. It is up to the developer

	- Authenticaiton scopes for Google Secret Manager
	- Authentication scopes for Google Secret Manager

	# check to see if the values are the same, and update if neede
	# check to see if the values are the same, and update if needed

-            return sorted(version_list['versions'], key=lambda d: d['name'])[-1]['name'].split('/')[-1]
+            versions_numbers = []
+            for version in version_list['versions']:
+                versions_numbers.append(version['name'].split('/')[-1])
+            return sorted(versions_numbers, key=int)[-1]

Adding support for Google Secret Manager for issue 543 #578

Adding support for Google Secret Manager for issue 543 #578

Conversation

dcostakos commented Jun 21, 2023

SUMMARY

ISSUE TYPE

COMPONENT NAME

ADDITIONAL INFORMATION

tze-dev Jun 22, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

toumorokoshi Jul 7, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

toumorokoshi left a comment

Choose a reason for hiding this comment

dcostakos commented Jul 13, 2023

toumorokoshi commented Jul 14, 2023

dcostakos commented Jul 31, 2023

dcostakos commented Nov 2, 2023

toumorokoshi commented Nov 7, 2023

dcostakos commented Dec 20, 2023

toumorokoshi commented Jan 3, 2024

dcostakos commented Jan 3, 2024

toumorokoshi commented Jan 4, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kuisathaverat commented Jan 29, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

AlexAkulov commented May 6, 2024

dcostakos commented May 7, 2024

martiniDB commented Jul 15, 2024

azameeracn commented Aug 1, 2024

runofthemill commented Sep 13, 2024

tze-dev Jun 22, 2023 •

edited

Loading

toumorokoshi Jul 7, 2023 •

edited

Loading