Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Cleanup some sonarcloud reported issues #345

Merged
merged 1 commit into from
Jan 22, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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 ");
Expand All @@ -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) {
Expand All @@ -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");
}
Expand All @@ -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();
Expand Down Expand Up @@ -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);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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());
}
}
22 changes: 13 additions & 9 deletions onyxia-api/src/main/java/fr/insee/onyxia/api/Application.java
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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)
Expand All @@ -31,16 +36,15 @@ public void onApplicationEvent(ApplicationEnvironmentPreparedEvent event) {
ConfigurableEnvironment environment = event.getEnvironment();
MutablePropertySources propertySources = environment.getPropertySources();
Map<String, Object> 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));
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,6 @@
public class Catalogs {
private List<CatalogWrapper> catalogs = new ArrayList<>();

public Catalogs() {}

public CatalogWrapper getCatalogById(final String id) {
for (final CatalogWrapper cw : catalogs) {
if (cw.getId().equals(id)) {
Expand Down
Original file line number Diff line number Diff line change
@@ -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()));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand All @@ -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);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we just remove this parameter, @olevitt ? Onyxia-web doesn't use it, e.g. for the 'delete all' onyxia-web send multiple delete calls (one for each service)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it's ok to keep it as it may be useful for other clients (such as an admin tool that would cleanup)

@RequestParam(value = "bulk", required = false) Optional<Boolean> 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;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;
}

Expand Down Expand Up @@ -170,13 +169,14 @@ private void addCustomOnyxiaProperties(Pkg pkg) {
property.setProperties(new HashMap<>());
Map<String, Property> 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");
Expand All @@ -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");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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")
Expand Down