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

Use ConnectionConfig values when establishing session to initial contact point #106

Merged
merged 1 commit into from
Nov 14, 2022
Merged

Use ConnectionConfig values when establishing session to initial contact point #106

merged 1 commit into from
Nov 14, 2022

Conversation

zimnx
Copy link
Collaborator

@zimnx zimnx commented Nov 2, 2022

When host information was missing, driver used resolved IP address as TLS.ServerName.
Instead it should connect to Server specified in ConnectionConfig and use NodeDomain as SNI.

@zimnx zimnx force-pushed the mz/initial-contact-point-sni branch 2 times, most recently from a141f76 to 9337308 Compare November 3, 2022 08:48
@zimnx zimnx requested a review from tnozicka November 3, 2022 08:48
@zimnx zimnx added the bug label Nov 3, 2022
@zimnx zimnx mentioned this pull request Nov 4, 2022
Merged
@zimnx zimnx force-pushed the mz/initial-contact-point-sni branch from 9337308 to 9d84173 Compare November 4, 2022 10:57
tnozicka
tnozicka reviewed Nov 4, 2022
View reviewed changes
host_source.go Outdated Show resolved Hide resolved
scyllacloud/hostdialer.go Outdated Show resolved Hide resolved
scyllacloud/hostdialer.go Outdated Show resolved Hide resolved
scyllacloud/hostdialer_test.go Outdated Show resolved Hide resolved
scyllacloud/hostdialer_test.go Outdated Show resolved Hide resolved
scyllacloud/hostdialer_test.go Outdated Show resolved Hide resolved
@zimnx zimnx force-pushed the mz/initial-contact-point-sni branch 2 times, most recently from 1db79ed to 6d12207 Compare November 4, 2022 16:18
tnozicka
tnozicka reviewed Nov 7, 2022
View reviewed changes
scyllacloud/hostdialer.go Outdated Show resolved Hide resolved
scyllacloud/hostdialer.go Outdated Show resolved Hide resolved
scyllacloud/hostdialer_test.go Outdated Show resolved Hide resolved
scyllacloud/hostdialer_test.go Outdated Show resolved Hide resolved
@zimnx zimnx force-pushed the mz/initial-contact-point-sni branch 2 times, most recently from 2819177 to 6a58023 Compare November 7, 2022 14:50
@zimnx zimnx requested a review from tnozicka November 7, 2022 15:07
fruch added a commit to fruch/python-driver that referenced this pull request Nov 8, 2022
@fruch
scylla/cloud.py: support using ip address in server
0e2dd4b 
Align with scylladb/gocql#106, so:
When host information was missing, driver used resolved IP address as TLS.ServerName.
Instead it should connect to Server specified in ConnectionConfig and use NodeDomain as SNI.

Depends: scylladb/scylla-ccm#412
Ref: scylladb/gocql#106
@fruch fruch mentioned this pull request Nov 8, 2022
Merged
tnozicka
tnozicka reviewed Nov 8, 2022
View reviewed changes
scyllacloud/config.go Outdated Show resolved Hide resolved
scyllacloud/hostdialer.go Outdated Show resolved Hide resolved
scyllacloud/hostdialer.go Outdated Show resolved Hide resolved
scyllacloud/hostdialer.go Outdated Show resolved Hide resolved
scyllacloud/hostdialer.go Outdated Show resolved Hide resolved
scyllacloud/hostdialer_test.go Outdated Show resolved Hide resolved
@fruch fruch mentioned this pull request Nov 8, 2022
Closed
@zimnx zimnx force-pushed the mz/initial-contact-point-sni branch 3 times, most recently from 64bad46 to 414bd04 Compare November 8, 2022 16:56
@vponomaryov vponomaryov mentioned this pull request Nov 9, 2022
Closed
7 tasks
@zimnx zimnx requested a review from tnozicka November 9, 2022 11:51
fruch added a commit to fruch/python-driver that referenced this pull request Nov 9, 2022
@fruch
scylla/cloud.py: support using ip address in server
79adf48 
Align with scylladb/gocql#106, so:
When host information was missing, driver used resolved IP address as TLS.ServerName.
Instead it should connect to Server specified in ConnectionConfig and use NodeDomain as SNI.

Depends: scylladb/scylla-ccm#412
Ref: scylladb/gocql#106
fruch added a commit to scylladb/python-driver that referenced this pull request Nov 9, 2022
@fruch
scylla/cloud.py: support using ip address in server
1d35a13 
Align with scylladb/gocql#106, so:
When host information was missing, driver used resolved IP address as TLS.ServerName.
Instead it should connect to Server specified in ConnectionConfig and use NodeDomain as SNI.

Depends: scylladb/scylla-ccm#412
Ref: scylladb/gocql#106
tnozicka
tnozicka reviewed Nov 10, 2022
View reviewed changes
scyllacloud/config.go Outdated Show resolved Hide resolved
@zimnx zimnx requested a review from tnozicka November 10, 2022 13:27
@zimnx
UPSTREAM: <carry>: Use ConnectionConfig fields when establishing sess…
a8f6507 
…ion to initial contact point

When host information was missing, driver used resolved IP address as TLS.ServerName
Instead it should connect to Server specified in ConnectionConfig and use NodeDomain
as SNI.
@zimnx zimnx force-pushed the mz/initial-contact-point-sni branch from 414bd04 to a8f6507 Compare November 10, 2022 15:49
@zimnx zimnx requested a review from tnozicka November 10, 2022 15:54
tnozicka
tnozicka approved these changes Nov 11, 2022
View reviewed changes
Copy link
Member

@tnozicka tnozicka left a comment

Choose a reason for hiding this comment

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

one optional nit

/lgtm
/approve

thanks!

tnozicka
tnozicka reviewed Nov 11, 2022
View reviewed changes
scyllacloud/hostdialer_test.go
return cc
}(),
hostInfo: &gocql.HostInfo{},
expectedError: fmt.Errorf("can't get client certificate from configuration: %w", fmt.Errorf("can't get current auth info: %w", fmt.Errorf("can't get current context config: %w", fmt.Errorf("current context can't be empty")))),
Copy link
Member

Choose a reason for hiding this comment

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

nit (global)

Suggested change
expectedError: fmt.Errorf("can't get client certificate from configuration: %w", fmt.Errorf("can't get current auth info: %w", fmt.Errorf("can't get current context config: %w", fmt.Errorf("current context can't be empty")))),
expectedError: fmt.Errorf("can't get client certificate from configuration: %w", fmt.Errorf("can't get current auth info: %w", fmt.Errorf("can't get current context config: %w", errors.New("current context can't be empty")))),

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Leaving as is, underlying errors can be obtain by using errors.Unwrap(fmt.Errorf("... %w ....", ..., err, ....)), and errors.Is supports both.

fruch
fruch approved these changes Nov 14, 2022
View reviewed changes
Copy link

@fruch fruch left a comment

Choose a reason for hiding this comment

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

LGTM

@zimnx zimnx merged commit b42f4b9 into scylladb:master Nov 14, 2022
fruch added a commit to fruch/scylla-bench that referenced this pull request Nov 15, 2022
@fruch
update to gocql==v1.7.3
30e2a72 
for serverless work we need the fix from:
scylladb/gocql#106

so we can run with diret ip address for testsing
@fruch fruch mentioned this pull request Nov 15, 2022
Merged
fruch added a commit to scylladb/scylla-bench that referenced this pull request Nov 15, 2022
@fruch
update to gocql==v1.7.3
d63d297 
for serverless work we need the fix from:
scylladb/gocql#106

so we can run with diret ip address for testsing
avelanarius added a commit to avelanarius/java-driver that referenced this pull request Nov 15, 2022
@avelanarius
Connect to server specified in serverless bundle
90bd3f7 
Before this change, the driver, when connecting to a serverless cluster,
would try to resolve host-id.* domains as a proxy IP for each node.

This commit aligns the behavior with scylladb/python-driver#177 and
scylladb/gocql#106. Now, the "Server" field of bundle is the single
hostname/IP the driver will use for proxy connection.

This simplifies the setup we had in integration tests, removing the
necessity of hacks caused by previous behavior (driver trying to
resolve host-id.* domains).

Fixes scylladb#164
Fixes scylladb#167
Fixes scylladb#169
@avelanarius avelanarius mentioned this pull request Nov 15, 2022
Merged
avelanarius added a commit to avelanarius/java-driver that referenced this pull request Nov 15, 2022
@avelanarius
Connect to server specified in serverless bundle
5fe8b3b 
Before this change, the driver, when connecting to a serverless cluster,
would try to resolve host-id.* domains as a proxy IP for each node.

This commit aligns the behavior with scylladb/python-driver#177 and
scylladb/gocql#106. Now, the "Server" field of bundle is the single
hostname/IP the driver will use for proxy connection.

This simplifies the setup we had in integration tests, removing the
necessity of hacks caused by previous behavior (driver trying to
resolve host-id.* domains).

Fixes scylladb#164
Fixes scylladb#167
Fixes scylladb#169
avelanarius added a commit to avelanarius/java-driver that referenced this pull request Nov 15, 2022
@avelanarius
Connect to server specified in serverless bundle
73614fc 
Before this change, the driver, when connecting to a serverless cluster,
would try to resolve host-id.* domains as a proxy IP for each node.

This commit aligns the behavior with scylladb/python-driver#177 and
scylladb/gocql#106. Now, the "Server" field of bundle is the single
hostname/IP the driver will use for proxy connection.

This simplifies the setup we had in integration tests, removing the
necessity of hacks to work around previous behavior (driver trying to
resolve host-id.* domains).

Fixes scylladb#164
Fixes scylladb#167
Fixes scylladb#169
avelanarius added a commit to avelanarius/java-driver that referenced this pull request Nov 23, 2022
@avelanarius
Connect to server specified in serverless bundle
d75744b 
Before this change, the driver, when connecting to a serverless cluster,
would try to resolve host-id.* domains as a proxy IP for each node.

This commit aligns the behavior with scylladb/python-driver#177 and
scylladb/gocql#106. Now, the "Server" field of bundle is the single
hostname/IP the driver will use for proxy connection.

This simplifies the setup we had in integration tests, removing the
necessity of hacks to work around previous behavior (driver trying to
resolve host-id.* domains).

Fixes scylladb#164
Fixes scylladb#167
Fixes scylladb#169
avelanarius added a commit to scylladb/java-driver that referenced this pull request Nov 23, 2022
@avelanarius
Connect to server specified in serverless bundle
f5fc83d 
Before this change, the driver, when connecting to a serverless cluster,
would try to resolve host-id.* domains as a proxy IP for each node.

This commit aligns the behavior with scylladb/python-driver#177 and
scylladb/gocql#106. Now, the "Server" field of bundle is the single
hostname/IP the driver will use for proxy connection.

This simplifies the setup we had in integration tests, removing the
necessity of hacks to work around previous behavior (driver trying to
resolve host-id.* domains).

Fixes #164
Fixes #167
Fixes #169
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@tnozicka tnozicka tnozicka approved these changes

@fruch fruch fruch approved these changes

Assignees
No one assigned
Labels
bug
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@zimnx @fruch @tnozicka