Skip to content

Commit

Permalink
Declare credential helpers to be a stable feature. (#18752)
Browse files Browse the repository at this point in the history
Closes #18615.

PiperOrigin-RevId: 539607278
Change-Id: I250250452db923ba132f1445c46b0112b175e505

Co-authored-by: Tiago Quelhas <tjgq@google.com>
  • Loading branch information
iancha1992 and tjgq committed Jun 23, 2023
1 parent b752c2f commit 351188d
Show file tree
Hide file tree
Showing 4 changed files with 21 additions and 18 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -141,7 +141,8 @@ public class AuthAndTLSOptions extends OptionsBase {
public Duration grpcKeepaliveTimeout;

@Option(
name = "experimental_credential_helper",
name = "credential_helper",
oldName = "experimental_credential_helper",
defaultValue = "null",
allowMultiple = true,
converter = UnresolvedScopedCredentialHelperConverter.class,
Expand All @@ -160,7 +161,8 @@ public class AuthAndTLSOptions extends OptionsBase {
public List<UnresolvedScopedCredentialHelper> credentialHelpers;

@Option(
name = "experimental_credential_helper_timeout",
name = "credential_helper_timeout",
oldName = "experimental_credential_helper_timeout",
defaultValue = "10s",
converter = DurationConverter.class,
documentationCategory = OptionDocumentationCategory.UNCATEGORIZED,
Expand All @@ -172,7 +174,8 @@ public class AuthAndTLSOptions extends OptionsBase {
public Duration credentialHelperTimeout;

@Option(
name = "experimental_credential_helper_cache_duration",
name = "credential_helper_cache_duration",
oldName = "experimental_credential_helper_cache_duration",
defaultValue = "30m",
converter = DurationConverter.class,
documentationCategory = OptionDocumentationCategory.UNCATEGORIZED,
Expand All @@ -184,7 +187,7 @@ public class AuthAndTLSOptions extends OptionsBase {
+ " of this flag.")
public Duration credentialHelperCacheTimeout;

/** One of the values of the `--experimental_credential_helper` flag. */
/** One of the values of the `--credential_helper` flag. */
@AutoValue
public abstract static class UnresolvedScopedCredentialHelper {
/** Returns the scope of the credential helper (if any). */
Expand All @@ -194,7 +197,7 @@ public abstract static class UnresolvedScopedCredentialHelper {
public abstract String getPath();
}

/** A {@link Converter} for the `--experimental_credential_helper` flag. */
/** A {@link Converter} for the `--credential_helper` flag. */
public static final class UnresolvedScopedCredentialHelperConverter
extends Converter.Contextless<UnresolvedScopedCredentialHelper> {
public static final UnresolvedScopedCredentialHelperConverter INSTANCE =
Expand Down
4 changes: 2 additions & 2 deletions src/test/py/bazel/bzlmod/bzlmod_credentials_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -125,7 +125,7 @@ def testCredentialsFromHelper(self):
) as static_server:
_, stdout, _ = self.RunBazel([
'run',
'--experimental_credential_helper=%workspace%/credhelper',
'--credential_helper=%workspace%/credhelper',
'--registry=' + static_server.getURL(),
'--registry=https://bcr.bazel.build',
'//:main',
Expand Down Expand Up @@ -156,7 +156,7 @@ def testCredentialsFromHelperOverrideNetrc(self):
_, stdout, _ = self.RunBazel(
[
'run',
'--experimental_credential_helper=%workspace%/credhelper',
'--credential_helper=%workspace%/credhelper',
'--registry=' + static_server.getURL(),
'--registry=https://bcr.bazel.build',
'//:main',
Expand Down
12 changes: 6 additions & 6 deletions src/test/shell/bazel/remote/remote_execution_test.sh
Original file line number Diff line number Diff line change
Expand Up @@ -108,15 +108,15 @@ function test_credential_helper_remote_cache() {

bazel build \
--remote_cache=grpc://localhost:${worker_port} \
--experimental_credential_helper="${TEST_TMPDIR}/credhelper" \
--credential_helper="${TEST_TMPDIR}/credhelper" \
//a:a >& $TEST_log || fail "Build with credentials should have succeeded"

# First build should have called helper for 4 distinct URIs.
expect_credential_helper_calls 4

bazel build \
--remote_cache=grpc://localhost:${worker_port} \
--experimental_credential_helper="${TEST_TMPDIR}/credhelper" \
--credential_helper="${TEST_TMPDIR}/credhelper" \
//a:b >& $TEST_log || fail "Build with credentials should have succeeded"

# Second build should have hit the credentials cache.
Expand All @@ -138,7 +138,7 @@ function test_credential_helper_remote_execution() {
bazel build \
--spawn_strategy=remote \
--remote_executor=grpc://localhost:${worker_port} \
--experimental_credential_helper="${TEST_TMPDIR}/credhelper" \
--credential_helper="${TEST_TMPDIR}/credhelper" \
//a:a >& $TEST_log || fail "Build with credentials should have succeeded"

# First build should have called helper for 5 distinct URIs.
Expand All @@ -147,7 +147,7 @@ function test_credential_helper_remote_execution() {
bazel build \
--spawn_strategy=remote \
--remote_executor=grpc://localhost:${worker_port} \
--experimental_credential_helper="${TEST_TMPDIR}/credhelper" \
--credential_helper="${TEST_TMPDIR}/credhelper" \
//a:b >& $TEST_log || fail "Build with credentials should have succeeded"

# Second build should have hit the credentials cache.
Expand All @@ -160,7 +160,7 @@ function test_credential_helper_clear_cache() {
bazel build \
--spawn_strategy=remote \
--remote_executor=grpc://localhost:${worker_port} \
--experimental_credential_helper="${TEST_TMPDIR}/credhelper" \
--credential_helper="${TEST_TMPDIR}/credhelper" \
//a:a >& $TEST_log || fail "Build with credentials should have succeeded"

expect_credential_helper_calls 5
Expand All @@ -170,7 +170,7 @@ function test_credential_helper_clear_cache() {
bazel build \
--spawn_strategy=remote \
--remote_executor=grpc://localhost:${worker_port} \
--experimental_credential_helper="${TEST_TMPDIR}/credhelper" \
--credential_helper="${TEST_TMPDIR}/credhelper" \
//a:b >& $TEST_log || fail "Build with credentials should have succeeded"

# Build after clean should have called helper again.
Expand Down
10 changes: 5 additions & 5 deletions src/test/shell/bazel/starlark_repository_test.sh
Original file line number Diff line number Diff line change
Expand Up @@ -1801,12 +1801,12 @@ function test_auth_from_credential_helper() {
bazel build //:it \
&& fail "Expected failure when downloading repo without credential helper"

bazel build --experimental_credential_helper="${TEST_TMPDIR}/credhelper" //:it \
bazel build --credential_helper="${TEST_TMPDIR}/credhelper" //:it \
|| fail "Expected success when downloading repo with credential helper"

expect_credential_helper_calls 1

bazel build --experimental_credential_helper="${TEST_TMPDIR}/credhelper" //:it \
bazel build --credential_helper="${TEST_TMPDIR}/credhelper" //:it \
|| fail "Expected success when downloading repo with credential helper"

expect_credential_helper_calls 1 # expect credentials to have been cached
Expand All @@ -1822,7 +1822,7 @@ function test_auth_from_credential_helper_overrides_starlark() {

setup_auth baduser badpass

bazel build --experimental_credential_helper="${TEST_TMPDIR}/credhelper" //:it \
bazel build --credential_helper="${TEST_TMPDIR}/credhelper" //:it \
|| fail "Expected success when downloading repo with credential helper overriding basic auth"
}

Expand Down Expand Up @@ -2217,7 +2217,7 @@ genrule(
cmd = "cp $< $@",
)
EOF
bazel build --experimental_credential_helper="${TEST_TMPDIR}/credhelper" //:it \
bazel build --credential_helper="${TEST_TMPDIR}/credhelper" //:it \
|| fail "Expected success despite needing a file behind credential helper"
}

Expand Down Expand Up @@ -2264,7 +2264,7 @@ genrule(
cmd = "cp $< $@",
)
EOF
bazel build --experimental_credential_helper="${TEST_TMPDIR}/credhelper" //:it \
bazel build --credential_helper="${TEST_TMPDIR}/credhelper" //:it \
|| fail "Expected success despite needing a file behind credential helper"
}

Expand Down

0 comments on commit 351188d

Please sign in to comment.