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

Add optional argument for overriding get_tls_context() parameters #8275

Merged
merged 11 commits into from
Jan 7, 2021

Conversation

yzhan289
Copy link
Contributor

@yzhan289 yzhan289 commented Dec 30, 2020

What does this PR do?

This PR adds the option for get_tls_context() to add an overrides argument to set config values.

This is intended to be compatible with integrations that hardcode the values of SSL context config values in their checks.

Note that the overrides argument takes precedent over reading the values for the respective config value.

This change can affect

This can also be applied to recently updated SSL context checks, such as

Motivation

Some older integrations specifically set values for their SSL context without reading the config values. However the get_tls_context() function abstracts the creation of the SSL context away without allowing the check to manually set config values that should stay the same. This could affect the intended functionality of the check while lacking visibility.

Additional Notes

SSL context and TLS context in my other PRs are referring to the same things.

Review checklist (to be filled by reviewers)

  • Feature or bugfix MUST have appropriate tests (unit, integration, e2e)
  • PR title must be written as a CHANGELOG entry (see why)
  • Files changes must correspond to the primary purpose of the PR as described in the title (small unrelated changes should have their own PR)
  • PR must have changelog/ and integration/ labels attached

@yzhan289 yzhan289 merged commit 3f3b108 into master Jan 7, 2021
@yzhan289 yzhan289 deleted the az/tls_context_overrides branch January 7, 2021 21:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants