From 1f9d0069b0a546a16236e14f0c2296ae9c76e7cd Mon Sep 17 00:00:00 2001 From: mohamedlajmileanix Date: Tue, 13 Aug 2024 00:28:39 +0200 Subject: [PATCH 1/8] CID-2874: Support .yaml and .yml extensions for manifest file. And removing redundant logs. --- .../githubagent/dto/ManifestFileUpdateDto.kt | 2 +- .../leanix/githubagent/dto/RepositoryDto.kt | 7 +- .../services/GitHubGraphQLService.kt | 26 +++-- .../services/GitHubScanningService.kt | 4 +- .../services/WebhookEventService.kt | 94 ++++++++++++------- .../leanix/githubagent/shared/Constants.kt | 5 +- .../resources/graphql/GetRepositories.graphql | 10 +- .../GetRepositoryWithManifestContent.graphql | 4 +- .../services/GitHubScanningServiceTest.kt | 12 ++- .../services/WebhookEventServiceTest.kt | 3 +- 10 files changed, 117 insertions(+), 50 deletions(-) diff --git a/src/main/kotlin/net/leanix/githubagent/dto/ManifestFileUpdateDto.kt b/src/main/kotlin/net/leanix/githubagent/dto/ManifestFileUpdateDto.kt index a0d34b3..961af24 100644 --- a/src/main/kotlin/net/leanix/githubagent/dto/ManifestFileUpdateDto.kt +++ b/src/main/kotlin/net/leanix/githubagent/dto/ManifestFileUpdateDto.kt @@ -3,7 +3,7 @@ package net.leanix.githubagent.dto data class ManifestFileUpdateDto( val repositoryFullName: String, val action: ManifestFileAction, - val manifestContent: String? + val manifestFile: ManifestFileDto ) enum class ManifestFileAction { diff --git a/src/main/kotlin/net/leanix/githubagent/dto/RepositoryDto.kt b/src/main/kotlin/net/leanix/githubagent/dto/RepositoryDto.kt index 824e4ae..ec6fbdb 100644 --- a/src/main/kotlin/net/leanix/githubagent/dto/RepositoryDto.kt +++ b/src/main/kotlin/net/leanix/githubagent/dto/RepositoryDto.kt @@ -13,5 +13,10 @@ data class RepositoryDto( val updatedAt: String, val languages: List, val topics: List, - val manifestFileContent: String?, + val manifestFile: ManifestFileDto? +) + +data class ManifestFileDto( + val path: String?, + val content: String? ) diff --git a/src/main/kotlin/net/leanix/githubagent/services/GitHubGraphQLService.kt b/src/main/kotlin/net/leanix/githubagent/services/GitHubGraphQLService.kt index 91e5243..42cac0c 100644 --- a/src/main/kotlin/net/leanix/githubagent/services/GitHubGraphQLService.kt +++ b/src/main/kotlin/net/leanix/githubagent/services/GitHubGraphQLService.kt @@ -3,13 +3,14 @@ package net.leanix.githubagent.services import com.expediagroup.graphql.client.spring.GraphQLWebClient import kotlinx.coroutines.runBlocking import net.leanix.githubagent.config.GitHubEnterpriseProperties +import net.leanix.githubagent.dto.ManifestFileDto import net.leanix.githubagent.dto.PagedRepositories import net.leanix.githubagent.dto.RepositoryDto import net.leanix.githubagent.exceptions.GraphQLApiException import net.leanix.githubagent.graphql.data.GetRepositories import net.leanix.githubagent.graphql.data.GetRepositoryManifestContent import net.leanix.githubagent.graphql.data.getrepositories.Blob -import net.leanix.githubagent.shared.MANIFEST_FILE_NAME +import net.leanix.githubagent.shared.ManifestFileName import org.slf4j.LoggerFactory import org.springframework.stereotype.Component import org.springframework.web.reactive.function.client.WebClient @@ -34,7 +35,8 @@ class GitHubGraphQLService( GetRepositories.Variables( pageCount = PAGE_COUNT, cursor = cursor, - expression = "HEAD:${gitHubEnterpriseProperties.manifestFileDirectory}$MANIFEST_FILE_NAME" + "HEAD:${gitHubEnterpriseProperties.manifestFileDirectory}${ManifestFileName.YAML.fileName}", + "HEAD:${gitHubEnterpriseProperties.manifestFileDirectory}${ManifestFileName.YML.fileName}" ) ) @@ -61,7 +63,19 @@ class GitHubGraphQLService( updatedAt = it.updatedAt, languages = it.languages!!.nodes!!.map { language -> language!!.name }, topics = it.repositoryTopics.nodes!!.map { topic -> topic!!.topic.name }, - manifestFileContent = (it.`object` as Blob?)?.text + manifestFile = if (it.manifestYaml != null) { + ManifestFileDto( + path = ManifestFileName.YAML.fileName, + content = (it.manifestYaml as Blob).text.toString() + ) + } else if (it.manifestYml != null) { + ManifestFileDto( + path = ManifestFileName.YML.fileName, + content = (it.manifestYml as Blob).text.toString() + ) + } else { + null + } ) } ) @@ -71,7 +85,7 @@ class GitHubGraphQLService( fun getManifestFileContent( owner: String, repositoryName: String, - filePath: String, + fileName: String, token: String ): String { val client = buildGitHubGraphQLClient(token) @@ -80,7 +94,7 @@ class GitHubGraphQLService( GetRepositoryManifestContent.Variables( owner = owner, repositoryName = repositoryName, - filePath = filePath + manifestFilePath = "HEAD:$fileName" ) ) @@ -93,7 +107,7 @@ class GitHubGraphQLService( throw GraphQLApiException(result.errors!!) } else { ( - result.data!!.repository!!.`object` + result.data!!.repository!!.manifestFile as net.leanix.githubagent.graphql.`data`.getrepositorymanifestcontent.Blob ).text.toString() } diff --git a/src/main/kotlin/net/leanix/githubagent/services/GitHubScanningService.kt b/src/main/kotlin/net/leanix/githubagent/services/GitHubScanningService.kt index ab6402f..41f9339 100644 --- a/src/main/kotlin/net/leanix/githubagent/services/GitHubScanningService.kt +++ b/src/main/kotlin/net/leanix/githubagent/services/GitHubScanningService.kt @@ -27,7 +27,6 @@ class GitHubScanningService( val installations = getInstallations(jwtToken.toString()) fetchAndSendOrganisationsData(installations) installations.forEach { installation -> - logger.info("Fetching repositories for organisation ${installation.account.login}") fetchAndSendRepositoriesData(installation) } }.onFailure { @@ -74,7 +73,6 @@ class GitHubScanningService( token = installationToken, cursor = cursor ) - logger.info("Sending page $page of repositories") webSocketService.sendMessage( "${cachingService.get("runId")}/repositories", repositoriesPage.repositories @@ -83,6 +81,6 @@ class GitHubScanningService( totalRepos += repositoriesPage.repositories.size page++ } while (repositoriesPage.hasNextPage) - logger.info("Fetched $totalRepos repositories") + logger.info("Fetched $totalRepos repositories data from organisation ${installation.account.login}") } } diff --git a/src/main/kotlin/net/leanix/githubagent/services/WebhookEventService.kt b/src/main/kotlin/net/leanix/githubagent/services/WebhookEventService.kt index 8144153..1d564ab 100644 --- a/src/main/kotlin/net/leanix/githubagent/services/WebhookEventService.kt +++ b/src/main/kotlin/net/leanix/githubagent/services/WebhookEventService.kt @@ -4,9 +4,11 @@ import com.fasterxml.jackson.module.kotlin.jacksonObjectMapper import com.fasterxml.jackson.module.kotlin.readValue import net.leanix.githubagent.config.GitHubEnterpriseProperties import net.leanix.githubagent.dto.ManifestFileAction +import net.leanix.githubagent.dto.ManifestFileDto import net.leanix.githubagent.dto.ManifestFileUpdateDto +import net.leanix.githubagent.dto.PushEventCommit import net.leanix.githubagent.dto.PushEventPayload -import net.leanix.githubagent.shared.MANIFEST_FILE_NAME +import net.leanix.githubagent.shared.ManifestFileName import org.slf4j.LoggerFactory import org.springframework.stereotype.Service @@ -47,39 +49,67 @@ class WebhookEventService( } if (pushEventPayload.ref == "refs/heads/${pushEventPayload.repository.defaultBranch}") { - when { - MANIFEST_FILE_NAME in headCommit.added -> { - logger.info("Manifest file added to repository $repositoryFullName") - val fileContent = getManifestFileContent(organizationName, repositoryName, installationToken) - sendManifestData(repositoryFullName, ManifestFileAction.ADDED, fileContent) - } - MANIFEST_FILE_NAME in headCommit.modified -> { - logger.info("Manifest file modified in repository $repositoryFullName") - val fileContent = getManifestFileContent(organizationName, repositoryName, installationToken) - sendManifestData(repositoryFullName, ManifestFileAction.MODIFIED, fileContent) - } - MANIFEST_FILE_NAME in headCommit.removed -> { - logger.info("Manifest file removed from repository $repositoryFullName") - sendManifestData(repositoryFullName, ManifestFileAction.REMOVED, null) - } - } + handleManifestFileChanges( + "${gitHubEnterpriseProperties.manifestFileDirectory}${ManifestFileName.YAML.fileName}", + headCommit, + repositoryFullName, + organizationName, + repositoryName, + installationToken + ) + handleManifestFileChanges( + "${gitHubEnterpriseProperties.manifestFileDirectory}${ManifestFileName.YML.fileName}", + headCommit, + repositoryFullName, + organizationName, + repositoryName, + installationToken + ) } } - private fun getManifestFileContent(organizationName: String, repositoryName: String, token: String): String { - return gitHubGraphQLService.getManifestFileContent( - owner = organizationName, - repositoryName, - "HEAD:${gitHubEnterpriseProperties.manifestFileDirectory}$MANIFEST_FILE_NAME", - token - ) - } - - private fun sendManifestData(repositoryFullName: String, action: ManifestFileAction, manifestContent: String?) { - logger.info("Sending manifest file update event for repository $repositoryFullName") - webSocketService.sendMessage( - "/events/manifestFile", - ManifestFileUpdateDto(repositoryFullName, action, manifestContent) - ) + @SuppressWarnings("LongParameterList") + private fun handleManifestFileChanges( + manifestFilePath: String, + headCommit: PushEventCommit, + repositoryFullName: String, + organizationName: String, + repositoryName: String, + installationToken: String + ) { + when (manifestFilePath) { + in headCommit.added, in headCommit.modified -> { + val action = when (manifestFilePath) { + in headCommit.added -> ManifestFileAction.ADDED + else -> ManifestFileAction.MODIFIED + } + logger.info("Manifest file $action in repository $repositoryFullName") + val fileContent = gitHubGraphQLService.getManifestFileContent( + organizationName, + repositoryName, + manifestFilePath, + installationToken + ) + webSocketService.sendMessage( + "/events/manifestFile", + ManifestFileUpdateDto( + repositoryFullName, + action, + ManifestFileDto(manifestFilePath, fileContent) + ) + ) + } + in headCommit.removed -> { + logger.info("Manifest file ${ManifestFileAction.REMOVED} from repository $repositoryFullName") + webSocketService.sendMessage( + "/events/manifestFile", + ManifestFileUpdateDto( + repositoryFullName, + ManifestFileAction.REMOVED, + ManifestFileDto(null, null) + ) + ) + } + } } } diff --git a/src/main/kotlin/net/leanix/githubagent/shared/Constants.kt b/src/main/kotlin/net/leanix/githubagent/shared/Constants.kt index d41f649..68fc080 100644 --- a/src/main/kotlin/net/leanix/githubagent/shared/Constants.kt +++ b/src/main/kotlin/net/leanix/githubagent/shared/Constants.kt @@ -2,7 +2,10 @@ package net.leanix.githubagent.shared const val TOPIC_PREFIX = "/app/ghe/" -const val MANIFEST_FILE_NAME = "leanix.yaml" +enum class ManifestFileName(val fileName: String) { + YAML("leanix.yaml"), + YML("leanix.yml") +} val SUPPORTED_EVENT_TYPES = listOf( "REPOSITORY", diff --git a/src/main/resources/graphql/GetRepositories.graphql b/src/main/resources/graphql/GetRepositories.graphql index 5abbe52..1293aff 100644 --- a/src/main/resources/graphql/GetRepositories.graphql +++ b/src/main/resources/graphql/GetRepositories.graphql @@ -1,4 +1,4 @@ -query GetRepositories($pageCount: Int!, $cursor: String, $expression: String!) { +query GetRepositories($pageCount: Int!, $cursor: String, $manifestYamlPath: String!, $manifestYmlPath: String!) { viewer { repositories(first: $pageCount, after: $cursor) { pageInfo { @@ -28,7 +28,13 @@ query GetRepositories($pageCount: Int!, $cursor: String, $expression: String!) { } } } - object(expression: $expression) { + manifestYaml: object(expression: $manifestYamlPath) { + __typename + ... on Blob { + text + } + } + manifestYml: object(expression: $manifestYmlPath) { __typename ... on Blob { text diff --git a/src/main/resources/graphql/GetRepositoryWithManifestContent.graphql b/src/main/resources/graphql/GetRepositoryWithManifestContent.graphql index 56f21a0..bbb489c 100644 --- a/src/main/resources/graphql/GetRepositoryWithManifestContent.graphql +++ b/src/main/resources/graphql/GetRepositoryWithManifestContent.graphql @@ -1,6 +1,6 @@ -query GetRepositoryManifestContent($owner: String!, $repositoryName: String!, $filePath: String!) { +query GetRepositoryManifestContent($owner: String!, $repositoryName: String!, $manifestFilePath: String!) { repository(owner: $owner, name: $repositoryName) { - object(expression: $filePath) { + manifestFile: object(expression: $manifestFilePath) { __typename ... on Blob { text diff --git a/src/test/kotlin/net/leanix/githubagent/services/GitHubScanningServiceTest.kt b/src/test/kotlin/net/leanix/githubagent/services/GitHubScanningServiceTest.kt index 6253db6..f66cef8 100644 --- a/src/test/kotlin/net/leanix/githubagent/services/GitHubScanningServiceTest.kt +++ b/src/test/kotlin/net/leanix/githubagent/services/GitHubScanningServiceTest.kt @@ -7,6 +7,7 @@ import net.leanix.githubagent.client.GitHubClient import net.leanix.githubagent.dto.Account import net.leanix.githubagent.dto.Installation import net.leanix.githubagent.dto.InstallationTokenResponse +import net.leanix.githubagent.dto.ManifestFileDto import net.leanix.githubagent.dto.Organization import net.leanix.githubagent.dto.PagedRepositories import net.leanix.githubagent.dto.RepositoryDto @@ -95,7 +96,16 @@ class GitHubScanningServiceTest { updatedAt = "2024-01-01T00:00:00Z", languages = listOf("Kotlin", "Java"), topics = listOf("test", "example"), - manifestFileContent = "dependencies { implementation 'com.example:example-lib:1.0.0' }" + manifestFile = ManifestFileDto( + "leanix.yaml", + "version: 1\n" + + "services:\n" + + " - name: disputes-service-v1\n" + + " externalId: disputes-service-v1\n" + + " description: |\n" + + " A microservice responsible for payment disputes.\n" + + " This service handles payment transactions." + ) ) ), hasNextPage = false, diff --git a/src/test/kotlin/net/leanix/githubagent/services/WebhookEventServiceTest.kt b/src/test/kotlin/net/leanix/githubagent/services/WebhookEventServiceTest.kt index fe41c46..dfd7630 100644 --- a/src/test/kotlin/net/leanix/githubagent/services/WebhookEventServiceTest.kt +++ b/src/test/kotlin/net/leanix/githubagent/services/WebhookEventServiceTest.kt @@ -4,6 +4,7 @@ import com.ninjasquad.springmockk.MockkBean import io.mockk.every import io.mockk.verify import net.leanix.githubagent.dto.ManifestFileAction +import net.leanix.githubagent.dto.ManifestFileDto import net.leanix.githubagent.dto.ManifestFileUpdateDto import org.junit.jupiter.api.BeforeEach import org.junit.jupiter.api.Test @@ -90,7 +91,7 @@ class WebhookEventServiceTest { ManifestFileUpdateDto( "owner/repo", ManifestFileAction.MODIFIED, - "content" + ManifestFileDto("leanix.yaml", "content") ) ) } From 401418fa4d3310bcac22f60284de81bb6471d343 Mon Sep 17 00:00:00 2001 From: mohamedlajmileanix Date: Tue, 13 Aug 2024 00:41:38 +0200 Subject: [PATCH 2/8] CID-2874: Add tests. --- .../services/GitHubScanningServiceTest.kt | 8 +--- .../services/WebhookEventServiceTest.kt | 48 +++++++++++++++++++ 2 files changed, 49 insertions(+), 7 deletions(-) diff --git a/src/test/kotlin/net/leanix/githubagent/services/GitHubScanningServiceTest.kt b/src/test/kotlin/net/leanix/githubagent/services/GitHubScanningServiceTest.kt index f66cef8..9f26638 100644 --- a/src/test/kotlin/net/leanix/githubagent/services/GitHubScanningServiceTest.kt +++ b/src/test/kotlin/net/leanix/githubagent/services/GitHubScanningServiceTest.kt @@ -98,13 +98,7 @@ class GitHubScanningServiceTest { topics = listOf("test", "example"), manifestFile = ManifestFileDto( "leanix.yaml", - "version: 1\n" + - "services:\n" + - " - name: disputes-service-v1\n" + - " externalId: disputes-service-v1\n" + - " description: |\n" + - " A microservice responsible for payment disputes.\n" + - " This service handles payment transactions." + "content" ) ) ), diff --git a/src/test/kotlin/net/leanix/githubagent/services/WebhookEventServiceTest.kt b/src/test/kotlin/net/leanix/githubagent/services/WebhookEventServiceTest.kt index dfd7630..5416663 100644 --- a/src/test/kotlin/net/leanix/githubagent/services/WebhookEventServiceTest.kt +++ b/src/test/kotlin/net/leanix/githubagent/services/WebhookEventServiceTest.kt @@ -119,4 +119,52 @@ class WebhookEventServiceTest { verify(exactly = 1) { webSocketService.sendMessage("/events/other", payload) } } + + @Test + fun `should send updates for yaml manifest file`() { + val manifestFilePath = "leanix.yaml" + val payload = createPushEventPayload(manifestFilePath) + every { cachingService.get(any()) } returns "token" + every { gitHubGraphQLService.getManifestFileContent(any(), any(), manifestFilePath, any()) } returns "content" + + webhookEventService.consumeWebhookEvent("PUSH", payload) + + verify { webSocketService.sendMessage(any(), any()) } + } + + @Test + fun `should send updates for yml manifest file`() { + val manifestFilePath = "leanix.yml" + val payload = createPushEventPayload(manifestFilePath) + every { cachingService.get(any()) } returns "token" + every { gitHubGraphQLService.getManifestFileContent(any(), any(), manifestFilePath, any()) } returns "content" + + webhookEventService.consumeWebhookEvent("PUSH", payload) + + verify { webSocketService.sendMessage(any(), any()) } + } + + private fun createPushEventPayload(manifestFileName: String): String { + return """ + { + "ref": "refs/heads/main", + "repository": { + "name": "repo", + "full_name": "org/repo", + "default_branch": "main", + "owner": { + "name": "org" + } + }, + "head_commit": { + "added": ["$manifestFileName"], + "modified": [], + "removed": [] + }, + "installation": { + "id": 1 + } + } + """ + } } From 4b3243bea53f7af29cdb80c625cd1fa2d94c1c6b Mon Sep 17 00:00:00 2001 From: mohamedlajmileanix Date: Tue, 13 Aug 2024 00:47:06 +0200 Subject: [PATCH 3/8] CID-2874: Update README file. --- README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.md b/README.md index 91abfa3..eb2075b 100644 --- a/README.md +++ b/README.md @@ -27,7 +27,7 @@ The SAP LeanIX agent discovers self-built software in self-hosted GitHub Enterpr - `GITHUB_ENTERPRISE_BASE_URL`: The base URL of your GitHub Enterprise Server instance. - `GITHUB_APP_ID`: The ID of your GitHub App. - `PEM_FILE`: The path to your GitHub App's PEM file inside the Docker container. - - `MANIFEST_FILE_DIRECTORY`: The directory path where the manifest files are stored in each repository. Manifest files are crucial for microservice discovery as they provide essential information about the service. For more information, see [Microservice Discovery Through a Manifest File](https://docs-eam.leanix.net/reference/microservice-discovery-manifest-file) in our documentation. + - `MANIFEST_FILE_DIRECTORY`: The directory path where the manifest files are stored in each repository. Manifest files are crucial for microservice discovery as they provide essential information about the service. For more information, see [Microservice Discovery Through a Manifest File](https://docs-eam.leanix.net/reference/microservice-discovery-manifest-file) in our documentation. **If both .yaml and .yml files exist in a repository, only the .yaml file will be used.** - `WEBHOOK_SECRET`: The secret used to validate incoming webhook events from GitHub. (Optional, but recommended. [Needs to be set in the GitHub App settings first](https://docs.github.com/en/enterprise-server@3.8/webhooks/using-webhooks/validating-webhook-deliveries).) 5. **Start the agent**: To start the agent, run the following Docker command. Replace the variables in angle brackets with your actual values. From f52f71257e3106da2b96455e6f2202e5c5cf835c Mon Sep 17 00:00:00 2001 From: mohamedlajmileanix Date: Tue, 13 Aug 2024 16:46:24 +0200 Subject: [PATCH 4/8] CID-2874: addressing PR comments. Ignore yml file is yaml file exists --- .../githubagent/dto/ManifestFileUpdateDto.kt | 2 +- .../leanix/githubagent/dto/RepositoryDto.kt | 7 +- .../services/GitHubGraphQLService.kt | 15 +-- .../services/WebhookEventService.kt | 125 ++++++++++++------ .../services/GitHubScanningServiceTest.kt | 6 +- .../services/WebhookEventServiceTest.kt | 26 ++-- 6 files changed, 110 insertions(+), 71 deletions(-) diff --git a/src/main/kotlin/net/leanix/githubagent/dto/ManifestFileUpdateDto.kt b/src/main/kotlin/net/leanix/githubagent/dto/ManifestFileUpdateDto.kt index 961af24..35b85d0 100644 --- a/src/main/kotlin/net/leanix/githubagent/dto/ManifestFileUpdateDto.kt +++ b/src/main/kotlin/net/leanix/githubagent/dto/ManifestFileUpdateDto.kt @@ -3,7 +3,7 @@ package net.leanix.githubagent.dto data class ManifestFileUpdateDto( val repositoryFullName: String, val action: ManifestFileAction, - val manifestFile: ManifestFileDto + val manifestFileContent: String? ) enum class ManifestFileAction { diff --git a/src/main/kotlin/net/leanix/githubagent/dto/RepositoryDto.kt b/src/main/kotlin/net/leanix/githubagent/dto/RepositoryDto.kt index ec6fbdb..848a45a 100644 --- a/src/main/kotlin/net/leanix/githubagent/dto/RepositoryDto.kt +++ b/src/main/kotlin/net/leanix/githubagent/dto/RepositoryDto.kt @@ -13,10 +13,5 @@ data class RepositoryDto( val updatedAt: String, val languages: List, val topics: List, - val manifestFile: ManifestFileDto? -) - -data class ManifestFileDto( - val path: String?, - val content: String? + val manifestFileContent: String? ) diff --git a/src/main/kotlin/net/leanix/githubagent/services/GitHubGraphQLService.kt b/src/main/kotlin/net/leanix/githubagent/services/GitHubGraphQLService.kt index 42cac0c..2c256d2 100644 --- a/src/main/kotlin/net/leanix/githubagent/services/GitHubGraphQLService.kt +++ b/src/main/kotlin/net/leanix/githubagent/services/GitHubGraphQLService.kt @@ -3,7 +3,6 @@ package net.leanix.githubagent.services import com.expediagroup.graphql.client.spring.GraphQLWebClient import kotlinx.coroutines.runBlocking import net.leanix.githubagent.config.GitHubEnterpriseProperties -import net.leanix.githubagent.dto.ManifestFileDto import net.leanix.githubagent.dto.PagedRepositories import net.leanix.githubagent.dto.RepositoryDto import net.leanix.githubagent.exceptions.GraphQLApiException @@ -63,16 +62,10 @@ class GitHubGraphQLService( updatedAt = it.updatedAt, languages = it.languages!!.nodes!!.map { language -> language!!.name }, topics = it.repositoryTopics.nodes!!.map { topic -> topic!!.topic.name }, - manifestFile = if (it.manifestYaml != null) { - ManifestFileDto( - path = ManifestFileName.YAML.fileName, - content = (it.manifestYaml as Blob).text.toString() - ) + manifestFileContent = if (it.manifestYaml != null) { + (it.manifestYaml as Blob).text.toString() } else if (it.manifestYml != null) { - ManifestFileDto( - path = ManifestFileName.YML.fileName, - content = (it.manifestYml as Blob).text.toString() - ) + (it.manifestYml as Blob).text.toString() } else { null } @@ -87,7 +80,7 @@ class GitHubGraphQLService( repositoryName: String, fileName: String, token: String - ): String { + ): String? { val client = buildGitHubGraphQLClient(token) val query = GetRepositoryManifestContent( diff --git a/src/main/kotlin/net/leanix/githubagent/services/WebhookEventService.kt b/src/main/kotlin/net/leanix/githubagent/services/WebhookEventService.kt index 1d564ab..d2dbc9a 100644 --- a/src/main/kotlin/net/leanix/githubagent/services/WebhookEventService.kt +++ b/src/main/kotlin/net/leanix/githubagent/services/WebhookEventService.kt @@ -4,7 +4,6 @@ import com.fasterxml.jackson.module.kotlin.jacksonObjectMapper import com.fasterxml.jackson.module.kotlin.readValue import net.leanix.githubagent.config.GitHubEnterpriseProperties import net.leanix.githubagent.dto.ManifestFileAction -import net.leanix.githubagent.dto.ManifestFileDto import net.leanix.githubagent.dto.ManifestFileUpdateDto import net.leanix.githubagent.dto.PushEventCommit import net.leanix.githubagent.dto.PushEventPayload @@ -39,7 +38,7 @@ class WebhookEventService( val repositoryName = pushEventPayload.repository.name val repositoryFullName = pushEventPayload.repository.fullName val headCommit = pushEventPayload.headCommit - val organizationName = pushEventPayload.repository.owner.name + val owner = pushEventPayload.repository.owner.name var installationToken = cachingService.get("installationToken:${pushEventPayload.installation.id}")?.toString() if (installationToken == null) { @@ -50,18 +49,9 @@ class WebhookEventService( if (pushEventPayload.ref == "refs/heads/${pushEventPayload.repository.defaultBranch}") { handleManifestFileChanges( - "${gitHubEnterpriseProperties.manifestFileDirectory}${ManifestFileName.YAML.fileName}", - headCommit, - repositoryFullName, - organizationName, - repositoryName, - installationToken - ) - handleManifestFileChanges( - "${gitHubEnterpriseProperties.manifestFileDirectory}${ManifestFileName.YML.fileName}", headCommit, repositoryFullName, - organizationName, + owner, repositoryName, installationToken ) @@ -70,46 +60,101 @@ class WebhookEventService( @SuppressWarnings("LongParameterList") private fun handleManifestFileChanges( - manifestFilePath: String, headCommit: PushEventCommit, repositoryFullName: String, - organizationName: String, + owner: String, repositoryName: String, installationToken: String ) { + val isYAMLFileUpdated = + isManifestFileUpdated( + headCommit, + "${gitHubEnterpriseProperties.manifestFileDirectory}${ManifestFileName.YAML.fileName}" + ) + val isYMLFileUpdated = + isManifestFileUpdated( + headCommit, + "${gitHubEnterpriseProperties.manifestFileDirectory}${ManifestFileName.YML.fileName}" + ) + + // ignore updates on YML file if YAML file exists + if (!isYAMLFileUpdated && isYMLFileUpdated) { + val yamlFileContent = gitHubGraphQLService.getManifestFileContent( + owner, + repositoryName, + "${gitHubEnterpriseProperties.manifestFileDirectory}${ManifestFileName.YAML.fileName}", + installationToken + ) + if (yamlFileContent != null) return + } + + val manifestFilePath = if (isYAMLFileUpdated) { + "${gitHubEnterpriseProperties.manifestFileDirectory}${ManifestFileName.YAML.fileName}" + } else if (isYMLFileUpdated) { + "${gitHubEnterpriseProperties.manifestFileDirectory}${ManifestFileName.YML.fileName}" + } else { + return + } + when (manifestFilePath) { in headCommit.added, in headCommit.modified -> { - val action = when (manifestFilePath) { - in headCommit.added -> ManifestFileAction.ADDED - else -> ManifestFileAction.MODIFIED - } - logger.info("Manifest file $action in repository $repositoryFullName") - val fileContent = gitHubGraphQLService.getManifestFileContent( - organizationName, + handleAddedOrModifiedManifestFile( + headCommit, + repositoryFullName, + owner, repositoryName, - manifestFilePath, - installationToken - ) - webSocketService.sendMessage( - "/events/manifestFile", - ManifestFileUpdateDto( - repositoryFullName, - action, - ManifestFileDto(manifestFilePath, fileContent) - ) + installationToken, + manifestFilePath ) } in headCommit.removed -> { - logger.info("Manifest file ${ManifestFileAction.REMOVED} from repository $repositoryFullName") - webSocketService.sendMessage( - "/events/manifestFile", - ManifestFileUpdateDto( - repositoryFullName, - ManifestFileAction.REMOVED, - ManifestFileDto(null, null) - ) - ) + handleRemovedManifestFile(repositoryFullName) } } } + + private fun isManifestFileUpdated(headCommit: PushEventCommit, fileName: String): Boolean { + return headCommit.added.any { it == fileName } || + headCommit.modified.any { it == fileName } || + headCommit.removed.any { it == fileName } + } + + @SuppressWarnings("LongParameterList") + private fun handleAddedOrModifiedManifestFile( + headCommit: PushEventCommit, + repositoryFullName: String, + owner: String, + repositoryName: String, + installationToken: String, + manifestFilePath: String + ) { + val action = if (manifestFilePath in headCommit.added) ManifestFileAction.ADDED else ManifestFileAction.MODIFIED + logger.info("Manifest file $action in repository $repositoryFullName") + val fileContent = gitHubGraphQLService.getManifestFileContent( + owner, + repositoryName, + manifestFilePath, + installationToken + ) + webSocketService.sendMessage( + "/events/manifestFile", + ManifestFileUpdateDto( + repositoryFullName, + action, + fileContent + ) + ) + } + + private fun handleRemovedManifestFile(repositoryFullName: String) { + logger.info("Manifest file ${ManifestFileAction.REMOVED} from repository $repositoryFullName") + webSocketService.sendMessage( + "/events/manifestFile", + ManifestFileUpdateDto( + repositoryFullName, + ManifestFileAction.REMOVED, + null + ) + ) + } } diff --git a/src/test/kotlin/net/leanix/githubagent/services/GitHubScanningServiceTest.kt b/src/test/kotlin/net/leanix/githubagent/services/GitHubScanningServiceTest.kt index 9f26638..0e8cb0a 100644 --- a/src/test/kotlin/net/leanix/githubagent/services/GitHubScanningServiceTest.kt +++ b/src/test/kotlin/net/leanix/githubagent/services/GitHubScanningServiceTest.kt @@ -7,7 +7,6 @@ import net.leanix.githubagent.client.GitHubClient import net.leanix.githubagent.dto.Account import net.leanix.githubagent.dto.Installation import net.leanix.githubagent.dto.InstallationTokenResponse -import net.leanix.githubagent.dto.ManifestFileDto import net.leanix.githubagent.dto.Organization import net.leanix.githubagent.dto.PagedRepositories import net.leanix.githubagent.dto.RepositoryDto @@ -96,10 +95,7 @@ class GitHubScanningServiceTest { updatedAt = "2024-01-01T00:00:00Z", languages = listOf("Kotlin", "Java"), topics = listOf("test", "example"), - manifestFile = ManifestFileDto( - "leanix.yaml", - "content" - ) + manifestFileContent = "content" ) ), hasNextPage = false, diff --git a/src/test/kotlin/net/leanix/githubagent/services/WebhookEventServiceTest.kt b/src/test/kotlin/net/leanix/githubagent/services/WebhookEventServiceTest.kt index 5416663..754227a 100644 --- a/src/test/kotlin/net/leanix/githubagent/services/WebhookEventServiceTest.kt +++ b/src/test/kotlin/net/leanix/githubagent/services/WebhookEventServiceTest.kt @@ -4,7 +4,6 @@ import com.ninjasquad.springmockk.MockkBean import io.mockk.every import io.mockk.verify import net.leanix.githubagent.dto.ManifestFileAction -import net.leanix.githubagent.dto.ManifestFileDto import net.leanix.githubagent.dto.ManifestFileUpdateDto import org.junit.jupiter.api.BeforeEach import org.junit.jupiter.api.Test @@ -35,6 +34,8 @@ class WebhookEventServiceTest { fun setUp() { every { gitHubAuthenticationService.refreshTokens() } returns Unit every { webSocketService.sendMessage(any(), any()) } returns Unit + every { cachingService.get(any()) } returns "token" + every { gitHubGraphQLService.getManifestFileContent(any(), any(), any(), any()) } returns "content" } @Test @@ -64,9 +65,6 @@ class WebhookEventServiceTest { @Test fun `should process push event`() { - every { cachingService.get(any()) } returns "token" - every { gitHubGraphQLService.getManifestFileContent(any(), any(), any(), any()) } returns "content" - val payload = """{ "repository": { "name": "repo", @@ -91,7 +89,7 @@ class WebhookEventServiceTest { ManifestFileUpdateDto( "owner/repo", ManifestFileAction.MODIFIED, - ManifestFileDto("leanix.yaml", "content") + "content" ) ) } @@ -124,8 +122,6 @@ class WebhookEventServiceTest { fun `should send updates for yaml manifest file`() { val manifestFilePath = "leanix.yaml" val payload = createPushEventPayload(manifestFilePath) - every { cachingService.get(any()) } returns "token" - every { gitHubGraphQLService.getManifestFileContent(any(), any(), manifestFilePath, any()) } returns "content" webhookEventService.consumeWebhookEvent("PUSH", payload) @@ -136,7 +132,7 @@ class WebhookEventServiceTest { fun `should send updates for yml manifest file`() { val manifestFilePath = "leanix.yml" val payload = createPushEventPayload(manifestFilePath) - every { cachingService.get(any()) } returns "token" + every { gitHubGraphQLService.getManifestFileContent(any(), any(), "leanix.yaml", any()) } returns null every { gitHubGraphQLService.getManifestFileContent(any(), any(), manifestFilePath, any()) } returns "content" webhookEventService.consumeWebhookEvent("PUSH", payload) @@ -144,6 +140,20 @@ class WebhookEventServiceTest { verify { webSocketService.sendMessage(any(), any()) } } + @Test + fun `should ignore yml file changes if yaml file exist in repository`() { + val manifestFilePath = "leanix.yml" + val payload = createPushEventPayload(manifestFilePath) + + every { + gitHubGraphQLService.getManifestFileContent("test-owner", "test-repo", "manifest.yaml", "token") + } returns "content" + + webhookEventService.consumeWebhookEvent("PUSH", payload) + + verify(exactly = 0) { webSocketService.sendMessage(any(), any()) } + } + private fun createPushEventPayload(manifestFileName: String): String { return """ { From e37310c720c04838aa9fb46f5791433d2db675cd Mon Sep 17 00:00:00 2001 From: mohamedlajmileanix Date: Tue, 13 Aug 2024 16:48:06 +0200 Subject: [PATCH 5/8] CID-2874: undo unnecessary change --- src/main/kotlin/net/leanix/githubagent/dto/RepositoryDto.kt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/main/kotlin/net/leanix/githubagent/dto/RepositoryDto.kt b/src/main/kotlin/net/leanix/githubagent/dto/RepositoryDto.kt index 848a45a..824e4ae 100644 --- a/src/main/kotlin/net/leanix/githubagent/dto/RepositoryDto.kt +++ b/src/main/kotlin/net/leanix/githubagent/dto/RepositoryDto.kt @@ -13,5 +13,5 @@ data class RepositoryDto( val updatedAt: String, val languages: List, val topics: List, - val manifestFileContent: String? + val manifestFileContent: String?, ) From c986c7e8a360d5aabee6cf58de5309ebb110e379 Mon Sep 17 00:00:00 2001 From: mohamedlajmileanix Date: Tue, 13 Aug 2024 16:49:00 +0200 Subject: [PATCH 6/8] CID-2874: undo unnecessary change --- .../net/leanix/githubagent/services/GitHubGraphQLService.kt | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/main/kotlin/net/leanix/githubagent/services/GitHubGraphQLService.kt b/src/main/kotlin/net/leanix/githubagent/services/GitHubGraphQLService.kt index 2c256d2..85afda9 100644 --- a/src/main/kotlin/net/leanix/githubagent/services/GitHubGraphQLService.kt +++ b/src/main/kotlin/net/leanix/githubagent/services/GitHubGraphQLService.kt @@ -78,7 +78,7 @@ class GitHubGraphQLService( fun getManifestFileContent( owner: String, repositoryName: String, - fileName: String, + filePath: String, token: String ): String? { val client = buildGitHubGraphQLClient(token) @@ -87,7 +87,7 @@ class GitHubGraphQLService( GetRepositoryManifestContent.Variables( owner = owner, repositoryName = repositoryName, - manifestFilePath = "HEAD:$fileName" + manifestFilePath = "HEAD:$filePath" ) ) From a06173ed7674a8b0c50a158ad99fdf3e562b6bbf Mon Sep 17 00:00:00 2001 From: mohamedlajmileanix Date: Tue, 13 Aug 2024 16:53:14 +0200 Subject: [PATCH 7/8] CID-2874: refactor code --- .../services/WebhookEventService.kt | 21 +++++++------------ 1 file changed, 8 insertions(+), 13 deletions(-) diff --git a/src/main/kotlin/net/leanix/githubagent/services/WebhookEventService.kt b/src/main/kotlin/net/leanix/githubagent/services/WebhookEventService.kt index d2dbc9a..94d692f 100644 --- a/src/main/kotlin/net/leanix/githubagent/services/WebhookEventService.kt +++ b/src/main/kotlin/net/leanix/githubagent/services/WebhookEventService.kt @@ -66,32 +66,27 @@ class WebhookEventService( repositoryName: String, installationToken: String ) { - val isYAMLFileUpdated = - isManifestFileUpdated( - headCommit, - "${gitHubEnterpriseProperties.manifestFileDirectory}${ManifestFileName.YAML.fileName}" - ) - val isYMLFileUpdated = - isManifestFileUpdated( - headCommit, - "${gitHubEnterpriseProperties.manifestFileDirectory}${ManifestFileName.YML.fileName}" - ) + val yamlFileName = "${gitHubEnterpriseProperties.manifestFileDirectory}${ManifestFileName.YAML.fileName}" + val ymlFileName = "${gitHubEnterpriseProperties.manifestFileDirectory}${ManifestFileName.YML.fileName}" + + val isYAMLFileUpdated = isManifestFileUpdated(headCommit, yamlFileName) + val isYMLFileUpdated = isManifestFileUpdated(headCommit, ymlFileName) // ignore updates on YML file if YAML file exists if (!isYAMLFileUpdated && isYMLFileUpdated) { val yamlFileContent = gitHubGraphQLService.getManifestFileContent( owner, repositoryName, - "${gitHubEnterpriseProperties.manifestFileDirectory}${ManifestFileName.YAML.fileName}", + yamlFileName, installationToken ) if (yamlFileContent != null) return } val manifestFilePath = if (isYAMLFileUpdated) { - "${gitHubEnterpriseProperties.manifestFileDirectory}${ManifestFileName.YAML.fileName}" + yamlFileName } else if (isYMLFileUpdated) { - "${gitHubEnterpriseProperties.manifestFileDirectory}${ManifestFileName.YML.fileName}" + ymlFileName } else { return } From 2d4d205fbd4800939db2cb8f13feee0169f70935 Mon Sep 17 00:00:00 2001 From: mohamedlajmileanix Date: Tue, 13 Aug 2024 16:58:12 +0200 Subject: [PATCH 8/8] CID-2874: refactor code --- .../services/WebhookEventService.kt | 66 +++++++++++-------- 1 file changed, 39 insertions(+), 27 deletions(-) diff --git a/src/main/kotlin/net/leanix/githubagent/services/WebhookEventService.kt b/src/main/kotlin/net/leanix/githubagent/services/WebhookEventService.kt index 94d692f..180f4b6 100644 --- a/src/main/kotlin/net/leanix/githubagent/services/WebhookEventService.kt +++ b/src/main/kotlin/net/leanix/githubagent/services/WebhookEventService.kt @@ -40,12 +40,7 @@ class WebhookEventService( val headCommit = pushEventPayload.headCommit val owner = pushEventPayload.repository.owner.name - var installationToken = cachingService.get("installationToken:${pushEventPayload.installation.id}")?.toString() - if (installationToken == null) { - gitHubAuthenticationService.refreshTokens() - installationToken = cachingService.get("installationToken:${pushEventPayload.installation.id}")?.toString() - require(installationToken != null) { "Installation token not found/ expired" } - } + val installationToken = getInstallationToken(pushEventPayload.installation.id) if (pushEventPayload.ref == "refs/heads/${pushEventPayload.repository.defaultBranch}") { handleManifestFileChanges( @@ -72,7 +67,6 @@ class WebhookEventService( val isYAMLFileUpdated = isManifestFileUpdated(headCommit, yamlFileName) val isYMLFileUpdated = isManifestFileUpdated(headCommit, ymlFileName) - // ignore updates on YML file if YAML file exists if (!isYAMLFileUpdated && isYMLFileUpdated) { val yamlFileContent = gitHubGraphQLService.getManifestFileContent( owner, @@ -83,29 +77,34 @@ class WebhookEventService( if (yamlFileContent != null) return } - val manifestFilePath = if (isYAMLFileUpdated) { - yamlFileName - } else if (isYMLFileUpdated) { - ymlFileName - } else { - return + val manifestFilePath = determineManifestFilePath(isYAMLFileUpdated, isYMLFileUpdated, yamlFileName, ymlFileName) + manifestFilePath?.let { + when (it) { + in headCommit.added, in headCommit.modified -> { + handleAddedOrModifiedManifestFile( + headCommit, + repositoryFullName, + owner, + repositoryName, + installationToken, + it + ) + } + in headCommit.removed -> { + handleRemovedManifestFile(repositoryFullName) + } + } } + } - when (manifestFilePath) { - in headCommit.added, in headCommit.modified -> { - handleAddedOrModifiedManifestFile( - headCommit, - repositoryFullName, - owner, - repositoryName, - installationToken, - manifestFilePath - ) - } - in headCommit.removed -> { - handleRemovedManifestFile(repositoryFullName) - } + private fun getInstallationToken(installationId: Int): String { + var installationToken = cachingService.get("installationToken:$installationId")?.toString() + if (installationToken == null) { + gitHubAuthenticationService.refreshTokens() + installationToken = cachingService.get("installationToken:$installationId")?.toString() + require(installationToken != null) { "Installation token not found/ expired" } } + return installationToken } private fun isManifestFileUpdated(headCommit: PushEventCommit, fileName: String): Boolean { @@ -114,6 +113,19 @@ class WebhookEventService( headCommit.removed.any { it == fileName } } + private fun determineManifestFilePath( + isYAMLFileUpdated: Boolean, + isYMLFileUpdated: Boolean, + yamlFileName: String, + ymlFileName: String + ): String? { + return when { + isYAMLFileUpdated -> yamlFileName + isYMLFileUpdated -> ymlFileName + else -> null + } + } + @SuppressWarnings("LongParameterList") private fun handleAddedOrModifiedManifestFile( headCommit: PushEventCommit,