diff options
7 files changed, 124 insertions, 19 deletions
diff --git a/config-model-api/abi-spec.json b/config-model-api/abi-spec.json index 15e53492860..28b07fcd94f 100644 --- a/config-model-api/abi-spec.json +++ b/config-model-api/abi-spec.json @@ -280,6 +280,19 @@ ], "fields": [] }, + "com.yahoo.config.application.api.DeploymentSpec$DeprecatedElement": { + "superClass": "java.lang.Object", + "interfaces": [], + "attributes": [ + "public" + ], + "methods": [ + "public void <init>(java.lang.String, java.util.List, java.lang.String)", + "public java.lang.String humanReadableString()", + "public java.lang.String toString()" + ], + "fields": [] + }, "com.yahoo.config.application.api.DeploymentSpec$ParallelSteps": { "superClass": "com.yahoo.config.application.api.DeploymentSpec$Steps", "interfaces": [], @@ -375,7 +388,7 @@ "public" ], "methods": [ - "public void <init>(java.util.List, java.util.Optional, java.util.Optional, java.util.Optional, java.util.List, java.lang.String)", + "public void <init>(java.util.List, java.util.Optional, java.util.Optional, java.util.Optional, java.util.List, java.lang.String, java.util.List)", "public java.util.Optional majorVersion()", "public java.util.List steps()", "public java.util.Optional athenzDomain()", @@ -387,6 +400,7 @@ "public java.util.List instanceNames()", "public java.util.List instances()", "public java.util.List endpoints()", + "public java.util.List deprecatedElements()", "public static com.yahoo.config.application.api.DeploymentSpec fromXml(java.io.Reader)", "public static com.yahoo.config.application.api.DeploymentSpec fromXml(java.lang.String)", "public static com.yahoo.config.application.api.DeploymentSpec fromXml(java.lang.String, boolean)", diff --git a/config-model-api/src/main/java/com/yahoo/config/application/api/DeploymentSpec.java b/config-model-api/src/main/java/com/yahoo/config/application/api/DeploymentSpec.java index 97ece3a675e..88363db6e49 100644 --- a/config-model-api/src/main/java/com/yahoo/config/application/api/DeploymentSpec.java +++ b/config-model-api/src/main/java/com/yahoo/config/application/api/DeploymentSpec.java @@ -39,7 +39,8 @@ public class DeploymentSpec { Optional.empty(), Optional.empty(), List.of(), - "<deployment version='1.0'/>"); + "<deployment version='1.0'/>", + List.of()); private final List<Step> steps; @@ -48,6 +49,7 @@ public class DeploymentSpec { private final Optional<AthenzDomain> athenzDomain; private final Optional<AthenzService> athenzService; private final List<Endpoint> endpoints; + private final List<DeprecatedElement> deprecatedElements; private final String xmlForm; @@ -56,13 +58,15 @@ public class DeploymentSpec { Optional<AthenzDomain> athenzDomain, Optional<AthenzService> athenzService, List<Endpoint> endpoints, - String xmlForm) { + String xmlForm, + List<DeprecatedElement> deprecatedElements) { this.steps = List.copyOf(Objects.requireNonNull(steps)); this.majorVersion = Objects.requireNonNull(majorVersion); this.athenzDomain = Objects.requireNonNull(athenzDomain); this.athenzService = Objects.requireNonNull(athenzService); this.xmlForm = Objects.requireNonNull(xmlForm); this.endpoints = List.copyOf(Objects.requireNonNull(endpoints)); + this.deprecatedElements = List.copyOf(Objects.requireNonNull(deprecatedElements)); validateTotalDelay(steps); validateUpgradePoliciesOfIncreasingConservativeness(steps); validateAthenz(); @@ -201,6 +205,11 @@ public class DeploymentSpec { return endpoints; } + /** Returns the deprecated elements used when creating this */ + public List<DeprecatedElement> deprecatedElements() { + return deprecatedElements; + } + private static List<DeploymentInstanceSpec> instances(List<DeploymentSpec.Step> steps) { return steps.stream() .flatMap(DeploymentSpec::flatten) @@ -576,5 +585,37 @@ public class DeploymentSpec { } + /** + * Represents a deprecated XML element in {@link com.yahoo.config.application.api.DeploymentSpec}, or the deprecated + * attribute(s) of an element. + */ + public static class DeprecatedElement { + + private final String tagName; + private final List<String> attributes; + private final String message; + + public DeprecatedElement(String tagName, List<String> attributes, String message) { + this.tagName = Objects.requireNonNull(tagName); + this.attributes = Objects.requireNonNull(attributes); + this.message = Objects.requireNonNull(message); + if (message.isBlank()) throw new IllegalArgumentException("message must be non-empty"); + } + + public String humanReadableString() { + if (attributes.isEmpty()) { + return "Element '" + tagName + "' is deprecated. " + message; + } + return "Element '" + tagName + "' contains deprecated attribute" + (attributes.size() > 1 ? "s" : "") + ": " + + attributes.stream().map(attr -> "'" + attr + "'").collect(Collectors.joining(", ")) + + ". " + message; + } + + @Override + public String toString() { + return humanReadableString(); + } + + } } diff --git a/config-model-api/src/main/java/com/yahoo/config/application/api/xml/DeploymentSpecXmlReader.java b/config-model-api/src/main/java/com/yahoo/config/application/api/xml/DeploymentSpecXmlReader.java index 07b8462f4d1..aa985cd48bd 100644 --- a/config-model-api/src/main/java/com/yahoo/config/application/api/xml/DeploymentSpecXmlReader.java +++ b/config-model-api/src/main/java/com/yahoo/config/application/api/xml/DeploymentSpecXmlReader.java @@ -6,6 +6,7 @@ import com.yahoo.config.application.api.DeploymentSpec; import com.yahoo.config.application.api.DeploymentSpec.DeclaredTest; import com.yahoo.config.application.api.DeploymentSpec.DeclaredZone; import com.yahoo.config.application.api.DeploymentSpec.Delay; +import com.yahoo.config.application.api.DeploymentSpec.DeprecatedElement; import com.yahoo.config.application.api.DeploymentSpec.ParallelSteps; import com.yahoo.config.application.api.DeploymentSpec.Step; import com.yahoo.config.application.api.DeploymentSpec.Steps; @@ -66,6 +67,7 @@ public class DeploymentSpecXmlReader { private static final String testerFlavorAttribute = "tester-flavor"; private final boolean validate; + private final List<DeprecatedElement> deprecatedElements = new ArrayList<>(); /** Creates a validating reader */ public DeploymentSpecXmlReader() { @@ -92,6 +94,7 @@ public class DeploymentSpecXmlReader { /** Reads a deployment spec from XML */ public DeploymentSpec read(String xmlForm) { + deprecatedElements.clear(); Element root = XML.getDocument(xmlForm).getDocumentElement(); if ( ! root.getTagName().equals(deploymentTag)) illegal("The root tag must be <deployment>"); @@ -126,7 +129,8 @@ public class DeploymentSpecXmlReader { stringAttribute(athenzDomainAttribute, root).map(AthenzDomain::from), stringAttribute(athenzServiceAttribute, root).map(AthenzService::from), applicationEndpoints, - xmlForm); + xmlForm, + deprecatedElements); } /** @@ -404,6 +408,7 @@ public class DeploymentSpecXmlReader { private Optional<String> readGlobalServiceId(Element environmentTag) { String globalServiceId = environmentTag.getAttribute("global-service-id"); if (globalServiceId == null || globalServiceId.isEmpty()) return Optional.empty(); + deprecate(environmentTag, List.of("global-service-id"), "See https://cloud.vespa.ai/en/reference/routing#deprecated-syntax"); return Optional.of(globalServiceId); } @@ -478,12 +483,17 @@ public class DeploymentSpecXmlReader { private boolean readActive(Element regionTag) { String activeValue = regionTag.getAttribute("active"); if ("".equals(activeValue)) return true; // Default to active + deprecate(regionTag, List.of("active"), "See https://cloud.vespa.ai/en/reference/routing#deprecated-syntax"); if ("true".equals(activeValue)) return true; if ("false".equals(activeValue)) return false; throw new IllegalArgumentException("Value of 'active' attribute in region tag must be 'true' or 'false' " + "to control whether this region should receive traffic from the global endpoint of this application"); } + private void deprecate(Element element, List<String> attributes, String message) { + deprecatedElements.add(new DeprecatedElement(element.getTagName(), attributes, message)); + } + private static boolean isEmptySpec(Element root) { if ( ! XML.getChildren(root).isEmpty()) return false; return root.getAttributes().getLength() == 0 diff --git a/config-model/src/main/java/com/yahoo/vespa/model/container/xml/ContainerModelBuilder.java b/config-model/src/main/java/com/yahoo/vespa/model/container/xml/ContainerModelBuilder.java index d1091829cc5..73138d15559 100644 --- a/config-model/src/main/java/com/yahoo/vespa/model/container/xml/ContainerModelBuilder.java +++ b/config-model/src/main/java/com/yahoo/vespa/model/container/xml/ContainerModelBuilder.java @@ -209,7 +209,7 @@ public class ContainerModelBuilder extends ConfigModelBuilder<ContainerModel> { addServerProviders(deployState, spec, cluster); // Must be added after nodes: - addAthensCopperArgos(cluster, context); + addDeploymentSpecConfig(cluster, context, deployState.getDeployLogger()); addZooKeeper(cluster, spec); addParameterStoreValidationHandler(cluster, deployState); @@ -307,19 +307,23 @@ public class ContainerModelBuilder extends ConfigModelBuilder<ContainerModel> { cluster.addComponent(cloudSecretStore); } - private void addAthensCopperArgos(ApplicationContainerCluster cluster, ConfigModelContext context) { + private void addDeploymentSpecConfig(ApplicationContainerCluster cluster, ConfigModelContext context, DeployLogger deployLogger) { if ( ! context.getDeployState().isHosted()) return; - app.getDeployment().map(DeploymentSpec::fromXml) - .ifPresent(deploymentSpec -> { - addIdentityProvider(cluster, - context.getDeployState().getProperties().configServerSpecs(), - context.getDeployState().getProperties().loadBalancerName(), - context.getDeployState().getProperties().ztsUrl(), - context.getDeployState().getProperties().athenzDnsSuffix(), - context.getDeployState().zone(), - deploymentSpec); - addRotationProperties(cluster, context.getDeployState().zone(), context.getDeployState().getEndpoints(), deploymentSpec); - }); + Optional<DeploymentSpec> deploymentSpec = app.getDeployment().map(DeploymentSpec::fromXml); + if (deploymentSpec.isEmpty()) return; + + for (var deprecatedElement : deploymentSpec.get().deprecatedElements()) { + deployLogger.log(WARNING, deprecatedElement.humanReadableString()); + } + + addIdentityProvider(cluster, + context.getDeployState().getProperties().configServerSpecs(), + context.getDeployState().getProperties().loadBalancerName(), + context.getDeployState().getProperties().ztsUrl(), + context.getDeployState().getProperties().athenzDnsSuffix(), + context.getDeployState().zone(), + deploymentSpec.get()); + addRotationProperties(cluster, context.getDeployState().zone(), context.getDeployState().getEndpoints(), deploymentSpec.get()); } private void addRotationProperties(ApplicationContainerCluster cluster, Zone zone, Set<ContainerEndpoint> endpoints, DeploymentSpec spec) { diff --git a/config-model/src/main/resources/schema/deployment.rnc b/config-model/src/main/resources/schema/deployment.rnc index f24750bde8b..51a286a13c8 100644 --- a/config-model/src/main/resources/schema/deployment.rnc +++ b/config-model/src/main/resources/schema/deployment.rnc @@ -101,7 +101,7 @@ ProdTest = element test { } Region = element region { - attribute active { xsd:boolean } & + attribute active { xsd:boolean }? & attribute athenz-service { xsd:string }? & text } diff --git a/config-model/src/test/cfg/application/app_invalid_deployment_xml/deployment.xml b/config-model/src/test/cfg/application/app_invalid_deployment_xml/deployment.xml index ac72067e9e7..738a3397aad 100644 --- a/config-model/src/test/cfg/application/app_invalid_deployment_xml/deployment.xml +++ b/config-model/src/test/cfg/application/app_invalid_deployment_xml/deployment.xml @@ -4,6 +4,6 @@ <staging/> <prod global-service-id="query"> <region>us-east-3</region> - <region active="false">us-west-1</region> + <region invalid="invalid">us-west-1</region> </prod> </deployment> diff --git a/config-model/src/test/java/com/yahoo/vespa/model/container/xml/ContainerModelBuilderTest.java b/config-model/src/test/java/com/yahoo/vespa/model/container/xml/ContainerModelBuilderTest.java index f02e99c5820..8ceb74c3d7e 100644 --- a/config-model/src/test/java/com/yahoo/vespa/model/container/xml/ContainerModelBuilderTest.java +++ b/config-model/src/test/java/com/yahoo/vespa/model/container/xml/ContainerModelBuilderTest.java @@ -1060,6 +1060,42 @@ public class ContainerModelBuilderTest extends ContainerModelBuilderTestBase { } } + @Test + public void logs_deployment_spec_deprecations() throws Exception { + String containerService = joinLines("<container id='foo' version='1.0'>", + " <nodes>", + " <node hostalias='host1' />", + " </nodes>", + "</container>"); + String deploymentXml = joinLines("<deployment version='1.0'>", + " <prod global-service-id='foo'>", + " <region active='true'>us-east-1</region>", + " </prod>", + "</deployment>"); + + ApplicationPackage applicationPackage = new MockApplicationPackage.Builder() + .withServices(containerService) + .withDeploymentSpec(deploymentXml) + .build(); + + TestLogger logger = new TestLogger(); + DeployState deployState = new DeployState.Builder() + .applicationPackage(applicationPackage) + .zone(new Zone(Environment.prod, RegionName.from("us-east-1"))) + .properties(new TestProperties().setHostedVespa(true)) + .deployLogger(logger) + .build(); + + createModel(root, deployState, null, DomBuilderTest.parse(containerService)); + assertFalse(logger.msgs.isEmpty()); + assertEquals(Level.WARNING, logger.msgs.get(0).getFirst()); + assertEquals(Level.WARNING, logger.msgs.get(1).getFirst()); + assertEquals("Element 'prod' contains deprecated attribute: 'global-service-id'. See https://cloud.vespa.ai/en/reference/routing#deprecated-syntax", + logger.msgs.get(0).getSecond()); + assertEquals("Element 'region' contains deprecated attribute: 'active'. See https://cloud.vespa.ai/en/reference/routing#deprecated-syntax", + logger.msgs.get(1).getSecond()); + } + private void assertComponentConfigured(ApplicationContainerCluster cluster, String componentId) { Component<?, ?> component = cluster.getComponentsMap().get(ComponentId.fromString(componentId)); assertNotNull(component); |