Skip to content

Commit

Permalink
flipped a flag 'incompatible_tls_enabled_removed'
Browse files Browse the repository at this point in the history
Set `--incompatible_tls_enabled_removed` to true by default.

RELNOTES: by default all remote connections considered to be via `gRPC` with TLS enabled, unless other specified. To disable TLS use `grpc://` prefix for you endpoints. All remote connections via `gRPC` affected - `--remote_cache`, `--remote_executor` or `--bes_backend`. http cache/executor is not affected. See #8061 for details.

Fixes #8061.

Closes #9062.

PiperOrigin-RevId: 262124742
  • Loading branch information
ishikhman authored and copybara-github committed Aug 7, 2019
1 parent a300ae8 commit d2de8ee
Show file tree
Hide file tree
Showing 3 changed files with 17 additions and 66 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -66,26 +66,23 @@ public class AuthAndTLSOptions extends OptionsBase {
name = "tls_enabled",
defaultValue = "false",
documentationCategory = OptionDocumentationCategory.UNCATEGORIZED,
deprecationWarning =
"Deprecated. Please specify a valid protocol in the URL (https or grpcs).",
deprecationWarning = "No-op. See #8061 for details.",
effectTags = {OptionEffectTag.UNKNOWN},
help =
"DEPRECATED. Specifies whether to use TLS for remote execution/caching and "
+ "the build event service (BES). See #8061 for details.")
help = "No-op. See #8061 for details.")
public boolean tlsEnabled;

@Deprecated
@Option(
name = "incompatible_tls_enabled_removed",
defaultValue = "false",
defaultValue = "true",
documentationCategory = OptionDocumentationCategory.UNCATEGORIZED,
deprecationWarning = "No-op. See #8061 for details.",
effectTags = {OptionEffectTag.UNKNOWN},
metadataTags = {
OptionMetadataTag.INCOMPATIBLE_CHANGE,
OptionMetadataTag.TRIGGERED_BY_ALL_INCOMPATIBLE_CHANGES
},
help =
"If set to true, bazel will handle --tls_enabled as a not existing flag."
+ "See #8061 for details.")
help = "No-op. See #8061 for details.")
public boolean incompatibleTlsEnabledRemoved;

@Option(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -50,15 +50,15 @@ public static ManagedChannel newChannel(String target, AuthAndTLSOptions options
Preconditions.checkNotNull(interceptors);

final SslContext sslContext =
isTlsEnabled(target, options) ? createSSlContext(options.tlsCertificate) : null;
isTlsEnabled(target) ? createSSlContext(options.tlsCertificate) : null;

String targetUrl = convertTargetScheme(target);

try {
NettyChannelBuilder builder =
NettyChannelBuilder.forTarget(targetUrl)
.negotiationType(
isTlsEnabled(target, options) ? NegotiationType.TLS : NegotiationType.PLAINTEXT)
isTlsEnabled(target) ? NegotiationType.TLS : NegotiationType.PLAINTEXT)
.defaultLoadBalancingPolicy("round_robin")
.intercept(interceptors);
if (sslContext != null) {
Expand Down Expand Up @@ -86,18 +86,11 @@ private static String convertTargetScheme(String target) {
return target.replace("grpcs://", "").replace("grpc://", "");
}

// TODO(ishikhman) remove options.tlsEnabled flag usage when an incompatible flag is flipped
private static boolean isTlsEnabled(String target, AuthAndTLSOptions options) {
if (options.incompatibleTlsEnabledRemoved && options.tlsEnabled) {
throw new IllegalArgumentException("flag --tls_enabled was not found");
}
if (options.incompatibleTlsEnabledRemoved) {
// 'grpcs://' or empty prefix => TLS-enabled
// when no schema prefix is provided in URL, bazel will treat it as a gRPC request with TLS
// enabled
return !target.startsWith("grpc://");
}
return target.startsWith("grpcs") || options.tlsEnabled;
private static boolean isTlsEnabled(String target) {
// 'grpcs://' or empty prefix => TLS-enabled
// when no schema prefix is provided in URL, bazel will treat it as a gRPC request with TLS
// enabled
return !target.startsWith("grpc://");
}

private static SslContext createSSlContext(@Nullable String rootCert) throws IOException {
Expand Down
47 changes: 4 additions & 43 deletions src/test/shell/bazel/remote/remote_execution_tls_test.sh
Original file line number Diff line number Diff line change
Expand Up @@ -86,16 +86,15 @@ function test_remote_grpcs_cache() {
|| fail "Failed to build //a:foo with grpcs remote cache"
}

function test_remote_grpc_cache_with_legacy_tls_enabled() {
# Test that if default scheme for --remote_cache flag with --tls_enabled, remote cache works.
function test_remote_grpc_cache() {
# Test that if default scheme for --remote_cache flag, remote cache works.
_prepareBasicRule

bazel build \
--remote_cache=localhost:${worker_port} \
--tls_enabled=true \
--tls_certificate="${cert_path}/ca.crt" \
//a:foo \
|| fail "Failed to build //a:foo with grpc --tls_enabled remote cache"
|| fail "Failed to build //a:foo with grpc remote cache"
}

function test_remote_https_cache() {
Expand All @@ -109,50 +108,12 @@ function test_remote_https_cache() {
|| fail "Failed to build //a:foo with https remote cache"
}

function test_remote_cache_with_incompatible_tls_enabled_removed_default_scheme() {
# Test that if default scheme for --remote_cache flag with --incompatible_tls_enabled_removed, remote cache works.
_prepareBasicRule

bazel build \
--remote_cache=localhost:${worker_port} \
--incompatible_tls_enabled_removed=true \
--tls_certificate="${cert_path}/ca.crt" \
//a:foo \
|| fail "Failed to build //a:foo with default(grpcs) remote cache"
}

function test_remote_cache_with_incompatible_tls_enabled_removed_grpcs_scheme() {
# Test that if 'grpcs' scheme for --remote_cache flag with --incompatible_tls_enabled_removed, remote cache works.
_prepareBasicRule

bazel build \
--remote_cache=grpcs://localhost:${worker_port} \
--incompatible_tls_enabled_removed=true \
--tls_certificate="${cert_path}/ca.crt" \
//a:foo \
|| fail "Failed to build //a:foo with grpcs remote cache"
}

function test_remote_cache_with_incompatible_tls_enabled_removed_grpc_scheme() {
# Test that if 'grpc' scheme for --remote_cache flag with --incompatible_tls_enabled_removed, remote cache fails.
_prepareBasicRule

bazel build \
--remote_cache=grpc://localhost:${worker_port} \
--incompatible_tls_enabled_removed=true \
--tls_certificate="${cert_path}/ca.crt" \
//a:foo \
&& fail "Expected test failure" || true
}

function test_remote_cache_with_incompatible_tls_enabled_removed() {
# Test that if --incompatible_tls_enabled_removed=true and --tls_enabled=true an error is thrown
# Test that if 'grpc' scheme for --remote_cache flag, remote cache fails.
_prepareBasicRule

bazel build \
--remote_cache=grpc://localhost:${worker_port} \
--tls_enabled=true \
--incompatible_tls_enabled_removed=true \
--tls_certificate="${cert_path}/ca.crt" \
//a:foo \
&& fail "Expected test failure" || true
Expand Down

0 comments on commit d2de8ee

Please sign in to comment.