ansible-powerscale
ansible-powerscale copied to clipboard
Add s3_key module
Description
This PR adds the module s3_key which allows the management of S3 keys for users.
GitHub Issues
Fixes #207
Checklist:
- [x] I have performed a self-review of my own code to ensure there are no formatting, pep8, linting, or security issues
- [x] I have performed Ansible Sanity test using --docker default
- [x] I have verified that new and existing unit tests pass locally with my changes
- [x] I have not allowed coverage numbers to degenerate
- [x] I have maintained at least 90% code coverage
- [x] I have commented my code, particularly in hard-to-understand areas
- [x] I have made corresponding changes to the documentation
- [x] I have added tests that prove my fix is effective or that my feature works
- [x] Backward compatibility is not broken
How Has This Been Tested?
- [X] manual testing by building the collection locally
- [x] automated tests
HELP: I would like to add automated tests but did not manage to execute the existing tests locally. Is there any documentation I missed describing how the test should be executed locally?
HELP: I would like to add automated tests but did not manage to execute the existing tests locally. Is there any documentation I missed describing how the test should be executed locally?
@fpfuetsch, please see Testing Ansible and Collections for details on how to go about running the ansible sanity, unit and integration test cases. Also, see Testing collections.
@fpfuetsch, you might want to fix the below module documentation error which is leading to ansible-sanity failing:
ERROR! module dellemc.powerscale.s3_key missing documentation (or could not parse documentation): dellemc.powerscale.s3_key did not contain a DOCUMENTATION attribute (/home/runner/work/ansible-powerscale/ansible-powerscale/.tox/sanity-py3.10-2.17/tmp/collections/ansible_collections/dellemc/powerscale/plugins/modules/s3_key.py). Unable to parse documentation in python file '/home/runner/work/ansible-powerscale/ansible-powerscale/.tox/sanity-py3.10-2.17/tmp/collections/ansible_collections/dellemc/powerscale/plugins/modules/s3_key.py': f-string: unmatched '(' (<unknown>, line 304). f-string: unmatched '(' (<unknown>, line 304)
Codecov Report
:x: Patch coverage is 92.30769% with 19 lines in your changes missing coverage. Please review.
:white_check_mark: Project coverage is 90.38%. Comparing base (da078f2) to head (d9acda7).
| Files with missing lines | Patch % | Lines |
|---|---|---|
| plugins/modules/s3_key.py | 85.12% | 10 Missing and 8 partials :warning: |
| tests/unit/plugins/module_utils/mock_s3_key_api.py | 95.23% | 0 Missing and 1 partial :warning: |
Additional details and impacted files
@@ Coverage Diff @@
## main #208 +/- ##
==========================================
+ Coverage 90.35% 90.38% +0.02%
==========================================
Files 143 146 +3
Lines 16766 17013 +247
Branches 2317 2336 +19
==========================================
+ Hits 15149 15377 +228
- Misses 946 956 +10
- Partials 671 680 +9
| Flag | Coverage Ξ | |
|---|---|---|
| units | 90.38% <92.30%> (+0.02%) |
:arrow_up: |
Flags with carried forward coverage won't be shown. Click here to find out more.
:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.
:rocket: New features to boost your workflow:
- :snowflake: Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
@anupamaloke Okay will have a look.
What do you think about the argument existing_key_overwrite: true/false to trigger the replacement of existing S3 keys. The thing I do not like about it that it violates idempotency. An alternative I could think about is to provide some kind of timestamp which is compared with the existing key's creation date to decide if a new key should be created. π€
@anupamaloke Okay will have a look.
Thank you! It seems there are a few more in the latest sanity run and if you could fix them as well?
ERROR: Found 4 yamllint issue(s) which need to be resolved:
ERROR: plugins/modules/s3_key.py:33:9: error: DOCUMENTATION: syntax error: could not find expected ':' (syntax)
ERROR: plugins/modules/s3_key.py:33:9: unparsable-with-libyaml: DOCUMENTATION: while scanning a simple key - could not find expected ':'
ERROR: plugins/modules/s3_key.py:115:24: error: RETURN: syntax error: could not find expected ':' (syntax)
ERROR: plugins/modules/s3_key.py:115:24: unparsable-with-libyaml: RETURN: while scanning a simple key - could not find expected ':'
What do you think about the argument
existing_key_overwrite: true/falseto trigger the replacement of existing S3 keys. The thing I do not like about it that it violates idempotency. An alternative I could think about is to provide some kind of timestamp which is compared with the existing key's creation date to decide if a new key should be created. π€
Yeah, this is the challenge with idempotency for all the modules that we have for configuring passwods/secrets/creds. One way I think of handling it would be like how it has been done in the ansible.builtin.user module with the update_password argument.
On the similar lines, we can introduce a new argument called update_s3_key (may be rename the existing_key_overwrite argument). Then, users will be able to use the argument choices - update_s3_key: always or update_s3_key: oncreate - to control the key update workflow as well as the idempotency. Let me know what you think.
@anupamaloke Thanks for your input. Tests were added and existing_key_overwrite was changed to generate_new_key: 'always'/'if_not_present'
@anupamaloke it would be nice if you could approve the workflow run for the latest commit :)
@anupamaloke hopefully fixed now all sanity and linting errors
@anupamaloke What are the next steps now? :)
@anupamaloke @ShrinidhiRao15 @gokul-srivathsan anybody there? πΆβπ«οΈ
@fpfuetsch , will check this.