From d7912e3d1284ee879b2fa62f3376b5cd21b05a31 Mon Sep 17 00:00:00 2001 From: Stefan Bethke Date: Fri, 19 Jul 2024 09:51:34 +0200 Subject: [PATCH] Improve error handling for segments Previously, a missing primarySegment or an empty additionalSegments entry might have caused a NullPointerException. Now, missing values are reported so the source can be identified. --- .../ingress/OnlyLangUrlMappingBuilder.java | 82 +++++++++++-------- 1 file changed, 48 insertions(+), 34 deletions(-) diff --git a/src/main/java/com/tsystemsmms/cmcc/cmccoperator/ingress/OnlyLangUrlMappingBuilder.java b/src/main/java/com/tsystemsmms/cmcc/cmccoperator/ingress/OnlyLangUrlMappingBuilder.java index 69f6431..2c5a366 100644 --- a/src/main/java/com/tsystemsmms/cmcc/cmccoperator/ingress/OnlyLangUrlMappingBuilder.java +++ b/src/main/java/com/tsystemsmms/cmcc/cmccoperator/ingress/OnlyLangUrlMappingBuilder.java @@ -79,33 +79,37 @@ public Collection buildLiveResources(SiteMapping siteMapp String suffix = i == 0 ? "" : Character.toString('a' - 1 + i); if (fqdn.isBlank()) fqdn = concatOptional(getDefaults().getNamePrefix(), site) + "." + getDefaults().getIngressDomain(); - ingresses.addAll(ingressBuilderFactory.builder(targetState, liveName(site, "home", suffix), fqdn, tls) - .responseTimeout(responseTimeout) - .uploadSize(uploadSize) - .pathPattern("/", serviceName) - .redirect("$scheme://" + fqdn + "/" + getLanguage(siteMapping.getPrimarySegment()), HttpStatus.MOVED_PERMANENTLY.value()) - .build()); - ingresses.addAll(ingressBuilderFactory.builder(targetState, liveName(site, "blueprint", suffix), fqdn, tls) - .responseTimeout(responseTimeout) - .uploadSize(uploadSize) - .pathPrefix("/blueprint", serviceName).build()); - ingresses.addAll(ingressBuilderFactory.builder(targetState, liveName(site, "all", suffix), fqdn, tls) - .responseTimeout(responseTimeout) - .uploadSize(uploadSize) - .pathPattern("/(" + handlerPattern + ")(.*)", serviceName).rewrite("/blueprint/servlet/$1$2").build()); - ingresses.addAll(ingressBuilderFactory.builder(targetState, liveName(site, "language", suffix), fqdn, tls) - .responseTimeout(responseTimeout) - .uploadSize(uploadSize) - .pathPattern("/(" + languagePattern + ")(.*)", serviceName).rewrite("/blueprint/servlet/" + getReplacement(siteMapping.getPrimarySegment()) + "$2").build()); - ingresses.addAll(ingressBuilderFactory.builder(targetState, liveName(site, "default", suffix), fqdn, tls) - .responseTimeout(responseTimeout) - .uploadSize(uploadSize) - .pathPattern("/(.+)", serviceName) - .redirect("$scheme://" + fqdn + "/" + getLanguage(siteMapping.getPrimarySegment()) + "/$1", HttpStatus.MOVED_PERMANENTLY.value()).build()); - ingresses.addAll(ingressBuilderFactory.builder(targetState, liveName(site, "seo", suffix), fqdn, tls) - .responseTimeout(responseTimeout) - .uploadSize(uploadSize) - .pathPattern("/(robots\\.txt|sitemap.*\\.xml)", serviceName).rewrite(getTargetState().getCmcc().getSpec().getWith().getIngressSeoHandler() + "/" + siteMapping.getPrimarySegment() + "/$1").build()); + try { + ingresses.addAll(ingressBuilderFactory.builder(targetState, liveName(site, "home", suffix), fqdn, tls) + .responseTimeout(responseTimeout) + .uploadSize(uploadSize) + .pathPattern("/", serviceName) + .redirect("$scheme://" + fqdn + "/" + getLanguage(siteMapping.getPrimarySegment()), HttpStatus.MOVED_PERMANENTLY.value()) + .build()); + ingresses.addAll(ingressBuilderFactory.builder(targetState, liveName(site, "blueprint", suffix), fqdn, tls) + .responseTimeout(responseTimeout) + .uploadSize(uploadSize) + .pathPrefix("/blueprint", serviceName).build()); + ingresses.addAll(ingressBuilderFactory.builder(targetState, liveName(site, "all", suffix), fqdn, tls) + .responseTimeout(responseTimeout) + .uploadSize(uploadSize) + .pathPattern("/(" + handlerPattern + ")(.*)", serviceName).rewrite("/blueprint/servlet/$1$2").build()); + ingresses.addAll(ingressBuilderFactory.builder(targetState, liveName(site, "language", suffix), fqdn, tls) + .responseTimeout(responseTimeout) + .uploadSize(uploadSize) + .pathPattern("/(" + languagePattern + ")(.*)", serviceName).rewrite("/blueprint/servlet/" + getReplacement(siteMapping.getPrimarySegment()) + "$2").build()); + ingresses.addAll(ingressBuilderFactory.builder(targetState, liveName(site, "default", suffix), fqdn, tls) + .responseTimeout(responseTimeout) + .uploadSize(uploadSize) + .pathPattern("/(.+)", serviceName) + .redirect("$scheme://" + fqdn + "/" + getLanguage(siteMapping.getPrimarySegment()) + "/$1", HttpStatus.MOVED_PERMANENTLY.value()).build()); + ingresses.addAll(ingressBuilderFactory.builder(targetState, liveName(site, "seo", suffix), fqdn, tls) + .responseTimeout(responseTimeout) + .uploadSize(uploadSize) + .pathPattern("/(robots\\.txt|sitemap.*\\.xml)", serviceName).rewrite(getTargetState().getCmcc().getSpec().getWith().getIngressSeoHandler() + "/" + siteMapping.getPrimarySegment() + "/$1").build()); + } catch (CustomResourceConfigError e) { + throw new CustomResourceConfigError("Site " + site + "/" + fqdn + ": " + e.getMessage()); + } } return ingresses; @@ -116,20 +120,20 @@ public String buildLiveUrl(SiteMapping siteMapping, String segment) { String fqdn = concatOptional(getDefaults().getNamePrefix(), siteMapping.getHostname()) + "." + getDefaults().getIngressDomain(); if (!siteMapping.getFqdn().isBlank()) fqdn = siteMapping.getFqdn(); - return "https://" + fqdn + "/" + getLanguage(segment); + try { + return "https://" + fqdn + "/" + getLanguage(segment); + } catch (CustomResourceConfigError e) { + throw new CustomResourceConfigError("Site " + siteMapping.getHostname() + "/" + fqdn + ": " + e.getMessage()); + } } private String getLanguage(String segment) { - String[] parts = segment.split("-"); - if (parts.length < 3) - throw new CustomResourceConfigError("Segment \"" + segment + "\" in site mapping is too short, needs to have at least three parts separated by -"); + String[] parts = splitSegment(segment); return parts[parts.length - 2]; } private String getReplacement(String segment) { - String[] parts = segment.split("-"); - if (parts.length < 3) - throw new CustomResourceConfigError("Segment \"" + segment + "\" in site mapping is too short, needs to have at least three parts separated by -"); + String[] parts = splitSegment(segment); parts[parts.length - 2] = "$1"; return String.join("-", parts); } @@ -137,4 +141,14 @@ private String getReplacement(String segment) { private String getPrimaryReplacement() { return ""; } + + private String[] splitSegment(String segment) { + if (segment == null || segment.isEmpty()) { + throw new CustomResourceConfigError("Segment in site mapping must be set and have at least three parts separated by -"); + } + String[] parts = segment.split("-"); + if (parts.length < 3) + throw new CustomResourceConfigError("Segment \"" + segment + "\" in site mapping is too short, needs to have at least three parts separated by -"); + return parts; + } }