Skip to content

Use correct HostnameVerifier when verify-hostnames set to true#20076

Merged
Praveen2112 merged 1 commit intotrinodb:masterfrom
Praveen2112:praveen/elasticsearch_host_verifier_fix
Dec 12, 2023
Merged

Use correct HostnameVerifier when verify-hostnames set to true#20076
Praveen2112 merged 1 commit intotrinodb:masterfrom
Praveen2112:praveen/elasticsearch_host_verifier_fix

Conversation

@Praveen2112
Copy link
Copy Markdown
Member

Description

Previously we were using NoopHostnameVerifier which turns off hostname verification, and if the HostnameVerifier is not set it uses DefaultHostnameVerifier which performs hostname verification

Additional context and related issues

Release notes

( ) This is not user-visible or is docs only, and no release notes are required.
( ) Release notes are required. Please propose a release note for me.
(x) Release notes are required, with the following suggested text:

# Elasticsearch
* Use correct HostnameVerifier when verify-hostnames set to true.

Previously we were using `NoopHostnameVerifier` which turns off hostname verification,
if the HostnameVerifier is not set it uses `DefaultHostnameVerifier` which performs
hostname verification
Copy link
Copy Markdown
Member

@kokosing kokosing left a comment

Choose a reason for hiding this comment

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

% comment

@Praveen2112 Praveen2112 merged commit de329bd into trinodb:master Dec 12, 2023
@github-actions github-actions bot added this to the 435 milestone Dec 12, 2023
@mosabua
Copy link
Copy Markdown
Member

mosabua commented Dec 14, 2023

I think this is potentially a breaking change @martint although the property defaults to true and is also not documented

@Praveen2112 can you send docs?

I also reworded the release notes entry.

@martint
Copy link
Copy Markdown
Member

martint commented Dec 14, 2023

It's a bug fix, really, so if someone were depending on the incorrect behavior it would appear to be a breaking change. I don't think it warrants being marked as a breaking change.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Development

Successfully merging this pull request may close these issues.

7 participants