Skip to content

Commit 7afff16

Browse files
authored
fix: Fix re-authentication when used with etcd server 3.5.x (#76)
This unifies the "reauth required" logic between KV and watch requests and ensures that it covers the new response codes/messages. Thanks to Kei Sugano sgnk@jp.ibm.com for help with the unit test additions. Signed-off-by: Nick Hill <nickhill@us.ibm.com>
1 parent 7f74e9a commit 7afff16

File tree

11 files changed

+236
-39
lines changed

11 files changed

+236
-39
lines changed

.travis.yml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ jdk:
66
env:
77
jobs:
88
- TARGET_ETCD_VERSION=v3.4.20
9-
- TARGET_ETCD_VERSION=v3.5.9
9+
- TARGET_ETCD_VERSION=v3.5.10
1010
global:
1111
- secure: "XB13D71N/3iWgKh/+3TDdqLYQN3hBzD+BvUYSbkB/Pgyl1mjI7CVj03IxeVl7AvLkP8O9t4WTVNxutIMXdEap01cc+cvd8S1aCKgRwbMh3dSHbRowjXB9XccX7h7JE09gkmBxHQWu5lIZqdSG2dWat4YjD0i/IdFDDFtmDV1VKVLnjEvxroHBJJDMo5mYFbrQZB0D+5W2v52JpdMw54tF/vsQ6ThlMN9faEuuD+HQBiEvslMmbl+d9eiDxPhHkdf4fUQr0OBz65V/ggvOd0aP8fRpD+G9bKdnjM+9kkvuCFOXTSlxJzhNO4JGIEJErXumGFjjsu5QwFD+RKrKaTl1V5/6IrkKgmyn/ZVFgI8ZiullwTyxei+7mHd1LAt+DW/GDrCG/Q3s3jrgfc9iMemgaMojrK6iOgtHAYYhQwDuOwdxLeOmqvgBEWvbe7nsebLQG6LOhJPEI7UjcSDXH9m7ag/CpP9q3qEOChpQWGkMo0eWLOmOMujdbLWqFaqNs/ot5yZJXb+BNW3opO40KFm0kG1arw3V/4LB+GZ4Wh52ScCZugBshUa8hB1J4pLlFhyi09IaxiqKCqAHfAtwvLQ2SY1VtBkyryViHX1VSIfPa4LpaHUIarFMZk11Vq3SmCvXrKhl1Vt7iVHDQMGBtO+abmfmGSZTgb3twEdcLc1Mc8="
1212
- secure: "FggRkH5Ttnz/AfHdwcxy5tnkqA42XwuRx4aDEmaz+CuK+hfKbwIQiqli0PzUQqbPJtEReU85vvyRjbC5lAOvFzQKTrCU3shbXXGq9PGiFVvVNsFB+93akAwtEZYl9CDjRzuYqNiS9hrpze4qMLVZsbRUGSXDVGNzK27//zA1WXbb/htqa4xTphmxLG6CrHI8L65tbbC2WY+qIa5eQHJDLwVRVAisXT/LDpj8ky69I9bRZZbyirkwKsgf7UGyneqqjO/d+3IOfc6ppGPQhe1IeL1XuVtiul9OtksfSBsAttgro7nRgbe2szU+Um2JKcv6dxqqFZJi7SN6TfWpN2f16bU88mJHmyZAILr6YQAHkiWM48vwcZR2MVYH0FI0M1c7xmsDeG6bAQ6H0kWKdLr0NfX1/vm+TPc+OHaCsnTlpcnQx5mcTi4Cztr/oTWMYsrocfuoNOy31gosnLspmjFcqUbjreWvArdZNXmVzj0hp6ewNz/bLszqtI57LdoAMc/bDs8rqElpCr8r12vsQQNgmmKigTlZIe8jNoPnJw00atzg7Dn2XqTHxNnFhtMOhEfmX7fEQ4du1LW0H8RntESjdAIeO8kXttraGmHP5Au7b78zN2WKc21NLBVuwAmcm69G6qcGFdBhuY6xup7aipG0ilB7M+/jllcD4mVAPn3lLGQ="

build-env/install-etcd.sh

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@
33
# Fail on any error
44
set -e
55

6-
ETCD_VERSION=${TARGET_ETCD_VERSION:-v3.5.9}
6+
ETCD_VERSION=${TARGET_ETCD_VERSION:-v3.5.10}
77

88
INSTALL_DIR="${1:-$HOME/etcd}"
99

cd/deploy.sh

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
#!/usr/bin/env bash
22

33
echo TRAVIS_JDK_VERSION="$TRAVIS_JDK_VERSION"
4-
if [ "$TRAVIS_BRANCH" = 'main' ] && [ "$TRAVIS_PULL_REQUEST" == 'false' ] && [ "$TRAVIS_JDK_VERSION" == 'openjdk11' ] && [ "$TARGET_ETCD_VERSION" == 'v3.5.9' ]; then
4+
if [ "$TRAVIS_BRANCH" = 'main' ] && [ "$TRAVIS_PULL_REQUEST" == 'false' ] && [ "$TRAVIS_JDK_VERSION" == 'openjdk17' ] && [ "$TARGET_ETCD_VERSION" == 'v3.5.10' ]; then
55
echo "deploying release to central repository"
66

77
# prepare key for signing

src/main/java/com/ibm/etcd/client/EtcdClient.java

Lines changed: 24 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -586,22 +586,23 @@ public boolean isClosed() {
586586
// See https://godoc.org/github.com/coreos/etcd/auth#pkg-variables
587587
// and https://godoc.org/github.com/coreos/etcd/etcdserver/api/v3rpc/rpctypes#pkg-variables
588588
private static final String[] AUTH_FAILURE_MESSAGES = {
589-
// These will be prefixed with "etcdserver: "
590-
"root user does not exist",
591-
"root user does not have root role",
592-
"user name already exists",
593-
"user name is empty",
594-
"user name not found",
595-
"role name already exists",
596-
"role name not found",
597-
"role name is empty",
598-
"authentication failed, invalid user ID or password",
599-
"permission denied",
600-
"role is not granted to the user",
601-
"permission is not granted to the role",
602-
"authentication is not enabled",
603-
"invalid auth token",
604-
"invalid auth management"
589+
// These will be prefixed with "etcdserver: "
590+
"root user does not exist",
591+
"root user does not have root role",
592+
"user name already exists",
593+
"user name is empty",
594+
"user name not found",
595+
"role name already exists",
596+
"role name not found",
597+
"role name is empty",
598+
"authentication failed, invalid user ID or password",
599+
"permission denied",
600+
"role is not granted to the user",
601+
"permission is not granted to the role",
602+
"authentication is not enabled",
603+
"invalid auth token",
604+
"invalid auth management",
605+
"revision of auth store is old"
605606
};
606607

607608
private Status lastAuthFailStatus;
@@ -613,11 +614,13 @@ protected static boolean reauthRequired(Throwable error) {
613614
}
614615
Status status = Status.fromThrowable(error);
615616
Code code = status.getCode();
616-
return code == Code.UNAUTHENTICATED
617-
|| (OTHER_AUTH_FAILURE_CODES.contains(code) &&
618-
(startsWith(status.getDescription(), "auth: ")
619-
|| endsWith(status.getDescription(), AUTH_FAILURE_MESSAGES)))
620-
|| (code == Code.CANCELLED && reauthRequired(error.getCause()));
617+
return reauthRequired(code, status.getDescription()) ||
618+
(code == Code.CANCELLED && reauthRequired(error.getCause()));
619+
}
620+
621+
public static boolean reauthRequired(Code code, String message) {
622+
return code == Code.UNAUTHENTICATED || (OTHER_AUTH_FAILURE_CODES.contains(code) &&
623+
(startsWith(message, "auth: ") || endsWith(message, AUTH_FAILURE_MESSAGES)));
621624
}
622625

623626
// This is only called in a synchronized context

src/main/java/com/ibm/etcd/client/GrpcClient.java

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,7 @@
3434
import java.util.concurrent.locks.LockSupport;
3535
import java.util.function.Function;
3636
import java.util.function.Supplier;
37+
import java.util.regex.Pattern;
3738

3839
import org.slf4j.Logger;
3940
import org.slf4j.LoggerFactory;
@@ -72,6 +73,8 @@ public class GrpcClient {
7273

7374
private static final Logger logger = LoggerFactory.getLogger(GrpcClient.class);
7475

76+
private static final Pattern CAMELCASE_PATT = Pattern.compile("([a-z])([A-Z]+)");
77+
7578
private final long defaultTimeoutMs;
7679

7780
public interface AuthProvider {
@@ -115,6 +118,15 @@ default CallCredentials refreshCredentials(Throwable trigger) {
115118
// modified only by reauthenticate() method
116119
private CallOptions callOptions = CallOptions.DEFAULT; // volatile tbd - lazy probably ok
117120

121+
// parse gRPC code from go formatted string (camel-case)
122+
public static Code parseGoCodeString(String codeString) {
123+
try {
124+
return Code.valueOf(CAMELCASE_PATT.matcher(codeString).replaceAll("$1_$2").toUpperCase());
125+
} catch (IllegalArgumentException _) {
126+
return Code.UNKNOWN;
127+
}
128+
}
129+
118130
/**
119131
* @deprecated use other constructor
120132
*/

src/main/java/com/ibm/etcd/client/watch/EtcdWatchClient.java

Lines changed: 22 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -25,9 +25,12 @@
2525
import java.util.concurrent.CancellationException;
2626
import java.util.concurrent.ConcurrentLinkedQueue;
2727
import java.util.concurrent.Executor;
28+
import java.util.regex.Matcher;
29+
import java.util.regex.Pattern;
2830

2931
import javax.annotation.concurrent.GuardedBy;
3032

33+
import com.ibm.etcd.client.EtcdClient;
3134
import org.slf4j.Logger;
3235
import org.slf4j.LoggerFactory;
3336

@@ -50,15 +53,15 @@
5053
import io.grpc.stub.StreamObserver;
5154

5255
/**
53-
*
56+
*
5457
*/
5558
public final class EtcdWatchClient implements Closeable {
5659

5760
private static final Logger logger = LoggerFactory.getLogger(EtcdWatchClient.class);
5861

5962
private static final Exception CANCEL_EXCEPTION = new CancellationException();
6063

61-
private static final String UNAUTH_REASON_PREFIX = "rpc error: code = PermissionDenied";
64+
private static final Pattern CANCEL_REASON_PATT = Pattern.compile("rpc error: code = (\\w+) desc = (.*)");
6265

6366
private static final MethodDescriptor<WatchRequest,WatchResponse> METHOD_WATCH =
6467
WatchGrpc.getWatchMethod();
@@ -109,7 +112,7 @@ final class WatcherRecord {
109112
long upToRevision;
110113
long watchId = -2L; // -2 only pre-creation, >= -1 after
111114
boolean lastCreateFailedAuth;
112-
115+
113116
boolean userCancelled, finished;
114117
volatile boolean vUserCancelled;
115118

@@ -176,9 +179,8 @@ public void processWatchEvents(final WatchResponse wr) {
176179
public boolean processCreatedResponse(WatchResponse wr, boolean cancelled) {
177180
long newWatchId = wr.getWatchId();
178181
if (cancelled || newWatchId == -1L) {
179-
String reason = wr.getCancelReason();
180-
if (reason != null && reason.startsWith(UNAUTH_REASON_PREFIX)
181-
&& !lastCreateFailedAuth) {
182+
String cancelReason = wr.getCancelReason();
183+
if (!lastCreateFailedAuth && reauthRequired(cancelReason)) {
182184
// If watch creation fails due to an authentication issue (likely
183185
// expired), we trigger a reauth+refresh of the stream by sending
184186
// an UNAUTHENTICATED error. This watch must first be re-created
@@ -190,7 +192,7 @@ public boolean processCreatedResponse(WatchResponse wr, boolean cancelled) {
190192
if (reqStream != null) {
191193
lastCreateFailedAuth = true;
192194
reqStream.onError(Code.UNAUTHENTICATED
193-
.toStatus().withDescription(reason).asException());
195+
.toStatus().withDescription(cancelReason).asException());
194196
}
195197
}
196198
}
@@ -284,6 +286,18 @@ private void completeCreateFuture(boolean created, Exception error) {
284286
}
285287
}
286288

289+
static boolean reauthRequired(String cancelReason) {
290+
if (cancelReason != null) {
291+
Matcher m = CANCEL_REASON_PATT.matcher(cancelReason);
292+
if (m.matches()) {
293+
Code code = GrpcClient.parseGoCodeString(m.group(1));
294+
String message = m.group(2);
295+
return EtcdClient.reauthRequired(code, message);
296+
}
297+
}
298+
return false;
299+
}
300+
287301
// @Override
288302
public Watch watch(WatchCreateRequest createReq, StreamObserver<WatchUpdate> observer) {
289303
return watch(createReq, observer, null);
@@ -559,4 +573,4 @@ public void close() {
559573
}
560574
});
561575
}
562-
}
576+
}

src/test/java/com/ibm/etcd/client/EtcdTestSuite.java

Lines changed: 52 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -19,9 +19,12 @@
1919
import java.io.IOException;
2020
import java.io.InputStreamReader;
2121
import java.io.Reader;
22+
import java.nio.file.Files;
23+
import java.nio.file.Path;
2224
import java.util.ArrayList;
2325
import java.util.Arrays;
2426
import java.util.List;
27+
import java.util.UUID;
2528
import java.util.concurrent.ExecutorService;
2629
import java.util.concurrent.Executors;
2730
import java.util.concurrent.TimeUnit;
@@ -50,31 +53,55 @@
5053
})
5154
public class EtcdTestSuite {
5255

53-
static Process etcdProcess, etcdTlsProcess, etcdTlsCaProcess;
56+
static Process etcdProcess, etcdTlsProcess, etcdTlsCaProcess, etcdJwtProcess;
5457

5558
static final String etcdCommand;
59+
static final String etcdctlCommand;
5660
static {
5761
String etcd = System.getenv("ETCD_CMD");
5862
etcdCommand = etcd != null ? etcd : "etcd";
63+
String etcdctl = System.getenv("ETCDCTL_CMD");
64+
etcdctlCommand = etcdctl != null ? etcdctl : "etcdctl";
5965
}
6066

6167
static final String clientKey = EtcdTestSuite.class.getResource("/client.key").getFile();
6268
static final String clientCert = EtcdTestSuite.class.getResource("/client.crt").getFile();
6369
static final String serverKey = EtcdTestSuite.class.getResource("/server.key").getFile();
6470
static final String serverCert = EtcdTestSuite.class.getResource("/server.crt").getFile();
71+
static final String jwtKey = EtcdTestSuite.class.getResource("/jwt_RS256.key").getFile(); // openssl genrsa -out jwt_RS256.key 4096
72+
static final String jwtPub = EtcdTestSuite.class.getResource("/jwt_RS256.pub").getFile(); // openssl rsa -in jwt_RS256.key -pubout > jwt_RS256.pub
73+
74+
static final String userName = "root";
75+
static final String userPwd = UUID.randomUUID().toString();
6576

6677
@BeforeClass
6778
public static void setUp() throws Exception {
6879
etcdProcess = startProcess();
80+
6981
etcdTlsProcess = startProcess("--cert-file=" + serverCert,
7082
"--key-file=" + serverKey, "--listen-client-urls=https://localhost:2360",
7183
"--listen-peer-urls=http://localhost:2361",
7284
"--advertise-client-urls=https://localhost:2360", "--name=tls");
73-
etcdTlsCaProcess = null; startProcess("--cert-file=" + serverCert,
85+
86+
etcdTlsCaProcess = startProcess("--cert-file=" + serverCert,
7487
"--key-file=" + serverKey, "--listen-client-urls=https://localhost:2362",
7588
"--listen-peer-urls=http://localhost:2363",
7689
"--advertise-client-urls=https://localhost:2362", "--name=tls-ca",
7790
"--trusted-ca-file=" + clientCert, "--client-cert-auth");
91+
92+
Path tmpDir = Files.createTempDirectory(null);
93+
tmpDir.toFile().deleteOnExit();
94+
etcdJwtProcess = startProcess("--auth-token=jwt,pub-key=" + jwtPub + ",priv-key=" + jwtKey + ",sign-method=RS256,ttl=4s",
95+
"--listen-client-urls=http://localhost:2365",
96+
"--listen-peer-urls=http://localhost:2364",
97+
"--advertise-client-urls=http://localhost:2365",
98+
"--data-dir=" + tmpDir);
99+
executeCtlCommand("--endpoints=localhost:2365",
100+
"user", "add", userName, "--new-user-password", userPwd);
101+
executeCtlCommand("--endpoints=localhost:2365",
102+
"user", "grant-role", userName, "root");
103+
executeCtlCommand("--endpoints=localhost:2365",
104+
"auth", "enable");
78105
}
79106

80107
private static Process startProcess(String... cmdline) throws Exception {
@@ -98,11 +125,34 @@ private static Process startProcess(String... cmdline) throws Exception {
98125
}
99126
}
100127

128+
private static void executeCtlCommand(String... cmdline) throws Exception {
129+
boolean ok = false;
130+
Process p = null;
131+
try {
132+
List<String> cmd = new ArrayList<>();
133+
cmd.add(etcdctlCommand);
134+
cmd.addAll(Arrays.asList(cmdline));
135+
p = new ProcessBuilder(cmd)
136+
.redirectErrorStream(true).start();
137+
waitForStartup(p);
138+
p.waitFor(30L, TimeUnit.SECONDS);
139+
System.out.println("etcdctl exit value:" + p.exitValue());
140+
ok = true;
141+
} catch (IOException e) {
142+
System.out.println("Failed to execute etcdctl: " + e);
143+
} finally {
144+
if (!ok) {
145+
tearDown(p);
146+
}
147+
}
148+
}
149+
101150
@AfterClass
102151
public static void tearDown() {
103152
tearDown(etcdProcess);
104153
tearDown(etcdTlsProcess);
105154
tearDown(etcdTlsCaProcess);
155+
tearDown(etcdJwtProcess);
106156
}
107157

108158
public static void tearDown(Process process) {

src/test/java/com/ibm/etcd/client/JsonConfigTest.java

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -109,6 +109,7 @@ public void testLifecycle() throws Exception {
109109

110110
EtcdClient c4 = config.getClient();
111111
assertNotSame(c1, c4);
112+
c4.close();
112113
}
113114

114115
void runBasicTests(EtcdClusterConfig config) throws Exception {

0 commit comments

Comments
 (0)