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

Persist SSL configuration for appgateway #746

Merged
merged 10 commits into from
Apr 15, 2022

Conversation

l3ender
Copy link
Contributor

@l3ender l3ender commented Jan 27, 2022

SUMMARY

This PR resolves a bug where SSL policy configuration is not set for an application gateway.

Resolves #287.

ISSUE TYPE
  • Bugfix Pull Request
COMPONENT NAME

azure_rm_appgateway

ADDITIONAL INFORMATION

The module input was not being provided to the parameters of the API. I have corrected that while ensuring idempotency, added SSL policy configuration as output in the info module, and added test coverage.

Because the configuration was never applied before, the test cases also needed to be updated because they were using a combination of SSL parameters not supported by the API:

Error creating the Application Gateway instance: Azure Error: ApplicationGatewaySslPolicyDisabledSslPolicyAndNewSslPropertiesCannotBeSpecifiedTogether
Message: Properties 'PolicyName', 'PolicyType', 'MinProtocolVersion' and 'CipherSuites' have been introduced for 'SslPolicy' in api-version '2019-06-01'. 'DisabledSslProtocols' cannot be used with the newly introduced properties. Use 'MinProtocolVersion' along with PolicyType 'Custom' instead of 'DisabledSslProtocols' for 'SslPolicy' for Application Gateway 'xxx'.

Error creating the Application Gateway instance: Azure Error: ApplicationGatewaySslPolicyPropertyNotSupportedForPolicyType
Message: Specified PolicyType 'Predefined' does not support property 'CipherSuites' for 'SslPolicy' for Application Gateway 'xxx'.

@l3ender
Copy link
Contributor Author

l3ender commented Jan 27, 2022

@Fred-sun Please review. Thank you!

@l3ender
Copy link
Contributor Author

l3ender commented Jan 31, 2022

Sorry for the few additional commits after requesting review, but it should be good to go now. Thanks!

@Fred-sun Fred-sun added enhancement New feature or request medium_priority Medium priority new_feature New feature requirments work in In trying to solve, or in working with contributors labels Feb 12, 2022
@Fred-sun
Copy link
Collaborator

@l3ender Please!

ERROR: plugins/modules/azure_rm_appgateway_info.py:0:0: return-syntax-error: RETURN.gateways.contains.ssl_policy.description: required key not provided @ data['gateways']['contains']['ssl_policy']['description']. Got None

@l3ender
Copy link
Contributor Author

l3ender commented Feb 13, 2022

@Fred-sun - it is now corrected; thank you!

@l3ender
Copy link
Contributor Author

l3ender commented Feb 26, 2022

Hi @Fred-sun, anything else needed for this PR? Thank you!

@Fred-sun
Copy link
Collaborator

@l3ender I will push forward the merger of this PR as soon as possible. Thank you very much!

@l3ender
Copy link
Contributor Author

l3ender commented Mar 23, 2022

@Fred-sun I think this PR is good to release. Let me know if I can add anything else--thank you!

@Fred-sun Fred-sun added ready_for_review The PR has been modified and can be reviewed and merged and removed work in In trying to solve, or in working with contributors labels Apr 8, 2022
@xuzhang3
Copy link
Collaborator

LGTM

1 similar comment
@xuzhang3
Copy link
Collaborator

LGTM

@xuzhang3 xuzhang3 merged commit 3342043 into ansible-collections:dev Apr 15, 2022
@xuzhang3 xuzhang3 deleted the appgw-ssl-config branch April 15, 2022 03:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request medium_priority Medium priority new_feature New feature requirments ready_for_review The PR has been modified and can be reviewed and merged
Projects
None yet
Development

Successfully merging this pull request may close these issues.

azure_rm_appgateway ssl_policy settings have no effect
4 participants