[7.1.0] [credentialhelper] Respect expires field from helper (#21429)

This was recently specified in EngFlow/credential-helper-spec#2. RELNOTES[NEW]: Bazel now respects `expires` from Credential Helpers. Closes #21249. PiperOrigin-RevId: 608208538 Change-Id: Id168f654093c7491a40364e3988af66ad1767443 Co-authored-by: Yannic Bonenberger <contact@yannic-bonenberger.com>
bazelbuild · Feb 20, 2024 · 9229bbb · 9229bbb
1 parent 9fd20ed
commit 9229bbb
Show file tree

Hide file tree

Showing 10 changed files with 188 additions and 34 deletions.
diff --git a/src/main/java/com/google/devtools/build/lib/authandtls/AuthAndTLSOptions.java b/src/main/java/com/google/devtools/build/lib/authandtls/AuthAndTLSOptions.java
@@ -180,10 +180,8 @@ public class AuthAndTLSOptions extends OptionsBase {
       documentationCategory = OptionDocumentationCategory.UNCATEGORIZED,
       effectTags = {OptionEffectTag.UNKNOWN},
       help =
-          "The duration for which credentials supplied by a credential helper are cached.\n\n"
-              + "Invoking with a different value will adjust the lifetime of preexisting entries;"
-              + " pass zero to clear the cache. A clean command always clears the cache, regardless"
-              + " of this flag.")
+          "The default duration for which credentials supplied by a credential helper are cached if"
+              + " the helper does not provide when the credentials expire.")
   public Duration credentialHelperCacheTimeout;
 
   /** One of the values of the `--credential_helper` flag. */

diff --git a/src/main/java/com/google/devtools/build/lib/authandtls/GoogleAuthUtils.java b/src/main/java/com/google/devtools/build/lib/authandtls/GoogleAuthUtils.java
@@ -20,11 +20,10 @@
 import com.google.common.annotations.VisibleForTesting;
 import com.google.common.base.Preconditions;
 import com.google.common.base.Strings;
-import com.google.common.collect.ImmutableList;
-import com.google.common.collect.ImmutableMap;
 import com.google.devtools.build.lib.authandtls.credentialhelper.CredentialHelperCredentials;
 import com.google.devtools.build.lib.authandtls.credentialhelper.CredentialHelperEnvironment;
 import com.google.devtools.build.lib.authandtls.credentialhelper.CredentialHelperProvider;
+import com.google.devtools.build.lib.authandtls.credentialhelper.GetCredentialsResponse;
 import com.google.devtools.build.lib.events.Event;
 import com.google.devtools.build.lib.runtime.CommandLinePathFactory;
 import com.google.devtools.build.lib.vfs.FileSystem;
@@ -252,7 +251,7 @@ public static CallCredentialsProvider newCallCredentialsProvider(@Nullable Crede
    */
   public static Credentials newCredentials(
       CredentialHelperEnvironment credentialHelperEnvironment,
-      Cache<URI, ImmutableMap<String, ImmutableList<String>>> credentialCache,
+      Cache<URI, GetCredentialsResponse> credentialCache,
       CommandLinePathFactory commandLinePathFactory,
       FileSystem fileSystem,
       AuthAndTLSOptions authAndTlsOptions)

diff --git a/src/main/java/com/google/devtools/build/lib/authandtls/credentialhelper/BUILD b/src/main/java/com/google/devtools/build/lib/authandtls/credentialhelper/BUILD
@@ -18,6 +18,7 @@ java_library(
         "SystemMillisTicker.java",
     ],
     deps = [
+        ":credentialhelper",
         "//src/main/java/com/google/devtools/build/lib:runtime",
         "//src/main/java/com/google/devtools/build/lib/authandtls",
         "//src/main/java/com/google/devtools/common/options",

diff --git a/...java/com/google/devtools/build/lib/authandtls/credentialhelper/CredentialCacheExpiry.java b/...java/com/google/devtools/build/lib/authandtls/credentialhelper/CredentialCacheExpiry.java
@@ -0,0 +1,74 @@
+// Copyright 2024 The Bazel Authors. All rights reserved.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//    http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package com.google.devtools.build.lib.authandtls.credentialhelper;
+
+import com.github.benmanes.caffeine.cache.Expiry;
+import com.google.common.base.Preconditions;
+import com.google.errorprone.annotations.CanIgnoreReturnValue;
+import java.net.URI;
+import java.time.Duration;
+import java.time.Instant;
+
+final class CredentialCacheExpiry implements Expiry<URI, GetCredentialsResponse> {
+  private Duration defaultCacheDuration = Duration.ZERO;
+
+  /**
+   * Sets the default cache duration for {@link GetCredentialsResponse}s that don't set {@code
+   * expiry}.
+   */
+  public void setDefaultCacheDuration(Duration duration) {
+    this.defaultCacheDuration = Preconditions.checkNotNull(duration);
+  }
+
+  private Duration getExpirationTime(GetCredentialsResponse response) {
+    Preconditions.checkNotNull(response);
+
+    var expires = response.getExpires();
+    if (expires.isEmpty()) {
+      return defaultCacheDuration;
+    }
+
+    var now = Instant.now();
+    return Duration.between(expires.get(), now);
+  }
+
+  @Override
+  public long expireAfterCreate(URI uri, GetCredentialsResponse response, long currentTime) {
+    Preconditions.checkNotNull(uri);
+    Preconditions.checkNotNull(response);
+
+    return getExpirationTime(response).toNanos();
+  }
+
+  @Override
+  public long expireAfterUpdate(
+      URI uri, GetCredentialsResponse response, long currentTime, long currentDuration) {
+    Preconditions.checkNotNull(uri);
+    Preconditions.checkNotNull(response);
+
+    return getExpirationTime(response).toNanos();
+  }
+
+  @CanIgnoreReturnValue
+  @Override
+  public long expireAfterRead(
+      URI uri, GetCredentialsResponse response, long currentTime, long currentDuration) {
+    Preconditions.checkNotNull(uri);
+    Preconditions.checkNotNull(response);
+
+    // We don't extend the duration on access.
+    return currentDuration;
+  }
+}
diff --git a/...om/google/devtools/build/lib/authandtls/credentialhelper/CredentialHelperCredentials.java b/...om/google/devtools/build/lib/authandtls/credentialhelper/CredentialHelperCredentials.java
@@ -17,7 +17,6 @@
 import com.github.benmanes.caffeine.cache.Cache;
 import com.google.auth.Credentials;
 import com.google.common.base.Preconditions;
-import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableMap;
 import java.io.IOException;
 import java.net.URI;
@@ -33,7 +32,7 @@
 public class CredentialHelperCredentials extends Credentials {
   private final CredentialHelperProvider credentialHelperProvider;
   private final CredentialHelperEnvironment credentialHelperEnvironment;
-  private final Cache<URI, ImmutableMap<String, ImmutableList<String>>> credentialCache;
+  private final Cache<URI, GetCredentialsResponse> credentialCache;
   private final Optional<Credentials> fallbackCredentials;
 
   /** Wraps around an {@link IOException} so we can smuggle it through {@link Cache#get}. */
@@ -53,7 +52,7 @@ IOException getWrapped() {
   public CredentialHelperCredentials(
       CredentialHelperProvider credentialHelperProvider,
       CredentialHelperEnvironment credentialHelperEnvironment,
-      Cache<URI, ImmutableMap<String, ImmutableList<String>>> credentialCache,
+      Cache<URI, GetCredentialsResponse> credentialCache,
       Optional<Credentials> fallbackCredentials) {
     this.credentialHelperProvider = Preconditions.checkNotNull(credentialHelperProvider);
     this.credentialHelperEnvironment = Preconditions.checkNotNull(credentialHelperEnvironment);
@@ -75,14 +74,14 @@ public String getAuthenticationType() {
   public Map<String, List<String>> getRequestMetadata(URI uri) throws IOException {
     Preconditions.checkNotNull(uri);
 
-    ImmutableMap<String, ImmutableList<String>> credentials;
+    GetCredentialsResponse response;
     try {
-      credentials = credentialCache.get(uri, this::getCredentialsFromHelper);
+      response = credentialCache.get(uri, this::getCredentialsFromHelper);
     } catch (WrappedIOException e) {
       throw e.getWrapped();
     }
-    if (credentials != null) {
-      return (Map) credentials;
+    if (response != null) {
+      return (Map) response.getHeaders();
     }
 
     if (fallbackCredentials.isPresent()) {
@@ -93,7 +92,7 @@ public Map<String, List<String>> getRequestMetadata(URI uri) throws IOException
   }
 
   @Nullable
-  private ImmutableMap<String, ImmutableList<String>> getCredentialsFromHelper(URI uri) {
+  private GetCredentialsResponse getCredentialsFromHelper(URI uri) {
     Preconditions.checkNotNull(uri);
 
     Optional<CredentialHelper> maybeCredentialHelper =
@@ -113,7 +112,7 @@ private ImmutableMap<String, ImmutableList<String>> getCredentialsFromHelper(URI
       return null;
     }
 
-    return response.getHeaders();
+    return response;
   }
 
   @Override

diff --git a/...main/java/com/google/devtools/build/lib/authandtls/credentialhelper/CredentialModule.java b/...main/java/com/google/devtools/build/lib/authandtls/credentialhelper/CredentialModule.java
@@ -17,24 +17,20 @@
 import com.github.benmanes.caffeine.cache.Cache;
 import com.github.benmanes.caffeine.cache.Caffeine;
 import com.google.common.collect.ImmutableList;
-import com.google.common.collect.ImmutableMap;
 import com.google.devtools.build.lib.authandtls.AuthAndTLSOptions;
 import com.google.devtools.build.lib.runtime.BlazeModule;
 import com.google.devtools.build.lib.runtime.CommandEnvironment;
 import com.google.devtools.common.options.OptionsBase;
 import java.net.URI;
-import java.time.Duration;
 
 /** A module whose sole purpose is to hold the credential cache which is shared by other modules. */
 public class CredentialModule extends BlazeModule {
-  private final Cache<URI, ImmutableMap<String, ImmutableList<String>>> credentialCache =
-      Caffeine.newBuilder()
-          .expireAfterWrite(Duration.ZERO)
-          .ticker(SystemMillisTicker.INSTANCE)
-          .build();
+  private final CredentialCacheExpiry cacheExpiry = new CredentialCacheExpiry();
+  private final Cache<URI, GetCredentialsResponse> credentialCache =
+      Caffeine.newBuilder().ticker(SystemMillisTicker.INSTANCE).expireAfter(cacheExpiry).build();
 
   /** Returns the credential cache. */
-  public Cache<URI, ImmutableMap<String, ImmutableList<String>>> getCredentialCache() {
+  public Cache<URI, GetCredentialsResponse> getCredentialCache() {
     return credentialCache;
   }
 
@@ -47,11 +43,7 @@ public Iterable<Class<? extends OptionsBase>> getCommonCommandOptions() {
   public void beforeCommand(CommandEnvironment env) {
     // Update the cache expiration policy according to the command options.
     AuthAndTLSOptions authAndTlsOptions = env.getOptions().getOptions(AuthAndTLSOptions.class);
-    credentialCache
-        .policy()
-        .expireAfterWrite()
-        .get()
-        .setExpiresAfter(authAndTlsOptions.credentialHelperCacheTimeout);
+    cacheExpiry.setDefaultCacheDuration(authAndTlsOptions.credentialHelperCacheTimeout);
 
     // Clear the cache on clean.
     if (env.getCommand().name().equals("clean")) {

diff --git a/...ava/com/google/devtools/build/lib/authandtls/credentialhelper/GetCredentialsResponse.java b/...ava/com/google/devtools/build/lib/authandtls/credentialhelper/GetCredentialsResponse.java
@@ -26,22 +26,40 @@
 import com.google.gson.stream.JsonToken;
 import com.google.gson.stream.JsonWriter;
 import java.io.IOException;
+import java.time.DateTimeException;
+import java.time.Instant;
+import java.time.ZoneId;
+import java.time.ZoneOffset;
+import java.time.format.DateTimeFormatter;
+import java.time.format.ResolverStyle;
 import java.util.Locale;
 import java.util.Map;
+import java.util.Optional;
 
 /**
  * Response from the {@code get} command of the <a
  * href="https://github.com/bazelbuild/proposals/blob/main/designs/2022-06-07-bazel-credential-helpers.md#proposal">Credential
  * Helper Protocol</a>.
+ *
+ * <p>See the <a
+ * href="https://github.com/EngFlow/credential-helper-spec/blob/main/schemas/get-credentials-response.schema.json">specification</a>.
  */
 @AutoValue
 @AutoValue.CopyAnnotations
 @Immutable
 @JsonAdapter(GetCredentialsResponse.GsonTypeAdapter.class)
 public abstract class GetCredentialsResponse {
+  public static final DateTimeFormatter RFC_3339_FORMATTER =
+      DateTimeFormatter.ofPattern("yyyy-MM-dd'T'HH:mm:ssXXX")
+          .withZone(ZoneId.from(ZoneOffset.UTC))
+          .withResolverStyle(ResolverStyle.LENIENT);
+
   /** Returns the headers to attach to the request. */
   public abstract ImmutableMap<String, ImmutableList<String>> getHeaders();
 
+  /** Returns the time the credentials expire and must be revalidated. */
+  public abstract Optional<Instant> getExpires();
+
   /** Returns a new builder for {@link GetCredentialsRequest}. */
   public static Builder newBuilder() {
     return new AutoValue_GetCredentialsResponse.Builder();
@@ -52,6 +70,8 @@ public static Builder newBuilder() {
   public abstract static class Builder {
     public abstract ImmutableMap.Builder<String, ImmutableList<String>> headersBuilder();
 
+    public abstract Builder setExpires(Instant instant);
+
     /** Returns the newly constructed {@link GetCredentialsResponse}. */
     public abstract GetCredentialsResponse build();
   }
@@ -80,6 +100,13 @@ public void write(JsonWriter writer, GetCredentialsResponse response) throws IOE
         }
         writer.endObject();
       }
+
+      var expires = response.getExpires();
+      if (expires.isPresent()) {
+        writer.name("expires");
+        writer.value(RFC_3339_FORMATTER.format(expires.get()));
+      }
+
       writer.endObject();
     }
 
@@ -141,6 +168,25 @@ public GetCredentialsResponse read(JsonReader reader) throws IOException {
             reader.endObject();
             break;
 
+          case "expires":
+            if (reader.peek() != JsonToken.STRING) {
+              throw new JsonSyntaxException(
+                  String.format(
+                      Locale.US,
+                      "Expected value of 'expires' to be a string, got %s",
+                      reader.peek()));
+            }
+            try {
+              response.setExpires(Instant.from(RFC_3339_FORMATTER.parse(reader.nextString())));
+            } catch (DateTimeException e) {
+              throw new JsonSyntaxException(
+                  String.format(
+                      Locale.US,
+                      "Expected value of 'expires' to be a RFC 3339 formatted timestamp: %s",
+                      e.getMessage()));
+            }
+            break;
+
           default:
             // We intentionally ignore unknown keys to achieve forward compatibility with responses
             // coming from newer tools.

diff --git a/src/main/java/com/google/devtools/build/lib/remote/RemoteModule.java b/src/main/java/com/google/devtools/build/lib/remote/RemoteModule.java
@@ -25,7 +25,6 @@
 import com.google.common.base.Strings;
 import com.google.common.base.Throwables;
 import com.google.common.collect.ImmutableList;
-import com.google.common.collect.ImmutableMap;
 import com.google.common.collect.ImmutableSet;
 import com.google.common.util.concurrent.ListeningScheduledExecutorService;
 import com.google.common.util.concurrent.MoreExecutors;
@@ -44,6 +43,7 @@
 import com.google.devtools.build.lib.authandtls.GoogleAuthUtils;
 import com.google.devtools.build.lib.authandtls.credentialhelper.CredentialHelperEnvironment;
 import com.google.devtools.build.lib.authandtls.credentialhelper.CredentialModule;
+import com.google.devtools.build.lib.authandtls.credentialhelper.GetCredentialsResponse;
 import com.google.devtools.build.lib.bazel.repository.downloader.Downloader;
 import com.google.devtools.build.lib.bazel.repository.downloader.HttpDownloader;
 import com.google.devtools.build.lib.buildeventstream.BuildEventArtifactUploader;
@@ -1100,7 +1100,7 @@ RemoteActionContextProvider getActionContextProvider() {
   @VisibleForTesting
   static Credentials createCredentials(
       CredentialHelperEnvironment credentialHelperEnvironment,
-      Cache<URI, ImmutableMap<String, ImmutableList<String>>> credentialCache,
+      Cache<URI, GetCredentialsResponse> credentialCache,
       CommandLinePathFactory commandLinePathFactory,
       FileSystem fileSystem,
       AuthAndTLSOptions authAndTlsOptions,