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.

Sign up for GitHub

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

Jump to bottom

Allow Insecure HTTPS #2913

Merged
merged 7 commits into from
Oct 25, 2024
Merged

Allow Insecure HTTPS #2913

merged 7 commits into from
Oct 25, 2024

Conversation

kmitul
Copy link
Contributor

@kmitul kmitul commented Oct 23, 2024

Closes #2491

Updated the verification logic for server certificates to allow for configurable settings. The verify_certs option now defaults to True but can be set to False in the configuration. This change enhances flexibility, allowing users to opt out of certificate verification in environments with varying security requirements.

Checklists

Pre-Review Checklist

  • this PR does NOT contain credentials of any kind, such as API keys or username/passwords (double check config.yml.example)
  • this PR has a meaningful title
  • this PR links to all relevant github issues that it fixes or partially addresses
  • this PR has a thorough description
  • Tested the changes locally
  • Considered corresponding documentation changes
  • Contributed any configuration settings changes to the configuration reference

Changes Requiring Extra Attention

  • Security-related changes (encryption, TLS, SSRF, etc)

Release Note

@cla-checker-service CLA Checker Service
Copy link

cla-checker-service bot commented Oct 23, 2024 •
edited
Loading

💚 CLA has been signed

@kmitul kmitul changed the title [WIP] Allow Insecure HTTPS Allow Insecure HTTPS Oct 23, 2024
@kmitul kmitul marked this pull request as ready for review October 23, 2024 06:52
@kmitul kmitul requested a review from a team as a code owner October 23, 2024 06:52
jedrazb
jedrazb reviewed Oct 23, 2024
View reviewed changes
Copy link
Member

@jedrazb jedrazb left a comment

Choose a reason for hiding this comment

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

Great job @kmitul thank you for looking into this 👍 I’ve left a couple of small suggestions. The main concern is highlighting that we don’t want this setting to be used in production, so the comments should be adjusted accordingly.

Also, please make sure you sign the contributor agreement, as we can’t merge your PR without it.

connectors/es/client.py Show resolved Hide resolved
config.yml.example Show resolved Hide resolved
docs/DOCKER.md Outdated Show resolved Hide resolved
docs/DOCKER.md Outdated Show resolved Hide resolved
connectors/es/client.py Outdated Show resolved Hide resolved
@jedrazb
Copy link
Member

jedrazb commented Oct 23, 2024

buildkite test this

@jedrazb
Copy link
Member

jedrazb commented Oct 23, 2024

You need to run make autoformat to make our linter happy (this reminds me we should perhaps make this a pre-commit hook)

@kmitul
Copy link
Contributor Author

kmitul commented Oct 24, 2024

Hi @jedrazb :)
Thank you for review and suggestions 👍
I have made the necessary fixes!

@jedrazb
Copy link
Member

jedrazb commented Oct 24, 2024

buildkite test this

jedrazb
jedrazb approved these changes Oct 24, 2024
View reviewed changes
Copy link
Member

@jedrazb jedrazb left a comment

Choose a reason for hiding this comment

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

LGTM! Thank you for contributing to the repo :)

@seanstory seanstory enabled auto-merge (squash) October 24, 2024 18:58
@seanstory
Copy link
Member

Awesome stuff. Make sure to check out our contributor program, to get rewards! https://www.elastic.co/community/contributor/submissions

@jedrazb
Copy link
Member

jedrazb commented Oct 25, 2024

buildkite test this

@jedrazb
Copy link
Member

jedrazb commented Oct 25, 2024

buildkite test this

@seanstory seanstory merged commit ece146e into elastic:main Oct 25, 2024
2 checks passed
@github-actions GitHub Actions
Copy link

💔 Failed to create backport PR(s)

The backport operation could not be completed due to the following error:
There are no branches to backport to. Aborting.

The backport PRs will be merged automatically after passing CI.

To backport manually run:
backport --pr 2913 --autoMerge --autoMergeMethod squash

@kmitul kmitul deleted the allow-insecure-https branch October 29, 2024 06:13
jedrazb pushed a commit that referenced this pull request Dec 10, 2024
@kmitul
Allow Insecure HTTPS (#2913)
d2ded88 
(cherry picked from commit ece146e)
@jedrazb jedrazb mentioned this pull request Dec 10, 2024
Merged
jedrazb pushed a commit that referenced this pull request Dec 10, 2024
@kmitul
Allow Insecure HTTPS (#2913)
3ba1d60 
(cherry picked from commit ece146e)
@jedrazb jedrazb mentioned this pull request Dec 10, 2024
Merged
@jedrazb
Copy link
Member

jedrazb commented Dec 10, 2024

💚 All backports created successfully

Status Branch Result
✅ 8.x
✅ 8.17

Note: Successful backport PRs will be merged automatically after passing CI.

Questions ?

Please refer to the Backport tool documentation

jedrazb added a commit that referenced this pull request Dec 24, 2024
@jedrazb @kmitul @artem-shelkovnikov
[8.x] Allow Insecure HTTPS (#2913) (#3025)
785fd66 
Co-authored-by: Mitul Kataria <53906546+kmitul@users.noreply.github.com>
Co-authored-by: Artem Shelkovnikov <artem.shelkovnikov@elastic.co>
jedrazb added a commit that referenced this pull request Dec 24, 2024
@jedrazb @kmitul @artem-shelkovnikov
[8.17] Allow Insecure HTTPS (#2913) (#3026)
48b9566 
Co-authored-by: Mitul Kataria <53906546+kmitul@users.noreply.github.com>
Co-authored-by: Artem Shelkovnikov <artem.shelkovnikov@elastic.co>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jedrazb jedrazb jedrazb approved these changes

Assignees
No one assigned
Labels
auto-backport v8.17.1 v8.18.0 v9.0.0
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Allow insecure HTTPS
3 participants
@kmitul @jedrazb @seanstory