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.

Sign up for GitHub

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

Refactoring #466

Merged
merged 8 commits into from
Aug 1, 2024
Merged

Refactoring #466

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
f97be2d
Use try-with-resources for streams
johnksv Jul 22, 2024
2b5b5d1
Replace system out with logger
johnksv Jul 22, 2024
e0eb7bd
Introduce constants for repeating strings
johnksv Jul 23, 2024
8bc20dd
Simplify boolean expression
johnksv Jul 23, 2024
0f9271c
Use streams toList instead collect
johnksv Jul 23, 2024
e742233
Remove static modifier for field that belong to bean
johnksv Jul 23, 2024
fe4dbb9
Define string constants for repeating values
johnksv Jul 23, 2024
135cc7e
Refactor creation of Quota objects from map
johnksv Jul 23, 2024
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
2 changes: 1 addition & 1 deletion onyxia-api/src/main/java/fr/insee/onyxia/api/configuration/metrics/CustomMetrics.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@
@Service
public class CustomMetrics implements MeterBinder {

private static Counter counter;
private Counter counter;

public void plusUn() {
counter.increment();
Expand Down
12 changes: 5 additions & 7 deletions onyxia-api/src/main/java/fr/insee/onyxia/api/controller/api/mylab/QuotaController.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -76,10 +76,8 @@ public QuotaUsage getQuota(
}

final QuotaUsage quotaUsage = new QuotaUsage();
final Quota spec = new Quota();
final Quota usage = new Quota();
mapKubQuotaToQuota(resourceQuota.getStatus().getHard(), spec);
mapKubQuotaToQuota(resourceQuota.getStatus().getUsed(), usage);
final Quota spec = kubQuotaToQuota(resourceQuota.getStatus().getHard());
final Quota usage = kubQuotaToQuota(resourceQuota.getStatus().getUsed());
quotaUsage.setSpec(spec);
quotaUsage.setUsage(usage);

Expand Down Expand Up @@ -199,10 +197,10 @@ private Owner getOwner(Region region, Project project) {
return owner;
}

private void mapKubQuotaToQuota(Map<String, Quantity> resourceQuota, Quota quota) {
private Quota kubQuotaToQuota(Map<String, Quantity> resourceQuota) {
final Map<String, String> rawData = new HashMap<>();
resourceQuota.entrySet().forEach(e -> rawData.put(e.getKey(), e.getValue().toString()));
quota.loadFromMap(rawData);
resourceQuota.forEach((key, value) -> rawData.put(key, value.toString()));
return Quota.from(rawData);
}

public KubernetesService getKubernetesService() {
Expand Down
46 changes: 18 additions & 28 deletions onyxia-api/src/main/java/fr/insee/onyxia/api/dao/universe/CatalogLoader.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,11 +10,7 @@
import fr.insee.onyxia.model.catalog.Pkg;
import fr.insee.onyxia.model.helm.Chart;
import fr.insee.onyxia.model.helm.Repository;
import java.io.ByteArrayOutputStream;
import java.io.IOException;
import java.io.InputStream;
import java.io.InputStreamReader;
import java.io.Reader;
import java.io.*;
import java.util.Iterator;
import java.util.List;
import java.util.Optional;
Expand Down Expand Up @@ -173,38 +169,32 @@ private void refreshPackage(CatalogWrapper cw, Pkg pkg)
}

public void extractDataFromTgz(InputStream in, Chart chart) throws IOException {
GzipCompressorInputStream gzipIn = new GzipCompressorInputStream(in);
// HelmConfig config = null;
try (TarArchiveInputStream tarIn = new TarArchiveInputStream(gzipIn)) {
String chartName = chart.getName();

try (GzipCompressorInputStream gzipIn = new GzipCompressorInputStream(in);
TarArchiveInputStream tarIn = new TarArchiveInputStream(gzipIn)) {

TarArchiveEntry entry;

while ((entry = tarIn.getNextEntry()) != null) {
if (entry.getName().endsWith(chart.getName() + "/values.schema.json")
&& !entry.getName()
.endsWith("charts/" + chart.getName() + "/values.schema.json")) {
String entryName = entry.getName();
if (entryName.endsWith(chartName + "/values.schema.json")
&& !entryName.endsWith("charts/" + chartName + "/values.schema.json")) {
// TODO : mutualize objectmapper
ObjectMapper mapper = new ObjectMapper();
mapper.configure(JsonParser.Feature.AUTO_CLOSE_SOURCE, false);
mapper.configure(DeserializationFeature.FAIL_ON_UNKNOWN_PROPERTIES, false);
ByteArrayOutputStream baos = new ByteArrayOutputStream();
byte[] buffer = new byte[1024];
int len;
while ((len = tarIn.read(buffer)) != -1) {
baos.write(buffer, 0, len);

try (ByteArrayOutputStream baos = new ByteArrayOutputStream()) {
tarIn.transferTo(baos);
chart.setConfig(mapper.readTree(baos.toString(UTF_8)));
}
chart.setConfig(mapper.readTree(baos.toString("UTF-8")));
}
if (entry.getName().endsWith(chart.getName() + "/values.yaml")
&& !entry.getName()
.endsWith("charts/" + chart.getName() + "/values.yaml")) {
ByteArrayOutputStream baos = new ByteArrayOutputStream();
byte[] buffer = new byte[1024];
int len;
while ((len = tarIn.read(buffer)) != -1) {
baos.write(buffer, 0, len);
} else if (entryName.endsWith(chartName + "/values.yaml")
&& !entryName.endsWith("charts/" + chartName + "/values.yaml")) {
try (ByteArrayOutputStream baos = new ByteArrayOutputStream()) {
tarIn.transferTo(baos);
chart.setDefaultValues(baos.toString());
}
byte[] fileContent = baos.toByteArray();
chart.setDefaultValues(new String(fileContent));
}
}
}
Expand Down
65 changes: 29 additions & 36 deletions onyxia-api/src/main/java/fr/insee/onyxia/api/services/impl/HelmAppsService.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,6 @@
import java.util.*;
import java.util.concurrent.CompletableFuture;
import java.util.concurrent.TimeoutException;
import java.util.stream.Collectors;
import org.apache.commons.lang3.StringUtils;
import org.apache.commons.lang3.time.FastDateFormat;
import org.slf4j.Logger;
Expand Down Expand Up @@ -67,6 +66,10 @@ public class HelmAppsService implements AppsService {
final OnyxiaEventPublisher onyxiaEventPublisher;

public static final String ONYXIA_SECRET_PREFIX = "sh.onyxia.release.v1.";
private static final String CATALOG = "catalog";
private static final String OWNER = "owner";
private static final String FRIENDLY_NAME = "friendlyName";
private static final String SHARE = "share";

@Autowired
public HelmAppsService(
Expand Down Expand Up @@ -131,13 +134,12 @@ public Collection<Object> installApp(
requestDTO.getFriendlyName());
onyxiaEventPublisher.publishEvent(installServiceEvent);
Map<String, String> metadata = new HashMap<>();
metadata.put("catalog", Base64Utils.base64Encode(catalogId));
metadata.put("owner", Base64Utils.base64Encode(user.getIdep()));
metadata.put(CATALOG, Base64Utils.base64Encode(catalogId));
metadata.put(OWNER, Base64Utils.base64Encode(user.getIdep()));
if (requestDTO.getFriendlyName() != null) {
metadata.put(
"friendlyName", Base64Utils.base64Encode(requestDTO.getFriendlyName()));
metadata.put(FRIENDLY_NAME, Base64Utils.base64Encode(requestDTO.getFriendlyName()));
}
metadata.put("share", Base64Utils.base64Encode(String.valueOf(requestDTO.isShare())));
metadata.put(SHARE, Base64Utils.base64Encode(String.valueOf(requestDTO.isShare())));
kubernetesService.createOnyxiaSecret(
region, namespaceId, requestDTO.getName(), metadata);
return List.of(res.getManifest());
Expand Down Expand Up @@ -182,18 +184,13 @@ public CompletableFuture<ServicesListing> getUserServices(
installedCharts.parallelStream()
.map(release -> getHelmApp(region, user, release))
.filter(
service -> {
boolean canUserSeeThisService = false;
if (project.getGroup() == null
|| service.isShare()
|| user.getIdep()
.equalsIgnoreCase(service.getOwner())) {
// Personal group
canUserSeeThisService = true;
}
return canUserSeeThisService;
})
.collect(Collectors.toList());
service ->
// Check if user can see this service
project.getGroup() == null
|| service.isShare()
|| user.getIdep()
.equalsIgnoreCase(service.getOwner()))
.toList();
ServicesListing listing = new ServicesListing();
listing.setApps(services);
return CompletableFuture.completedFuture(listing);
Expand Down Expand Up @@ -307,18 +304,18 @@ private Service getHelmApp(Region region, User user, HelmLs release) {
.get();
if (secret != null && secret.getData() != null) {
Map<String, String> data = secret.getData();
if (data.containsKey("friendlyName")) {
service.setFriendlyName(Base64Utils.base64Decode(data.get("friendlyName")));
if (data.containsKey(FRIENDLY_NAME)) {
service.setFriendlyName(Base64Utils.base64Decode(data.get(FRIENDLY_NAME)));
}
if (data.containsKey("owner")) {
service.setOwner(Base64Utils.base64Decode(data.get("owner")));
if (data.containsKey(OWNER)) {
service.setOwner(Base64Utils.base64Decode(data.get(OWNER)));
}
if (data.containsKey("catalog")) {
service.setCatalogId(Base64Utils.base64Decode(data.get("catalog")));
if (data.containsKey(CATALOG)) {
service.setCatalogId(Base64Utils.base64Decode(data.get(CATALOG)));
}
if (data.containsKey("share")) {
if (data.containsKey(SHARE)) {
service.setShare(
Boolean.parseBoolean(Base64Utils.base64Decode(data.get("share"))));
Boolean.parseBoolean(Base64Utils.base64Decode(data.get(SHARE))));
}
}
} catch (Exception e) {
Expand All @@ -341,8 +338,7 @@ private Service getHelmApp(Region region, User user, HelmLs release) {
Map<String, String> result = new HashMap<>();
node.fields()
.forEachRemaining(
currentNode ->
mapAppender(result, currentNode, new ArrayList<String>()));
currentNode -> mapAppender(result, currentNode, new ArrayList<>()));
service.setEnv(result);
service.setSuspendable(service.getEnv().containsKey(SUSPEND_KEY));
if (service.getEnv().containsKey(SUSPEND_KEY)) {
Expand All @@ -364,13 +360,13 @@ private Service getHelmApp(Region region, User user, HelmLs release) {
public void rename(
Region region, Project project, User user, String serviceId, String friendlyName)
throws IOException, InterruptedException, TimeoutException {
patchOnyxiaSecret(region, project, user, serviceId, Map.of("friendlyName", friendlyName));
patchOnyxiaSecret(region, project, user, serviceId, Map.of(FRIENDLY_NAME, friendlyName));
}

@Override
public void share(Region region, Project project, User user, String serviceId, boolean share)
throws IOException, InterruptedException, TimeoutException {
patchOnyxiaSecret(region, project, user, serviceId, Map.of("share", String.valueOf(share)));
patchOnyxiaSecret(region, project, user, serviceId, Map.of(SHARE, String.valueOf(share)));
}

private void patchOnyxiaSecret(
Expand All @@ -386,10 +382,7 @@ private void patchOnyxiaSecret(
if (secret != null) {
Map<String, String> secretData =
secret.getData() != null ? secret.getData() : new HashMap<>();
data.forEach(
(k, v) -> {
secretData.put(k, Base64Utils.base64Encode(v));
});
data.forEach((k, v) -> secretData.put(k, Base64Utils.base64Encode(v)));
secret.setData(secretData);
if (secret.getMetadata().getManagedFields() != null) {
secret.getMetadata().getManagedFields().clear();
Expand All @@ -401,7 +394,7 @@ private void patchOnyxiaSecret(
.serverSideApply();
} else {
Map<String, String> metadata = new HashMap<>();
metadata.put("owner", user.getIdep());
metadata.put(OWNER, user.getIdep());
metadata.putAll(data);
kubernetesService.createOnyxiaSecret(region, namespaceId, serviceId, metadata);
}
Expand Down Expand Up @@ -590,7 +583,7 @@ private Service getServiceFromRelease(
currentTask.setStatus(status);
return currentTask;
})
.collect(Collectors.toList()));
.toList());

return service;
}
Expand Down
24 changes: 15 additions & 9 deletions onyxia-api/src/main/java/fr/insee/onyxia/api/services/impl/ServiceUrlResolver.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,15 @@
import java.util.ArrayList;
import java.util.List;
import java.util.stream.Stream;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

public final class ServiceUrlResolver {

private ServiceUrlResolver() {}

private static final Logger LOGGER = LoggerFactory.getLogger(ServiceUrlResolver.class);

static List<String> getServiceUrls(Region region, String manifest, KubernetesClient client) {
Region.Expose expose = region.getServices().getExpose();
boolean isIstioEnabled = expose.getIstio() != null && expose.getIstio().isEnabled();
Expand Down Expand Up @@ -49,9 +56,8 @@ static List<String> getServiceUrls(Region region, String manifest, KubernetesCli
.getPath()))
.toList());
} catch (Exception e) {
System.out.println(
"Warning : could not read urls from ingress "
+ ingress.getFullResourceName());
LOGGER.warn(
"Could not read urls from ingress {}", ingress.getFullResourceName());
}
}
}
Expand All @@ -66,9 +72,9 @@ static List<String> getServiceUrls(Region region, String manifest, KubernetesCli
try {
urls.add(resource.get("spec", "host"));
} catch (Exception e) {
System.out.println(
"Warning : could not read urls from OpenShift Route "
+ resource.getFullResourceName());
LOGGER.warn(
"Could not read urls from OpenShift Route {}",
resource.getFullResourceName());
}
}
}
Expand All @@ -85,9 +91,9 @@ static List<String> getServiceUrls(Region region, String manifest, KubernetesCli
// One should consider to add support for 'spec/http[*]/match[*]/uri/prefix'
urls.addAll(resource.get("spec", "hosts"));
} catch (Exception e) {
System.out.println(
"Warning : could not read urls from Istio Virtual Service "
+ resource.getFullResourceName());
LOGGER.warn(
"Could not read urls from Istio Virtual Service {}",
resource.getFullResourceName());
}
}
}
Expand Down
Loading
Loading