Feat: Update GCP-SM Plugin for Empty Secret Creation#706
Conversation
|
Hello @Michael-Burke I see that you have a sort of test case already, would you mind adding to/modifying the integration tests? The test entrypoint is in tests/integration/targets/gcp_secret_manager/tasks/main.yml |
|
@thekad Great catch! Added and updated to include the new test cases of:
|
|
hello @Michael-Burke apologies in advance but quick question: are you using AI to write this feature? Asking because we don't have any policy/guidelines for AI assisted contributions (yet) and I believe that would be something worth considering first cc @SirGitsalot |
|
hello @thekad, I did use AI to assist, but mainly its prediction and autocomplete feature. The python isn't that complicated so I didn't ask it to write it for me. Happy to re-write / implement if that'll be an issue. |
|
@Michael-Burke I don't think there is a problem, we just don't have a contribution policy for such cases, and we should clear those up before merging is all. I'll leave that to @SirGitsalot as he's the project admin. |
|
The policy is that a PR "does need to be a contributor’s original creation" but it's OK to use "coding assistance tools" so this PR is fine. |
|
@Michael-Burke there's a failure in the sanity tests, you can quickly run these in your workstation to make sure they will pass e.g. |
|
@thekad Thank you! I've updated the code to fix the failing sanity tests. Please let me know If there is anything else that's failing: ❯ ansible-test sanity --python-interpreter $(which python3) gcp_secret_manager
Running sanity test "action-plugin-docs"
Running sanity test "ansible-doc"
Running sanity test "changelog"
Running sanity test "compile" on Python 3.13
Running sanity test "empty-init"
Running sanity test "ignores"
Running sanity test "import" on Python 3.13
Running sanity test "line-endings"
Running sanity test "no-assert"
Running sanity test "no-get-exception"
Running sanity test "no-illegal-filenames"
Running sanity test "no-smart-quotes"
Running sanity test "pep8"
Running sanity test "pslint"
Running sanity test "pylint"
Running sanity test "replace-urlopen"
Running sanity test "runtime-metadata"
Running sanity test "shebang"
Running sanity test "shellcheck"
Running sanity test "symlinks"
Running sanity test "use-argspec-type-path"
Running sanity test "use-compat-six"
Running sanity test "validate-modules"
Running sanity test "yamllint" |
|
@Michael-Burke can you rebase please? |
@thekad rebase should be complete; I also re-ran the sanity tests which are still passing. Let me know if there is anything else I can help with or do. Thank you! ➜ ansible-test sanity --python-interpreter $(which python3) gcp_secret_manager
Running sanity test "action-plugin-docs"
Running sanity test "ansible-doc"
Running sanity test "changelog"
Running sanity test "compile" on Python 3.13
Running sanity test "empty-init"
Running sanity test "ignores"
Running sanity test "import" on Python 3.13
Running sanity test "line-endings"
Running sanity test "no-assert"
Running sanity test "no-get-exception"
Running sanity test "no-illegal-filenames"
Running sanity test "no-smart-quotes"
Running sanity test "pep8"
Running sanity test "pslint"
Running sanity test "pylint"
Running sanity test "replace-urlopen"
Running sanity test "runtime-metadata"
Running sanity test "shebang"
Running sanity test "shellcheck"
Running sanity test "symlinks"
Running sanity test "use-argspec-type-path"
Running sanity test "use-compat-six"
Running sanity test "validate-modules"
Running sanity test "yamllint" |
|
@Michael-Burke sanity tests look good, but integration tests are failing with this exception: If you're curious, you can run the integration tests yourself:
(or you could build your own integration test playbook and then amend the integration tests) |
|



SUMMARY
The current plugin
gcp_secret_manager.pywill fail when fetching an empty secret from GCP Secrets Manager. It will also fail when attempting to create an empty secret since it requires creation WITH a value.GCP Secret Manager API current allows the creation of an empty secret (no version or values). My changes are to support the fetching of an empty secret (not just the value) and creation of an empty secret while maintaining all current functionality of the module.
minor_changes: proper secret existence check; added empty secret initialization
ISSUE TYPE
COMPONENT NAME
gcp_secret_manager
ADDITIONAL INFORMATION
BEFORE CHANGES
The 409 is due to the fact that the module currently attempts to fetch if the VERSION (with its value) exists, which returns FALSE since its an empty secret. This means when it goes to "update" a pre-existing secret, it 409's since its attempting to create a resource that already exists. I update this so it fetches if the SECRET exists, and then we handle updating / setting the version information outside the creation depending on if a secret exists or not.
AFTER CHANGES