From 69630375f32ab5fe21b7510620a358d8e1de75ba Mon Sep 17 00:00:00 2001 From: Johannes Rudolph Date: Fri, 21 Jun 2024 21:10:52 +0200 Subject: [PATCH 1/2] refactor: enforce that all GitHandler operations are managed via lock remove components from Spring Context so that they cannot be accidentally injected directly and bypass the operation context --- .../dockerosb/persistence/CatalogRepository.kt | 16 +++------------- .../dockerosb/persistence/GitHandlerService.kt | 7 +++---- .../persistence/GitOperationContextFactory.kt | 4 +++- .../persistence/ServiceInstanceRepository.kt | 5 +---- .../meshcloud/dockerosb/ServiceBrokerFixture.kt | 5 ++++- 5 files changed, 14 insertions(+), 23 deletions(-) diff --git a/src/main/kotlin/io/meshcloud/dockerosb/persistence/CatalogRepository.kt b/src/main/kotlin/io/meshcloud/dockerosb/persistence/CatalogRepository.kt index 1d8d584..c55aa82 100644 --- a/src/main/kotlin/io/meshcloud/dockerosb/persistence/CatalogRepository.kt +++ b/src/main/kotlin/io/meshcloud/dockerosb/persistence/CatalogRepository.kt @@ -3,23 +3,13 @@ package io.meshcloud.dockerosb.persistence import io.meshcloud.dockerosb.config.MeshServiceDefinition import mu.KotlinLogging import org.springframework.cloud.servicebroker.model.catalog.Catalog -import org.springframework.cloud.servicebroker.model.catalog.ServiceDefinition -import org.springframework.context.annotation.Bean -import org.springframework.context.annotation.Configuration private val log = KotlinLogging.logger { } -@Configuration class CatalogRepository( private val yamlHandler: YamlHandler, private val gitHandler: GitHandler ) { - - @Bean - fun catalog(): Catalog { - return getCatalog() - } - fun getCatalog(): Catalog { val catalogYml = gitHandler.fileInRepo("catalog.yml") @@ -35,11 +25,11 @@ class CatalogRepository( throw IllegalArgumentException("ServiceDefinitionId cannot contain any characters other than a-z, A-Z, 0-9 and - in your catalog!") else return Catalog.builder() - .serviceDefinitions(catalog.services) - .build() + .serviceDefinitions(catalog.services) + .build() } class YamlCatalog( val services: List ) -} +} \ No newline at end of file diff --git a/src/main/kotlin/io/meshcloud/dockerosb/persistence/GitHandlerService.kt b/src/main/kotlin/io/meshcloud/dockerosb/persistence/GitHandlerService.kt index 90b363f..e765829 100644 --- a/src/main/kotlin/io/meshcloud/dockerosb/persistence/GitHandlerService.kt +++ b/src/main/kotlin/io/meshcloud/dockerosb/persistence/GitHandlerService.kt @@ -16,10 +16,9 @@ import java.io.File private val log = KotlinLogging.logger {} /** - * Note: consumers should use this only via a [GitOperationContext] + * Note: consumers should use this only via a [GitOperationContext] to manage concurrent access to the git repo */ -@Service -class GitHandlerService( +open class GitHandlerService( private val gitConfig: GitConfig ) : GitHandler { @@ -246,7 +245,7 @@ class GitHandlerService( .call() } - protected fun push() { + protected open fun push() { val pushCommand = git.push() gitConfig.username?.let { diff --git a/src/main/kotlin/io/meshcloud/dockerosb/persistence/GitOperationContextFactory.kt b/src/main/kotlin/io/meshcloud/dockerosb/persistence/GitOperationContextFactory.kt index 465367f..ecd11c7 100644 --- a/src/main/kotlin/io/meshcloud/dockerosb/persistence/GitOperationContextFactory.kt +++ b/src/main/kotlin/io/meshcloud/dockerosb/persistence/GitOperationContextFactory.kt @@ -1,5 +1,6 @@ package io.meshcloud.dockerosb.persistence +import io.meshcloud.dockerosb.config.GitConfig import org.springframework.stereotype.Service import java.util.concurrent.locks.ReentrantLock @@ -14,9 +15,10 @@ import java.util.concurrent.locks.ReentrantLock */ @Service class GitOperationContextFactory( - private val gitHandler: GitHandler, + gitConfig: GitConfig, private val yamlHandler: YamlHandler ) { + private val gitHandler = GitHandlerService(gitConfig) // we have exactly one git operation that may be active at any time private val lock = ReentrantLock(true) diff --git a/src/main/kotlin/io/meshcloud/dockerosb/persistence/ServiceInstanceRepository.kt b/src/main/kotlin/io/meshcloud/dockerosb/persistence/ServiceInstanceRepository.kt index 04998ba..4b61196 100644 --- a/src/main/kotlin/io/meshcloud/dockerosb/persistence/ServiceInstanceRepository.kt +++ b/src/main/kotlin/io/meshcloud/dockerosb/persistence/ServiceInstanceRepository.kt @@ -3,12 +3,9 @@ package io.meshcloud.dockerosb.persistence import io.meshcloud.dockerosb.model.ServiceInstance import io.meshcloud.dockerosb.model.Status import org.springframework.cloud.servicebroker.model.instance.OperationState -import org.springframework.jmx.support.MetricType -import org.springframework.stereotype.Component import java.io.File -import java.time.Instant -@Component + class ServiceInstanceRepository(private val yamlHandler: YamlHandler, private val gitHandler: GitHandler) { fun createServiceInstance(serviceInstance: ServiceInstance) { diff --git a/src/test/kotlin/io/meshcloud/dockerosb/ServiceBrokerFixture.kt b/src/test/kotlin/io/meshcloud/dockerosb/ServiceBrokerFixture.kt index 2341319..d227412 100644 --- a/src/test/kotlin/io/meshcloud/dockerosb/ServiceBrokerFixture.kt +++ b/src/test/kotlin/io/meshcloud/dockerosb/ServiceBrokerFixture.kt @@ -36,9 +36,12 @@ class ServiceBrokerFixture(catalogPath: String) : Closeable { val yamlHandler = YamlHandler() + /** + * an unsafe git handler + */ val gitHandler = GitHandlerService(gitConfig) - val contextFactory = GitOperationContextFactory(gitHandler, yamlHandler) + val contextFactory = GitOperationContextFactory(gitConfig, yamlHandler) val builder = TestDataBuilder(catalogPath, yamlHandler) From b1490276f42ea402a8ffbf44e93a02bdac4f8570 Mon Sep 17 00:00:00 2001 From: Johannes Rudolph Date: Fri, 21 Jun 2024 21:32:14 +0200 Subject: [PATCH 2/2] refactor: simplify counting local commits with library function --- .../meshcloud/dockerosb/persistence/GitHandlerService.kt | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/src/main/kotlin/io/meshcloud/dockerosb/persistence/GitHandlerService.kt b/src/main/kotlin/io/meshcloud/dockerosb/persistence/GitHandlerService.kt index e765829..4a1cf80 100644 --- a/src/main/kotlin/io/meshcloud/dockerosb/persistence/GitHandlerService.kt +++ b/src/main/kotlin/io/meshcloud/dockerosb/persistence/GitHandlerService.kt @@ -190,12 +190,10 @@ open class GitHandlerService( fun hasLocalCommits(): Boolean { val origin = git.repository.resolve("origin/${gitConfig.remoteBranch}") - val head = git.getRepository().resolve("HEAD") + val head = git.repository.resolve("HEAD") - var count = 0 - for (entry in git.log().addRange(origin, head).call()) { - ++count - } + val range = git.log().addRange(origin, head).call() + val count = range.count() if (count > 0) { log.info { "Your branch is ahead of 'origin/${gitConfig.remoteBranch}' by $count commit(s)." }