From 26e26aa8300fe079a32d70b6403f74fd69cbd3d0 Mon Sep 17 00:00:00 2001 From: John Kasper Svergja Date: Wed, 10 Jan 2024 09:40:07 +0100 Subject: [PATCH] Cleanup sonarcloud reported issues A good basis for further cleanup later as well - https://sonarcloud.io/project/issues?impactSeverities=HIGH&resolved=false&id=InseeFrLab_onyxia-api&open=AYyCBqZxRJnw-QF9_Gfr - https://sonarcloud.io/project/issues?impactSeverities=HIGH&resolved=false&id=InseeFrLab_onyxia-api&open=AYyCBqZxRJnw-QF9_Gfq - https://sonarcloud.io/project/issues?impactSeverities=HIGH&resolved=false&id=InseeFrLab_onyxia-api&open=AYyCBqYTRJnw-QF9_Ge4 - https://sonarcloud.io/project/issues?impactSeverities=HIGH&resolved=false&id=InseeFrLab_onyxia-api&open=AYyCBqWBRJnw-QF9_Gdg - https://sonarcloud.io/project/issues?impactSeverities=HIGH&resolved=false&id=InseeFrLab_onyxia-api&open=AYyCBqV9RJnw-QF9_Gdd - https://sonarcloud.io/project/issues?impactSeverities=HIGH&resolved=false&id=InseeFrLab_onyxia-api&open=AYyCBqV9RJnw-QF9_Gde - https://sonarcloud.io/project/issues?impactSeverities=HIGH&resolved=false&id=InseeFrLab_onyxia-api&open=AYyCBqVoRJnw-QF9_GdT - https://sonarcloud.io/project/issues?impactSeverities=HIGH&resolved=false&id=InseeFrLab_onyxia-api&open=AYyCBqXdRJnw-QF9_Gei - https://sonarcloud.io/project/issues?impactSeverities=HIGH&resolved=false&id=InseeFrLab_onyxia-api&open=AYyCBqXuRJnw-QF9_Geq - https://sonarcloud.io/project/issues?impactSeverities=HIGH&resolved=false&id=InseeFrLab_onyxia-api&open=AYyCBqYKRJnw-QF9_Ge0 --- .../service/HelmInstallService.java | 33 ++++++++----------- .../service/HelmVersionService.java | 8 ++--- .../java/fr/insee/onyxia/api/Application.java | 22 ++++++++----- .../onyxia/api/configuration/Catalogs.java | 2 -- .../api/configuration/CatalogsLoader.java | 15 ++------- .../api/configuration/proxy/SetProxy.java | 14 +++++--- .../controller/api/mylab/MyLabController.java | 4 +-- .../api/controller/pub/CatalogController.java | 12 +++---- .../api/security/NoSecurityUserProvider.java | 5 +-- 9 files changed, 49 insertions(+), 66 deletions(-) diff --git a/helm-wrapper/src/main/java/io/github/inseefrlab/helmwrapper/service/HelmInstallService.java b/helm-wrapper/src/main/java/io/github/inseefrlab/helmwrapper/service/HelmInstallService.java index 06ec1445..306fdaf6 100644 --- a/helm-wrapper/src/main/java/io/github/inseefrlab/helmwrapper/service/HelmInstallService.java +++ b/helm-wrapper/src/main/java/io/github/inseefrlab/helmwrapper/service/HelmInstallService.java @@ -2,8 +2,6 @@ import static io.github.inseefrlab.helmwrapper.utils.Command.safeConcat; -import com.fasterxml.jackson.core.JsonProcessingException; -import com.fasterxml.jackson.databind.JsonMappingException; import com.fasterxml.jackson.databind.ObjectMapper; import io.github.inseefrlab.helmwrapper.configuration.HelmConfiguration; import io.github.inseefrlab.helmwrapper.model.HelmInstaller; @@ -31,9 +29,10 @@ public class HelmInstallService { private final Pattern helmNamePattern = Pattern.compile("^[a-z0-9]([-a-z0-9]*[a-z0-9])?(\\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*$"); - private HelmReleaseInfoParser helmReleaseInfoParser = new HelmReleaseInfoParser(); - - public HelmInstallService() {} + private final HelmReleaseInfoParser helmReleaseInfoParser = new HelmReleaseInfoParser(); + private static final String VALUES_INFO_TYPE = "values"; + private static final String MANIFEST_INFO_TYPE = "manifest"; + private static final String NOTES_INFO_TYPE = "notes"; public HelmInstaller installChart( HelmConfiguration configuration, @@ -104,12 +103,7 @@ public int uninstaller(HelmConfiguration configuration, String name, String name } public HelmLs[] listChartInstall(HelmConfiguration configuration, String namespace) - throws JsonMappingException, - InvalidExitValueException, - JsonProcessingException, - IOException, - InterruptedException, - TimeoutException { + throws InvalidExitValueException, IOException, InterruptedException, TimeoutException { StringBuilder command = new StringBuilder("helm ls"); if (namespace != null) { command.append(" -n "); @@ -124,15 +118,15 @@ public HelmLs[] listChartInstall(HelmConfiguration configuration, String namespa } public String getManifest(HelmConfiguration configuration, String id, String namespace) { - return getReleaseInfo(configuration, "manifest", id, namespace); + return getReleaseInfo(configuration, MANIFEST_INFO_TYPE, id, namespace); } public String getValues(HelmConfiguration configuration, String id, String namespace) { - return getReleaseInfo(configuration, "values", id, namespace); + return getReleaseInfo(configuration, VALUES_INFO_TYPE, id, namespace); } public String getNotes(HelmConfiguration configuration, String id, String namespace) { - return getReleaseInfo(configuration, "notes", id, namespace); + return getReleaseInfo(configuration, NOTES_INFO_TYPE, id, namespace); } public HelmReleaseInfo getAll(HelmConfiguration configuration, String id, String namespace) { @@ -152,7 +146,7 @@ public HelmReleaseInfo getAll(HelmConfiguration configuration, String id, String private String getReleaseInfo( HelmConfiguration configuration, String infoType, String id, String namespace) { - if (!List.of("manifest", "notes", "values").contains(infoType)) { + if (!List.of(MANIFEST_INFO_TYPE, NOTES_INFO_TYPE, VALUES_INFO_TYPE).contains(infoType)) { throw new IllegalArgumentException( "Invalid info type " + infoType + ", should be manifest, notes or values"); } @@ -161,11 +155,11 @@ private String getReleaseInfo( safeConcat(command, id); command.append(" --namespace "); safeConcat(command, namespace); - if (infoType.equals("notes")) { + if (infoType.equals(NOTES_INFO_TYPE)) { return Command.executeAndGetResponseAsRaw(configuration, command.toString()) .getOutput() .getString(); - } else if (infoType.equals("values")) { + } else if (infoType.equals(VALUES_INFO_TYPE)) { return Command.executeAndGetResponseAsJson(configuration, command.toString()) .getOutput() .getString(); @@ -222,13 +216,12 @@ public HelmLs getAppById(HelmConfiguration configuration, String appId, String n | IOException | InterruptedException | TimeoutException e) { - // TODO Auto-generated catch block - e.printStackTrace(); + LOGGER.warn("Exception occurred when getting app by id", e); } return null; } - public class MultipleServiceFound extends Exception { + public static class MultipleServiceFound extends Exception { public MultipleServiceFound(String s) { super(s); diff --git a/helm-wrapper/src/main/java/io/github/inseefrlab/helmwrapper/service/HelmVersionService.java b/helm-wrapper/src/main/java/io/github/inseefrlab/helmwrapper/service/HelmVersionService.java index 698b1b55..3dccd7d2 100644 --- a/helm-wrapper/src/main/java/io/github/inseefrlab/helmwrapper/service/HelmVersionService.java +++ b/helm-wrapper/src/main/java/io/github/inseefrlab/helmwrapper/service/HelmVersionService.java @@ -10,10 +10,8 @@ public class HelmVersionService { public String getVersion() throws InvalidExitValueException, IOException, InterruptedException, TimeoutException { - String version = - Command.executeAndGetResponseAsRaw("helm version --template={{.Version}}") - .getOutput() - .getString(StandardCharsets.UTF_8.name()); - return version; + return Command.executeAndGetResponseAsRaw("helm version --template={{.Version}}") + .getOutput() + .getString(StandardCharsets.UTF_8.name()); } } diff --git a/onyxia-api/src/main/java/fr/insee/onyxia/api/Application.java b/onyxia-api/src/main/java/fr/insee/onyxia/api/Application.java index 7f981233..32da4b50 100644 --- a/onyxia-api/src/main/java/fr/insee/onyxia/api/Application.java +++ b/onyxia-api/src/main/java/fr/insee/onyxia/api/Application.java @@ -3,6 +3,8 @@ import java.util.HashMap; import java.util.Map; import org.apache.commons.lang3.StringUtils; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; import org.springframework.boot.autoconfigure.SpringBootApplication; import org.springframework.boot.builder.SpringApplicationBuilder; import org.springframework.boot.context.event.ApplicationEnvironmentPreparedEvent; @@ -17,6 +19,9 @@ @EnableAsync(proxyTargetClass = true) public class Application { + // logger + private static final Logger LOGGER = LoggerFactory.getLogger(Application.class); + public static void main(String[] args) { ConfigurableApplicationContext context = new SpringApplicationBuilder(Application.class) @@ -31,16 +36,15 @@ public void onApplicationEvent(ApplicationEnvironmentPreparedEvent event) { ConfigurableEnvironment environment = event.getEnvironment(); MutablePropertySources propertySources = environment.getPropertySources(); Map myMap = new HashMap<>(); - System.out.println(environment.getProperty("oidc.jwk-uri")); - if (StringUtils.isNotEmpty(environment.getProperty("oidc.issuer-uri"))) { - myMap.put( - "spring.security.oauth2.resourceserver.jwt.issuer-uri", - environment.getProperty("oidc.issuer-uri")); + String oidcJwkUri = environment.getProperty("oidc.jwk-uri"); + String oidcIssuerUri = environment.getProperty("oidc.issuer-uri"); + LOGGER.info("oidc properties, jwk-uri: {}, issuer-uri: {}", oidcJwkUri, oidcIssuerUri); + + if (StringUtils.isNotEmpty(oidcIssuerUri)) { + myMap.put("spring.security.oauth2.resourceserver.jwt.issuer-uri", oidcIssuerUri); } - if (StringUtils.isNotEmpty(environment.getProperty("oidc.jwk-uri"))) { - myMap.put( - "spring.security.oauth2.resourceserver.jwt.jwk-set-uri", - environment.getProperty("oidc.jwk-uri")); + if (StringUtils.isNotEmpty(oidcJwkUri)) { + myMap.put("spring.security.oauth2.resourceserver.jwt.jwk-set-uri", oidcJwkUri); } propertySources.addFirst(new MapPropertySource("ALIASES", myMap)); } diff --git a/onyxia-api/src/main/java/fr/insee/onyxia/api/configuration/Catalogs.java b/onyxia-api/src/main/java/fr/insee/onyxia/api/configuration/Catalogs.java index a24cb7c9..6a49e8c1 100644 --- a/onyxia-api/src/main/java/fr/insee/onyxia/api/configuration/Catalogs.java +++ b/onyxia-api/src/main/java/fr/insee/onyxia/api/configuration/Catalogs.java @@ -8,8 +8,6 @@ public class Catalogs { private List catalogs = new ArrayList<>(); - public Catalogs() {} - public CatalogWrapper getCatalogById(final String id) { for (final CatalogWrapper cw : catalogs) { if (cw.getId().equals(id)) { diff --git a/onyxia-api/src/main/java/fr/insee/onyxia/api/configuration/CatalogsLoader.java b/onyxia-api/src/main/java/fr/insee/onyxia/api/configuration/CatalogsLoader.java index 5eb82332..369faac3 100644 --- a/onyxia-api/src/main/java/fr/insee/onyxia/api/configuration/CatalogsLoader.java +++ b/onyxia-api/src/main/java/fr/insee/onyxia/api/configuration/CatalogsLoader.java @@ -1,32 +1,21 @@ package fr.insee.onyxia.api.configuration; -import com.fasterxml.jackson.databind.ObjectMapper; import fr.insee.onyxia.api.configuration.properties.CatalogsConfiguration; import org.slf4j.Logger; import org.slf4j.LoggerFactory; -import org.springframework.beans.factory.annotation.Autowired; import org.springframework.beans.factory.config.ConfigurableBeanFactory; import org.springframework.context.annotation.Bean; import org.springframework.context.annotation.Configuration; import org.springframework.context.annotation.Scope; -import org.springframework.core.io.ResourceLoader; @Configuration public class CatalogsLoader { - - @Autowired private ResourceLoader resourceLoader; - - @Autowired private ObjectMapper mapper; - - @Autowired private CatalogFilter catalogFilter; - - @Autowired private CatalogsConfiguration catalogsConfiguration; - private static final Logger LOGGER = LoggerFactory.getLogger(CatalogsLoader.class); @Bean @Scope(ConfigurableBeanFactory.SCOPE_SINGLETON) - public Catalogs catalogs() { + public Catalogs catalogs( + CatalogFilter catalogFilter, CatalogsConfiguration catalogsConfiguration) { Catalogs catalogs = new Catalogs(); catalogs.setCatalogs(catalogsConfiguration.getResolvedCatalogs()); catalogs.setCatalogs(catalogFilter.filterCatalogs(catalogs.getCatalogs())); diff --git a/onyxia-api/src/main/java/fr/insee/onyxia/api/configuration/proxy/SetProxy.java b/onyxia-api/src/main/java/fr/insee/onyxia/api/configuration/proxy/SetProxy.java index b9b39815..31e1bfef 100644 --- a/onyxia-api/src/main/java/fr/insee/onyxia/api/configuration/proxy/SetProxy.java +++ b/onyxia-api/src/main/java/fr/insee/onyxia/api/configuration/proxy/SetProxy.java @@ -2,12 +2,16 @@ import jakarta.annotation.PostConstruct; import org.apache.commons.lang3.StringUtils; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; import org.springframework.beans.factory.annotation.Value; import org.springframework.context.annotation.Configuration; @Configuration public class SetProxy { + private static final Logger LOGGER = LoggerFactory.getLogger(SetProxy.class); + @Value("${http.proxyHost}") private String httpProxyHost; @@ -26,21 +30,21 @@ public class SetProxy { @PostConstruct public void setProxy() { if (StringUtils.isNotEmpty(httpProxyHost)) { - System.out.println("Using proxy host : " + httpProxyHost); + LOGGER.info("Using proxy host : {}", httpProxyHost); System.setProperty("http.proxyHost", httpProxyHost); System.setProperty("https.proxyHost", httpProxyHost); if (StringUtils.isNotEmpty(httpProxyPort)) { - System.out.println("Using proxy port : " + httpProxyPort); + LOGGER.info("Using proxy port : {}", httpProxyPort); System.setProperty("http.proxyPort", httpProxyPort); System.setProperty("https.proxyPort", httpProxyPort); } if (StringUtils.isNotEmpty(noProxy)) { - System.out.println("No proxy : " + noProxy); - System.setProperty("http.nonProxyHosts", noProxy.replaceAll(",", "\\|")); + LOGGER.info("No proxy : {}", noProxy); + System.setProperty("http.nonProxyHosts", noProxy.replace(",", "|")); System.setProperty("no_proxy", noProxy); } if (StringUtils.isNotEmpty(proxyUsername)) { - System.out.println("Proxy username : " + proxyUsername); + LOGGER.info("Proxy username : {}", proxyUsername); System.setProperty("http.proxyUser", proxyUsername); if (StringUtils.isNotEmpty(proxyPassword)) { System.setProperty("http.proxyPassword", proxyPassword); 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 62d46861..c6126ffe 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 @@ -190,11 +190,11 @@ public UninstallService destroyApp( @Parameter(hidden = true) Region region, @Parameter(hidden = true) Project project, @RequestParam(value = "path", required = false) String path, - @RequestParam(value = "bulk", required = false) boolean bulk) + @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); + region, project, userProvider.getUser(region), path, bulk.orElse(false)); } return null; } diff --git a/onyxia-api/src/main/java/fr/insee/onyxia/api/controller/pub/CatalogController.java b/onyxia-api/src/main/java/fr/insee/onyxia/api/controller/pub/CatalogController.java index 889debee..c89de8c1 100644 --- a/onyxia-api/src/main/java/fr/insee/onyxia/api/controller/pub/CatalogController.java +++ b/onyxia-api/src/main/java/fr/insee/onyxia/api/controller/pub/CatalogController.java @@ -18,7 +18,6 @@ import java.util.HashMap; import java.util.List; import java.util.Map; -import java.util.stream.Collectors; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.web.bind.annotation.GetMapping; import org.springframework.web.bind.annotation.PathVariable; @@ -41,8 +40,8 @@ public Catalogs getCatalogs(@Parameter(hidden = true) Region region) { Catalogs filteredCatalogs = new Catalogs(); filteredCatalogs.setCatalogs( catalogService.getCatalogs().getCatalogs().stream() - .filter((catalog) -> isCatalogEnabled(region, catalog)) - .collect(Collectors.toList())); + .filter(catalog -> isCatalogEnabled(region, catalog)) + .toList()); return filteredCatalogs; } @@ -170,13 +169,14 @@ private void addCustomOnyxiaProperties(Pkg pkg) { property.setProperties(new HashMap<>()); Map onyxiaProperties = new HashMap<>(); Property customName = new Property(); - customName.setType("string"); + String stringType = "string"; + customName.setType(stringType); customName.setDescription("Service custom name"); customName.setDefaut(pkg.getName()); customName.setTitle("Custom name"); onyxiaProperties.put("friendlyName", customName); Property userDefinedValues = new Property(); - userDefinedValues.setType("string"); + userDefinedValues.setType(stringType); userDefinedValues.setDescription("Values defined by the end user"); userDefinedValues.setDefaut(""); userDefinedValues.setTitle("User defined values"); @@ -185,7 +185,7 @@ private void addCustomOnyxiaProperties(Pkg pkg) { userDefinedValues.setXonyxia(xonyxiaUserDefinedValues); onyxiaProperties.put("userDefinedValues", userDefinedValues); Property owner = new Property(); - owner.setType("string"); + owner.setType(stringType); owner.setDescription("Owner of the chart"); owner.setDefaut("owner"); owner.setTitle("Owner"); diff --git a/onyxia-api/src/main/java/fr/insee/onyxia/api/security/NoSecurityUserProvider.java b/onyxia-api/src/main/java/fr/insee/onyxia/api/security/NoSecurityUserProvider.java index ea97c5a7..34eed467 100644 --- a/onyxia-api/src/main/java/fr/insee/onyxia/api/security/NoSecurityUserProvider.java +++ b/onyxia-api/src/main/java/fr/insee/onyxia/api/security/NoSecurityUserProvider.java @@ -4,7 +4,6 @@ import fr.insee.onyxia.api.services.utils.HttpRequestUtils; import fr.insee.onyxia.model.User; import fr.insee.onyxia.model.region.Region; -import org.springframework.beans.factory.annotation.Autowired; import org.springframework.boot.autoconfigure.condition.ConditionalOnExpression; import org.springframework.context.annotation.Bean; import org.springframework.context.annotation.Configuration; @@ -15,10 +14,8 @@ @ConditionalOnExpression("'${authentication.mode}' == 'none' or '${authentication.mode}' == ''") public class NoSecurityUserProvider { - @Autowired private HttpRequestUtils httpRequestUtils; - @Bean - public UserProvider getUserProvider() { + public UserProvider getUserProvider(HttpRequestUtils httpRequestUtils) { return (Region region) -> User.newInstance() .setEmail("toto@tld.fr")