Onepassword lookup add service accounts #6660

Domi-cc · 2023-06-09T13:27:22Z

SUMMARY

add new service_account_token for onepassword lookup

Fixes #6635

minor_changes:

onepassword lookup - adds service_account_token parameters for supporting 1password service accounts

ISSUE TYPE

Feature Pull Request

COMPONENT NAME

onepassword

ADDITIONAL INFORMATION

lookup('community.general.onepassword', 'KITT', vault='Servers', service_account_token='ops_foobar....')

…ount token is set

ansibullbot · 2023-06-09T13:30:00Z

cc @Rylon @azenk @samdoran @scottsb
click here for bot help

felixfontein

Thanks for your contribution! Can you please add a changelog fragment? Thanks.

plugins/lookup/onepassword.py

plugins/lookup/onepassword_raw.py

plugins/modules/onepassword_info.py

Co-authored-by: Felix Fontein <felix@fontein.de>

jansagurna · 2023-06-12T20:44:11Z

/rebuild

jansagurna · 2023-06-13T16:51:31Z

@samdoran i hope i fixed all your comments please ping me if i missed one.

plugins/lookup/onepassword_raw.py

Co-authored-by: Felix Fontein <felix@fontein.de>

samdoran · 2023-06-13T19:12:25Z

@jansagurna There are a few minor ones.

The main thing that should still be addressed is the changes to _check_required_params. I think that method should be left as is and the caller in each class should decide if it needs to be called. That provides the cleanest separation of duties.

# Something like this
if not self.service_account_token:
    self._check_required_params(['some', 'params'])

jansagurna · 2023-06-13T19:25:59Z

@samdoran i removed the check directly from the _check_required_params and added a check before try to login. is this fine for you?

Domi-cc · 2023-06-14T17:19:44Z

are we good here? or do you need any assistance?

plugins/lookup/onepassword.py

samdoran · 2023-06-14T16:40:56Z

plugins/lookup/onepassword.py

+        if self.service_account_token:
+            self._check_required_params(['service_account_token'])


The service_account_token attribute can only be set if it was passed as a parameter, so it's not necessary to check for only that parameter in a separate call.

Reading through this more, this method shouldn't be called at all if service_account_token was passed to the lookup. It will either appear to be logged in already or fail before it gets here (if the changes in my other suggestion are made).

All the changes to this method can be removed.

removed the self._check_required_params(['service_account_token'])

plugins/lookup/onepassword.py

Co-authored-by: Sam Doran <github@samdoran.com>

samdoran

Hopefully these are the last few changes. Thank you for all your work.

samdoran · 2023-06-14T18:37:35Z

plugins/lookup/onepassword.py

+        if self.service_account_token:
+            environment_update = {"OP_SERVICE_ACCOUNT_TOKEN": self.service_account_token}
+            args = [
+                "whoami",
+            ]

-        args = [
-            "account", "add", "--raw",
-            "--address", "{0}.{1}".format(self.subdomain, self.domain),
-            "--email", to_bytes(self.username),
-            "--signin",
-        ]
+            return self._run(args, environment_update=environment_update)
+        else:


These changes aren't needed since full_signin will be skipped if there is service token. If there is a problem with the service token, the lookup will exit before getting here.

Removed these changes.

plugins/lookup/onepassword.py

Co-authored-by: Sam Doran <github@samdoran.com>

samdoran

Look great. Thank you for your contribution.

samdoran · 2023-06-15T14:22:00Z

shipit

patchback · 2023-06-15T17:18:16Z

Backport to stable-7: 💚 backport PR created

✅ Backport PR branch: patchback/backports/stable-7/473e557c2f31425495fa5deed514835bdc508315/pr-6660

Backported as #6710

🤖 @patchback
I'm built with octomachinery and
my source is open — https://github.com/sanitizers/patchback-github-app.

* add service account token and bypass required fields when service account token is set * add token to base class * add Info * add service_account_token * add service_account_token * add documentation * add service_account_token * fix E111: indentation is not a multiple of 4 * fix lint problems * Update plugins/lookup/onepassword_raw.py Co-authored-by: Felix Fontein <felix@fontein.de> * Update plugins/modules/onepassword_info.py Co-authored-by: Felix Fontein <felix@fontein.de> * Update plugins/lookup/onepassword.py Co-authored-by: Felix Fontein <felix@fontein.de> * add changelog fragment * change type service_account_token to align to domain option * add fragment value * Update changelogs/fragments/6660-onepassword-lookup-service-account.yaml Co-authored-by: Felix Fontein <felix@fontein.de> * Update plugins/lookup/onepassword.py Co-authored-by: Felix Fontein <felix@fontein.de> * remove service_account_token from onepassword_info.py * adjust V1 to raise error if service_account_token is set * adjust V1 to raise error if service_account_token is set * adjust V1 to raise error if service_account_token is set * adjust if assert_logged_in * Update plugins/lookup/onepassword.py Co-authored-by: Sam Doran <github@samdoran.com> * Update plugins/lookup/onepassword.py Co-authored-by: Sam Doran <github@samdoran.com> * remove double return * remove new line * remove new line * remove new line * remove spaces * remove new line * remove spaces * Update plugins/lookup/onepassword_raw.py Co-authored-by: Felix Fontein <felix@fontein.de> * add _check_required_params * Update plugins/lookup/onepassword.py Co-authored-by: Sam Doran <github@samdoran.com> * Update plugins/lookup/onepassword.py Co-authored-by: Sam Doran <github@samdoran.com> * remove _check_required_params * remove spaces * Update plugins/lookup/onepassword.py Co-authored-by: Sam Doran <github@samdoran.com> * remove code --------- Co-authored-by: Jan Sagurna <jan.sagurna@sag-solutions.com> Co-authored-by: Jan Sagurna <58932831+jansagurna@users.noreply.github.com> Co-authored-by: Felix Fontein <felix@fontein.de> Co-authored-by: Sam Doran <github@samdoran.com> (cherry picked from commit 473e557)

felixfontein · 2023-06-15T17:18:33Z

@Domi-cc @jansagurna thanks a lot for working on this!
@samdoran thanks a lot for reviewing!

… accounts (#6710) Onepassword lookup add service accounts (#6660) * add service account token and bypass required fields when service account token is set * add token to base class * add Info * add service_account_token * add service_account_token * add documentation * add service_account_token * fix E111: indentation is not a multiple of 4 * fix lint problems * Update plugins/lookup/onepassword_raw.py Co-authored-by: Felix Fontein <felix@fontein.de> * Update plugins/modules/onepassword_info.py Co-authored-by: Felix Fontein <felix@fontein.de> * Update plugins/lookup/onepassword.py Co-authored-by: Felix Fontein <felix@fontein.de> * add changelog fragment * change type service_account_token to align to domain option * add fragment value * Update changelogs/fragments/6660-onepassword-lookup-service-account.yaml Co-authored-by: Felix Fontein <felix@fontein.de> * Update plugins/lookup/onepassword.py Co-authored-by: Felix Fontein <felix@fontein.de> * remove service_account_token from onepassword_info.py * adjust V1 to raise error if service_account_token is set * adjust V1 to raise error if service_account_token is set * adjust V1 to raise error if service_account_token is set * adjust if assert_logged_in * Update plugins/lookup/onepassword.py Co-authored-by: Sam Doran <github@samdoran.com> * Update plugins/lookup/onepassword.py Co-authored-by: Sam Doran <github@samdoran.com> * remove double return * remove new line * remove new line * remove new line * remove spaces * remove new line * remove spaces * Update plugins/lookup/onepassword_raw.py Co-authored-by: Felix Fontein <felix@fontein.de> * add _check_required_params * Update plugins/lookup/onepassword.py Co-authored-by: Sam Doran <github@samdoran.com> * Update plugins/lookup/onepassword.py Co-authored-by: Sam Doran <github@samdoran.com> * remove _check_required_params * remove spaces * Update plugins/lookup/onepassword.py Co-authored-by: Sam Doran <github@samdoran.com> * remove code --------- Co-authored-by: Jan Sagurna <jan.sagurna@sag-solutions.com> Co-authored-by: Jan Sagurna <58932831+jansagurna@users.noreply.github.com> Co-authored-by: Felix Fontein <felix@fontein.de> Co-authored-by: Sam Doran <github@samdoran.com> (cherry picked from commit 473e557) Co-authored-by: Dominik Haßelkuss <Domi-cc@users.noreply.github.com>

Domi-cc and others added 8 commits June 8, 2023 20:28

add service account token and bypass required fields when service acc…

9070a5a

…ount token is set

add token to base class

eb6352e

add Info

07ba8cf

add service_account_token

e831a25

add service_account_token

095562a

add documentation

f7cb8ed

add service_account_token

f007397

fix E111: indentation is not a multiple of 4

375d6d5

ansibullbot added feature This issue/PR relates to a feature request lookup lookup plugin module module new_contributor Help guide this first time contributor plugins plugin (any type) labels Jun 9, 2023

fix lint problems

90abfb7

felixfontein added check-before-release PR will be looked at again shortly before release and merged if possible. backport-7 Automatically create a backport for the stable-7 branch labels Jun 12, 2023

felixfontein reviewed Jun 12, 2023

View reviewed changes

plugins/lookup/onepassword.py Outdated Show resolved Hide resolved

plugins/lookup/onepassword_raw.py Outdated Show resolved Hide resolved

plugins/modules/onepassword_info.py Outdated Show resolved Hide resolved

jansagurna and others added 3 commits June 12, 2023 22:34

Update plugins/lookup/onepassword_raw.py

ae94bbf

Co-authored-by: Felix Fontein <felix@fontein.de>

Update plugins/modules/onepassword_info.py

4ab33a6

Co-authored-by: Felix Fontein <felix@fontein.de>

Update plugins/lookup/onepassword.py

d12b22a

Co-authored-by: Felix Fontein <felix@fontein.de>

ansibullbot added needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR and removed needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR labels Jun 12, 2023

add changelog fragment

9c15c70

This comment was marked as outdated.

Sign in to view

ansibullbot added the needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR label Jun 12, 2023

jansagurna added 2 commits June 12, 2023 22:58

change type service_account_token to align to domain option

a67bfea

add fragment value

7163369

ansibullbot removed the needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR label Jun 12, 2023

remove spaces

5e7f8f7

felixfontein reviewed Jun 13, 2023

View reviewed changes

plugins/lookup/onepassword_raw.py Outdated Show resolved Hide resolved

Update plugins/lookup/onepassword_raw.py

a0bfb65

Co-authored-by: Felix Fontein <felix@fontein.de>

add _check_required_params

ce5c38f

samdoran suggested changes Jun 14, 2023

View reviewed changes

ansibullbot added the needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR label Jun 14, 2023

jansagurna and others added 4 commits June 14, 2023 20:18

Update plugins/lookup/onepassword.py

990328f

Co-authored-by: Sam Doran <github@samdoran.com>

Update plugins/lookup/onepassword.py

7cc6c5f

Co-authored-by: Sam Doran <github@samdoran.com>

remove _check_required_params

df8a5a9

remove spaces

b1fae64

ansibullbot removed the needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR label Jun 14, 2023

samdoran suggested changes Jun 14, 2023

View reviewed changes

jansagurna and others added 2 commits June 14, 2023 20:44

Update plugins/lookup/onepassword.py

48b36b8

Co-authored-by: Sam Doran <github@samdoran.com>

remove code

4346b29

samdoran approved these changes Jun 15, 2023

View reviewed changes

felixfontein merged commit 473e557 into ansible-collections:main Jun 15, 2023
136 checks passed

patchback bot mentioned this pull request Jun 15, 2023

[PR #6660/473e557c backport][stable-7] Onepassword lookup add service accounts #6710

Merged

felixfontein removed the check-before-release PR will be looked at again shortly before release and merged if possible. label Jun 15, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Onepassword lookup add service accounts #6660

Onepassword lookup add service accounts #6660

Domi-cc commented Jun 9, 2023

ansibullbot commented Jun 9, 2023

felixfontein left a comment

jansagurna commented Jun 12, 2023

This comment was marked as outdated.

jansagurna commented Jun 13, 2023

samdoran commented Jun 13, 2023

jansagurna commented Jun 13, 2023

Domi-cc commented Jun 14, 2023

samdoran Jun 14, 2023

jansagurna Jun 14, 2023

samdoran left a comment

samdoran Jun 14, 2023

jansagurna Jun 15, 2023

samdoran left a comment

samdoran commented Jun 15, 2023

patchback bot commented Jun 15, 2023 •

edited

Loading

felixfontein commented Jun 15, 2023

		if self.service_account_token:
		self._check_required_params(['service_account_token'])

Onepassword lookup add service accounts #6660

Onepassword lookup add service accounts #6660

Conversation

Domi-cc commented Jun 9, 2023

SUMMARY

ISSUE TYPE

COMPONENT NAME

ADDITIONAL INFORMATION

ansibullbot commented Jun 9, 2023

felixfontein left a comment

Choose a reason for hiding this comment

jansagurna commented Jun 12, 2023

This comment was marked as outdated.

jansagurna commented Jun 13, 2023

samdoran commented Jun 13, 2023

jansagurna commented Jun 13, 2023

Domi-cc commented Jun 14, 2023

samdoran Jun 14, 2023

Choose a reason for hiding this comment

jansagurna Jun 14, 2023

Choose a reason for hiding this comment

samdoran left a comment

Choose a reason for hiding this comment

samdoran Jun 14, 2023

Choose a reason for hiding this comment

jansagurna Jun 15, 2023

Choose a reason for hiding this comment

samdoran left a comment

Choose a reason for hiding this comment

samdoran commented Jun 15, 2023

patchback bot commented Jun 15, 2023 • edited Loading

Backport to stable-7: 💚 backport PR created

felixfontein commented Jun 15, 2023

patchback bot commented Jun 15, 2023 •

edited

Loading