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.

Sign up for GitHub

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

flipped a flag 'incompatible_tls_enabled_removed' #9062

Closed
wants to merge 3 commits into from
Closed

flipped a flag 'incompatible_tls_enabled_removed' #9062

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
a8e636e
flipped a flag 'incompatible_tls_enabled_removed'
Aug 2, 2019
e712907
removed tls_enabled
Aug 2, 2019
c9a89de
deprecated and made no-op both --tls_enabled and --incompatible_tls_e…
Aug 5, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 1 addition & 14 deletions src/main/java/com/google/devtools/build/lib/authandtls/AuthAndTLSOptions.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -61,22 +61,9 @@ public class AuthAndTLSOptions extends OptionsBase {
)
public String googleCredentials;

@Deprecated
@Option(
name = "tls_enabled",
defaultValue = "false",
documentationCategory = OptionDocumentationCategory.UNCATEGORIZED,
deprecationWarning =
"Deprecated. Please specify a valid protocol in the URL (https or grpcs).",
effectTags = {OptionEffectTag.UNKNOWN},
help =
"DEPRECATED. Specifies whether to use TLS for remote execution/caching and "
+ "the build event service (BES). See #8061 for details.")
public boolean tlsEnabled;

@Option(
name = "incompatible_tls_enabled_removed",
defaultValue = "false",
defaultValue = "true",
documentationCategory = OptionDocumentationCategory.UNCATEGORIZED,
effectTags = {OptionEffectTag.UNKNOWN},
metadataTags = {
Expand Down
15 changes: 4 additions & 11 deletions src/main/java/com/google/devtools/build/lib/authandtls/GoogleAuthUtils.java
View file
Open in desktop
Original file line number Diff line number Diff line change
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;
// '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
View file
Open in desktop
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