diff --git a/src/main/groovy/com/cloudogu/gitops/application/orchestration/GitHandler.groovy b/src/main/groovy/com/cloudogu/gitops/application/orchestration/GitHandler.groovy index 9fd203348..29931a3b9 100644 --- a/src/main/groovy/com/cloudogu/gitops/application/orchestration/GitHandler.groovy +++ b/src/main/groovy/com/cloudogu/gitops/application/orchestration/GitHandler.groovy @@ -35,7 +35,7 @@ class GitHandler { config.scm.scmManager.internal = false config.scm.scmManager.urlForJenkins = config.scm.scmManager.url } else { - log.debug("Setting configs for internal SCM-Manager") + log.debug('Setting configs for internal SCM-Manager') config.scm.scmManager.internal = true config.scm.scmManager.urlForJenkins = "http://scmm.${config.application.namePrefix}${config.scm.scmManager.namespace}.svc.cluster.local/scm" @@ -73,19 +73,19 @@ class GitHandler { return tenant } - throw new IllegalStateException("No SCM provider found.") + throw new IllegalStateException('No SCM provider found.') } private GitProvider createTenantScmProvider() { switch (config.scm.scmProviderType) { case ScmProviderType.GITLAB: return new GitlabProvider(config, config.scm.gitlab) - case ScmProviderType.SCM_MANAGER: return new ScmManagerProvider(config, config.scm.scmManager, k8sClient, - networkingUtils) + networkingUtils, + config.application.namePrefix ?: '') default: throw new IllegalArgumentException("Unsupported SCM provider found in TenantSCM: ${config.scm.scmProviderType}") @@ -96,12 +96,12 @@ class GitHandler { switch (config.multiTenant.scmProviderType) { case ScmProviderType.GITLAB: return new GitlabProvider(config, config.multiTenant.gitlab) - case ScmProviderType.SCM_MANAGER: return new ScmManagerProvider(config, config.multiTenant.scmManager, k8sClient, - networkingUtils) + networkingUtils, + centralScmManagerServicePrefix()) default: throw new IllegalArgumentException("Unsupported SCM-Central provider: ${config.multiTenant.scmProviderType}") @@ -109,23 +109,19 @@ class GitHandler { } private void setupExternalRepositoriesIfPossible() { - final String namePrefix = (config.application.namePrefix ?: "").trim() + final String namePrefix = (config.application.namePrefix ?: '').trim() final boolean repositorySetupBlockedByInternalScmBootstrap = isRepositorySetupBlockedByInternalScmBootstrap() - log.info( - "Evaluating repository setup: centralConfigured={}, tenantConfigured={}, namePrefix='{}', repositorySetupBlockedByInternalScmBootstrap={}", + log.info("Evaluating repository setup: centralConfigured={}, tenantConfigured={}, namePrefix='{}', repositorySetupBlockedByInternalScmBootstrap={}", central != null, tenant != null, namePrefix, - repositorySetupBlockedByInternalScmBootstrap - ) + repositorySetupBlockedByInternalScmBootstrap) if (repositorySetupBlockedByInternalScmBootstrap) { - log.info( - "Skipping repository setup because the configured internal SCM-Manager is not deployed yet. " + - "Repository setup can continue immediately when an external SCM-Manager is configured. namePrefix='{}'", - namePrefix - ) + log.info('Skipping repository setup because the configured internal SCM-Manager is not deployed yet. ' + + "Repository setup can continue immediately when an external SCM-Manager is configured. namePrefix='{}'", + namePrefix) return } @@ -138,13 +134,14 @@ class GitHandler { setupRepos(tenant, namePrefix) } } + private boolean isRepositorySetupBlockedByInternalScmBootstrap() { - config.scm.scmProviderType == ScmProviderType.SCM_MANAGER && config.scm.scmManager?.internal + return config.scm.scmProviderType == ScmProviderType.SCM_MANAGER && config.scm.scmManager?.internal } - static void setupRepos(GitProvider gitProvider, String namePrefix = "") { - gitProvider.createRepository(withOrgPrefix(namePrefix, "argocd/cluster-resources"), - "GitOps repo for basic cluster-resources") + static void setupRepos(GitProvider gitProvider, String namePrefix = '') { + gitProvider.createRepository(withOrgPrefix(namePrefix, 'argocd/cluster-resources'), + 'GitOps repo for basic cluster-resources') } static String withOrgPrefix(String prefix, String repoPath) { @@ -154,4 +151,16 @@ class GitHandler { return prefix + repoPath } + + + private String centralScmManagerServicePrefix() { + def namespace = (config.multiTenant.scmManager.namespace ?: '').strip() + def baseNamespace = 'scm-manager' + + if (namespace == baseNamespace || !namespace.endsWith(baseNamespace)) { + return '' + } + + return namespace.substring(0, namespace.length() - baseNamespace.length()) + } } \ No newline at end of file diff --git a/src/main/groovy/com/cloudogu/gitops/infrastructure/git/providers/GitProvider.groovy b/src/main/groovy/com/cloudogu/gitops/infrastructure/git/providers/GitProvider.groovy index 1415a59d3..b529f63dd 100644 --- a/src/main/groovy/com/cloudogu/gitops/infrastructure/git/providers/GitProvider.groovy +++ b/src/main/groovy/com/cloudogu/gitops/infrastructure/git/providers/GitProvider.groovy @@ -24,23 +24,6 @@ interface GitProvider { URI prometheusMetricsEndpoint() - /** - * Deletes the given repository on the provider, if supported. - * Note: This capability is not used by the current destruction flow, - * which talks directly to provider-specific clients (e.g. ScmManagerApiClient).*/ - void deleteRepository(String namespace, String repository, boolean prefixNamespace) - - /** - * Deletes a user account on the provider, if supported. - * Note: Not used by the current destruction flow; kept as an optional capability - * on the GitProvider abstraction */ - void deleteUser(String name) - - /** - * Sets the default branch of a repository, if supported by the provider; - * kept as an optional capability on the GitProvider abstraction */ - void setDefaultBranch(String repoTarget, String branch) - String getUrl() String getProtocol() diff --git a/src/main/groovy/com/cloudogu/gitops/infrastructure/git/providers/gitlab/GitlabProvider.groovy b/src/main/groovy/com/cloudogu/gitops/infrastructure/git/providers/gitlab/GitlabProvider.groovy index 53f023c0a..974dc61cf 100644 --- a/src/main/groovy/com/cloudogu/gitops/infrastructure/git/providers/gitlab/GitlabProvider.groovy +++ b/src/main/groovy/com/cloudogu/gitops/infrastructure/git/providers/gitlab/GitlabProvider.groovy @@ -135,30 +135,6 @@ class GitlabProvider implements GitProvider { return null } - /** - * No-op by design. GitLab repository deletion is not managed through this abstraction. - * Kept for interface compatibility only.*/ - @Override - void deleteRepository(String namespace, String repository, boolean prefixNamespace) { - // intentionally left blank - } - - /** - * No-op by design. User deletion is not supported or handled through this provider. - * Kept for interface compatibility only.*/ - @Override - void deleteUser(String name) { - // intentionally left blank - } - - /** - * No-op by design. Default branch management is not implemented via this abstraction. - * Kept for interface compatibility only.*/ - @Override - void setDefaultBranch(String repoTarget, String branch) { - // intentionally left blank - } - private Group parentGroup() { String raw = gitlabConfig?.parentGroupId?.trim() if (!raw) throw new IllegalArgumentException("--gitlab-group-id is required") diff --git a/src/main/groovy/com/cloudogu/gitops/infrastructure/git/providers/scmmanager/ScmManagerProvider.groovy b/src/main/groovy/com/cloudogu/gitops/infrastructure/git/providers/scmmanager/ScmManagerProvider.groovy index 2656b05c5..8d8d5eeb3 100644 --- a/src/main/groovy/com/cloudogu/gitops/infrastructure/git/providers/scmmanager/ScmManagerProvider.groovy +++ b/src/main/groovy/com/cloudogu/gitops/infrastructure/git/providers/scmmanager/ScmManagerProvider.groovy @@ -28,9 +28,10 @@ class ScmManagerProvider implements GitProvider { Config config ScmManagerProvider(Config config, - ScmManagerConfig scmmConfig, - K8sClient k8sClient, - NetworkingUtils networkingUtils) { + ScmManagerConfig scmmConfig, + K8sClient k8sClient, + NetworkingUtils networkingUtils, + String servicePrefix = '') { this.scmmConfig = scmmConfig this.config = config this.k8sClient = k8sClient @@ -39,7 +40,8 @@ class ScmManagerProvider implements GitProvider { this.urls = new ScmManagerUrlResolver(this.config, this.scmmConfig, this.k8sClient, - this.networkingUtils) + this.networkingUtils, + servicePrefix) } ScmManagerApiClient getApiClient() { @@ -56,7 +58,7 @@ class ScmManagerProvider implements GitProvider { boolean createRepository(String repoTarget, String description, boolean initialize = true) { def repoNamespace = repoTarget.split('/', 2)[0] def repoName = repoTarget.split('/', 2)[1] - def repo = new Repository(repoNamespace, repoName, description ?: "") + def repo = new Repository(repoNamespace, repoName, description ?: '') Response response = getApiClient().repositoryApi().create(repo, initialize).execute() return handle201or409(response, "Repository ${repoNamespace}/${repoName}") @@ -125,21 +127,6 @@ class ScmManagerProvider implements GitProvider { return urls.prometheusEndpoint() } - @Override - void deleteRepository(String namespace, String repository, boolean prefixNamespace) { - // intentionally left blank - } - - @Override - void deleteUser(String name) { - // intentionally left blank - } - - @Override - void setDefaultBranch(String repoTarget, String branch) { - // intentionally left blank - } - private static Permission.Role mapToScmManager(AccessRole role) { switch (role) { case AccessRole.READ: @@ -158,32 +145,17 @@ class ScmManagerProvider implements GitProvider { } } - private static boolean handle201or409(Response response, String what) { - int code = response.code() - - if (code == 409) { - log.debug("${what} already exists - ignoring HTTP 409") - return false + private static boolean handle201or409(Response response, String resourceName) { + if (response.code() == 201) { + log.debug("${resourceName} created successfully") + return true } - if (code != 201) { - throw new RuntimeException("Could not create ${what}. HTTP Details: ${response.code()} ${response.message()}: ${response.errorBody()?.string()}") + if (response.code() == 409) { + log.debug("${resourceName} already exists") + return false } - return true - } - - /** - * Test-only constructor.*/ - ScmManagerProvider(Config config, - ScmManagerConfig scmmConfig, - ScmManagerUrlResolver urls, - ScmManagerApiClient apiClient) { - this.scmmConfig = Objects.requireNonNull(scmmConfig, "scmmConfig must not be null") - this.config = Objects.requireNonNull(config, "config must not be null") - this.urls = Objects.requireNonNull(urls, "urls must not be null") - this.apiClient = apiClient ?: new ScmManagerApiClient(urls.clientApiBase().toString(), - scmmConfig.credentials, - config.application.insecure) + throw new RuntimeException("Failed to create ${resourceName}. HTTP Status: ${response.code()} - ${response.message()}") } } \ No newline at end of file diff --git a/src/main/groovy/com/cloudogu/gitops/infrastructure/git/providers/scmmanager/ScmManagerUrlResolver.groovy b/src/main/groovy/com/cloudogu/gitops/infrastructure/git/providers/scmmanager/ScmManagerUrlResolver.groovy index b5aac3d46..10b45e7d8 100644 --- a/src/main/groovy/com/cloudogu/gitops/infrastructure/git/providers/scmmanager/ScmManagerUrlResolver.groovy +++ b/src/main/groovy/com/cloudogu/gitops/infrastructure/git/providers/scmmanager/ScmManagerUrlResolver.groovy @@ -14,43 +14,49 @@ class ScmManagerUrlResolver { private final ScmManagerConfig scmm private final K8sClient k8s private final NetworkingUtils net + private final String servicePrefix private URI cachedClusterBind private final String releaseName = 'scmm' - ScmManagerUrlResolver(Config config, ScmManagerConfig scmm, K8sClient k8s, NetworkingUtils net) { + ScmManagerUrlResolver(Config config, + ScmManagerConfig scmm, + K8sClient k8s, + NetworkingUtils net, + String servicePrefix = '') { this.config = config this.scmm = scmm this.k8s = k8s this.net = net + this.servicePrefix = servicePrefix ?: '' } // ---------- Public API used by ScmManager ---------- /** Client base …/scm (no trailing slash) */ URI clientBase() { - noTrailSlash(ensureScm(clientBaseRaw())) + return noTrailSlash(ensureScm(clientBaseRaw())) } /** Client API base …/scm/api/ */ URI clientApiBase() { - withSlash(clientBase()).resolve("api/") + return withSlash(clientBase()).resolve('api/') } /** Client repo base …/scm/repo (no trailing slash) */ URI clientRepoBase() { - noTrailSlash(withSlash(clientBase()).resolve("${root()}/")) + return noTrailSlash(withSlash(clientBase()).resolve("${root()}/")) } /** In-cluster base …/scm (no trailing slash) */ URI inClusterBase() { - noTrailSlash(ensureScm(inClusterBaseRaw())) + return noTrailSlash(ensureScm(inClusterBaseRaw())) } /** In-cluster repo prefix …/scm/repo/[] */ String inClusterRepoPrefix() { - def prefix = (config.application.namePrefix ?: "").strip() + def prefix = (config.application.namePrefix ?: '').strip() def base = withSlash(inClusterBase()) def url = withSlash(base.resolve(root())) @@ -60,21 +66,20 @@ class ScmManagerUrlResolver { /** In-cluster repo URL …/scm/repo// */ String inClusterRepoUrl(String repoTarget) { def repo = repoTarget.strip() - noTrailSlash(withSlash(inClusterBase()).resolve("${root()}/${repo}/")).toString() + return noTrailSlash(withSlash(inClusterBase()).resolve("${root()}/${repo}/")).toString() } /** Client repo URL …/scm/repo// (no trailing slash) */ String clientRepoUrl(String repoTarget) { def repo = repoTarget.strip() - noTrailSlash(withSlash(clientRepoBase()).resolve("${repo}/")).toString() + return noTrailSlash(withSlash(clientRepoBase()).resolve("${repo}/")).toString() } /** …/scm/api/v2/metrics/prometheus */ URI prometheusEndpoint() { - withSlash(clientBase()).resolve("api/v2/metrics/prometheus") + return withSlash(clientBase()).resolve('api/v2/metrics/prometheus') } - // ---------- Base resolution ---------- private URI clientBaseRaw() { @@ -87,32 +92,29 @@ class ScmManagerUrlResolver { } private URI serviceDnsBase() { - def namespace = (scmm.namespace ?: "scm-manager").strip() - URI.create("http://${serviceName()}.${namespace}.svc.cluster.local") + return URI.create("http://${serviceName()}.${serviceNamespace()}.svc.cluster.local") } private URI externalBase() { - def url = (scmm.url ?: "").strip() + def url = (scmm.url ?: '').strip() if (url) return URI.create(url) - def ingress = (scmm.ingress ?: "").strip() + def ingress = (scmm.ingress ?: '').strip() if (ingress) return URI.create("http://${ingress}") - throw new IllegalArgumentException("Either scmm.url or scmm.ingress must be set when internal=false") + throw new IllegalArgumentException('Either scmm.url or scmm.ingress must be set when internal=false') } private URI nodePortBase() { if (cachedClusterBind) return cachedClusterBind - def namespace = (scmm.namespace ?: "scm-manager").strip() - - final def port = k8s.waitForNodePort(serviceName(), namespace) + final def port = k8s.waitForNodePort(serviceName(), serviceNamespace()) final def host = net.findClusterBindAddress() cachedClusterBind = new URI("http://${host}:${port}") return cachedClusterBind } private String serviceName() { - def prefix = (config.application.namePrefix ?: '').strip() + def prefix = servicePrefix.strip() if (prefix) { return "${prefix}${releaseName}" @@ -121,6 +123,17 @@ class ScmManagerUrlResolver { return releaseName } + private String serviceNamespace() { + def namespace = (scmm.namespace ?: 'scm-manager').strip() + def prefix = servicePrefix.strip() + + if (prefix && !namespace.startsWith(prefix)) { + return "${prefix}${namespace}" + } + + return namespace + } + // ---------- Helpers ---------- private String root() { @@ -129,17 +142,17 @@ class ScmManagerUrlResolver { private static URI ensureScm(URI u) { def us = withSlash(u) - def path = us.path ?: "" - path.endsWith("/scm/") ? us : us.resolve("scm/") + def path = us.path ?: '' + return path.endsWith('/scm/') ? us : us.resolve('scm/') } private static URI withSlash(URI u) { def s = u.toString() - s.endsWith('/') ? u : URI.create(s + '/') + return s.endsWith('/') ? u : URI.create(s + '/') } private static URI noTrailSlash(URI u) { def s = u.toString() - s.endsWith('/') ? URI.create(s.substring(0, s.length() - 1)) : u + return s.endsWith('/') ? URI.create(s[0..-2]) : u } } \ No newline at end of file diff --git a/src/test/groovy/com/cloudogu/gitops/infrastructure/git/providers/scmmanager/ScmManagerProviderTest.groovy b/src/test/groovy/com/cloudogu/gitops/infrastructure/git/providers/scmmanager/ScmManagerProviderTest.groovy index 90316cb1e..2dc3c667f 100644 --- a/src/test/groovy/com/cloudogu/gitops/infrastructure/git/providers/scmmanager/ScmManagerProviderTest.groovy +++ b/src/test/groovy/com/cloudogu/gitops/infrastructure/git/providers/scmmanager/ScmManagerProviderTest.groovy @@ -35,52 +35,55 @@ class ScmManagerProviderTest { @Mock ScmManagerConfig scmmCfg @Mock - K8sClient k8s - @Mock - NetworkingUtils net - @Mock ScmManagerUrlResolver urls @Mock ScmManagerApiClient apiClient @Mock RepositoryApi repoApi + @Mock + K8sClient k8s + @Mock + NetworkingUtils net @BeforeEach void setup() { config = new Config(application: new Config.ApplicationSchema(insecure: false, - namePrefix: "fv40-", + namePrefix: 'fv40-', runningInsideK8s: true)) - lenient().when(scmmCfg.getCredentials()).thenReturn(new Credentials("user", "password")) - lenient().when(scmmCfg.getGitOpsUsername()).thenReturn("gitops-bot") + lenient().when(scmmCfg.getCredentials()).thenReturn(new Credentials('user', 'password')) + lenient().when(scmmCfg.getGitOpsUsername()).thenReturn('gitops-bot') - lenient().when(urls.inClusterBase()).thenReturn(new URI("http://scmm.ns.svc.cluster.local/scm")) - lenient().when(urls.inClusterRepoPrefix()).thenReturn("http://scmm.ns.svc.cluster.local/scm/repo/fv40-") - lenient().when(urls.clientApiBase()).thenReturn(new URI("http://nodeport/scm/api/v2/")) + lenient().when(urls.inClusterBase()).thenReturn(new URI('http://scmm.ns.svc.cluster.local/scm')) + lenient().when(urls.inClusterRepoPrefix()).thenReturn('http://scmm.ns.svc.cluster.local/scm/repo/fv40-') + lenient().when(urls.clientApiBase()).thenReturn(new URI('http://nodeport/scm/api/v2/')) lenient().when(apiClient.repositoryApi()).thenReturn(repoApi) } - private ScmManagerProvider newSchManager() { - return new ScmManagerProvider(config, scmmCfg, urls, apiClient) + private ScmManagerProvider newScmManager() { + def scmManager = new ScmManagerProvider(config, scmmCfg, k8s, net, 'fv40-') + scmManager.urls = urls + scmManager.apiClient = apiClient + return scmManager } private static Call callReturningSuccess(int code) { def call = mock(Call) when(call.execute()).thenReturn(Response.success(code, null)) - call + return call } private static Call callReturningError(int code) { def call = mock(Call) def body = new RealResponseBody('ignored', 0, mock(BufferedSource)) when(call.execute()).thenReturn(Response.error(code, body)) - call + return call } @Test void 'createRepository returns true on 201 and false on subsequent 409 for the same repo'() { - def scmManager = newSchManager() + def scmManager = newScmManager() def created = callReturningSuccess(201) def conflict = callReturningError(409) @@ -89,79 +92,91 @@ class ScmManagerProviderTest { when(repoApi.create(any(Repository), anyBoolean())) .thenAnswer(inv -> { Repository r = inv.getArgument(0) - if (seen.contains(r.fullRepoName)) return conflict + if (seen.contains(r.fullRepoName)) { + return conflict + } + seen.add(r.fullRepoName) return created }) - assertTrue(scmManager.createRepository("team/demo", "Demo repo", true)) - assertFalse(scmManager.createRepository("team/demo", "Demo repo", true)) // 409 - assertTrue(scmManager.createRepository("team/other", null, false)) // neuer Name -> 201 + assertTrue(scmManager.createRepository('team/demo', 'Demo repo', true)) + assertFalse(scmManager.createRepository('team/demo', 'Demo repo', true)) + assertTrue(scmManager.createRepository('team/other', null, false)) verify(repoApi, times(3)).create(any(Repository), anyBoolean()) } @Test void 'setRepositoryPermission maps MAINTAIN to WRITE and handles 201 409'() { - def scmManager = newSchManager() + def scmManager = newScmManager() def created = callReturningSuccess(201) def conflict = callReturningError(409) def seen = new HashSet() - // key: ns/name when(repoApi.createPermission(anyString(), anyString(), any(Permission))) .thenAnswer(inv -> { String namespace = inv.getArgument(0) String repoName = inv.getArgument(1) - String key = namespace + "/" + repoName - if (seen.contains(key)) return conflict + String key = namespace + '/' + repoName + + if (seen.contains(key)) { + return conflict + } + seen.add(key) return created }) - assertDoesNotThrow({ -> scmManager.setRepositoryPermission("namespace/repo1", "devs", AccessRole.MAINTAIN, Scope.GROUP) + assertDoesNotThrow({ -> scmManager.setRepositoryPermission('namespace/repo1', 'devs', AccessRole.MAINTAIN, Scope.GROUP) } as Executable) - assertDoesNotThrow({ -> scmManager.setRepositoryPermission("namespace/repo1", "devs", AccessRole.MAINTAIN, Scope.GROUP) + assertDoesNotThrow({ -> scmManager.setRepositoryPermission('namespace/repo1', 'devs', AccessRole.MAINTAIN, Scope.GROUP) } as Executable) - verify(repoApi, atLeastOnce()) - .createPermission(eq("namespace"), eq("repo1"), argThat { Permission p -> p.groupPermission && p.role == Permission.Role.WRITE }) + + verify(repoApi, atLeastOnce()).createPermission(eq('namespace'), + eq('repo1'), + argThat { Permission p -> p.groupPermission && p.role == Permission.Role.WRITE + }) } @Test - void 'url, repoPrefix, repoUrl variants, protocol and host come from UrlResolver'() { - when(urls.inClusterRepoUrl(anyString())).thenAnswer(a -> "http://scmm.ns.svc.cluster.local/scm/repo/" + a.getArgument(0)) - when(urls.clientRepoUrl(anyString())).thenAnswer(a -> "http://nodeport/scm/repo/" + a.getArgument(0)) + void 'url repoPrefix repoUrl variants protocol and host come from UrlResolver'() { + when(urls.inClusterRepoUrl(anyString())).thenAnswer(a -> 'http://scmm.ns.svc.cluster.local/scm/repo/' + a.getArgument(0)) + when(urls.clientRepoUrl(anyString())).thenAnswer(a -> 'http://nodeport/scm/repo/' + a.getArgument(0)) - def scmManager = newSchManager() + def scmManager = newScmManager() - assertEquals("http://scmm.ns.svc.cluster.local/scm", scmManager.url) - assertEquals("http://scmm.ns.svc.cluster.local/scm/repo/fv40-", scmManager.repoPrefix()) + assertEquals('http://scmm.ns.svc.cluster.local/scm', scmManager.url) + assertEquals('http://scmm.ns.svc.cluster.local/scm/repo/fv40-', scmManager.repoPrefix()) - assertEquals("http://scmm.ns.svc.cluster.local/scm/repo/team/app", - scmManager.repoUrl("team/app", RepoUrlScope.IN_CLUSTER)) - assertEquals("http://nodeport/scm/repo/team/app", - scmManager.repoUrl("team/app", RepoUrlScope.CLIENT)) + assertEquals('http://scmm.ns.svc.cluster.local/scm/repo/team/app', + scmManager.repoUrl('team/app', RepoUrlScope.IN_CLUSTER)) + assertEquals('http://nodeport/scm/repo/team/app', + scmManager.repoUrl('team/app', RepoUrlScope.CLIENT)) - assertEquals("http", scmManager.protocol) - assertEquals("scmm.ns.svc.cluster.local", scmManager.host) + assertEquals('http', scmManager.protocol) + assertEquals('scmm.ns.svc.cluster.local', scmManager.host) } @Test void 'prometheusMetricsEndpoint is delegated to UrlResolver'() { - when(urls.prometheusEndpoint()).thenReturn(new URI("http://nodeport/scm/api/v2/metrics/prometheus")) - def scmManager = newSchManager() - assertEquals(new URI("http://nodeport/scm/api/v2/metrics/prometheus"), scmManager.prometheusMetricsEndpoint()) + when(urls.prometheusEndpoint()).thenReturn(new URI('http://nodeport/scm/api/v2/metrics/prometheus')) + + def scmManager = newScmManager() + + assertEquals(new URI('http://nodeport/scm/api/v2/metrics/prometheus'), + scmManager.prometheusMetricsEndpoint()) } - // Credentials & GitOps-User @Test void 'credentials and gitOpsUsername come from ScmManagerConfig'() { - def scmManager = newSchManager() - assertEquals("user", scmManager.credentials.username) - assertEquals("password", scmManager.credentials.password) - assertEquals("gitops-bot", scmManager.gitOpsUsername) + def scmManager = newScmManager() + + assertEquals('user', scmManager.credentials.username) + assertEquals('password', scmManager.credentials.password) + assertEquals('gitops-bot', scmManager.gitOpsUsername) } } \ No newline at end of file diff --git a/src/test/groovy/com/cloudogu/gitops/infrastructure/git/providers/scmmanager/ScmManagerUrlResolverTest.groovy b/src/test/groovy/com/cloudogu/gitops/infrastructure/git/providers/scmmanager/ScmManagerUrlResolverTest.groovy index 2366ac89c..7c7fdac73 100644 --- a/src/test/groovy/com/cloudogu/gitops/infrastructure/git/providers/scmmanager/ScmManagerUrlResolverTest.groovy +++ b/src/test/groovy/com/cloudogu/gitops/infrastructure/git/providers/scmmanager/ScmManagerUrlResolverTest.groovy @@ -1,7 +1,6 @@ package com.cloudogu.gitops.infrastructure.git.providers.scmmanager import static org.junit.jupiter.api.Assertions.* -import static org.mockito.ArgumentMatchers.any import static org.mockito.ArgumentMatchers.eq import static org.mockito.Mockito.* @@ -16,12 +15,14 @@ import org.junit.jupiter.api.extension.ExtendWith import org.mockito.Mock import org.mockito.junit.jupiter.MockitoExtension -@ExtendWith(MockitoExtension.class) +@ExtendWith(MockitoExtension) class ScmManagerUrlResolverTest { + private Config config @Mock private K8sClient k8s + @Mock private NetworkingUtils net @@ -31,127 +32,227 @@ class ScmManagerUrlResolverTest { runningInsideK8s: false)) } - private ScmManagerUrlResolver resolverWith(Map args = [:]) { - def scmmCofig = new ScmTenantSchema.ScmManagerTenantConfig() - scmmCofig.internal = (args.containsKey('internal') ? args.internal : true) - scmmCofig.namespace = (args.containsKey('namespace') ? args.namespace : "scm-manager") - scmmCofig.url = (args.containsKey('url') ? args.url : "") - scmmCofig.ingress = (args.containsKey('ingress') ? args.ingress : "") + private ScmManagerUrlResolver resolverWith(Map args = [:], String servicePrefix = 'fv40-') { + def scmmConfig = new ScmTenantSchema.ScmManagerTenantConfig() + scmmConfig.internal = (args.containsKey('internal') ? args.internal : true) + scmmConfig.namespace = (args.containsKey('namespace') ? args.namespace : 'scm-manager') + scmmConfig.url = (args.containsKey('url') ? args.url : '') + scmmConfig.ingress = (args.containsKey('ingress') ? args.ingress : '') - return new ScmManagerUrlResolver(config, scmmCofig, k8s, net) + return new ScmManagerUrlResolver(config, scmmConfig, k8s, net, servicePrefix) } // ---------- Client base & API ---------- + @Test - void "clientBase(): internal + outside K8s uses NodePort and appends 'scm' (no trailing slash) and only resolves NodePort once"() { - when(k8s.waitForNodePort(eq('fv40-scmm'), any())).thenReturn("30080") - when(net.findClusterBindAddress()).thenReturn("10.0.0.1") + void "clientBase(): tenant internal outside K8s uses prefixed NodePort lookup and appends 'scm' only once"() { + when(k8s.waitForNodePort('fv40-scmm', 'fv40-scm-manager')).thenReturn('30080') + when(net.findClusterBindAddress()).thenReturn('10.0.0.1') def r = resolverWith() URI base1 = r.clientBase() URI base2 = r.clientBase() - assertEquals("http://10.0.0.1:30080/scm", base1.toString()) + assertEquals('http://10.0.0.1:30080/scm', base1.toString()) assertEquals(base1, base2) - verify(k8s, times(1)).waitForNodePort("fv40-scmm", "scm-manager") + verify(k8s, times(1)).waitForNodePort('fv40-scmm', 'fv40-scm-manager') verify(net, times(1)).findClusterBindAddress() verifyNoMoreInteractions(k8s, net) } + @Test + void "clientBase(): central internal outside K8s keeps unprefixed service name and namespace"() { + when(k8s.waitForNodePort('scmm', 'scm-manager')).thenReturn('30080') + when(net.findClusterBindAddress()).thenReturn('10.0.0.1') + + def r = resolverWith([:], '') + + assertEquals('http://10.0.0.1:30080/scm', r.clientBase().toString()) + + verify(k8s).waitForNodePort('scmm', 'scm-manager') + verify(net).findClusterBindAddress() + verifyNoMoreInteractions(k8s, net) + } + @Test void "clientApiBase(): appends 'api' to the client base"() { - when(k8s.waitForNodePort("fv40-scmm", "scm-manager")).thenReturn("30080") - when(net.findClusterBindAddress()).thenReturn("10.0.0.1") + when(k8s.waitForNodePort('fv40-scmm', 'fv40-scm-manager')).thenReturn('30080') + when(net.findClusterBindAddress()).thenReturn('10.0.0.1') - var urlResolver = resolverWith() - assertEquals("http://10.0.0.1:30080/scm/api/", urlResolver.clientApiBase().toString()) + def urlResolver = resolverWith() + + assertEquals('http://10.0.0.1:30080/scm/api/', urlResolver.clientApiBase().toString()) } // ---------- Repo base & URLs ---------- + @Test void "clientRepoUrl(): trims repoTarget and removes trailing slash"() { - when(k8s.waitForNodePort("fv40-scmm", "scm-manager")).thenReturn("30080") - when(net.findClusterBindAddress()).thenReturn("10.0.0.1") + when(k8s.waitForNodePort('fv40-scmm', 'fv40-scm-manager')).thenReturn('30080') + when(net.findClusterBindAddress()).thenReturn('10.0.0.1') + + def urlResolver = resolverWith() - var urlResolver = resolverWith() - assertEquals("http://10.0.0.1:30080/scm/repo/ns/project", - urlResolver.clientRepoUrl(" ns/project ")) + assertEquals('http://10.0.0.1:30080/scm/repo/ns/project', + urlResolver.clientRepoUrl(' ns/project ')) } // ---------- In-cluster base & URLs ---------- + @Test - void "inClusterBase(): internal uses service DNS "() { - def r = resolverWith(namespace: "custom-ns", internal: true) - assertEquals("http://fv40-scmm.custom-ns.svc.cluster.local/scm", r.inClusterBase().toString()) + void "inClusterBase(): tenant internal uses prefixed service DNS"() { + config.application.runningInsideK8s = true + + def r = resolverWith() + + assertEquals('http://fv40-scmm.fv40-scm-manager.svc.cluster.local/scm', + r.inClusterBase().toString()) + } + + @Test + void "inClusterBase(): tenant internal prefixes custom namespace when needed"() { + config.application.runningInsideK8s = true + + def r = resolverWith(namespace: 'custom-ns') + + assertEquals('http://fv40-scmm.fv40-custom-ns.svc.cluster.local/scm', + r.inClusterBase().toString()) + } + + @Test + void "inClusterBase(): tenant internal does not duplicate already prefixed namespace"() { + config.application.runningInsideK8s = true + + def r = resolverWith(namespace: 'fv40-scm-manager') + + assertEquals('http://fv40-scmm.fv40-scm-manager.svc.cluster.local/scm', + r.inClusterBase().toString()) + } + + @Test + void "inClusterBase(): central internal uses unprefixed service DNS"() { + config.application.runningInsideK8s = true + + def r = resolverWith([:], '') + + assertEquals('http://scmm.scm-manager.svc.cluster.local/scm', + r.inClusterBase().toString()) } @Test void "inClusterBase(): external uses external base + 'scm'"() { - var r = resolverWith(internal: false, url: "https://fv40-scmm.external") - assertEquals("https://fv40-scmm.external/scm", r.inClusterBase().toString()) + def r = resolverWith(internal: false, url: 'https://fv40-scmm.external') + + assertEquals('https://fv40-scmm.external/scm', r.inClusterBase().toString()) } @Test - void "inClusterRepoUrl(): builds full in-cluster repo URL without trailing slash"() { - var urlResolver = resolverWith() - assertEquals("http://fv40-scmm.scm-manager.svc.cluster.local/scm/repo/admin/admin", - urlResolver.inClusterRepoUrl("admin/admin")) + void "inClusterRepoUrl(): builds full tenant in-cluster repo URL without trailing slash"() { + config.application.runningInsideK8s = true + + def urlResolver = resolverWith() + + assertEquals('http://fv40-scmm.fv40-scm-manager.svc.cluster.local/scm/repo/admin/admin', + urlResolver.inClusterRepoUrl('admin/admin')) } @Test - void "inClusterRepoPrefix(): includes configured namePrefix (empty prefix yields base path)"() { - // with non-empty namePrefix - config.application.namePrefix = 'fv40-' - def r1 = resolverWith() - assertEquals('http://fv40-scmm.scm-manager.svc.cluster.local/scm/repo/fv40-', r1.inClusterRepoPrefix()) + void "inClusterRepoPrefix(): tenant service uses servicePrefix and repo namespace uses application namePrefix"() { + config.application.runningInsideK8s = true + + def r = resolverWith() + + assertEquals('http://fv40-scmm.fv40-scm-manager.svc.cluster.local/scm/repo/fv40-', + r.inClusterRepoPrefix()) + } - // with empty/blank namePrefix + @Test + void "inClusterRepoPrefix(): central service stays unprefixed but repo namespace still uses application namePrefix"() { + config.application.runningInsideK8s = true + + def r = resolverWith([:], '') + + assertEquals('http://scmm.scm-manager.svc.cluster.local/scm/repo/fv40-', + r.inClusterRepoPrefix()) + } + + @Test + void "inClusterRepoPrefix(): empty application namePrefix yields base repo path"() { + config.application.runningInsideK8s = true config.application.namePrefix = ' ' - def r2 = resolverWith() - assertEquals('http://scmm.scm-manager.svc.cluster.local/scm/repo/', r2.inClusterRepoPrefix()) + + def r = resolverWith([:], '') + + assertEquals('http://scmm.scm-manager.svc.cluster.local/scm/repo/', + r.inClusterRepoPrefix()) } // ---------- externalBase selection & error ---------- + @Test void "externalBase(): prefers 'url' over 'ingress'"() { def r = resolverWith(internal: false, url: 'https://scmm.external', ingress: 'ingress.example.org') + assertEquals('https://scmm.external/scm', r.inClusterBase().toString()) } @Test void "externalBase(): uses 'ingress' when 'url' is missing"() { def r = resolverWith(internal: false, url: null, ingress: 'ingress.example.org') + assertEquals('http://ingress.example.org/scm', r.inClusterBase().toString()) } @Test void "externalBase(): throws when neither 'url' nor 'ingress' is set"() { def r = resolverWith(internal: false, url: null, ingress: null) - def ex = assertThrows(IllegalArgumentException) { r.inClusterBase() } + + def ex = assertThrows(IllegalArgumentException) { + r.inClusterBase() + } + assertTrue(ex.message.contains('Either scmm.url or scmm.ingress must be set when internal=false')) } @Test - void "nodePortBase(): falls back to default namespace 'scm-manager' when none provided"() { - when(k8s.waitForNodePort(eq('fv40-scmm'), eq('scm-manager'))).thenReturn("30080") + void "nodePortBase(): tenant falls back to prefixed default namespace when none provided"() { + when(k8s.waitForNodePort(eq('fv40-scmm'), eq('fv40-scm-manager'))).thenReturn('30080') when(net.findClusterBindAddress()).thenReturn('10.0.0.1') def r = resolverWith(namespace: null) + + assertEquals('http://10.0.0.1:30080/scm', r.clientBase().toString()) + } + + @Test + void "nodePortBase(): central falls back to unprefixed default namespace when none provided"() { + when(k8s.waitForNodePort(eq('scmm'), eq('scm-manager'))).thenReturn('30080') + when(net.findClusterBindAddress()).thenReturn('10.0.0.1') + + def r = resolverWith([namespace: null], '') + assertEquals('http://10.0.0.1:30080/scm', r.clientBase().toString()) } // ---------- helpers behavior ---------- + @Test void "ensureScm(): adds 'scm' if missing and keeps it if present"() { def r1 = resolverWith(internal: false, url: 'https://fv40-scmm.localhost') assertEquals('https://fv40-scmm.localhost/scm', r1.clientBase().toString()) + + def r2 = resolverWith(internal: false, url: 'https://fv40-scmm.localhost/scm') + assertEquals('https://fv40-scmm.localhost/scm', r2.clientBase().toString()) } // ---------- prometheus endpoint ---------- + @Test - void "prometheusEndpoint(): resolves "() { + void "prometheusEndpoint(): resolves"() { def r = resolverWith(internal: false, url: 'https://fv40-scmm.localhost') - assertEquals('https://fv40-scmm.localhost/scm/api/v2/metrics/prometheus', r.prometheusEndpoint().toString()) + + assertEquals('https://fv40-scmm.localhost/scm/api/v2/metrics/prometheus', + r.prometheusEndpoint().toString()) } } \ No newline at end of file diff --git a/src/test/groovy/com/cloudogu/gitops/testhelper/git/GitlabMock.groovy b/src/test/groovy/com/cloudogu/gitops/testhelper/git/GitlabMock.groovy index 27f071d2c..057fac1ac 100644 --- a/src/test/groovy/com/cloudogu/gitops/testhelper/git/GitlabMock.groovy +++ b/src/test/groovy/com/cloudogu/gitops/testhelper/git/GitlabMock.groovy @@ -54,15 +54,6 @@ class GitlabMock implements GitProvider { return new Credentials("gitops", "gitops") } - @Override - void deleteRepository(String n, String r, boolean p) {} - - @Override - void deleteUser(String name) {} - - @Override - void setDefaultBranch(String target, String branch) {} - @Override String getUrl() { return base.toString() diff --git a/src/test/groovy/com/cloudogu/gitops/testhelper/git/ScmManagerMock.groovy b/src/test/groovy/com/cloudogu/gitops/testhelper/git/ScmManagerMock.groovy index bdd74d318..aedc33afd 100644 --- a/src/test/groovy/com/cloudogu/gitops/testhelper/git/ScmManagerMock.groovy +++ b/src/test/groovy/com/cloudogu/gitops/testhelper/git/ScmManagerMock.groovy @@ -86,21 +86,6 @@ class ScmManagerMock implements GitProvider { return prometheus } - @Override - void deleteRepository(String namespace, String repository, boolean prefixNamespace) { - - } - - @Override - void deleteUser(String name) { - - } - - @Override - void setDefaultBranch(String repoTarget, String branch) { - - } - /** In-cluster base …/scm (without trailing slash) */ @Override String getUrl() {