Skip to content

transport.publish_address should contain CNAME#45626

Merged
andrershov merged 3 commits intoelastic:masterfrom
andrershov:tcp_add_cname
Aug 16, 2019
Merged

transport.publish_address should contain CNAME#45626
andrershov merged 3 commits intoelastic:masterfrom
andrershov:tcp_add_cname

Conversation

@andrershov
Copy link
Copy Markdown
Contributor

This commit adds CNAME reporting for transport.publish_address same way it's done for http.publish_address.

  1. This PR goes to 7.x and master (8.0.0).
  2. Follow-up PR to master (8.0.0) will output cname anyway and log deprecation warning if system property is specified.
  3. Once 8 is released, a new follow-up can remove the property handling completely.

Relates #32806
Relates #39970

@andrershov andrershov added >bug :Distributed/Network Http and internode communication implementations v8.0.0 v7.4.0 labels Aug 15, 2019
@elasticmachine
Copy link
Copy Markdown
Collaborator

Pinging @elastic/es-distributed

Copy link
Copy Markdown
Contributor

@original-brownbear original-brownbear left a comment

Choose a reason for hiding this comment

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

LGTM :)

static final String PROFILES = "profiles";
}

private String getPublishAddressString(String propertyName, TransportAddress publishAddress){
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

NIT: maybe rather name this formatPublishAddressString, it's really weird if something is named get... and then used to print another object's properties :)

@andrershov andrershov merged commit e0a2558 into elastic:master Aug 16, 2019
andrershov pushed a commit that referenced this pull request Aug 16, 2019
This commit adds CNAME reporting for transport.publish_address same way
it's done for http.publish_address.

Relates #32806
Relates #39970

(cherry picked from commit e0a2558)
andrershov pushed a commit that referenced this pull request Aug 22, 2019
…erty is specified (#45662)

Follow-up of #45626.
Now we always output transport.publish_address with CNAME and log
deprecation warning if es.transport.cname_in_publish_address property
is specified.
This commit also adds a test which will fail once Elasticsearch version is
changed to 9, to make sure we remove the property when doing
reversioning.

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

Labels

>bug :Distributed/Network Http and internode communication implementations v7.4.0 v8.0.0-alpha1

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants