From 2e34d711f91e93292de4d57320af990bfe0894b5 Mon Sep 17 00:00:00 2001 From: John Kasper Svergja Date: Mon, 26 Aug 2024 10:32:13 +0200 Subject: [PATCH] Cleanup unused ServiceType We only support Kubernetes as ServiceType --- .../checks/CompatibilityChecks.java | 33 ++- .../properties/RegionsConfiguration.java | 43 ++-- .../controller/api/mylab/MyLabController.java | 230 ++++++++---------- .../api/services/impl/HelmAppsService.java | 1 - .../fr/insee/onyxia/model/region/Region.java | 9 - .../insee/onyxia/model/service/Service.java | 16 -- 6 files changed, 137 insertions(+), 195 deletions(-) diff --git a/onyxia-api/src/main/java/fr/insee/onyxia/api/configuration/checks/CompatibilityChecks.java b/onyxia-api/src/main/java/fr/insee/onyxia/api/configuration/checks/CompatibilityChecks.java index dba84792..b35b10a2 100644 --- a/onyxia-api/src/main/java/fr/insee/onyxia/api/configuration/checks/CompatibilityChecks.java +++ b/onyxia-api/src/main/java/fr/insee/onyxia/api/configuration/checks/CompatibilityChecks.java @@ -2,7 +2,6 @@ import fr.insee.onyxia.api.configuration.kubernetes.KubernetesClientProvider; import fr.insee.onyxia.api.configuration.properties.RegionsConfiguration; -import fr.insee.onyxia.model.service.Service; import io.fabric8.kubernetes.client.KubernetesClient; import io.github.inseefrlab.helmwrapper.service.HelmVersionService; import org.slf4j.Logger; @@ -63,24 +62,20 @@ public void checkKubernetesVersion() { .getResolvedRegions() .forEach( region -> { - if (region.getServices() - .getType() - .equals(Service.ServiceType.KUBERNETES)) { - KubernetesClient client = - kubernetesClientProvider.getRootClient(region); - try { - LOGGER.info( - "Region {} kubernetes v{}.{}", - region.getName(), - client.getKubernetesVersion().getMajor(), - client.getKubernetesVersion().getMinor()); - } catch (Exception e) { - LOGGER.error( - "Could not contact Kubernetes APIServer for region {} at {}", - region.getName(), - client.getMasterUrl(), - e); - } + KubernetesClient client = + kubernetesClientProvider.getRootClient(region); + try { + LOGGER.info( + "Region {} kubernetes v{}.{}", + region.getName(), + client.getKubernetesVersion().getMajor(), + client.getKubernetesVersion().getMinor()); + } catch (Exception e) { + LOGGER.error( + "Could not contact Kubernetes APIServer for region {} at {}", + region.getName(), + client.getMasterUrl(), + e); } }); } diff --git a/onyxia-api/src/main/java/fr/insee/onyxia/api/configuration/properties/RegionsConfiguration.java b/onyxia-api/src/main/java/fr/insee/onyxia/api/configuration/properties/RegionsConfiguration.java index be76cbb5..d9222d78 100644 --- a/onyxia-api/src/main/java/fr/insee/onyxia/api/configuration/properties/RegionsConfiguration.java +++ b/onyxia-api/src/main/java/fr/insee/onyxia/api/configuration/properties/RegionsConfiguration.java @@ -3,7 +3,6 @@ import com.fasterxml.jackson.databind.JsonNode; import com.fasterxml.jackson.databind.ObjectMapper; import fr.insee.onyxia.model.region.Region; -import fr.insee.onyxia.model.service.Service; import jakarta.annotation.PostConstruct; import java.io.IOException; import java.util.Arrays; @@ -44,30 +43,28 @@ public void load() throws Exception { resolvedRegions = Arrays.asList(mapper.readValue(regions, Region[].class)); resolvedRegions.forEach( region -> { - if (region.getServices().getType().equals(Service.ServiceType.KUBERNETES)) { - if (region.getServices() - .getAuthenticationMode() - .equals(Region.Services.AuthenticationMode.SERVICEACCOUNT)) { - LOGGER.warn( - "Using serviceAccount authentication for region {}. Onyxia will deploy services using it's own global permissions, this may be a security issue.", - region.getId()); - } + if (region.getServices() + .getAuthenticationMode() + .equals(Region.Services.AuthenticationMode.SERVICEACCOUNT)) { + LOGGER.warn( + "Using serviceAccount authentication for region {}. Onyxia will deploy services using it's own global permissions, this may be a security issue.", + region.getId()); + } - if (region.getServices() - .getAuthenticationMode() - .equals(Region.Services.AuthenticationMode.IMPERSONATE)) { - LOGGER.info( - "Using impersonation authentication for region {}.", - region.getId()); - } + if (region.getServices() + .getAuthenticationMode() + .equals(Region.Services.AuthenticationMode.IMPERSONATE)) { + LOGGER.info( + "Using impersonation authentication for region {}.", + region.getId()); + } - if (region.getServices() - .getAuthenticationMode() - .equals(Region.Services.AuthenticationMode.TOKEN_PASSTHROUGH)) { - LOGGER.info( - "Using token passthrough authentication for region {}. User token will be used by Onyxia to interact with the API Server.", - region.getId()); - } + if (region.getServices() + .getAuthenticationMode() + .equals(Region.Services.AuthenticationMode.TOKEN_PASSTHROUGH)) { + LOGGER.info( + "Using token passthrough authentication for region {}. User token will be used by Onyxia to interact with the API Server.", + region.getId()); } }); } diff --git a/onyxia-api/src/main/java/fr/insee/onyxia/api/controller/api/mylab/MyLabController.java b/onyxia-api/src/main/java/fr/insee/onyxia/api/controller/api/mylab/MyLabController.java index 8451c222..de64480f 100644 --- a/onyxia-api/src/main/java/fr/insee/onyxia/api/controller/api/mylab/MyLabController.java +++ b/onyxia-api/src/main/java/fr/insee/onyxia/api/controller/api/mylab/MyLabController.java @@ -89,9 +89,7 @@ public ServicesListing getMyServices( User user = userProvider.getUser(region); ServicesListing dto = new ServicesListing(); List> futures = new ArrayList<>(); - if (region.getServices().getType().equals(Service.ServiceType.KUBERNETES)) { - futures.add(helmAppsService.getUserServices(region, project, user, groupId)); - } + futures.add(helmAppsService.getUserServices(region, project, user, groupId)); for (var future : futures) { ServicesListing listing = future.get(); dto.getApps().addAll(listing.getApps()); @@ -138,11 +136,8 @@ public Service getApp( @Parameter(hidden = true) Project project, @RequestParam("serviceId") String serviceId) throws Exception { - if (Service.ServiceType.KUBERNETES.equals(region.getServices().getType())) { - return helmAppsService.getUserService( - region, project, userProvider.getUser(region), serviceId); - } - return null; + return helmAppsService.getUserService( + region, project, userProvider.getUser(region), serviceId); } @PostMapping("/app/rename") @@ -151,15 +146,12 @@ public void renameApp( @Parameter(hidden = true) Project project, @RequestBody RenameRequestDTO request) throws Exception { - if (Service.ServiceType.KUBERNETES.equals(region.getServices().getType())) { - User user = userProvider.getUser(region); - helmAppsService.rename( - region, - project, - userProvider.getUser(region), - request.getServiceID(), - request.getFriendlyName()); - } + helmAppsService.rename( + region, + project, + userProvider.getUser(region), + request.getServiceID(), + request.getFriendlyName()); } @PostMapping("/app/share") @@ -168,15 +160,12 @@ public void shareApp( @Parameter(hidden = true) Project project, @RequestBody ShareRequestDTO request) throws Exception { - if (Service.ServiceType.KUBERNETES.equals(region.getServices().getType())) { - User user = userProvider.getUser(region); - helmAppsService.share( - region, - project, - userProvider.getUser(region), - request.getServiceID(), - request.isShare()); - } + helmAppsService.share( + region, + project, + userProvider.getUser(region), + request.getServiceID(), + request.isShare()); } @PostMapping("/app/suspend") @@ -199,70 +188,67 @@ public void resumeApp( private void suspendOrResume(Region region, Project project, String serviceId, boolean suspend) throws Exception { - if (Service.ServiceType.KUBERNETES.equals(region.getServices().getType())) { - User user = userProvider.getUser(region); - Service userService = - helmAppsService.getUserService( - region, project, userProvider.getUser(region), serviceId); - if (!userService.isSuspendable()) { - throw new ServiceNotSuspendableException(); - } - String chart = userService.getChart(); - int split = chart.lastIndexOf('-'); - String chartName = chart.substring(0, split); - String version = chart.substring(split + 1); - String catalogId = userService.getCatalogId(); - // This code is for legacy compat for services that were created with Onyxia < v2.7.0 - // before introduction of Onyxia's secret - if (catalogId == null) { - List elligibleCatalogs = - catalogService.getCatalogs(region, user).getCatalogs().stream() - .filter( - catalog -> - catalog.getCatalog() - .getPackageByName(chartName) - .isPresent()) - .toList(); - if (elligibleCatalogs.isEmpty()) { - throw new NotFoundException(); - } - if (elligibleCatalogs.size() > 1) { - throw new IllegalStateException("Chart is present in multiple catalogs, abort"); - } - CatalogWrapper catalog = elligibleCatalogs.getFirst(); - catalogId = catalog.getId(); + User user = userProvider.getUser(region); + Service userService = + helmAppsService.getUserService( + region, project, userProvider.getUser(region), serviceId); + if (!userService.isSuspendable()) { + throw new ServiceNotSuspendableException(); + } + String chart = userService.getChart(); + int split = chart.lastIndexOf('-'); + String chartName = chart.substring(0, split); + String version = chart.substring(split + 1); + String catalogId = userService.getCatalogId(); + // This code is for legacy compat for services that were created with Onyxia < v2.7.0 + // before introduction of Onyxia's secret + if (catalogId == null) { + List elligibleCatalogs = + catalogService.getCatalogs(region, user).getCatalogs().stream() + .filter( + catalog -> + catalog.getCatalog() + .getPackageByName(chartName) + .isPresent()) + .toList(); + if (elligibleCatalogs.isEmpty()) { + throw new NotFoundException(); } - Optional catalog = catalogService.getCatalogById(catalogId, user); - if (catalog.isEmpty()) { - throw new IllegalStateException( - "Catalog " + catalogId + " is not available anymore"); + if (elligibleCatalogs.size() > 1) { + throw new IllegalStateException("Chart is present in multiple catalogs, abort"); } + CatalogWrapper catalog = elligibleCatalogs.getFirst(); + catalogId = catalog.getId(); + } + Optional catalog = catalogService.getCatalogById(catalogId, user); + if (catalog.isEmpty()) { + throw new IllegalStateException("Catalog " + catalogId + " is not available anymore"); + } - if (suspend) { - helmAppsService.suspend( - region, - project, - catalog.get().getId(), - chartName, - version, - user, - serviceId, - catalog.get().getSkipTlsVerify(), - catalog.get().getCaFile(), - false); - } else { - helmAppsService.resume( - region, - project, - catalog.get().getId(), - chartName, - version, - user, - serviceId, - catalog.get().getSkipTlsVerify(), - catalog.get().getCaFile(), - false); - } + if (suspend) { + helmAppsService.suspend( + region, + project, + catalog.get().getId(), + chartName, + version, + user, + serviceId, + catalog.get().getSkipTlsVerify(), + catalog.get().getCaFile(), + false); + } else { + helmAppsService.resume( + region, + project, + catalog.get().getId(), + chartName, + version, + user, + serviceId, + catalog.get().getSkipTlsVerify(), + catalog.get().getCaFile(), + false); } } @@ -299,11 +285,8 @@ public String getLogs( @Parameter(hidden = true) Project project, @RequestParam("serviceId") String serviceId, @RequestParam("taskId") String taskId) { - if (Service.ServiceType.KUBERNETES.equals(region.getServices().getType())) { - return helmAppsService.getLogs( - region, project, userProvider.getUser(region), serviceId, taskId); - } - return null; + return helmAppsService.getLogs( + region, project, userProvider.getUser(region), serviceId, taskId); } @Operation( @@ -327,36 +310,32 @@ public SseEmitter getEvents( @Parameter(hidden = true) Region region, @Parameter(hidden = true) Project project) throws Exception { - if (Service.ServiceType.KUBERNETES.equals(region.getServices().getType())) { - final SseEmitter emitter = new SseEmitter(); - final CustomWatcher watcher = new CustomWatcher(emitter, objectMapper); - final Watch watch = - helmAppsService.getEvents( - region, project, userProvider.getUser(region), watcher); - emitter.onCompletion( - new Runnable() { - @Override - public void run() { - watch.close(); - } - }); - emitter.onError( - new Consumer() { - @Override - public void accept(Throwable throwable) { - watch.close(); - } - }); - emitter.onTimeout( - new Runnable() { - @Override - public void run() { - watch.close(); - } - }); - return emitter; - } - return null; + final SseEmitter emitter = new SseEmitter(); + final CustomWatcher watcher = new CustomWatcher(emitter, objectMapper); + final Watch watch = + helmAppsService.getEvents(region, project, userProvider.getUser(region), watcher); + emitter.onCompletion( + new Runnable() { + @Override + public void run() { + watch.close(); + } + }); + emitter.onError( + new Consumer() { + @Override + public void accept(Throwable throwable) { + watch.close(); + } + }); + emitter.onTimeout( + new Runnable() { + @Override + public void run() { + watch.close(); + } + }); + return emitter; } public static class CustomWatcher implements Watcher { @@ -423,11 +402,8 @@ public UninstallService destroyApp( @RequestParam(value = "path", required = false) String path, @RequestParam(value = "bulk", required = false) Optional bulk) throws Exception { - if (Service.ServiceType.KUBERNETES.equals(region.getServices().getType())) { - return helmAppsService.destroyService( - region, project, userProvider.getUser(region), path, bulk.orElse(false)); - } - return null; + return helmAppsService.destroyService( + region, project, userProvider.getUser(region), path, bulk.orElse(false)); } @Operation( diff --git a/onyxia-api/src/main/java/fr/insee/onyxia/api/services/impl/HelmAppsService.java b/onyxia-api/src/main/java/fr/insee/onyxia/api/services/impl/HelmAppsService.java index 5be9b072..e7cb01ad 100644 --- a/onyxia-api/src/main/java/fr/insee/onyxia/api/services/impl/HelmAppsService.java +++ b/onyxia-api/src/main/java/fr/insee/onyxia/api/services/impl/HelmAppsService.java @@ -324,7 +324,6 @@ private Service getHelmApp(Region region, User user, HelmLs release) { service.setId(release.getName()); service.setName(release.getName()); service.setSubtitle(release.getChart()); - service.setType(Service.ServiceType.KUBERNETES); service.setName(release.getName()); service.setNamespace(release.getNamespace()); service.setRevision(release.getRevision()); diff --git a/onyxia-model/src/main/java/fr/insee/onyxia/model/region/Region.java b/onyxia-model/src/main/java/fr/insee/onyxia/model/region/Region.java index b13af9d4..45f63735 100644 --- a/onyxia-model/src/main/java/fr/insee/onyxia/model/region/Region.java +++ b/onyxia-model/src/main/java/fr/insee/onyxia/model/region/Region.java @@ -185,7 +185,6 @@ public void setOnyxiaAPI(OnyxiaAPI onyxiaAPI) { allowSetters = true) public static class Services { - private Service.ServiceType type; private boolean singleNamespace = true; private boolean allowNamespaceCreation = true; private Map namespaceLabels = new HashMap<>(); @@ -292,14 +291,6 @@ public void setUserNamespace(boolean userNamespace) { this.userNamespace = userNamespace; } - public Service.ServiceType getType() { - return type; - } - - public void setType(Service.ServiceType type) { - this.type = type; - } - public String getNamespacePrefix() { return namespacePrefix; } diff --git a/onyxia-model/src/main/java/fr/insee/onyxia/model/service/Service.java b/onyxia-model/src/main/java/fr/insee/onyxia/model/service/Service.java index 76294224..b33a68ad 100644 --- a/onyxia-model/src/main/java/fr/insee/onyxia/model/service/Service.java +++ b/onyxia-model/src/main/java/fr/insee/onyxia/model/service/Service.java @@ -29,9 +29,6 @@ public class Service { "State of the release (can be: unknown, deployed, uninstalled, superseded, failed, uninstalling, pending-install, pending-upgrade or pending-rollback)") private String status; - @Schema(description = "Fixed to kubernetes. This should be removed in v1.0") - private ServiceType type; - @Schema(description = "Urls are coming from ingress object") private List urls; @@ -159,14 +156,6 @@ public void setStatus(String status) { this.status = status; } - public ServiceType getType() { - return type; - } - - public void setType(ServiceType type) { - this.type = type; - } - public List getUrls() { return urls; } @@ -326,11 +315,6 @@ public static enum ServiceStatus { STOPPED; } - @Schema(description = "") - public static enum ServiceType { - KUBERNETES - } - @Schema(description = "") public static class Monitoring { private String url;