Skip to content

Commit be8769b

Browse files
jrhee17jrhee17
committed
address comments by ai
1 parent 167d50d commit be8769b

File tree

6 files changed

+43
-35
lines changed

6 files changed

+43
-35
lines changed

‎it/xds-client/src/test/java/com/linecorp/armeria/xds/it/DataSourceTest.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,9 +20,9 @@
2020
import static org.awaitility.Awaitility.await;
2121

2222
import java.io.File;
23-
import java.time.Duration;
2423
import java.nio.file.Files;
2524
import java.nio.file.StandardCopyOption;
25+
import java.time.Duration;
2626
import java.util.Base64;
2727
import java.util.concurrent.atomic.AtomicLong;
2828
import java.util.concurrent.atomic.AtomicReference;

‎it/xds-client/src/test/java/com/linecorp/armeria/xds/it/DynamicSecretTest.java‎

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -419,8 +419,7 @@ void sdsTlsCertificateOnly() throws Exception {
419419
assertThat(certSnapshot).isNotNull();
420420
assertThat(certSnapshot.tlsKeyPair()).isEqualTo(certificate1.tlsKeyPair());
421421
final CertificateValidationContextSnapshot validationContext = tlsSnapshot.validationContext();
422-
assertThat(validationContext).isNotNull();
423-
assertThat(validationContext.trustedCa()).isNull();
422+
assertThat(validationContext).isNull();
424423
}
425424
}
426425

@@ -521,8 +520,7 @@ void sdsValidationContextOnly() throws Exception {
521520
tlsSnapshot = listenerSnapshot.routeSnapshot().virtualHostSnapshots().get(0)
522521
.routeEntries().get(0).clusterSnapshot().transportSocket();
523522
final TlsCertificateSnapshot certSnapshot = tlsSnapshot.tlsCertificate();
524-
assertThat(certSnapshot).isNotNull();
525-
assertThat(certSnapshot.tlsKeyPair()).isNull();
523+
assertThat(certSnapshot).isNull();
526524
final CertificateValidationContextSnapshot validationContext = tlsSnapshot.validationContext();
527525
assertThat(validationContext).isNotNull();
528526
assertThat(validationContext.trustedCa()).containsExactly(certificate2.certificate());

‎it/xds-client/src/test/java/com/linecorp/armeria/xds/it/XdsResourceReader.java‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -69,6 +69,7 @@ public static <T extends GeneratedMessage> T fromYaml(String yaml, Class<T> claz
6969
.addEscape('\\', "\\\\")
7070
.addEscape('"', "\\\"")
7171
.addEscape('\n', "\\n")
72+
.addEscape('\r', "\\r")
7273
.build();
7374

7475
static String escapeMultiLine(String str) {

‎xds/src/main/java/com/linecorp/armeria/xds/DataSourceStream.java‎

Lines changed: 12 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -57,11 +57,13 @@ protected Subscription onStart(SnapshotWatcher<Optional<ByteString>> watcher) {
5757
}
5858
if (dataSource.hasInlineBytes()) {
5959
final byte[] bytes = dataSource.getInlineBytes().toByteArray();
60-
watcher.onUpdate(Optional.of(ByteString.copyFrom(bytes)), null);
61-
} else if (dataSource.hasInlineString()) {
60+
return SnapshotStream.just(Optional.of(ByteString.copyFrom(bytes))).subscribe(watcher);
61+
}
62+
if (dataSource.hasInlineString()) {
6263
final byte[] bytes = dataSource.getInlineString().getBytes(StandardCharsets.UTF_8);
63-
watcher.onUpdate(Optional.of(ByteString.copyFrom(bytes)), null);
64-
} else if (dataSource.hasEnvironmentVariable()) {
64+
return SnapshotStream.just(Optional.of(ByteString.copyFrom(bytes))).subscribe(watcher);
65+
}
66+
if (dataSource.hasEnvironmentVariable()) {
6567
final String envVar = dataSource.getEnvironmentVariable();
6668
final String envVarValue = System.getenv(envVar);
6769
if (envVarValue == null) {
@@ -70,8 +72,9 @@ protected Subscription onStart(SnapshotWatcher<Optional<ByteString>> watcher) {
7072
return SnapshotStream.<Optional<ByteString>>error(e).subscribe(watcher);
7173
}
7274
final byte[] bytes = envVarValue.getBytes(StandardCharsets.UTF_8);
73-
watcher.onUpdate(Optional.of(ByteString.copyFrom(bytes)), null);
74-
} else if (dataSource.hasFilename()) {
75+
return SnapshotStream.just(Optional.of(ByteString.copyFrom(bytes))).subscribe(watcher);
76+
}
77+
if (dataSource.hasFilename()) {
7578
final String filename = dataSource.getFilename();
7679
final Path filePath = Paths.get(filename).toAbsolutePath();
7780
Path dirPath = filePath.getParent();
@@ -99,11 +102,9 @@ protected Subscription onStart(SnapshotWatcher<Optional<ByteString>> watcher) {
99102
}
100103
});
101104
};
102-
} else {
103-
final IllegalArgumentException e = new IllegalArgumentException(
104-
String.format("Unexpected data source type '%s'", dataSource.getSpecifierCase()));
105-
return SnapshotStream.<Optional<ByteString>>error(e).subscribe(watcher);
106105
}
107-
return SnapshotStream.<ByteString>empty().subscribe(watcher);
106+
final IllegalArgumentException e = new IllegalArgumentException(
107+
String.format("Unexpected data source type '%s'", dataSource.getSpecifierCase()));
108+
return SnapshotStream.<Optional<ByteString>>error(e).subscribe(watcher);
108109
}
109110
}

‎xds/src/main/java/com/linecorp/armeria/xds/TransportSocketSnapshot.java‎

Lines changed: 9 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,8 @@
1616

1717
package com.linecorp.armeria.xds;
1818

19+
import java.util.Optional;
20+
1921
import com.linecorp.armeria.common.annotation.Nullable;
2022
import com.linecorp.armeria.common.annotation.UnstableApi;
2123

@@ -36,15 +38,17 @@ public final class TransportSocketSnapshot implements Snapshot<TransportSocket>
3638
private final CertificateValidationContextSnapshot validationContext;
3739

3840
TransportSocketSnapshot(TransportSocket transportSocket) {
39-
this(transportSocket, null, null);
41+
this.transportSocket = transportSocket;
42+
tlsCertificate = null;
43+
validationContext = null;
4044
}
4145

4246
TransportSocketSnapshot(TransportSocket transportSocket,
43-
@Nullable TlsCertificateSnapshot tlsCertificate,
44-
@Nullable CertificateValidationContextSnapshot validationContext) {
47+
Optional<TlsCertificateSnapshot> tlsCertificate,
48+
Optional<CertificateValidationContextSnapshot> validationContext) {
4549
this.transportSocket = transportSocket;
46-
this.tlsCertificate = tlsCertificate;
47-
this.validationContext = validationContext;
50+
this.tlsCertificate = tlsCertificate.orElse(null);
51+
this.validationContext = validationContext.orElse(null);
4852
}
4953

5054
@Override

‎xds/src/main/java/com/linecorp/armeria/xds/TransportSocketStream.java‎

Lines changed: 18 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -16,11 +16,12 @@
1616

1717
package com.linecorp.armeria.xds;
1818

19+
import java.util.Optional;
20+
1921
import com.linecorp.armeria.common.annotation.Nullable;
2022

2123
import io.envoyproxy.envoy.config.core.v3.ConfigSource;
2224
import io.envoyproxy.envoy.config.core.v3.TransportSocket;
23-
import io.envoyproxy.envoy.extensions.transport_sockets.tls.v3.CertificateValidationContext;
2425
import io.envoyproxy.envoy.extensions.transport_sockets.tls.v3.CommonTlsContext;
2526
import io.envoyproxy.envoy.extensions.transport_sockets.tls.v3.CommonTlsContext.CombinedCertificateValidationContext;
2627
import io.envoyproxy.envoy.extensions.transport_sockets.tls.v3.SdsSecretConfig;
@@ -52,47 +53,50 @@ protected Subscription onStart(SnapshotWatcher<TransportSocketSnapshot> watcher)
5253
UpstreamTlsContext.class);
5354
final CommonTlsContext commonTlsContext = tlsContext.getCommonTlsContext();
5455

55-
final SnapshotStream<CertificateValidationContextSnapshot> validationStream;
56+
final SnapshotStream<Optional<CertificateValidationContextSnapshot>> validationStream;
5657

5758
if (commonTlsContext.hasValidationContext()) {
5859
final Secret secret = Secret.newBuilder()
5960
.setValidationContext(commonTlsContext.getValidationContext())
6061
.build();
6162
final SecretStream secretStream = new SecretStream(secret, context);
62-
validationStream = secretStream.switchMap(
63-
resource -> new CertificateValidationContextStream(context, resource));
63+
validationStream = secretStream
64+
.switchMap(resource -> new CertificateValidationContextStream(context, resource))
65+
.map(Optional::of);
6466
} else if (commonTlsContext.hasValidationContextSdsSecretConfig()) {
6567
final SdsSecretConfig sdsConfig = commonTlsContext.getValidationContextSdsSecretConfig();
6668
final SecretStream secretStream = new SecretStream(sdsConfig, configSource, context);
67-
validationStream = secretStream.switchMap(
68-
resource -> new CertificateValidationContextStream(context, resource));
69+
validationStream = secretStream
70+
.switchMap(resource -> new CertificateValidationContextStream(context, resource))
71+
.map(Optional::of);
6972
} else if (commonTlsContext.hasCombinedValidationContext()) {
7073
final CombinedCertificateValidationContext combined =
7174
commonTlsContext.getCombinedValidationContext();
7275
final SdsSecretConfig sdsConfig = combined.getValidationContextSdsSecretConfig();
7376
final SecretStream secretStream = new SecretStream(sdsConfig, configSource, context);
7477
validationStream = secretStream.switchMap(resource -> new CertificateValidationContextStream(
75-
context, resource, combined.getDefaultValidationContext()));
78+
context, resource, combined.getDefaultValidationContext()))
79+
.map(Optional::of);
7680
} else {
77-
validationStream = SnapshotStream.just(new CertificateValidationContextSnapshot(
78-
CertificateValidationContext.getDefaultInstance()));
81+
validationStream = SnapshotStream.empty();
7982
}
8083

81-
final SnapshotStream<TlsCertificateSnapshot> tlsCertStream;
84+
final SnapshotStream<Optional<TlsCertificateSnapshot>> tlsCertStream;
8285
if (!commonTlsContext.getTlsCertificatesList().isEmpty()) {
8386
final TlsCertificate tlsCertificate = commonTlsContext.getTlsCertificatesList().get(0);
8487
final Secret secret = Secret.newBuilder().setTlsCertificate(tlsCertificate).build();
8588
final SecretStream secretStream = new SecretStream(secret, context);
86-
tlsCertStream = secretStream.switchMap(resource -> new TlsCertificateStream(context, resource));
89+
tlsCertStream = secretStream.switchMap(resource -> new TlsCertificateStream(context, resource))
90+
.map(Optional::of);
8791
} else if (!commonTlsContext.getTlsCertificateSdsSecretConfigsList().isEmpty()) {
8892
final SdsSecretConfig sdsConfig =
8993
commonTlsContext.getTlsCertificateSdsSecretConfigsList().get(0);
9094
final SecretStream secretStream = new SecretStream(sdsConfig, configSource, context);
91-
tlsCertStream = secretStream.switchMap(resource -> new TlsCertificateStream(context, resource));
95+
tlsCertStream = secretStream.switchMap(resource -> new TlsCertificateStream(context, resource))
96+
.map(Optional::of);
9297
} else {
9398
// static
94-
tlsCertStream = SnapshotStream.just(
95-
new TlsCertificateSnapshot(TlsCertificate.getDefaultInstance(), null));
99+
tlsCertStream = SnapshotStream.empty();
96100
}
97101

98102
final SnapshotStream<TransportSocketSnapshot> stream =

0 commit comments

Comments
 (0)