From 808199929f9936ccd5d2ee46673ab2ffaa821c4d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Leszczy=C5=84ski?= <2000michal@wp.pl> Date: Mon, 16 Sep 2024 13:36:28 +0200 Subject: [PATCH] fix(helthcheck): don't run alternator query ping when authentication is enforced This is a workaround for #4036. Ref #4036 --- pkg/ping/dynamoping/dynamoping.go | 24 +++++++++++++----------- pkg/service/healthcheck/service.go | 17 ++++++++++------- 2 files changed, 23 insertions(+), 18 deletions(-) diff --git a/pkg/ping/dynamoping/dynamoping.go b/pkg/ping/dynamoping/dynamoping.go index 257da4bc76..6359dceb88 100644 --- a/pkg/ping/dynamoping/dynamoping.go +++ b/pkg/ping/dynamoping/dynamoping.go @@ -24,8 +24,6 @@ import ( type Config struct { Addr string RequiresAuthentication bool - Username string - Password string Timeout time.Duration TLSConfig *tls.Config } @@ -71,10 +69,19 @@ var unauthorisedMessage = []string{ "The security token included in the request is invalid.", } +// ErrAlternatorQueryPingNotSupported is returned when alternator query ping is executed, +// but managed cluster enforces alternator authentication. +// See #4036 for more details. +var ErrAlternatorQueryPingNotSupported = errors.New("ScyllaDB Manager does not support alternator query ping when authentication is enforced") + // QueryPing checks if host is available, it returns RTT and error. Special errors // are ErrTimeout and ErrUnauthorised. Ping is based on executing // a real query. func QueryPing(ctx context.Context, config Config) (rtt time.Duration, err error) { + if config.RequiresAuthentication { + return 0, ErrAlternatorQueryPingNotSupported + } + t := timeutc.Now() defer func() { rtt = timeutc.Since(t) @@ -92,18 +99,13 @@ func QueryPing(ctx context.Context, config Config) (rtt time.Duration, err error sess := session.Must(session.NewSessionWithOptions(session.Options{ Config: aws.Config{ - Endpoint: aws.String(config.Addr), - Region: aws.String("scylla"), - HTTPClient: httpClient(config), + Endpoint: aws.String(config.Addr), + Region: aws.String("scylla"), + HTTPClient: httpClient(config), + Credentials: credentials.AnonymousCredentials, }, })) - if config.RequiresAuthentication && config.Username != "" && config.Password != "" { - sess.Config.Credentials = credentials.NewStaticCredentials(config.Username, config.Password, "") - } else { - sess.Config.Credentials = credentials.AnonymousCredentials - } - svc := dynamodb.New(sess) ctx, cancel := context.WithDeadline(ctx, t.Add(config.Timeout)) diff --git a/pkg/service/healthcheck/service.go b/pkg/service/healthcheck/service.go index 070acda63f..1213682963 100644 --- a/pkg/service/healthcheck/service.go +++ b/pkg/service/healthcheck/service.go @@ -300,15 +300,18 @@ func (s *Service) pingAlternator(ctx context.Context, _ uuid.UUID, host string, return 0, nil } - pingFunc := dynamoping.SimplePing - if queryPing, err := ni.SupportsAlternatorQuery(); err == nil && queryPing { - pingFunc = dynamoping.QueryPing - } - addr := ni.AlternatorAddr(host) config := dynamoping.Config{ - Addr: addr, - Timeout: timeout, + Addr: addr, + Timeout: timeout, + RequiresAuthentication: ni.AlternatorEnforceAuthorization, + } + + pingFunc := dynamoping.SimplePing + if !config.RequiresAuthentication { + if queryPing, err := ni.SupportsAlternatorQuery(); err == nil && queryPing { + pingFunc = dynamoping.QueryPing + } } tlsConfig := ni.AlternatorTLSConfig()