From 407cdd3a8759b1e7a4d70f77cdccf69f18ab266c Mon Sep 17 00:00:00 2001 From: chasegawa Date: Fri, 24 Feb 2023 11:47:00 -0700 Subject: [PATCH] SHIBUI-2334: Scheduler continually tries to overwrite unchanged metadata Adjust entity descriptor file writer to only write the those EDs that have no file or have been updated recently. Additionally added a locking mechanism so that multiple instances will not attempt to write while another process is doing so. --- backend/build.gradle | 3 + backend/src/main/app-resources/default.yml | 7 ++ backend/src/main/app-resources/release.yml | 6 ++ .../CoreShibUiConfiguration.java | 26 +++--- .../admin/ui/domain/util/Shedlock.java | 20 +++++ .../EntityDescriptorFilesScheduledTasks.java | 83 ++++++++++++++----- ...ternalMetadataProvidersScheduledTasks.java | 4 + .../MetadataProvidersScheduledTasks.java | 4 + ...yDescriptorFilesScheduledTasksTests.groovy | 4 +- gradle.properties | 2 + 10 files changed, 121 insertions(+), 38 deletions(-) create mode 100644 backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/domain/util/Shedlock.java diff --git a/backend/build.gradle b/backend/build.gradle index 5381c9222..a77f0cfbb 100644 --- a/backend/build.gradle +++ b/backend/build.gradle @@ -165,6 +165,9 @@ dependencies { implementation "org.apache.logging.log4j:log4j-to-slf4j:${project.'log4JVersion'}" implementation "org.apache.logging.log4j:log4j-api:${project.'log4JVersion'}" + implementation "net.javacrumbs.shedlock:shedlock-spring:${project.'shedlockVersion'}" + implementation "net.javacrumbs.shedlock:shedlock-provider-jdbc-template:${project.'shedlockVersion'}" + // TODO: figure out what this should really be runtimeOnly "org.springframework.boot:spring-boot-starter-tomcat:${project.'springbootVersion'}" diff --git a/backend/src/main/app-resources/default.yml b/backend/src/main/app-resources/default.yml index 08fdfb387..949a8e4b5 100644 --- a/backend/src/main/app-resources/default.yml +++ b/backend/src/main/app-resources/default.yml @@ -12,6 +12,13 @@ # default-rootuser:root ## need to include the encoding for the password - be sure to quote the entire value as shown # default-password: "{noop}foopassword" +## Maximum amount of time to lock the scheduled task - the default is 30m, but should be increased if there are a large (1500+) entities +## YMMV depending on the speed of connection to the DB, server processing capabilities etc. +# maxTask: +# lockTime: 30m +## The first time the scheduler executes on this node, write out the entity descriptors to file [true|false] (default is true) +# entityDescriptor: +# writeOnStartup: true # pac4j-enabled: true # pac4j: # keystorePath: "/etc/shibui/samlKeystore.jks" diff --git a/backend/src/main/app-resources/release.yml b/backend/src/main/app-resources/release.yml index 006b05506..672685672 100644 --- a/backend/src/main/app-resources/release.yml +++ b/backend/src/main/app-resources/release.yml @@ -12,6 +12,12 @@ # default-rootuser:root ## need to include the encoding for the password - be sure to quote the entire value as shown # default-password: "{noop}foopassword" +## Maximum amount of time to lock the scheduled task - the default is 30m, but should be increased if there are a large (1500+) entities +## YMMV depending on the speed of connection to the DB, server processing capabilities etc. +# maxTask: +# lockTime: 30m +## The first time the scheduler executes on this node, write out the entity descriptors to file [true|false] (default is true) +# writeOnStartup: true # pac4j-enabled: true # pac4j: # keystorePath: "/etc/shibui/samlKeystore.jks" diff --git a/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/configuration/CoreShibUiConfiguration.java b/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/configuration/CoreShibUiConfiguration.java index 8a3ef5aba..97444ac4d 100644 --- a/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/configuration/CoreShibUiConfiguration.java +++ b/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/configuration/CoreShibUiConfiguration.java @@ -39,6 +39,8 @@ import edu.internet2.tier.shibboleth.admin.util.EntityDescriptorConversionUtils; import edu.internet2.tier.shibboleth.admin.util.LuceneUtility; import edu.internet2.tier.shibboleth.admin.util.ModelRepresentationConversions; +import net.javacrumbs.shedlock.core.LockProvider; +import net.javacrumbs.shedlock.provider.jdbctemplate.JdbcTemplateLockProvider; import org.apache.lucene.analysis.Analyzer; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.beans.factory.annotation.Qualifier; @@ -52,7 +54,7 @@ import org.springframework.context.annotation.Import; import org.springframework.context.support.ResourceBundleMessageSource; import org.springframework.core.io.Resource; -import org.springframework.web.client.RestTemplate; +import org.springframework.jdbc.core.JdbcTemplate; import org.springframework.web.servlet.LocaleResolver; import org.springframework.web.servlet.config.annotation.InterceptorRegistry; import org.springframework.web.servlet.config.annotation.PathMatchConfigurer; @@ -61,7 +63,7 @@ import org.springframework.web.util.UrlPathHelper; import javax.servlet.http.HttpServletRequest; -import java.net.URL; +import javax.sql.DataSource; @Configuration @Import(SearchConfiguration.class) @@ -96,26 +98,19 @@ public AttributeUtility attributeUtility() { @Bean @ConditionalOnProperty(name = "shibui.metadata-dir") - public EntityDescriptorFilesScheduledTasks entityDescriptorFilesScheduledTasks( - EntityDescriptorRepository entityDescriptorRepository, - @Value("${shibui.metadata-dir}") final String metadataDir) { - return new EntityDescriptorFilesScheduledTasks(metadataDir, entityDescriptorRepository, openSamlObjects(), - fileWritingService()); + public EntityDescriptorFilesScheduledTasks entityDescriptorFilesScheduledTasks(EntityDescriptorRepository entityDescriptorRepository, @Value("${shibui.metadata-dir}") final String metadataDir) { + return new EntityDescriptorFilesScheduledTasks(metadataDir, entityDescriptorRepository, openSamlObjects(), fileWritingService()); } @Bean @ConditionalOnProperty(name = "shibui.metadataProviders.target") - public MetadataProvidersScheduledTasks metadataProvidersScheduledTasks( - @Value("${shibui.metadataProviders.target}") final Resource resource, - final MetadataResolverService metadataResolverService) { + public MetadataProvidersScheduledTasks metadataProvidersScheduledTasks(@Value("${shibui.metadataProviders.target}") final Resource resource, final MetadataResolverService metadataResolverService) { return new MetadataProvidersScheduledTasks(resource, metadataResolverService, fileWritingService()); } @Bean @ConditionalOnProperty(name = "shibui.external.metadataProviders.target") - public ExternalMetadataProvidersScheduledTasks externalMetadataProvidersScheduledTasks( - @Value("${shibui.external.metadataProviders.target}") final Resource resource, - final MetadataResolverService metadataResolverService) { + public ExternalMetadataProvidersScheduledTasks externalMetadataProvidersScheduledTasks(@Value("${shibui.external.metadataProviders.target}") final Resource resource, final MetadataResolverService metadataResolverService) { return new ExternalMetadataProvidersScheduledTasks(resource, metadataResolverService, fileWritingService()); } @@ -254,4 +249,9 @@ public DynamicRegistrationService dynamicRegistrationService(DynamicRegistration ShibRestTemplateDelegate delegate = new ShibRestTemplateDelegate(config); return new JPADynamicRegistrationServiceImpl(groupService, driRepo, ownershipRepo, delegate, permissionEvaluator, userService); } + + @Bean + public LockProvider lockProvider(DataSource dataSource) { + return new JdbcTemplateLockProvider(JdbcTemplateLockProvider.Configuration.builder().withJdbcTemplate(new JdbcTemplate(dataSource)).usingDbTime().build()); + } } \ No newline at end of file diff --git a/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/domain/util/Shedlock.java b/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/domain/util/Shedlock.java new file mode 100644 index 000000000..ecf8c87fb --- /dev/null +++ b/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/domain/util/Shedlock.java @@ -0,0 +1,20 @@ +package edu.internet2.tier.shibboleth.admin.ui.domain.util; + +import javax.persistence.Entity; +import javax.persistence.Id; +import java.util.Date; + +/** + * Generally speaking, this isn't used, but is here to ensure that the table gets created cleanly regardless of DB type + */ +@Entity +public class Shedlock { + @Id + String name; + + Date lockUntil; + + Date lockedAt; + + String locked_by; +} \ No newline at end of file diff --git a/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/scheduled/EntityDescriptorFilesScheduledTasks.java b/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/scheduled/EntityDescriptorFilesScheduledTasks.java index 35f819165..b338c2559 100644 --- a/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/scheduled/EntityDescriptorFilesScheduledTasks.java +++ b/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/scheduled/EntityDescriptorFilesScheduledTasks.java @@ -5,10 +5,13 @@ import edu.internet2.tier.shibboleth.admin.ui.opensaml.OpenSamlObjects; import edu.internet2.tier.shibboleth.admin.ui.repository.EntityDescriptorRepository; import edu.internet2.tier.shibboleth.admin.ui.service.FileWritingService; +import net.javacrumbs.shedlock.spring.annotation.EnableSchedulerLock; +import net.javacrumbs.shedlock.spring.annotation.SchedulerLock; import org.bouncycastle.util.encoders.Hex; import org.opensaml.core.xml.io.MarshallingException; import org.slf4j.Logger; import org.slf4j.LoggerFactory; +import org.springframework.beans.factory.annotation.Value; import org.springframework.boot.autoconfigure.condition.ConditionalOnProperty; import org.springframework.context.annotation.Configuration; import org.springframework.scheduling.annotation.Scheduled; @@ -21,12 +24,13 @@ import java.nio.file.Paths; import java.security.MessageDigest; import java.security.NoSuchAlgorithmException; +import java.time.LocalDateTime; +import java.util.HashMap; import java.util.Set; import java.util.stream.Stream; import static java.util.stream.Collectors.toSet; - /** * This class is wrapped with Spring's scheduling facility to produce background scheduled periodic tasks pertaining to * generating, cleaning, etc. of entity descriptor files conforming to Shibboleth's LocalDynamicMetadataProvider naming @@ -36,21 +40,22 @@ */ @Configuration @ConditionalOnProperty(name = "shibui.metadata-dir") +@EnableSchedulerLock(defaultLockAtMostFor = "${shibui.maxTask.lockTime:30m}") public class EntityDescriptorFilesScheduledTasks { - private static final Logger LOGGER = LoggerFactory.getLogger(EntityDescriptorFilesScheduledTasks.class); + private static final String SHA1HEX_FILENAME_TEMPLATE = "%s.xml"; + private static final String TARGET_FILE_TEMPLATE = "%s/%s"; - private String metadataDirName; + /** cache by entity id and the last time we wrote out (or started the system) */ + private static final HashMap LAST_WRITTEN_CACHE = new HashMap(); private EntityDescriptorRepository entityDescriptorRepository; - + private final FileWritingService fileWritingService; + private String metadataDirName; private OpenSamlObjects openSamlObjects; - private static final String SHA1HEX_FILENAME_TEMPLATE = "%s.xml"; - - private static final String TARGET_FILE_TEMPLATE = "%s/%s"; - - private final FileWritingService fileWritingService; + @Value("${shibui.entityDescriptor.writeOnStartup:true}") + private boolean writeOnStartup; public EntityDescriptorFilesScheduledTasks(String metadataDirName, EntityDescriptorRepository entityDescriptorRepository, @@ -63,28 +68,60 @@ public EntityDescriptorFilesScheduledTasks(String metadataDirName, } @Scheduled(fixedRateString = "${shibui.taskRunRate:30000}") + @SchedulerLock(name = "generateEntityDescriptorFiles") @Transactional(readOnly = true) - public void generateEntityDescriptorFiles() throws MarshallingException { + public void generateEntityDescriptorFiles() { this.entityDescriptorRepository.findAllStreamByServiceEnabled(true) .forEach(ed -> { Path targetFilePath = targetFilePathFor(toSha1HexString(ed.getEntityID())); - if (Files.exists(targetFilePath)) { - LOGGER.info("Overwriting entity descriptor file [{}] for entity id [{}]", targetFilePath, ed.getEntityID()); - } else { - LOGGER.info("Generating entity descriptor file [{}] for entity id [{}]", targetFilePath, ed.getEntityID()); - } - - try { - String xmlContent = this.openSamlObjects.marshalToXmlString(ed); - fileWritingService.write(targetFilePath, xmlContent); - } catch (MarshallingException | IOException e) { - //TODO: any other better way to handle it? - LOGGER.error("Error marshalling entity descriptor into a file {} - {}", ed.getEntityID(), e.getMessage()); + if (needToWriteUpdate(ed)) { + LOGGER.info("Generating/Overwriting entity descriptor file [{}] for entity id [{}]", targetFilePath, ed.getEntityID()); + writeFile(ed, targetFilePath); } }); } + /** + * @return boolean true if any of the conditions for writing a file are true + * ** The file does not exist + * OR + * ** Write on startup (ie if we don't have a record of the file being written (there are a couple of scenarios that fit this)) IF configured to do so + * OR + * ** The ed last update/change time is after the last written cache time + */ + private boolean needToWriteUpdate(EntityDescriptor ed) { + Path targetFilePath = targetFilePathFor(toSha1HexString(ed.getEntityID())); + // If the file has never been written, write it out + if (!Files.exists(targetFilePath)) { + return true; + } + // If we don't have an entry in the cache, add the entry and write it out only if configured to do so on startup + // Regardless, if we don't have a cached entry, cache one + if (!LAST_WRITTEN_CACHE.containsKey(ed.getEntityID())) { + LAST_WRITTEN_CACHE.put(ed.getEntityID(), LocalDateTime.now()); + return writeOnStartup; + } + // If the entity is in the cache, write out only if the entity's last modified time is after the last cache time. + if (LAST_WRITTEN_CACHE.get(ed.getEntityID()).isBefore(ed.getModifiedDate())) { + LAST_WRITTEN_CACHE.put(ed.getEntityID(), LocalDateTime.now()); + return true; + } + return false; + } + + private void writeFile(EntityDescriptor ed, Path targetFilePath) { + try { + String xmlContent = this.openSamlObjects.marshalToXmlString(ed); + fileWritingService.write(targetFilePath, xmlContent); + } catch (MarshallingException | IOException e) { + //TODO: any other better way to handle it? + LOGGER.error("Error marshalling entity descriptor into a file {} - {}", ed.getEntityID(), e.getMessage()); + } + LAST_WRITTEN_CACHE.put(ed.getEntityID(), LocalDateTime.now()); + } + @Scheduled(fixedDelayString = "${shibui.taskDelayRate:30000}") + @SchedulerLock(name = "removeDanglingEntityDescriptorFiles") @Transactional(readOnly = true) public void removeDanglingEntityDescriptorFiles() { Path targetDirPath = Paths.get(this.metadataDirName); @@ -135,4 +172,4 @@ private Path targetFilePathFor(String sha1HexBaseFilename) { String filenamePath = String.format(TARGET_FILE_TEMPLATE, this.metadataDirName, filename); return Paths.get(filenamePath); } -} +} \ No newline at end of file diff --git a/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/scheduled/ExternalMetadataProvidersScheduledTasks.java b/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/scheduled/ExternalMetadataProvidersScheduledTasks.java index b2e3c7d37..fd30535f3 100644 --- a/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/scheduled/ExternalMetadataProvidersScheduledTasks.java +++ b/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/scheduled/ExternalMetadataProvidersScheduledTasks.java @@ -2,6 +2,8 @@ import edu.internet2.tier.shibboleth.admin.ui.service.FileWritingService; import edu.internet2.tier.shibboleth.admin.ui.service.MetadataResolverService; +import net.javacrumbs.shedlock.spring.annotation.EnableSchedulerLock; +import net.javacrumbs.shedlock.spring.annotation.SchedulerLock; import org.slf4j.Logger; import org.slf4j.LoggerFactory; import org.springframework.boot.autoconfigure.condition.ConditionalOnProperty; @@ -22,6 +24,7 @@ @Configuration @ConditionalOnProperty("shibui.external.metadataProviders.target") +@EnableSchedulerLock(defaultLockAtMostFor = "30m") public class ExternalMetadataProvidersScheduledTasks { private static final Logger logger = LoggerFactory.getLogger(ExternalMetadataProvidersScheduledTasks.class); @@ -36,6 +39,7 @@ public ExternalMetadataProvidersScheduledTasks(Resource target, MetadataResolver } @Scheduled(fixedRateString = "${shibui.external.metadataProviders.taskRunRate:30000}") + @SchedulerLock(name = "generateExternalMetadataProvidersFile") @Transactional(readOnly = true) public void generateMetadataProvidersFile() { try (StringWriter os = new StringWriter()) { diff --git a/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/scheduled/MetadataProvidersScheduledTasks.java b/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/scheduled/MetadataProvidersScheduledTasks.java index e89db306b..fba10fafe 100644 --- a/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/scheduled/MetadataProvidersScheduledTasks.java +++ b/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/scheduled/MetadataProvidersScheduledTasks.java @@ -2,6 +2,8 @@ import edu.internet2.tier.shibboleth.admin.ui.service.FileWritingService; import edu.internet2.tier.shibboleth.admin.ui.service.MetadataResolverService; +import net.javacrumbs.shedlock.spring.annotation.EnableSchedulerLock; +import net.javacrumbs.shedlock.spring.annotation.SchedulerLock; import org.slf4j.Logger; import org.slf4j.LoggerFactory; import org.springframework.boot.autoconfigure.condition.ConditionalOnProperty; @@ -22,6 +24,7 @@ @Configuration @ConditionalOnProperty("shibui.metadataProviders.target") +@EnableSchedulerLock(defaultLockAtMostFor = "30m") public class MetadataProvidersScheduledTasks { private static final Logger logger = LoggerFactory.getLogger(MetadataProvidersScheduledTasks.class); @@ -36,6 +39,7 @@ public MetadataProvidersScheduledTasks(Resource target, MetadataResolverService } @Scheduled(fixedRateString = "${shibui.metadataProviders.taskRunRate:30000}") + @SchedulerLock(name = "generateMetadataProvidersFile") @Transactional(readOnly = true) public void generateMetadataProvidersFile() { try (StringWriter os = new StringWriter()) { diff --git a/backend/src/test/groovy/edu/internet2/tier/shibboleth/admin/ui/scheduled/EntityDescriptorFilesScheduledTasksTests.groovy b/backend/src/test/groovy/edu/internet2/tier/shibboleth/admin/ui/scheduled/EntityDescriptorFilesScheduledTasksTests.groovy index f25d67682..ed6b64092 100644 --- a/backend/src/test/groovy/edu/internet2/tier/shibboleth/admin/ui/scheduled/EntityDescriptorFilesScheduledTasksTests.groovy +++ b/backend/src/test/groovy/edu/internet2/tier/shibboleth/admin/ui/scheduled/EntityDescriptorFilesScheduledTasksTests.groovy @@ -113,7 +113,7 @@ class EntityDescriptorFilesScheduledTasksTests extends AbstractBaseDataJpaTest { entityDescriptorFilesScheduledTasks.removeDanglingEntityDescriptorFiles() then: - def files = new File(tempPath, file) - files.size() == 0 + def file2 = new File(tempPath, file) + !file2.exists() } } \ No newline at end of file diff --git a/gradle.properties b/gradle.properties index 26fc19c5b..c0054973e 100644 --- a/gradle.properties +++ b/gradle.properties @@ -12,6 +12,8 @@ lombokVersion=5.3.3.3 opensamlVersion=4.2.0 pac4JVersion=5.4.3 pac4jSpringSecurityVersion=7.0.3 +# update shedlock to 5.x when updating to java 17+Spring 6 etc +shedlockVersion=4.42.0 shibbolethVersion=4.2.1 shibOIDCVersion=2.1.0 springbootVersion=2.7.0