Skip to content
This repository has been archived by the owner on Dec 10, 2023. It is now read-only.

feat: replace custom cache with Google Guava used in Jenkins core project #184

Merged
merged 11 commits into from
May 13, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
1 change: 1 addition & 0 deletions casc/docker-compose.yml
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ services:
container_name: jenkins
networks:
- crowd_net
restart: always

volumes:
- jenkins_home:/var/jenkins_home
Expand Down
39 changes: 39 additions & 0 deletions pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -97,8 +97,27 @@
<groupId>org.apache.httpcomponents</groupId>
<artifactId>httpclient-cache</artifactId>
</exclusion>
<!-- <exclusion>
<groupId>com.atlassian.crowd</groupId>
<artifactId>atlassian-annotations</artifactId>
</exclusion>
<exclusion>
<groupId>com.atlassian.crowd</groupId>
<artifactId>atlassian-collectors-util</artifactId>
</exclusion> -->
</exclusions>
</dependency>
<dependency>
<groupId>com.atlassian.crowd</groupId>
<artifactId>crowd-integration-api</artifactId>
<version>${crowd-integration-client-rest.version}</version>
</dependency>
<dependency>
<groupId>com.atlassian.crowd</groupId>
<artifactId>crowd-integration-client-common</artifactId>
<version>${crowd-integration-client-rest.version}</version>
</dependency>

<dependency>
<groupId>org.jenkins-ci.plugins</groupId>
<artifactId>apache-httpcomponents-client-4-api</artifactId>
Expand All @@ -111,6 +130,26 @@
<groupId>io.jenkins.plugins</groupId>
<artifactId>jaxb</artifactId>
</dependency>

<dependency>
<groupId>com.google.guava</groupId>
<artifactId>guava</artifactId>
<exclusions>
<exclusion>
<groupId>com.google.errorprone</groupId>
<artifactId>error_prone_annotations</artifactId>
</exclusion>
<exclusion>
<groupId>com.google.j2objc</groupId>
<artifactId>j2objc-annotations</artifactId>
</exclusion>
<exclusion>
<groupId>org.checkerframework</groupId>
<artifactId>checker-qual</artifactId>
</exclusion>
</exclusions>
</dependency>

<!-- Tests -->
<dependency>
<groupId>junit</groupId>
Expand Down
124 changes: 31 additions & 93 deletions src/main/java/de/theit/jenkins/crowd/CrowdConfigurationService.java
Original file line number Diff line number Diff line change
Expand Up @@ -46,16 +46,16 @@
import com.atlassian.crowd.service.client.ClientProperties;
import com.atlassian.crowd.service.client.ClientPropertiesImpl;
import com.atlassian.crowd.service.client.CrowdClient;
import com.google.common.cache.Cache;
import com.google.common.cache.CacheBuilder;

import hudson.util.Secret;

import java.util.ArrayList;
import java.util.Collection;
import java.util.Comparator;
import java.util.HashSet;
import java.util.LinkedHashMap;
import java.util.List;
import java.util.Map;
import java.util.Properties;
import java.util.TreeSet;
import java.util.concurrent.TimeUnit;
Expand Down Expand Up @@ -144,15 +144,15 @@ public class CrowdConfigurationService {

private final Integer cacheTTL;

private CacheMap<String, Boolean> isGroupMemberCache = null;
private Cache<String, Boolean> isGroupMemberCache = null;

private CacheMap<String, User> userFromSSOTokenCache = null;
private Cache<String, User> userFromSSOTokenCache = null;

private CacheMap<String, User> userCache = null;
private Cache<String, User> userCache = null;

private CacheMap<String, Group> groupCache = null;
private Cache<String, Group> groupCache = null;

private CacheMap<String, Collection<GrantedAuthority>> authoritiesForUserCache = null;
private Cache<String, Collection<GrantedAuthority>> authoritiesForUserCache = null;

/**
* Creates a new Crowd configuration object.
Expand Down Expand Up @@ -208,16 +208,16 @@ public CrowdConfigurationService(String url, String applicationName, Secret pass
this.cacheTTL = cacheTTL;

if (cacheSize != null && cacheSize > 0) {
this.isGroupMemberCache = new CacheMap<>(cacheSize);
this.userFromSSOTokenCache = new CacheMap<>(cacheSize);
this.userCache = new CacheMap<>(cacheSize);
this.groupCache = new CacheMap<>(cacheSize);
this.authoritiesForUserCache = new CacheMap<>(cacheSize);
this.isGroupMemberCache = CacheBuilder.newBuilder().maximumSize(cacheSize).expireAfterWrite(cacheTTL, TimeUnit.MINUTES).build();
this.userFromSSOTokenCache = CacheBuilder.newBuilder().maximumSize(cacheSize).expireAfterWrite(cacheTTL, TimeUnit.MINUTES).build();
this.userCache = CacheBuilder.newBuilder().maximumSize(cacheSize).expireAfterWrite(cacheTTL, TimeUnit.MINUTES).build();
this.groupCache = CacheBuilder.newBuilder().maximumSize(cacheSize).expireAfterWrite(cacheTTL, TimeUnit.MINUTES).build();
this.authoritiesForUserCache = CacheBuilder.newBuilder().maximumSize(cacheSize).expireAfterWrite(cacheTTL, TimeUnit.MINUTES).build();
}

Properties props = getProperties(url, applicationName, Secret.toString(password), sessionValidationInterval,
Properties props = getProperties(url, applicationName, password, sessionValidationInterval,
useSSO, cookieDomain, cookieTokenkey, useProxy, httpProxyHost, httpProxyPort, httpProxyUsername,
Secret.toString(httpProxyPassword), socketTimeout, httpTimeout, httpMaxConnections);
httpProxyPassword, socketTimeout, httpTimeout, httpMaxConnections);
this.clientProperties = ClientPropertiesImpl.newInstanceFromProperties(props);
this.crowdClient = new RestCrowdClientFactory().newInstance(this.clientProperties);
this.tokenHelper = CrowdHttpTokenHelperImpl.getInstance(CrowdHttpValidationFactorExtractorImpl.getInstance());
Expand Down Expand Up @@ -247,8 +247,9 @@ public boolean isUseSSO() {
*/
public boolean isGroupMember(String username) {
if (username == null) {
return false; // prevent NPE
return false;
}

if (allowedGroupNames.isEmpty()) {
return true;
}
Expand All @@ -257,7 +258,7 @@ public boolean isGroupMember(String username) {
Boolean retval = getValidValueFromCache(username, isGroupMemberCache);
if (retval != null) {
LOG.log(Level.FINEST, "isGroupMember() cache hit: {0}", username);
return retval;
return Boolean.TRUE.equals(retval);
}

LOG.log(Level.FINEST, "isGroupMember() cache hit MISS: {0}", username);
Expand All @@ -267,6 +268,9 @@ public boolean isGroupMember(String username) {
for (String group : this.allowedGroupNames) {
retval = isGroupMember(username, group);
if (retval) {
// If correct object was returned save it to cache
// checking if key is present is redundant
setValueToCache(username, retval, isGroupMemberCache);
break;
}
}
Expand All @@ -281,10 +285,6 @@ public boolean isGroupMember(String username) {
retval = null;
}

// If correct object was returned save it to cache
// checking if key is present is redundant
setValueToCache(username, retval, isGroupMemberCache);

return Boolean.TRUE.equals(retval);
}

Expand Down Expand Up @@ -441,8 +441,6 @@ public User getUser(String username) throws UserNotFoundException, OperationFail

LOG.log(Level.FINEST, "getUser() cache hit MISS: {0}", username);

LOG.log(Level.FINEST, "CrowdClient.getUser()");

ClassLoader orgContextClassLoader = null;
Thread currentThread = null;
if (IS_MIN_JAVA_11) {
Expand Down Expand Up @@ -604,15 +602,14 @@ public User findUserFromSSOToken(String token) throws OperationFailedException,
ApplicationPermissionException, InvalidTokenException {
// Load the entry from cache if it's valid return it
User retval = getValidValueFromCache(token, userFromSSOTokenCache);

if (retval != null) {
LOG.log(Level.FINEST, "findUserFromSSOToken() cache hit");
return retval;
}

LOG.log(Level.FINEST, "findUserFromSSOToken() cache hit MISS");

LOG.log(Level.FINEST, "CrowdClient.findUserFromSSOToken()");

ClassLoader orgContextClassLoader = null;
Thread currentThread = null;
if (IS_MIN_JAVA_11) {
Expand Down Expand Up @@ -773,7 +770,7 @@ private boolean isGroupMember(String username, String group)
InvalidAuthenticationException, OperationFailedException {
boolean retval = false;
if (isGroupActive(group)) {
LOG.log(Level.FINE, "Checking group membership for user '{0}' and group '{1}'...", new Object[] {username, group});
LOG.log(Level.FINE, "Checking group membership for user ''{0}'' and group ''{1}''...", new Object[] {username, group});

if (this.nestedGroups) {
if (isUserNestedGroupMember(username, group)) {
Expand All @@ -790,11 +787,11 @@ private boolean isGroupMember(String username, String group)
return retval;
}

private Properties getProperties(String url, String applicationName, String password,
private Properties getProperties(String url, String applicationName, Secret password,
int sessionValidationInterval, boolean useSSO,
String cookieDomain, String cookieTokenkey, Boolean useProxy,
String httpProxyHost, String httpProxyPort, String httpProxyUsername,
String httpProxyPassword, String socketTimeout,
Secret httpProxyPassword, String socketTimeout,
String httpTimeout, String httpMaxConnections) {
// for
// https://docs.atlassian.com/crowd/2.7.1/com/atlassian/crowd/service/client/ClientPropertiesImpl.html
Expand All @@ -805,7 +802,7 @@ private Properties getProperties(String url, String applicationName, String pass
crowdUrl += "/";
}
props.setProperty("application.name", applicationName);
props.setProperty("application.password", password);
props.setProperty("application.password", password.getPlainText());
props.setProperty("crowd.base.url", crowdUrl);
props.setProperty("application.login.url", crowdUrl + "console/");
props.setProperty("crowd.server.url", crowdUrl + "services/");
Expand All @@ -829,8 +826,8 @@ private Properties getProperties(String url, String applicationName, String pass
props.setProperty("http.proxy.port", httpProxyPort);
if (httpProxyUsername != null && !httpProxyUsername.equals(""))
props.setProperty("http.proxy.username", httpProxyUsername);
if (httpProxyPassword != null && !httpProxyPassword.equals(""))
props.setProperty("http.proxy.password", httpProxyPassword);
if (httpProxyPassword != null && !httpProxyPassword.getPlainText().equals(""))
props.setProperty("http.proxy.password", httpProxyPassword.getPlainText());
}

if (socketTimeout != null && !socketTimeout.equals(""))
Expand All @@ -843,79 +840,20 @@ private Properties getProperties(String url, String applicationName, String pass
return props;
}

private static class CacheEntry<T> {
private final long expires;
private final T value;

CacheEntry(int ttlSeconds, T value) {
this.expires = System.currentTimeMillis() + TimeUnit.SECONDS.toMillis(ttlSeconds);
this.value = value;
}

public T getValue() {
return value;
}

public boolean isValid() {
boolean isValid = System.currentTimeMillis() < expires;
LOG.log(Level.FINEST, "CacheEntry::isValid(): {0} -> {1}", new Object[] {isValid, value} );
return isValid;
}
}

/**
* While we could use Guava's CacheBuilder the method signature changes make
* using it problematic. Safer to roll our own and ensure compatibility across
* as wide a range of Jenkins versions as possible.
*
* @param <K> Key type
* @param <V> Cache entry type
*/
private static class CacheMap<K, V> extends LinkedHashMap<K, CacheEntry<V>> {

private static final long serialVersionUID = 1L;
private final int cacheSize;

CacheMap(int cacheSize) {
super(cacheSize + 1); // prevent realloc when hitting cache size limit
this.cacheSize = cacheSize;
}

@Override
protected boolean removeEldestEntry(Map.Entry<K, CacheEntry<V>> eldest) {
return size() > cacheSize || eldest.getValue() == null || !eldest.getValue().isValid();
}
}

private <T> T getValidValueFromCache(String key, CacheMap<String, T> cacheObj) {
private <V> V getValidValueFromCache(String key, Cache<String, V> cacheObj) {
if (!useCache || cacheObj == null) {
return null;
}

final CacheEntry<T> cached;
synchronized (this) {
cached = cacheObj.get(key);
}

if (cached != null && cached.isValid()) {
return cached.getValue();
} else {
return null;
}
return cacheObj.getIfPresent(key);
}

private <T> void setValueToCache(String key, T value, CacheMap<String, T> cacheObj) {
private <V> void setValueToCache(String key, V value, Cache<String,V> cacheObj) {
// Let's save the entry in the cache if necessary
if (!useCache || value == null) {
return;
}

synchronized (this) {
if (cacheObj == null) {
return;
}
cacheObj.put(key, new CacheEntry<>(cacheTTL, value));
LOG.log(Level.FINEST, "setValueToCache::cacheObj: {0}", cacheObj.toString());
}
cacheObj.put(key, value);
}
}
13 changes: 7 additions & 6 deletions src/main/java/de/theit/jenkins/crowd/CrowdSecurityRealm.java
Original file line number Diff line number Diff line change
Expand Up @@ -203,11 +203,11 @@ public CrowdSecurityRealm(String url, String applicationName, Secret password, S
String cookieTokenkey, Boolean useProxy, String httpProxyHost, String httpProxyPort,
String httpProxyUsername, Secret httpProxyPassword, String socketTimeout,
String httpTimeout, String httpMaxConnections, CacheConfiguration cache) {
this.cookieTokenkey = cookieTokenkey;
this.cookieTokenkey = StringUtils.trimToEmpty(cookieTokenkey);
this.useProxy = useProxy;
this.httpProxyHost = httpProxyHost;
this.httpProxyHost = StringUtils.trimToEmpty(httpProxyHost);
this.httpProxyPort = httpProxyPort;
this.httpProxyUsername = httpProxyUsername;
this.httpProxyUsername = StringUtils.trimToEmpty(httpProxyUsername);
this.httpProxyPassword = httpProxyPassword;
this.socketTimeout = socketTimeout;
this.httpTimeout = httpTimeout;
Expand All @@ -221,6 +221,10 @@ public CrowdSecurityRealm(String url, String applicationName, Secret password, S
this.useSSO = useSSO;
this.cookieDomain = cookieDomain;
this.cache = cache;

// If this constructor is called, make sure to re-save the configuration.
// This way, migrated secrets are persisted securely without user interaction.
this.getDescriptor().save();
}

/**
Expand Down Expand Up @@ -265,9 +269,6 @@ public CrowdSecurityRealm(String url, String applicationName, String password, S
useSSO,
cookieDomain, cookieTokenkey, useProxy, httpProxyHost, httpProxyPort, httpProxyUsername,
Secret.fromString(httpProxyPassword), socketTimeout, httpTimeout, httpMaxConnections, cache);
// If this constructor is called, make sure to re-save the configuration.
// This way, migrated secrets are persisted securely without user interaction.
getDescriptor().save();
}

/**
Expand Down