From 9df18e3066115f6084fab9b5c04ecd303084426c Mon Sep 17 00:00:00 2001 From: Jon Bratseth Date: Mon, 15 May 2023 11:56:15 +0200 Subject: Don't assume any environments are single-region --- .../config/application/OverrideProcessor.java | 6 +- .../config/application/PropertiesProcessor.java | 25 +++--- .../config/application/ValidationProcessor.java | 1 + .../java/com/yahoo/config/application/Xml.java | 14 ++-- .../config/application/IncludeProcessorTest.java | 1 + .../config/application/OverrideProcessorTest.java | 32 -------- .../config/application/XmlPreprocessorTest.java | 89 ++++++++++++++++++++-- .../src/test/resources/multienvapp/services.xml | 1 + .../com/yahoo/config/provision/Environment.java | 5 +- 9 files changed, 109 insertions(+), 65 deletions(-) diff --git a/config-application-package/src/main/java/com/yahoo/config/application/OverrideProcessor.java b/config-application-package/src/main/java/com/yahoo/config/application/OverrideProcessor.java index 169b1c8557b..f98f0524fea 100644 --- a/config-application-package/src/main/java/com/yahoo/config/application/OverrideProcessor.java +++ b/config-application-package/src/main/java/com/yahoo/config/application/OverrideProcessor.java @@ -169,11 +169,7 @@ class OverrideProcessor implements PreProcessor { } if ( ! elementRegions.isEmpty()) { // match region - // match region in multi-region environments only - if ( environment.isMultiRegion() && ! elementRegions.contains(region)) return false; - - // explicit region implies multi-region environment - if ( ! environment.isMultiRegion() && elementEnvironments.isEmpty() ) return false; + if ( ! elementRegions.contains(region)) return false; } if ( ! elementTags.isEmpty()) { // match tags diff --git a/config-application-package/src/main/java/com/yahoo/config/application/PropertiesProcessor.java b/config-application-package/src/main/java/com/yahoo/config/application/PropertiesProcessor.java index 2152a62228b..0c49e9f41d9 100644 --- a/config-application-package/src/main/java/com/yahoo/config/application/PropertiesProcessor.java +++ b/config-application-package/src/main/java/com/yahoo/config/application/PropertiesProcessor.java @@ -3,7 +3,11 @@ package com.yahoo.config.application; import java.util.logging.Level; import com.yahoo.text.XML; -import org.w3c.dom.*; +import org.w3c.dom.Document; +import org.w3c.dom.Element; +import org.w3c.dom.NamedNodeMap; +import org.w3c.dom.Node; +import org.w3c.dom.NodeList; import javax.xml.transform.TransformerException; import java.util.ArrayList; @@ -18,6 +22,7 @@ import java.util.logging.Logger; * @author hmusum */ class PropertiesProcessor implements PreProcessor { + private final static Logger log = Logger.getLogger(PropertiesProcessor.class.getName()); private final LinkedHashMap properties; @@ -27,7 +32,7 @@ class PropertiesProcessor implements PreProcessor { public Document process(Document input) throws TransformerException { Document doc = Xml.copyDocument(input); - final Document document = buildProperties(doc); + Document document = buildProperties(doc); applyProperties(document.getDocumentElement()); return document; } @@ -36,11 +41,9 @@ class PropertiesProcessor implements PreProcessor { NodeList list = input.getElementsByTagNameNS(XmlPreProcessor.preprocessNamespaceUri, "properties"); while (list.getLength() > 0) { Element propertiesElement = (Element) list.item(0); - //System.out.println("prop=" + propertiesElement); Element parent = (Element) propertiesElement.getParentNode(); for (Node node : XML.getChildren(propertiesElement)) { - //System.out.println("Found " + node.getNodeName() + ", " + node.getTextContent()); - final String propertyName = node.getNodeName(); + String propertyName = node.getNodeName(); if (properties.containsKey(propertyName)) { log.log(Level.WARNING, "Duplicate definition for property '" + propertyName + "' detected"); } @@ -53,8 +56,7 @@ class PropertiesProcessor implements PreProcessor { } private void applyProperties(Element parent) { - // System.out.println("applying properties for " + parent.getNodeName()); - final NamedNodeMap attributes = parent.getAttributes(); + NamedNodeMap attributes = parent.getAttributes(); for (int i = 0; i < attributes.getLength(); i++) { Node a = attributes.item(i); if (hasProperty(a)) { @@ -84,7 +86,7 @@ class PropertiesProcessor implements PreProcessor { // Use a list with keys sorted by length (longest key first) // Needed for replacing values where you have overlapping keys ArrayList keys = new ArrayList<>(properties.keySet()); - Collections.sort(keys, Collections.reverseOrder(Comparator.comparing(String::length))); + keys.sort(Collections.reverseOrder(Comparator.comparing(String::length))); for (String key : keys) { String value = properties.get(key); @@ -92,10 +94,10 @@ class PropertiesProcessor implements PreProcessor { // first, the else branch will only happen when there cannot be an exact // match, i.e. where you want to replace only parts of the attribute or node value if (propertyValue.equals("${" + key + "}")) { - final String regex = "\\$\\{" + key + "\\}"; + String regex = "\\$\\{" + key + "}"; return propertyValue.replaceAll(regex, value); } else if (propertyValue.contains(key)) { - return propertyValue.replaceAll("\\$\\{" + key + "\\}", value); + return propertyValue.replaceAll("\\$\\{" + key + "}", value); } } throw new IllegalArgumentException("Unable to find property replace in " + propertyValue); @@ -116,10 +118,11 @@ class PropertiesProcessor implements PreProcessor { } private static boolean hasProperty(String s) { - return s.matches("^.*\\$\\{.+\\}.*$"); + return s.matches("^.*\\$\\{.+}.*$"); } public LinkedHashMap getProperties() { return properties; } + } diff --git a/config-application-package/src/main/java/com/yahoo/config/application/ValidationProcessor.java b/config-application-package/src/main/java/com/yahoo/config/application/ValidationProcessor.java index 5f430f70584..3dd2af0b6ea 100644 --- a/config-application-package/src/main/java/com/yahoo/config/application/ValidationProcessor.java +++ b/config-application-package/src/main/java/com/yahoo/config/application/ValidationProcessor.java @@ -15,4 +15,5 @@ public class ValidationProcessor implements PreProcessor { throw new UnsupportedOperationException("XInclude not supported, use preprocess:include instead"); return input; } + } \ No newline at end of file diff --git a/config-application-package/src/main/java/com/yahoo/config/application/Xml.java b/config-application-package/src/main/java/com/yahoo/config/application/Xml.java index cf391f2dd0e..525da509de6 100644 --- a/config-application-package/src/main/java/com/yahoo/config/application/Xml.java +++ b/config-application-package/src/main/java/com/yahoo/config/application/Xml.java @@ -32,19 +32,18 @@ import java.util.logging.Logger; * @author hmusum */ public class Xml { + private static final Logger log = Logger.getLogger(Xml.class.getPackage().toString()); // Access to this needs to be synchronized (as it is in getDocumentBuilder() below) private static final DocumentBuilderFactory factory = createDocumentBuilderFactory(); public static Document getDocument(Reader reader) { - Document doc; try { - doc = getDocumentBuilder().parse(new InputSource(reader)); + return getDocumentBuilder().parse(new InputSource(reader)); } catch (SAXException | IOException e) { throw new IllegalArgumentException(e); } - return doc; } private static DocumentBuilderFactory createDocumentBuilderFactory() { @@ -67,7 +66,7 @@ public class Xml { /** * Creates a new XML document builder. * - * @return A new DocumentBuilder instance, or null if we fail to get one. + * @return a new DocumentBuilder instance, or null if we fail to get one. */ private static synchronized DocumentBuilder getDocumentBuilder() { try { @@ -114,7 +113,7 @@ public class Xml { } /** - * Utility method to get an XML element from a reader + * Utility method to get an XML element from a reader. * * @param reader the {@link Reader} to get an xml element from */ @@ -122,9 +121,7 @@ public class Xml { return XML.getDocument(reader).getDocumentElement(); } - /** - * @return The root element of each xml file under pathFromAppRoot/ in the app package - */ + /** Returns the root element of each xml file under pathFromAppRoot/ in the app package. */ public static List allElemsFromPath(ApplicationPackage app, String pathFromAppRoot) { List ret = new ArrayList<>(); List files = null; @@ -156,4 +153,5 @@ public class Xml { } return children; } + } diff --git a/config-application-package/src/test/java/com/yahoo/config/application/IncludeProcessorTest.java b/config-application-package/src/test/java/com/yahoo/config/application/IncludeProcessorTest.java index 697d8c208d3..d4c8884db11 100644 --- a/config-application-package/src/test/java/com/yahoo/config/application/IncludeProcessorTest.java +++ b/config-application-package/src/test/java/com/yahoo/config/application/IncludeProcessorTest.java @@ -45,6 +45,7 @@ public class IncludeProcessorTest { + 1 diff --git a/config-application-package/src/test/java/com/yahoo/config/application/OverrideProcessorTest.java b/config-application-package/src/test/java/com/yahoo/config/application/OverrideProcessorTest.java index ca074ef9704..4d972d15716 100644 --- a/config-application-package/src/test/java/com/yahoo/config/application/OverrideProcessorTest.java +++ b/config-application-package/src/test/java/com/yahoo/config/application/OverrideProcessorTest.java @@ -287,38 +287,6 @@ public class OverrideProcessorTest { assertOverride(Environment.from("test"), RegionName.from("us-west"), expected); } - @Test - public void testParsingInheritEnvironment() throws TransformerException { - String expected = - "" + - "" + - " " + - " " + - " " + - " " + - " 1" + - " " + - " \n" + - " \n" + - " \n" + - " " + - " " + - // node1 is specified for us-west but does not match because region overrides implies environment=prod - " " + - " " + - " " + - " " + - " " + - " " + - " " + - " " + - " " + - " " + - " " + - ""; - assertOverride(Environment.from("staging"), RegionName.from("us-west"), expected); - } - @Test(expected = IllegalArgumentException.class) public void testParsingDifferentEnvInParentAndChild() throws TransformerException { String in = "" + diff --git a/config-application-package/src/test/java/com/yahoo/config/application/XmlPreprocessorTest.java b/config-application-package/src/test/java/com/yahoo/config/application/XmlPreprocessorTest.java index 49dbacbae3d..bbccc8343a1 100644 --- a/config-application-package/src/test/java/com/yahoo/config/application/XmlPreprocessorTest.java +++ b/config-application-package/src/test/java/com/yahoo/config/application/XmlPreprocessorTest.java @@ -57,14 +57,14 @@ public class XmlPreprocessorTest { Tags.empty()).run()); // Difference from dev: node1 - // Difference from dev: no TestBar + // Difference from dev: no TestBar String expectedStaging = """ - + 1 @@ -91,7 +91,81 @@ public class XmlPreprocessorTest { RegionName.defaultName(), Tags.empty()).run()); - String expectedUsWest = + String expectedPerfUsWest = + """ + + + + + + + + 1 + + + + + + + + + + + + + + + """; + TestBase.assertDocument(expectedPerfUsWest, + new XmlPreProcessor(appDir, + services, + InstanceName.defaultName(), + Environment.perf, + RegionName.from("us-west"), + Tags.empty()).run()); + + String expectedPerfUsEastAndCentral = + """ + + + + + + + + + 1 + + + + + + + + + + + + + + + """; + TestBase.assertDocument(expectedPerfUsEastAndCentral, + new XmlPreProcessor(appDir, + services, + InstanceName.defaultName(), + Environment.perf, + RegionName.from("us-east"), + Tags.empty()).run()); + TestBase.assertDocument(expectedPerfUsEastAndCentral, + new XmlPreProcessor(appDir, + services, + InstanceName.defaultName(), + Environment.perf, + RegionName.from("us-central"), + Tags.empty()).run()); + + String expectedProdUsWest = """ @@ -125,7 +199,7 @@ public class XmlPreprocessorTest { """; - TestBase.assertDocument(expectedUsWest, + TestBase.assertDocument(expectedProdUsWest, new XmlPreProcessor(appDir, services, InstanceName.defaultName(), @@ -133,7 +207,7 @@ public class XmlPreprocessorTest { RegionName.from("us-west"), Tags.empty()).run()); - String expectedUsEastAndCentral = + String expectedProdUsEastAndCentral = """ @@ -142,6 +216,7 @@ public class XmlPreprocessorTest { + 1 @@ -166,14 +241,14 @@ public class XmlPreprocessorTest { """; - TestBase.assertDocument(expectedUsEastAndCentral, + TestBase.assertDocument(expectedProdUsEastAndCentral, new XmlPreProcessor(appDir, services, InstanceName.defaultName(), Environment.prod, RegionName.from("us-east"), Tags.empty()).run()); - TestBase.assertDocument(expectedUsEastAndCentral, + TestBase.assertDocument(expectedProdUsEastAndCentral, new XmlPreProcessor(appDir, services, InstanceName.defaultName(), diff --git a/config-application-package/src/test/resources/multienvapp/services.xml b/config-application-package/src/test/resources/multienvapp/services.xml index 450291fa609..5c0ad50d87d 100644 --- a/config-application-package/src/test/resources/multienvapp/services.xml +++ b/config-application-package/src/test/resources/multienvapp/services.xml @@ -22,6 +22,7 @@ + diff --git a/config-provisioning/src/main/java/com/yahoo/config/provision/Environment.java b/config-provisioning/src/main/java/com/yahoo/config/provision/Environment.java index 2d3d5e062f7..bbec1f14c13 100644 --- a/config-provisioning/src/main/java/com/yahoo/config/provision/Environment.java +++ b/config-provisioning/src/main/java/com/yahoo/config/provision/Environment.java @@ -34,8 +34,9 @@ public enum Environment { /** Returns whether this environment is production (prod) */ public boolean isProduction() { return this == prod; } - /** Returns whether this environment can exist in multiple regions */ - public boolean isMultiRegion() { return this == prod || this == dev; } + /** Returns whether this environment can exist in multiple regions. @deprecated they all are */ + @Deprecated // TODO: Remove after June 2023 + public boolean isMultiRegion() { return true; } /** Returns the prod environment. This is useful for non-hosted properties where we just need any consistent value */ public static Environment defaultEnvironment() { return prod; } -- cgit v1.2.3