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

fix for security group description crash #38108

Merged
merged 1 commit into from May 14, 2018

Conversation

zikalino
Copy link
Contributor

SUMMARY

Fixes a crash when existing description is not empty, and description not specified in task.

ISSUE TYPE
  • Bugfix Pull Request
COMPONENT NAME

azure_rm_securitygroup

ANSIBLE VERSION

2.5

ADDITIONAL INFORMATION

@zikalino
Copy link
Contributor Author

resolves #37658

@ansibot
Copy link
Contributor

ansibot commented Mar 29, 2018

@ansibot ansibot added azure bug This issue/PR relates to a bug. cloud committer_review In order to be merged, this PR must follow the certified review workflow. module This issue/PR relates to a module. needs_triage Needs a first human triage before being processed. owner_pr This PR is made by the module's maintainer. support:certified This issue/PR relates to certified code. labels Mar 29, 2018
@yuwzho
Copy link
Contributor

yuwzho commented Mar 30, 2018

Fixes #37658

@ansibot ansibot removed the needs_triage Needs a first human triage before being processed. label Mar 30, 2018
@yuwzho
Copy link
Contributor

yuwzho commented Mar 30, 2018

shipit

@ansibot ansibot added shipit This PR is ready to be merged by Core and removed committer_review In order to be merged, this PR must follow the certified review workflow. labels Mar 30, 2018
@resmo
Copy link
Contributor

resmo commented Mar 31, 2018

LGTM

@zikalino
Copy link
Contributor Author

zikalino commented Apr 2, 2018

@jborean93 could you check this?

@@ -400,7 +400,7 @@ def compare_rules(r, rule):
matched = True
if rule.get('description', None) != r['description']:
changed = True
r['description'] = rule['description']
r['description'] = rule.get('description', None)
if rule['protocol'] != r['protocol']:
Copy link
Contributor

Choose a reason for hiding this comment

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

will make the protocol also positional args?

Copy link
Contributor

Choose a reason for hiding this comment

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

is there any other parameter need same fix so that user will be able to update security group eventually

Copy link
Contributor Author

Choose a reason for hiding this comment

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

other params will be always there afaik

@jborean93 jborean93 self-requested a review April 12, 2018 00:21
@yungezz yungezz merged commit 5d2c23e into ansible:devel May 14, 2018
@zikalino zikalino deleted the fixing-security-group-crash branch May 14, 2018 07:20
oolongbrothers pushed a commit to oolongbrothers/ansible that referenced this pull request May 14, 2018
oolongbrothers pushed a commit to oolongbrothers/ansible that referenced this pull request May 14, 2018
tonal pushed a commit to tonal/ansible that referenced this pull request May 15, 2018
oolongbrothers pushed a commit to oolongbrothers/ansible that referenced this pull request May 15, 2018
oolongbrothers pushed a commit to oolongbrothers/ansible that referenced this pull request May 15, 2018
ilicmilan pushed a commit to ilicmilan/ansible that referenced this pull request Nov 7, 2018
@ansible ansible locked and limited conversation to collaborators May 14, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
azure bug This issue/PR relates to a bug. cloud module This issue/PR relates to a module. owner_pr This PR is made by the module's maintainer. shipit This PR is ready to be merged by Core support:certified This issue/PR relates to certified code.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants