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

Allow setting a custom Authenticator #1242

Merged
merged 2 commits into from
Nov 24, 2018
Merged

Conversation

alourie
Copy link
Contributor

@alourie alourie commented Nov 15, 2018

Fixes #1238
Fixes #1109

Signed-off-by: Alex Lourie djay.il@gmail.com

Signed-off-by: Alex Lourie <djay.il@gmail.com>
@alourie
Copy link
Contributor Author

alourie commented Nov 15, 2018

@Zariel, @magaldima something along these lines? If this looks the right direction, I'll add the failure when can't create the Authenticator and will add tests.

@magaldima
Copy link

I'm specifically worried now that the input host to the AuthProvider is hardcoded as host.ConnectAddress().String(). This doesn't solve my use case since I have to authenticate against a domain hostname such as devxxx12.company.com (which I include as part of the initialConnectHosts) instead of the IPV4 connect address hostname. It's possible to get around this by building an explicit mapping from IPV4 address to name myself in my custom Authenticator.

Copy link
Contributor

@Zariel Zariel left a comment

Choose a reason for hiding this comment

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

We should also check if a config has both set and error out during session creation.

cluster.go Outdated
Consistency Consistency // default consistency level (default: Quorum)
Compressor Compressor // compression algorithm (default: nil)
Authenticator Authenticator // authenticator (default: nil)
AuthProvider func(h string) (Authenticator, error) // an authenticator factory. Can be used to create alternative authenticators (default: nil)
Copy link
Contributor

Choose a reason for hiding this comment

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

I think this should take a HostInfo

conn.go Outdated
if cfg.AuthProvider != nil {
c.auth, err = cfg.AuthProvider(host.ConnectAddress().String())
if err != nil {
// Fail?
Copy link
Contributor

Choose a reason for hiding this comment

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

yes

Signed-off-by: Alex Lourie <djay.il@gmail.com>
@Zariel Zariel merged commit 70385f8 into apache:master Nov 24, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants