Skip to content
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

open-iscsi: adding mutual authentication support and updating authentication parameters description #3422

Merged
merged 9 commits into from
Sep 26, 2021

Conversation

ricsanfre
Copy link
Contributor

SUMMARY

Updating confusing description of authentication parameters included in documentation section
and adding the capability to specify mutual authentication between target and initiator

ISSUE TYPE
  • Feature Pull Request
COMPONENT NAME

plugins/modules/system/open_iscsi.py

ADDITIONAL INFORMATION

node_auth, node_pass and node_user parameters are used for target login, not for target discovery. Authentication can be set for both procedures (discovery and login) but module current implementation does not support discovery authentication and thus parameters descriptions are confusing.

Adding two new module parameters (node_user_in, node_pass_in) to support mutual authentication.

@ansibullbot ansibullbot added feature This issue/PR relates to a feature request module module new_contributor Help guide this first time contributor plugins plugin (any type) system labels Sep 23, 2021
@ansibullbot
Copy link
Collaborator

The test ansible-test sanity --test pep8 [explain] failed with 1 error:

plugins/modules/system/open_iscsi.py:212:1: W293: blank line contains whitespace

The test ansible-test sanity --test pylint [explain] failed with 1 error:

plugins/modules/system/open_iscsi.py:212:0: trailing-whitespace: Trailing whitespace

The test ansible-test sanity --test pep8 [explain] failed with 1 error:

plugins/modules/system/open_iscsi.py:212:1: W293: blank line contains whitespace

The test ansible-test sanity --test pep8 [explain] failed with 1 error:

plugins/modules/system/open_iscsi.py:212:1: W293: blank line contains whitespace

The test ansible-test sanity --test pylint [explain] failed with 1 error:

plugins/modules/system/open_iscsi.py:212:0: trailing-whitespace: Trailing whitespace

The test ansible-test sanity --test pylint [explain] failed with 1 error:

plugins/modules/system/open_iscsi.py:212:0: trailing-whitespace: Trailing whitespace

The test ansible-test sanity --test pep8 [explain] failed with 1 error:

plugins/modules/system/open_iscsi.py:212:1: W293: blank line contains whitespace

The test ansible-test sanity --test pylint [explain] failed with 1 error:

plugins/modules/system/open_iscsi.py:212:0: trailing-whitespace: Trailing whitespace

click here for bot help

@ansibullbot ansibullbot added ci_verified Push fixes to PR branch to re-run CI needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR labels Sep 23, 2021
@ansibullbot ansibullbot removed ci_verified Push fixes to PR branch to re-run CI needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR labels Sep 23, 2021
@ricsanfre ricsanfre changed the title Adding mutual authentication support and updating authentication parameters description open-iscsi: adding mutual authentication support and updating authentication parameters description Sep 23, 2021
Copy link
Collaborator

@Ajpantuso Ajpantuso left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for your contribution! Please add a changelog fragment as well.

plugins/modules/system/open_iscsi.py Show resolved Hide resolved
plugins/modules/system/open_iscsi.py Outdated Show resolved Hide resolved
plugins/modules/system/open_iscsi.py Outdated Show resolved Hide resolved
@ansibullbot ansibullbot added the needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR label Sep 23, 2021
@felixfontein felixfontein added backport-3 check-before-release PR will be looked at again shortly before release and merged if possible. labels Sep 24, 2021
ricsanfre and others added 3 commits September 24, 2021 10:17
Adding version_added to node_user_in parameter

Co-authored-by: Ajpantuso <ajpantuso@gmail.com>
adding version_added attibute to new parameter password_in

Co-authored-by: Ajpantuso <ajpantuso@gmail.com>
Co-authored-by: Ajpantuso <ajpantuso@gmail.com>
@ansibullbot ansibullbot added needs_ci This PR requires CI testing to be performed. Please close and re-open this PR to trigger CI and removed needs_ci This PR requires CI testing to be performed. Please close and re-open this PR to trigger CI needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR labels Sep 24, 2021
@ansibullbot
Copy link
Collaborator

The test ansible-test sanity --test changelog [explain] failed with 1 error:

changelogs/fragments/3422-open-iscsi-mutual-authentication-support.yam:0:0: extension must be one of: .yml, .yaml

The test ansible-test sanity --test changelog [explain] failed with 1 error:

changelogs/fragments/3422-open-iscsi-mutual-authentication-support.yam:0:0: extension must be one of: .yml, .yaml

The test ansible-test sanity --test changelog [explain] failed with 1 error:

changelogs/fragments/3422-open-iscsi-mutual-authentication-support.yam:0:0: extension must be one of: .yml, .yaml

click here for bot help

@ansibullbot ansibullbot added ci_verified Push fixes to PR branch to re-run CI needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR labels Sep 24, 2021
@ansibullbot ansibullbot added needs_ci This PR requires CI testing to be performed. Please close and re-open this PR to trigger CI and removed ci_verified Push fixes to PR branch to re-run CI labels Sep 24, 2021
@ansibullbot ansibullbot added needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR and removed needs_ci This PR requires CI testing to be performed. Please close and re-open this PR to trigger CI needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR labels Sep 24, 2021
Copy link
Collaborator

@Ajpantuso Ajpantuso left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Other than the superfluous Changelog item this looks good to me.

…port.yaml

Co-authored-by: Ajpantuso <ajpantuso@gmail.com>
Copy link
Collaborator

@Ajpantuso Ajpantuso left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One last changelog item.

…port.yaml

Co-authored-by: Ajpantuso <ajpantuso@gmail.com>
Copy link
Collaborator

@Ajpantuso Ajpantuso left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks!

@ansibullbot ansibullbot removed the needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR label Sep 25, 2021
@felixfontein felixfontein removed the check-before-release PR will be looked at again shortly before release and merged if possible. label Sep 26, 2021
@felixfontein felixfontein merged commit 43a9f09 into ansible-collections:main Sep 26, 2021
@patchback
Copy link

patchback bot commented Sep 26, 2021

Backport to stable-3: 💚 backport PR created

✅ Backport PR branch: patchback/backports/stable-3/43a9f09a175fb2503525f5f92ef091821ee27c26/pr-3422

Backported as #3447

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

patchback bot pushed a commit that referenced this pull request Sep 26, 2021
…ication parameters description (#3422)

* Adding mutual athentication support and changing doucumentation about authentication credentials

* Removing blank line with whitspaces

* Update plugins/modules/system/open_iscsi.py

Adding version_added to node_user_in parameter

Co-authored-by: Ajpantuso <ajpantuso@gmail.com>

* Update plugins/modules/system/open_iscsi.py

adding version_added attibute to new parameter password_in

Co-authored-by: Ajpantuso <ajpantuso@gmail.com>

* Update plugins/modules/system/open_iscsi.py

Co-authored-by: Ajpantuso <ajpantuso@gmail.com>

* Adding changelog fragment for #3422

* Rename 3422-open-iscsi-mutual-authentication-support.yam to 3422-open-iscsi-mutual-authentication-support.yaml

* Update changelogs/fragments/3422-open-iscsi-mutual-authentication-support.yaml

Co-authored-by: Ajpantuso <ajpantuso@gmail.com>

* Update changelogs/fragments/3422-open-iscsi-mutual-authentication-support.yaml

Co-authored-by: Ajpantuso <ajpantuso@gmail.com>

Co-authored-by: Ajpantuso <ajpantuso@gmail.com>
(cherry picked from commit 43a9f09)
@felixfontein
Copy link
Collaborator

@ricsanfre thanks for implementing this!
@Ajpantuso thanks for reviewing!

felixfontein pushed a commit that referenced this pull request Sep 26, 2021
…ication parameters description (#3422) (#3447)

* Adding mutual athentication support and changing doucumentation about authentication credentials

* Removing blank line with whitspaces

* Update plugins/modules/system/open_iscsi.py

Adding version_added to node_user_in parameter

Co-authored-by: Ajpantuso <ajpantuso@gmail.com>

* Update plugins/modules/system/open_iscsi.py

adding version_added attibute to new parameter password_in

Co-authored-by: Ajpantuso <ajpantuso@gmail.com>

* Update plugins/modules/system/open_iscsi.py

Co-authored-by: Ajpantuso <ajpantuso@gmail.com>

* Adding changelog fragment for #3422

* Rename 3422-open-iscsi-mutual-authentication-support.yam to 3422-open-iscsi-mutual-authentication-support.yaml

* Update changelogs/fragments/3422-open-iscsi-mutual-authentication-support.yaml

Co-authored-by: Ajpantuso <ajpantuso@gmail.com>

* Update changelogs/fragments/3422-open-iscsi-mutual-authentication-support.yaml

Co-authored-by: Ajpantuso <ajpantuso@gmail.com>

Co-authored-by: Ajpantuso <ajpantuso@gmail.com>
(cherry picked from commit 43a9f09)

Co-authored-by: Ricardo Sanchez <84853324+ricsanfre@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature This issue/PR relates to a feature request has_issue module module new_contributor Help guide this first time contributor plugins plugin (any type) system
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants