From 920c8468fbe3d45bd3c9018c00478dd2343cff7f Mon Sep 17 00:00:00 2001 From: Dee Kryvenko Date: Tue, 4 Oct 2022 15:17:25 -0700 Subject: [PATCH 1/7] Fix token caching for multi-cluster multi-namespace environments --- ...actVaultTokenCredentialWithExpiration.java | 142 ++++++++++++------ ...aultTokenCredentialWithExpirationTest.java | 25 ++- 2 files changed, 123 insertions(+), 44 deletions(-) diff --git a/src/main/java/com/datapipe/jenkins/vault/credentials/AbstractVaultTokenCredentialWithExpiration.java b/src/main/java/com/datapipe/jenkins/vault/credentials/AbstractVaultTokenCredentialWithExpiration.java index f4cbaf5b..e85c767e 100644 --- a/src/main/java/com/datapipe/jenkins/vault/credentials/AbstractVaultTokenCredentialWithExpiration.java +++ b/src/main/java/com/datapipe/jenkins/vault/credentials/AbstractVaultTokenCredentialWithExpiration.java @@ -4,7 +4,11 @@ import com.bettercloud.vault.VaultConfig; import com.bettercloud.vault.VaultException; import com.cloudbees.plugins.credentials.CredentialsScope; +import java.io.Serializable; import java.util.Calendar; +import java.util.Objects; +import java.util.concurrent.ConcurrentHashMap; +import java.util.concurrent.ConcurrentMap; import java.util.logging.Level; import java.util.logging.Logger; @@ -14,8 +18,97 @@ public abstract class AbstractVaultTokenCredentialWithExpiration private final static Logger LOGGER = Logger .getLogger(AbstractVaultTokenCredentialWithExpiration.class.getName()); - private Calendar tokenExpiry; - private String currentClientToken; + public static class CacheKey implements Serializable { + private final transient String vaultUrl; + private final transient String namespace; + + public CacheKey(String vaultUrl, String namespace) { + this.vaultUrl = vaultUrl; + this.namespace = namespace; + } + + @Override + public String toString() { + return String.format("vaultUrl=%s, namespace=%s", vaultUrl, namespace); + } + + @Override + public boolean equals(Object o) { + if (this == o) { + return true; + } + if (!(o instanceof CacheKey)) { + return false; + } + CacheKey cacheKey = (CacheKey) o; + return Objects.equals(vaultUrl, cacheKey.vaultUrl) && Objects.equals( + namespace, cacheKey.namespace); + } + + @Override + public int hashCode() { + return Objects.hash(vaultUrl, namespace); + } + } + + public static class TokenHolder implements Serializable { + private final transient CacheKey key; + private transient Calendar tokenExpiry; + private transient String token; + + public TokenHolder(CacheKey key) { + this.key = key; + } + + public synchronized Vault authorizeWithVault( + AbstractVaultTokenCredentialWithExpiration credentials, VaultConfig config) { + Vault vault = credentials.getVault(config); + if (tokenExpired()) { + token = credentials.getToken(vault); + config.token(token); + setTokenExpiry(vault); + } else { + config.token(token); + } + return vault; + } + + private void setTokenExpiry(Vault vault) { + int tokenTTL = 0; + try { + tokenTTL = (int) vault.auth().lookupSelf().getTTL(); + } catch (VaultException e) { + LOGGER.log(Level.WARNING, "Could not determine token expiration. " + + "Check if token is allowed to access auth/token/lookup-self. " + + "Assuming token TTL expired.", e); + } + tokenExpiry = Calendar.getInstance(); + tokenExpiry.add(Calendar.SECOND, tokenTTL); + } + + private boolean tokenExpired() { + if (tokenExpiry == null) { + return true; + } + + boolean result = true; + Calendar now = Calendar.getInstance(); + long timeDiffInMillis = now.getTimeInMillis() - tokenExpiry.getTimeInMillis(); + if (timeDiffInMillis < -2000L) { + // token will be valid for at least another 2s + result = false; + LOGGER.log(Level.FINE, String.format("Auth token (for key %s) is still valid", key)); + } else { + LOGGER.log(Level.FINE, + String.format("Auth token (for key %s) has to be re-issued (%d)", + key, timeDiffInMillis)); + } + + return result; + } + } + + private final transient ConcurrentMap cache = new ConcurrentHashMap<>(); protected AbstractVaultTokenCredentialWithExpiration(CredentialsScope scope, String id, String description) { @@ -26,50 +119,13 @@ protected AbstractVaultTokenCredentialWithExpiration(CredentialsScope scope, Str @Override public Vault authorizeWithVault(VaultConfig config) { - Vault vault = getVault(config); - if (tokenExpired()) { - currentClientToken = getToken(vault); - config.token(currentClientToken); - setTokenExpiry(vault); - } else { - config.token(currentClientToken); - } - return vault; + CacheKey key = new CacheKey(config.getAddress(), config.getNameSpace()); + cache.putIfAbsent(key, new TokenHolder(key)); + TokenHolder holder = cache.get(key); + return holder.authorizeWithVault(this, config); } protected Vault getVault(VaultConfig config) { return new Vault(config); } - - private void setTokenExpiry(Vault vault) { - int tokenTTL = 0; - try { - tokenTTL = (int) vault.auth().lookupSelf().getTTL(); - } catch (VaultException e) { - LOGGER.log(Level.WARNING, "Could not determine token expiration. " + - "Check if token is allowed to access auth/token/lookup-self. " + - "Assuming token TTL expired.", e); - } - tokenExpiry = Calendar.getInstance(); - tokenExpiry.add(Calendar.SECOND, tokenTTL); - } - - private boolean tokenExpired() { - if (tokenExpiry == null) { - return true; - } - - boolean result = true; - Calendar now = Calendar.getInstance(); - long timeDiffInMillis = now.getTimeInMillis() - tokenExpiry.getTimeInMillis(); - if (timeDiffInMillis < -2000L) { - // token will be valid for at least another 2s - result = false; - LOGGER.log(Level.FINE, "Auth token is still valid"); - } else { - LOGGER.log(Level.FINE, "Auth token has to be re-issued" + timeDiffInMillis); - } - - return result; - } } diff --git a/src/test/java/com/datapipe/jenkins/vault/credentials/AbstractVaultTokenCredentialWithExpirationTest.java b/src/test/java/com/datapipe/jenkins/vault/credentials/AbstractVaultTokenCredentialWithExpirationTest.java index af43f458..04174f34 100644 --- a/src/test/java/com/datapipe/jenkins/vault/credentials/AbstractVaultTokenCredentialWithExpirationTest.java +++ b/src/test/java/com/datapipe/jenkins/vault/credentials/AbstractVaultTokenCredentialWithExpirationTest.java @@ -52,13 +52,36 @@ public void shouldBeAbleToFetchTokenOnInit() throws VaultException { @Test public void shouldReuseTheExistingTokenIfNotExpired() throws VaultException { + when(authResponse.getAuthClientToken()).thenReturn("fakeToken1", "fakeToken2"); + when(auth.lookupSelf()).thenReturn(lookupResponse); + when(lookupResponse.getTTL()).thenReturn(5L); + + vaultTokenCredentialWithExpiration.authorizeWithVault(vaultConfig); + vaultTokenCredentialWithExpiration.authorizeWithVault(vaultConfig); + + verify(vaultConfig, times(2)).token("fakeToken1"); + } + + @Test + public void shouldCacheDifferentTokensPerServer() throws VaultException { + when(vaultConfig.getAddress()).thenReturn("http://first"); + when(vaultConfig.getNameSpace()).thenReturn(null); + + VaultConfig secondVaultConfig = mock(VaultConfig.class); + when(secondVaultConfig.getAddress()).thenReturn("http://second"); + when(secondVaultConfig.getNameSpace()).thenReturn("second"); + + when(authResponse.getAuthClientToken()).thenReturn("fakeToken1", "fakeToken2", "shouldNeverBeRequested"); when(auth.lookupSelf()).thenReturn(lookupResponse); when(lookupResponse.getTTL()).thenReturn(5L); vaultTokenCredentialWithExpiration.authorizeWithVault(vaultConfig); vaultTokenCredentialWithExpiration.authorizeWithVault(vaultConfig); + vaultTokenCredentialWithExpiration.authorizeWithVault(secondVaultConfig); + vaultTokenCredentialWithExpiration.authorizeWithVault(secondVaultConfig); - verify(vaultConfig, times(2)).token("fakeToken"); + verify(vaultConfig, times(2)).token("fakeToken1"); + verify(secondVaultConfig, times(2)).token("fakeToken2"); } @Test From 06aefddb4cad2c71705f2acce944b497c19bce57 Mon Sep 17 00:00:00 2001 From: Dee Kryvenko Date: Tue, 4 Oct 2022 15:23:51 -0700 Subject: [PATCH 2/7] Fix token caching for multi-cluster multi-namespace environments --- .../AbstractVaultTokenCredentialWithExpiration.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/main/java/com/datapipe/jenkins/vault/credentials/AbstractVaultTokenCredentialWithExpiration.java b/src/main/java/com/datapipe/jenkins/vault/credentials/AbstractVaultTokenCredentialWithExpiration.java index e85c767e..02da9d6b 100644 --- a/src/main/java/com/datapipe/jenkins/vault/credentials/AbstractVaultTokenCredentialWithExpiration.java +++ b/src/main/java/com/datapipe/jenkins/vault/credentials/AbstractVaultTokenCredentialWithExpiration.java @@ -78,7 +78,8 @@ private void setTokenExpiry(Vault vault) { try { tokenTTL = (int) vault.auth().lookupSelf().getTTL(); } catch (VaultException e) { - LOGGER.log(Level.WARNING, "Could not determine token expiration. " + + LOGGER.log(Level.WARNING, + String.format("Could not determine token expiration (for key %s). ", key) + "Check if token is allowed to access auth/token/lookup-self. " + "Assuming token TTL expired.", e); } From d878673f4c165b26e8756e0dda9c2398dc959ab2 Mon Sep 17 00:00:00 2001 From: Dee Kryvenko Date: Tue, 4 Oct 2022 17:32:43 -0700 Subject: [PATCH 3/7] CA-2586: Fix Vault plugin permission denied issue --- .../AbstractVaultTokenCredentialWithExpiration.java | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/src/main/java/com/datapipe/jenkins/vault/credentials/AbstractVaultTokenCredentialWithExpiration.java b/src/main/java/com/datapipe/jenkins/vault/credentials/AbstractVaultTokenCredentialWithExpiration.java index 02da9d6b..fe45173a 100644 --- a/src/main/java/com/datapipe/jenkins/vault/credentials/AbstractVaultTokenCredentialWithExpiration.java +++ b/src/main/java/com/datapipe/jenkins/vault/credentials/AbstractVaultTokenCredentialWithExpiration.java @@ -4,6 +4,7 @@ import com.bettercloud.vault.VaultConfig; import com.bettercloud.vault.VaultException; import com.cloudbees.plugins.credentials.CredentialsScope; +import java.io.IOException; import java.io.Serializable; import java.util.Calendar; import java.util.Objects; @@ -109,7 +110,7 @@ private boolean tokenExpired() { } } - private final transient ConcurrentMap cache = new ConcurrentHashMap<>(); + private transient ConcurrentMap cache = new ConcurrentHashMap<>(); protected AbstractVaultTokenCredentialWithExpiration(CredentialsScope scope, String id, String description) { @@ -129,4 +130,9 @@ public Vault authorizeWithVault(VaultConfig config) { protected Vault getVault(VaultConfig config) { return new Vault(config); } + + private void readObject(java.io.ObjectInputStream in) + throws IOException, ClassNotFoundException { + cache = new ConcurrentHashMap<>(); + } } From a80bc3c35490de289d69a6bef4cca5fa10c37d9c Mon Sep 17 00:00:00 2001 From: Dee Kryvenko Date: Tue, 4 Oct 2022 17:34:56 -0700 Subject: [PATCH 4/7] CA-2586: Fix Vault plugin permission denied issue --- pom.xml | 2 +- ...AbstractVaultTokenCredentialWithExpiration.java | 14 +++++++------- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/pom.xml b/pom.xml index 409b84fd..043bec6b 100644 --- a/pom.xml +++ b/pom.xml @@ -12,7 +12,7 @@ ${changelist} hpi - 9999-SNAPSHOT + 356.26310337f_CA-2586_b_3 2.303.3 2.0.0 jenkinsci/${project.artifactId} diff --git a/src/main/java/com/datapipe/jenkins/vault/credentials/AbstractVaultTokenCredentialWithExpiration.java b/src/main/java/com/datapipe/jenkins/vault/credentials/AbstractVaultTokenCredentialWithExpiration.java index fe45173a..c68e09a5 100644 --- a/src/main/java/com/datapipe/jenkins/vault/credentials/AbstractVaultTokenCredentialWithExpiration.java +++ b/src/main/java/com/datapipe/jenkins/vault/credentials/AbstractVaultTokenCredentialWithExpiration.java @@ -19,9 +19,9 @@ public abstract class AbstractVaultTokenCredentialWithExpiration private final static Logger LOGGER = Logger .getLogger(AbstractVaultTokenCredentialWithExpiration.class.getName()); - public static class CacheKey implements Serializable { - private final transient String vaultUrl; - private final transient String namespace; + public static class CacheKey { + private final String vaultUrl; + private final String namespace; public CacheKey(String vaultUrl, String namespace) { this.vaultUrl = vaultUrl; @@ -52,10 +52,10 @@ public int hashCode() { } } - public static class TokenHolder implements Serializable { - private final transient CacheKey key; - private transient Calendar tokenExpiry; - private transient String token; + public static class TokenHolder { + private final CacheKey key; + private Calendar tokenExpiry; + private String token; public TokenHolder(CacheKey key) { this.key = key; From ef5475adc7edce336c9cb449ffeab55e069e2c9a Mon Sep 17 00:00:00 2001 From: Dee Kryvenko Date: Tue, 4 Oct 2022 17:35:42 -0700 Subject: [PATCH 5/7] CA-2586: Fix Vault plugin permission denied issue --- pom.xml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pom.xml b/pom.xml index 043bec6b..409b84fd 100644 --- a/pom.xml +++ b/pom.xml @@ -12,7 +12,7 @@ ${changelist} hpi - 356.26310337f_CA-2586_b_3 + 9999-SNAPSHOT 2.303.3 2.0.0 jenkinsci/${project.artifactId} From 31438910b4dbeb8b9aadf2fc5bba9fe5fe63e074 Mon Sep 17 00:00:00 2001 From: Dee Kryvenko Date: Tue, 4 Oct 2022 18:27:06 -0700 Subject: [PATCH 6/7] CA-2586: Fix Vault plugin permission denied issue --- ...tractVaultTokenCredentialWithExpiration.java | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/src/main/java/com/datapipe/jenkins/vault/credentials/AbstractVaultTokenCredentialWithExpiration.java b/src/main/java/com/datapipe/jenkins/vault/credentials/AbstractVaultTokenCredentialWithExpiration.java index c68e09a5..17ea6fc1 100644 --- a/src/main/java/com/datapipe/jenkins/vault/credentials/AbstractVaultTokenCredentialWithExpiration.java +++ b/src/main/java/com/datapipe/jenkins/vault/credentials/AbstractVaultTokenCredentialWithExpiration.java @@ -4,8 +4,6 @@ import com.bettercloud.vault.VaultConfig; import com.bettercloud.vault.VaultException; import com.cloudbees.plugins.credentials.CredentialsScope; -import java.io.IOException; -import java.io.Serializable; import java.util.Calendar; import java.util.Objects; import java.util.concurrent.ConcurrentHashMap; @@ -110,7 +108,14 @@ private boolean tokenExpired() { } } - private transient ConcurrentMap cache = new ConcurrentHashMap<>(); + private ConcurrentMap cache = new ConcurrentHashMap<>(); + + private synchronized ConcurrentMap getCache() { + if (cache == null) { + cache = new ConcurrentHashMap<>(); + } + return cache; + } protected AbstractVaultTokenCredentialWithExpiration(CredentialsScope scope, String id, String description) { @@ -121,6 +126,7 @@ protected AbstractVaultTokenCredentialWithExpiration(CredentialsScope scope, Str @Override public Vault authorizeWithVault(VaultConfig config) { + ConcurrentMap cache = getCache(); CacheKey key = new CacheKey(config.getAddress(), config.getNameSpace()); cache.putIfAbsent(key, new TokenHolder(key)); TokenHolder holder = cache.get(key); @@ -130,9 +136,4 @@ public Vault authorizeWithVault(VaultConfig config) { protected Vault getVault(VaultConfig config) { return new Vault(config); } - - private void readObject(java.io.ObjectInputStream in) - throws IOException, ClassNotFoundException { - cache = new ConcurrentHashMap<>(); - } } From c00c96d6cfdc7f2c1daed5b52d83809da13eced7 Mon Sep 17 00:00:00 2001 From: Dee Kryvenko Date: Sat, 8 Oct 2022 14:06:04 -0700 Subject: [PATCH 7/7] New gpg